Roll FFmpeg for M37.

media/base/media_file_checker.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/base/media_file_checker.h"
 
 #include <map>
 
 #include "base/bind.h"
 #include "base/time/time.h"
@@ skipping 58 matching lines @@
     result = av_dup_packet(&packet);
     if (result < 0)
       break;
 
     std::map<int, AVCodecContext*>::const_iterator it =
         stream_contexts.find(packet.stream_index);
     if (it == stream_contexts.end()) {
       av_free_packet(&packet);
       continue;
     }
     AVCodecContext* av_context = it->second;

[wolenetz, 2014/05/16 19:45:58]

I'm confused a bit. Codesearch failed to indicate to me where we ensure
av_context.refcounted_frames is set to 1 in this code path.
So I'm not sure av_frame_unref() is always good to use here.

[DaleCurtis, 2014/05/16 19:54:26]

Looks like this may need to wait until the roll after all.  You can see that in
the new version of FFmpeg avcodec_get_frame_defaults() just calls
av_buffer_unref() regardless of that setting:

http://git.chromium.org/gitweb/?p=chromium/third_party/ffmpeg.git;a=blob;f=li...

 
     int frame_decoded = 0;
     if (av_context->codec_type == AVMEDIA_TYPE_AUDIO) {
       // A shallow copy of packet so we can slide packet.data as frames are
       // decoded; otherwise av_free_packet() will corrupt memory.
       AVPacket temp_packet = packet;
       do {
-        avcodec_get_frame_defaults(frame.get());

[wolenetz, 2014/05/16 19:45:58]

nit: In CL description, s/get_buffer_defaults/get_frame_defaults

[DaleCurtis, 2014/05/20 01:13:45]

Done.

         result = avcodec_decode_audio4(av_context, frame.get(), &frame_decoded,
                                        &temp_packet);
         if (result < 0)
           break;
+        av_frame_unref(frame.get());
         temp_packet.size -= result;
         temp_packet.data += result;
+        frame_decoded = 0;
       } while (temp_packet.size > 0);
     } else if (av_context->codec_type == AVMEDIA_TYPE_VIDEO) {
-      avcodec_get_frame_defaults(frame.get());
       result = avcodec_decode_video2(av_context, frame.get(), &frame_decoded,

[wolenetz, 2014/05/16 19:45:58]

(Possibly for a different CL): Per
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/ffmpeg...,
ILTM like we might not be protecting against overreads for damaged MPEG streams.
By design or am I missing something? 

[DaleCurtis, 2014/05/16 19:54:26]

DecoderBuffer automatically pads and zeros.

[wolenetz, 2014/05/20 22:37:10]

(Clarification from offline / CR reply comment in other file: ffmpeg packet
creation in this path pads (in ffmpeg code). In main media pipeline path (not in
this file), ffmpeg_demuxer.cc code using DecoderBuffer that similarly pads and
zeros is used).

                                      &packet);
+      if (result >= 0 && frame_decoded)

[wolenetz, 2014/05/16 19:45:58]

(Possibly for a different CL): Per
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/ffmpeg...,
some decoders may support multiple frames in a single AVPacket. For video, why
don't we use temp_packet, and update its size & data like we do for audio? Is
this possibly because the return value (unlike audio) is zero if no frame could
be decompressed, instead of number of bytes consumed regardless?

[DaleCurtis, 2014/05/16 19:54:26]

Hmm, that comment has changed I think. When I talked to Ronald about this in the
past FFmpeg only outputs 1 frame per packet.

We could add a check to ensure this is true though.

[DaleCurtis, 2014/05/20 01:13:45]

Done.

[wolenetz, 2014/05/20 22:37:10]

Clarification from offline: DCHECK is added to main media pipeline, but not this
checker.

+        av_frame_unref(frame.get());
     }
     av_free_packet(&packet);
   } while (base::TimeTicks::Now() < deadline && read_ok && result >= 0);
 
   return read_ok && (result == AVERROR_EOF || result >= 0);
 }
 
 }  // namespace media