Re-add AC3/EAC3 audio demuxing support

media/ffmpeg/ffmpeg_common.cc

 // Copyright (c) 2012 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/ffmpeg/ffmpeg_common.h"
 
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/sha1.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 48 matching lines @@
 int64_t ConvertToTimeBase(const AVRational& time_base,
                           const base::TimeDelta& timestamp) {
   return av_rescale_q(timestamp.InMicroseconds(), kMicrosBase, time_base);
 }
 
 // Converts an FFmpeg audio codec ID into its corresponding supported codec id.
 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:
       return kCodecVorbis;
     case AV_CODEC_ID_PCM_U8:
     case AV_CODEC_ID_PCM_S16LE:
     case AV_CODEC_ID_PCM_S24LE:
     case AV_CODEC_ID_PCM_S32LE:
     case AV_CODEC_ID_PCM_F32LE:
       return kCodecPCM;
@@ skipping 215 matching lines @@
   return AV_SAMPLE_FMT_NONE;
 }
 
 bool AVCodecContextToAudioDecoderConfig(const AVCodecContext* codec_context,
                                         bool is_encrypted,
                                         AudioDecoderConfig* config) {
   DCHECK_EQ(codec_context->codec_type, AVMEDIA_TYPE_AUDIO);
 
   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);
 
   int sample_rate = codec_context->sample_rate;
   if (codec == kCodecOpus) {

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

It looks like we handle exceptions here. I suggest doing the same for your new
exception and possibly converting this to a switch statement.

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

Done.

     // |codec_context->sample_fmt| is not set by FFmpeg because Opus decoding is
     // not enabled in FFmpeg.  It doesn't matter what value is set here, so long
     // as it's valid, the true sample format is selected inside the decoder.
     sample_format = kSampleFormatF32;
 
     // Always use 48kHz for OPUS.  Technically we should match to the highest
     // supported hardware sample rate among [8, 12, 16, 24, 48] kHz, but we
     // don't know the hardware sample rate at this point and those rates are
     // rarely used for output.  See the "Input Sample Rate" section of the spec:
     // http://tools.ietf.org/html/draft-terriberry-oggopus-01#page-11
@@ skipping 22 matching lines @@
                       codec_context->extradata + codec_context->extradata_size);
   }
   config->Initialize(codec,
                      sample_format,
                      channel_layout,
                      sample_rate,
                      extra_data,
                      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());
   }
 
   return true;
 }
 
 bool AVStreamToAudioDecoderConfig(const AVStream* stream,
                                   AudioDecoderConfig* config) {
   bool is_encrypted = false;
@@ skipping 323 matching lines @@
 }
 
 int32_t HashCodecName(const char* codec_name) {
   // Use the first 32-bits from the SHA1 hash as the identifier.
   int32_t hash;
   memcpy(&hash, base::SHA1HashString(codec_name).substr(0, 4).c_str(), 4);
   return hash;
 }
 
 }  // namespace media