Re-add AC3/EAC3 audio demuxing support

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 3d3a8b89957d36be73c1480b1a63788dee3049d9..5318a10958ff4eee1f24b09fe7ed34344db5f69f 100644
--- a/media/ffmpeg/ffmpeg_common.cc
+++ b/media/ffmpeg/ffmpeg_common.cc
@@ -67,6 +67,12 @@ static AudioCodec CodecIDToAudioCodec(AVCodecID codec_id) {
   switch (codec_id) {
     case AV_CODEC_ID_AAC:
       return kCodecAAC;
+#if defined(ENABLE_AC3_EAC3_AUDIO_DEMUXING)
+    case AV_CODEC_ID_AC3:
+      return kCodecAC3;
+    case AV_CODEC_ID_EAC3:
+      return kCodecEAC3;
+#endif
     case AV_CODEC_ID_MP3:
       return kCodecMP3;
     case AV_CODEC_ID_VORBIS:
@@ -323,6 +329,14 @@ bool AVCodecContextToAudioDecoderConfig(const AVCodecContext* codec_context,
     sample_rate = 48000;
   }
 
+  // For AC3/EAC3 we enable only demuxing, but not decoding (due to licensing

[ddorwin, 2016/01/07 19:12:43]

For a variety of reasons, including that we don't have a complex matrix of
"ffmpeg_branding" options. I think you can just drop the ().

[servolk, 2016/01/07 21:17:36]

Done.

+  // restrictions), so FFmpeg does not fill |sample_fmt|.
+  if (codec == kCodecAC3 || codec == kCodecEAC3) {

[ddorwin, 2016/01/07 19:12:43]

This should be else if.

[servolk, 2016/01/07 21:17:36]

Even if we wrap it into #if as suggested below? I think the difference between
if/'else if' in this case is really minor and I'd rather have a stand-alone
piece of code inside #if, rather than relying on another if to be above it.

+    // The spec for AC3/EAC3 audio ETSI TS 102 366, according to Sections F.3.1
+    // and F.5.1 sample_format for AC3/EAC3 must be 16.
+    sample_format = kSampleFormatS16;
+  }

[ddorwin, 2016/01/07 19:12:43]

Should this code be ifdef'd or should we have this "dead" code in all builds?

How is this different from line 70-75?

[servolk, 2016/01/07 21:17:36]

The difference is really minor, I think lines 70-75 were wrapped in #if to
guarantee CodecIDToAudioCodec never returns kCodecAC3/EAC3 for builds where
AC3/EAC3 are disabled, whereas this code would simply be dead, but harmless even
if somehow it got executed on the builds with AC3/EAC3 disabled. But I agree we
can wrap this into #if as well, it shouldn't make any difference.

+
   base::TimeDelta seek_preroll;
   if (codec_context->seek_preroll > 0) {
     seek_preroll = base::TimeDelta::FromMicroseconds(
@@ -352,8 +366,7 @@ bool AVCodecContextToAudioDecoderConfig(const AVCodecContext* codec_context,
                      is_encrypted,
                      seek_preroll,
                      codec_context->delay);
-
-  if (codec != kCodecOpus) {
+  if (codec != kCodecOpus && codec != kCodecAC3 && codec != kCodecEAC3) {

[ddorwin, 2016/01/07 19:12:43]

Suggest:
// Verify XXX for codecs for which FFmpeg provides a sample_fmt.

[servolk, 2016/01/07 21:17:36]

Done.

     DCHECK_EQ(av_get_bytes_per_sample(codec_context->sample_fmt) * 8,
               config->bits_per_channel());
   }