Roll src/third_party/ffmpeg/ 3c7a09882..cdf4accee (3188 commits).

media/ffmpeg/ffmpeg_common.cc

Index: media/ffmpeg/ffmpeg_common.cc
diff --git a/media/ffmpeg/ffmpeg_common.cc b/media/ffmpeg/ffmpeg_common.cc
index aaf3afcd5de754f1b2371deb3565ae466d3bd5bb..8122ad9748e1cce74148f5a873bc777661ef336c 100644
--- a/media/ffmpeg/ffmpeg_common.cc
+++ b/media/ffmpeg/ffmpeg_common.cc
@@ -22,6 +22,22 @@ namespace media {
 
 namespace {
 
+gfx::Size GetCodedSize(AVCodecContext* context) {

[DaleCurtis, 2016/11/14 17:58:20]

Do we need this function? It seems like we should remove coded size from the
VideoDecoderConfig; or at least for this roll just make it equal to
width/height.

It's only really used by ffmpeg to output the video frames AFAICT.

[wolenetz, 2016/11/15 02:15:35]

We record stats including coded size (perhaps we don't really need to). Perhaps
more importantly, we also create ffmpeg video decode contexts from video decoder
configs using coded size as part of that creation. It's also not clear to me
that VideoDecoderConfig::coded_size() is truly not needed by *all* of our
various video decoder's initializations (content_decryptor_delegate, gpu, vpx,
etc), not just ffmpeg, so would prefer not removing it during this roll.

+  DCHECK(context);
+
+  // Open a temporary decode, to try to get coded size if codec is supported.
+  AVCodec* codec = avcodec_find_decoder(context->codec_id);

[DaleCurtis, 2016/11/14 17:58:21]

Doesn't this need to be released?

[wolenetz, 2016/11/15 02:15:35]

No. Rather, the caller's management of |context|'s eventual close (using
ScopedPtrAVFreeContext) leads to closing the context as part of av_free_context
calling avcodec_close. That method sets codec to NULL (not releasing it). retval
of avcodec_find_decoder is a ptr to a global entry, so releasing it as part of
context free doesn't make sense.

Also, no LSAN/MSAN/ASAN errs on media_unittests or chrome (against both YT and
go/videotestmatrix); third_party/ffmpeg/docs/examples/demuxing_decoding.c does
similar...

+  if (codec && avcodec_open2(context, codec, NULL) >= 0 &&
+      context->coded_width > 0 && context->coded_height > 0) {
+    return gfx::Size(context->coded_width, context->coded_height);
+  }
+
+  // Otherwise, either ffmpeg does not support decode of the codec or opening
+  // the decoder failed. Default to the width/height. If a decoder other than
+  // ffmpeg is used, it will need to figure out the correct coded size.
+  return gfx::Size(context->width, context->height);
+}
+
 EncryptionScheme GetEncryptionScheme(const AVStream* stream) {
   AVDictionaryEntry* key =
       av_dict_get(stream->metadata, "enc_key_id", nullptr, 0);
@@ -407,8 +423,16 @@ bool AVCodecContextToAudioDecoderConfig(
 
 bool AVStreamToAudioDecoderConfig(const AVStream* stream,
                                   AudioDecoderConfig* config) {
+  std::unique_ptr<AVCodecContext, ScopedPtrAVFreeContext> codec_context(

[DaleCurtis, 2016/11/14 17:58:20]

Seems this could be extracted to a helper function since it's used in many
places.

[wolenetz, 2016/11/15 02:15:35]

SGTM. I was beginning to think similar :)
Done.

+      avcodec_alloc_context3(NULL));
+  if (!codec_context ||
+      avcodec_parameters_to_context(codec_context.get(), stream->codecpar) <
+          0) {
+    return false;
+  }
+
   return AVCodecContextToAudioDecoderConfig(
-      stream->codec, GetEncryptionScheme(stream), config);
+      codec_context.get(), GetEncryptionScheme(stream), config);
 }
 
 void AudioDecoderConfigToAVCodecContext(const AudioDecoderConfig& config,
@@ -441,19 +465,29 @@ void AudioDecoderConfigToAVCodecContext(const AudioDecoderConfig& config,
 
 bool AVStreamToVideoDecoderConfig(const AVStream* stream,
                                   VideoDecoderConfig* config) {
-  gfx::Size coded_size(stream->codec->coded_width, stream->codec->coded_height);
+  std::unique_ptr<AVCodecContext, ScopedPtrAVFreeContext> codec_context(
+      avcodec_alloc_context3(NULL));
+  if (!codec_context ||
+      avcodec_parameters_to_context(codec_context.get(), stream->codecpar) <
+          0) {
+    return false;
+  }
+
+  // This call will attempt to open a decoder to further populate
+  // |codec_context|.
+  gfx::Size coded_size(GetCodedSize(codec_context.get()));
 
   // TODO(vrk): This assumes decoded frame data starts at (0, 0), which is true
   // for now, but may not always be true forever. Fix this in the future.
-  gfx::Rect visible_rect(stream->codec->width, stream->codec->height);
+  gfx::Rect visible_rect(codec_context->width, codec_context->height);
 
   AVRational aspect_ratio = { 1, 1 };
   if (stream->sample_aspect_ratio.num)
     aspect_ratio = stream->sample_aspect_ratio;
-  else if (stream->codec->sample_aspect_ratio.num)
-    aspect_ratio = stream->codec->sample_aspect_ratio;
+  else if (codec_context->sample_aspect_ratio.num)
+    aspect_ratio = codec_context->sample_aspect_ratio;
 
-  VideoCodec codec = CodecIDToVideoCodec(stream->codec->codec_id);
+  VideoCodec codec = CodecIDToVideoCodec(codec_context->codec_id);
 
   VideoCodecProfile profile = VIDEO_CODEC_PROFILE_UNKNOWN;
   if (codec == kCodecVP8)
@@ -463,7 +497,7 @@ bool AVStreamToVideoDecoderConfig(const AVStream* stream,
     // crbug.com/592074
     profile = VP9PROFILE_PROFILE0;
   else
-    profile = ProfileIDToVideoCodecProfile(stream->codec->profile);
+    profile = ProfileIDToVideoCodecProfile(codec_context->profile);
 
   // Without the FFmpeg h264 decoder, AVFormat is unable to get the profile, so
   // default to baseline and let the VDA fail later if it doesn't support the
@@ -478,7 +512,7 @@ bool AVStreamToVideoDecoderConfig(const AVStream* stream,
       visible_rect.size(), aspect_ratio.num, aspect_ratio.den);
 
   VideoPixelFormat format =
-      AVPixelFormatToVideoPixelFormat(stream->codec->pix_fmt);
+      AVPixelFormatToVideoPixelFormat(codec_context->pix_fmt);
   // The format and coded size may be unknown if FFmpeg is compiled without
   // video decoders.
 #if defined(DISABLE_FFMPEG_VIDEO_DECODERS)
@@ -508,8 +542,8 @@ bool AVStreamToVideoDecoderConfig(const AVStream* stream,
   }
 
   // Prefer the color space found by libavcodec if available.
-  ColorSpace color_space = AVColorSpaceToColorSpace(stream->codec->colorspace,
-                                                    stream->codec->color_range);
+  ColorSpace color_space = AVColorSpaceToColorSpace(codec_context->colorspace,
+                                                    codec_context->color_range);
   if (color_space == COLOR_SPACE_UNSPECIFIED) {
     // Otherwise, assume that SD video is usually Rec.601, and HD is usually
     // Rec.709.
@@ -517,20 +551,20 @@ bool AVStreamToVideoDecoderConfig(const AVStream* stream,
                                                 : COLOR_SPACE_HD_REC709;
   }
 
-  // AVStream occasionally has invalid extra data. See http://crbug.com/517163
-  if ((stream->codec->extradata_size == 0) !=
-      (stream->codec->extradata == nullptr)) {
-    LOG(ERROR) << __func__
-               << (stream->codec->extradata == nullptr ? " NULL" : " Non-Null")
-               << " extra data cannot have size of "
-               << stream->codec->extradata_size << ".";
+  // AVCodecContext occasionally has invalid extra data. See
+  // http://crbug.com/517163
+  if (codec_context->extradata != nullptr &&
+      codec_context->extradata_size == 0) {
+    LOG(ERROR) << __func__ << " Non-Null extra data cannot have size of 0.";
     return false;
   }
+  CHECK_EQ(codec_context->extradata == nullptr,
+           codec_context->extradata_size == 0);
 
   std::vector<uint8_t> extra_data;
-  if (stream->codec->extradata_size > 0) {
-    extra_data.assign(stream->codec->extradata,
-                      stream->codec->extradata + stream->codec->extradata_size);
+  if (codec_context->extradata_size > 0) {
+    extra_data.assign(codec_context->extradata,
+                      codec_context->extradata + codec_context->extradata_size);
   }
   config->Initialize(codec, profile, format, color_space, coded_size,
                      visible_rect, natural_size, extra_data,
@@ -734,8 +768,12 @@ bool FFmpegUTCDateToTime(const char* date_utc, base::Time* out) {
   DCHECK(date_utc);
   DCHECK(out);
 
+  // Note, base::Time::FromUTCString(date_utc, out) would work for parsing a

[DaleCurtis, 2016/11/14 17:58:20]

Is it really worth having our own implementation? If there's an existing well
tested method, we should just use that; even if it's a bit more lenient -- it's
not like this is critical information :)

[wolenetz, 2016/11/15 02:15:35]

Yeah, I tried that first, and found the leniency failed our (strict) tests for
"invalid" times.
I've now changed to the base/time/time.h version of similar that is more
lenient, directly used from FFmpegDemuxer's ExtractTimelineOffset() helper. I
kept a subset of the unit tests (moved from ffmpeg_common_unittest to
ffmpeg_demuxer_unittest) to verify basic functionality of
base::Time::FromUTCString() as used by FFmpegDemuxer.

+  // valid FFmpegUTC strictly standardized |date_utc|, but it is more flexible
+  // to allowing missing pieces and having different formats. This method
+  // enforces stricter format expectation of FFmpeg.
   std::vector<base::StringPiece> fields = base::SplitStringPiece(
-      date_utc, " ", base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+      date_utc, "T", base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
   if (fields.size() != 2)
     return false;
 
@@ -744,26 +782,44 @@ bool FFmpegUTCDateToTime(const char* date_utc, base::Time* out) {
   if (date_fields.size() != 3)
     return false;
 
-  // TODO(acolwell): Update this parsing code when FFmpeg returns sub-second
-  // information.
   std::vector<base::StringPiece> time_fields = base::SplitStringPiece(
-      fields[1], ":", base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
-  if (time_fields.size() != 3)
+      fields[1], ".", base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+  if (time_fields.size() != 2)
+    return false;
+
+  std::vector<base::StringPiece> hms_fields = base::SplitStringPiece(
+      time_fields[0], ":", base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+  if (hms_fields.size() != 3)
+    return false;
+
+  std::vector<base::StringPiece> subsecond_fields = base::SplitStringPiece(
+      time_fields[1], "Z", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
+  if (subsecond_fields.size() != 2 || subsecond_fields[0].length() != 6 ||
+      !subsecond_fields[1].empty()) {
+    return false;
+  }
+
+  int microseconds;
+  if (!base::StringToInt(subsecond_fields[0], &microseconds))
     return false;
 
   base::Time::Exploded exploded;
-  exploded.millisecond = 0;
   // This field cannot be uninitialized. Unless not modified, make it 0 here
   // then.
   exploded.day_of_week = 0;
+
+  exploded.millisecond = microseconds / base::Time::kMicrosecondsPerMillisecond;
   if (base::StringToInt(date_fields[0], &exploded.year) &&
       base::StringToInt(date_fields[1], &exploded.month) &&
       base::StringToInt(date_fields[2], &exploded.day_of_month) &&
-      base::StringToInt(time_fields[0], &exploded.hour) &&
-      base::StringToInt(time_fields[1], &exploded.minute) &&
-      base::StringToInt(time_fields[2], &exploded.second)) {
-    if (base::Time::FromUTCExploded(exploded, out))
+      base::StringToInt(hms_fields[0], &exploded.hour) &&
+      base::StringToInt(hms_fields[1], &exploded.minute) &&
+      base::StringToInt(hms_fields[2], &exploded.second)) {
+    if (base::Time::FromUTCExploded(exploded, out)) {
+      *out += base::TimeDelta::FromMicroseconds(
+          microseconds % base::Time::kMicrosecondsPerMillisecond);
       return true;
+    }
   }
 
   return false;