Cleanup OPUS decoder. Remove extraneous transforms and copies.

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/basictypes.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 268 matching lines @@
   AudioCodec codec = CodecIDToAudioCodec(codec_context->codec_id);
 
   SampleFormat sample_format =
       AVSampleFormatToSampleFormat(codec_context->sample_fmt);
 
   ChannelLayout channel_layout = ChannelLayoutToChromeChannelLayout(
       codec_context->channel_layout, codec_context->channels);
 
   if (codec == kCodecOpus) {
     // |codec_context->sample_fmt| is not set by FFmpeg because Opus decoding is
-    // not enabled in FFmpeg, so we need to manually set the sample format.
+    // 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 = kSampleFormatS16;

[acolwell GONE FROM CHROMIUM, 2013/12/11 23:30:02]

This is probably a separate CL, but... It seems like sample format only matters
for PCM "codecs". Other ones appear to be dependent on the actual decoder. I
wonder if we should just remove this code all together and modify
AudioDecoderConfig to specify/allow kUnknownSampleFormat for non-PCM codecs.

[DaleCurtis, 2013/12/12 00:03:24]

Sounds reasonable, but for another CL.

   }
 
   base::TimeDelta seek_preroll;
   if (codec_context->seek_preroll > 0) {
     seek_preroll = base::TimeDelta::FromMicroseconds(
         codec_context->seek_preroll * 1000000.0 / codec_context->sample_rate);
   }
 
   base::TimeDelta codec_delay;
   if (codec_context->delay > 0) {
     codec_delay = base::TimeDelta::FromMicroseconds(
         codec_context->delay * 1000000.0 / codec_context->sample_rate);
   }
 
   config->Initialize(codec,
                      sample_format,
                      channel_layout,
                      codec_context->sample_rate,
                      codec_context->extradata,
                      codec_context->extradata_size,
                      is_encrypted,
                      record_stats,
                      seek_preroll,
                      codec_delay);
   if (codec != kCodecOpus) {

[acolwell GONE FROM CHROMIUM, 2013/12/11 23:30:02]

nit: Shouldn't this be checking for sample_format != kUnknownSampleFormat
instead?

[DaleCurtis, 2013/12/12 00:03:24]

Not unless we make the change you talk about above. No config should have
kUnknownSampleFormat by this point.  Did you mean we should DCHECK() that as
well?

[acolwell GONE FROM CHROMIUM, 2013/12/12 00:53:14]

ok. This was meant to go with the comment above.

     DCHECK_EQ(av_get_bytes_per_sample(codec_context->sample_fmt) * 8,
               config->bits_per_channel());
   }
 }
 
 void AVStreamToAudioDecoderConfig(
     const AVStream* stream,
     AudioDecoderConfig* config,
     bool record_stats) {
   bool is_encrypted = false;
@@ skipping 209 matching lines @@
       return PIX_FMT_YUVJ420P;
     case VideoFrame::YV12A:
       return PIX_FMT_YUVA420P;
     default:
       DVLOG(1) << "Unsupported VideoFrame::Format: " << video_format;
   }
   return PIX_FMT_NONE;
 }
 
 }  // namespace media