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 88c29b19e2275f1a5cbf6eec14b0a4ae41d69ed8..d36d7f63dafb7046667c0abcdaa379b76c59e2e7 100644
--- a/media/ffmpeg/ffmpeg_common.cc
+++ b/media/ffmpeg/ffmpeg_common.cc
@@ -66,6 +66,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:
@@ -301,8 +307,12 @@ bool AVCodecContextToAudioDecoderConfig(const AVCodecContext* codec_context,
 
   AudioCodec codec = CodecIDToAudioCodec(codec_context->codec_id);
 
-  SampleFormat sample_format = AVSampleFormatToSampleFormat(
-      codec_context->sample_fmt, codec_context->codec_id);
+  // For AC3/EAC3, FFmpeg does not fill |sample_fmt|.

[ddorwin, 2016/01/04 23:22:17]

Is this a bug that should be fixed upstream or because the decoder is not
enabled (see 322)?

[servolk, 2016/01/07 02:30:03]

That's because we don't enable decoder, to avoid any potential licensing
issues/fee. We enable only demuxing and pass demuxed data to underlying platform
and eventually to HDMI sink, the actual decoding/playback happens there.
Adjusted the comment to explain that.

+  SampleFormat sample_format =
+      (codec == kCodecAC3 || codec == kCodecEAC3)
+          ? kSampleFormatS16

[ddorwin, 2016/01/04 23:22:17]

How do we know S16 is always correct?

[servolk, 2016/01/07 02:30:03]

The spec for AC3/EAC3 audio is ETSI TS 102 366:
http://www.etsi.org/deliver/etsi_ts/102300_102399/102366/01.03.01_60/ts_10236...
Sections F.3.1 and F.5.1 there indicate that SampleSize should be 16.

+          : AVSampleFormatToSampleFormat(codec_context->sample_fmt,
+                                         codec_context->codec_id);
 
   ChannelLayout channel_layout = ChannelLayoutToChromeChannelLayout(
       codec_context->channel_layout, codec_context->channels);
@@ -351,8 +361,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/04 23:22:17]

I assume this is for the same reasons. Perhaps reference this in a comment.

A switch statement might be better too.

[servolk, 2016/01/07 02:30:04]

I think a switch would be awkward here, it would have to look like:
switch(codec) {
case kOpus:
case kCodecAC3:
case kCodecEAC3:
break;

default:
DCHECK...
}
}

Much more verbose than this if.

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