media: Opus support for WebM in Media Source

media/filters/opus_audio_decoder.cc

Index: media/filters/opus_audio_decoder.cc
diff --git a/media/filters/opus_audio_decoder.cc b/media/filters/opus_audio_decoder.cc
index 115799ab71193fe829265268c3898a5cbbc6cf95..66dc283ea1186de96bbd0d578f8cd3fda0247198 100644
--- a/media/filters/opus_audio_decoder.cc
+++ b/media/filters/opus_audio_decoder.cc
@@ -250,7 +250,6 @@ OpusAudioDecoder::OpusAudioDecoder(
       channel_layout_(CHANNEL_LAYOUT_NONE),
       samples_per_second_(0),
       last_input_timestamp_(kNoTimestamp()),
-      output_bytes_to_drop_(0),

[acolwell GONE FROM CHROMIUM, 2013/08/28 20:44:47]

nit:It looks like you didn't remove this from the .h.

[vignesh, 2013/08/29 21:37:31]

Done.

       skip_samples_(0) {
 }
 
@@ -457,10 +456,17 @@ bool OpusAudioDecoder::ConfigureDecoder() {
                   config,
                   &opus_header);
 
-  skip_samples_ = opus_header.skip_samples;
-
-  if (skip_samples_ > 0)
-    output_bytes_to_drop_ = skip_samples_ * config.bytes_per_frame();
+  if (!config.codec_delay()) {
+    skip_samples_ = opus_header.skip_samples;

[acolwell GONE FROM CHROMIUM, 2013/08/28 20:44:47]

Why do we special case this?

[vignesh, 2013/08/29 21:37:31]

In case of some old opus files, the container value might not exist. We can
still play those files by making use of the value from the codec private header.

+  } else {
+    // WebM container stores codec delay in nanoseconds. Convert it to samples.
+    skip_samples_ = config.codec_delay() * config.samples_per_second() /

[acolwell GONE FROM CHROMIUM, 2013/08/28 20:44:47]

nit: This looks like it could potentially overflow. At a minimum you should
return false if skip_samples_ becomes negative.

[vignesh, 2013/08/29 21:37:31]

Done.

+                    1000000000;
+    if (skip_samples_ != opus_header.skip_samples) {

[acolwell GONE FROM CHROMIUM, 2013/08/28 20:44:47]

Shouldn't the codec_delay() header always match the skip_samples field?

[vignesh, 2013/08/29 21:37:31]

As of now, both the fields should match. But some tool might eventually screw it
up. So I thought we could be more permissive and somehow try to play the file.
But after a second thought, seems like being strict in this case makes sense. So
i'll make the change accordingly.

+      DVLOG(1) << "Codec Delay in container does not match the value in Opus "
+               << "header. Using the value in container.";

[acolwell GONE FROM CHROMIUM, 2013/08/28 20:44:47]

nit: return false here if the expectation that these should always match.

[vignesh, 2013/08/29 21:37:31]

Done.

+    }
+  }
 
   uint8 channel_mapping[kMaxVorbisChannels];
   memcpy(&channel_mapping,
@@ -508,7 +514,7 @@ void OpusAudioDecoder::CloseDecoder() {
 void OpusAudioDecoder::ResetTimestampState() {
   output_timestamp_helper_->SetBaseTimestamp(kNoTimestamp());
   last_input_timestamp_ = kNoTimestamp();
-  output_bytes_to_drop_ = 0;
+  skip_samples_ = 0;
 }
 
 bool OpusAudioDecoder::Decode(const scoped_refptr<DecoderBuffer>& input,
@@ -539,16 +545,6 @@ bool OpusAudioDecoder::Decode(const scoped_refptr<DecoderBuffer>& input,
     output_timestamp_helper_->SetBaseTimestamp(input->timestamp());
   }
 
-  if (decoded_audio_size > 0 && output_bytes_to_drop_ > 0) {
-    int dropped_size = std::min(decoded_audio_size, output_bytes_to_drop_);
-    DCHECK_EQ(dropped_size % kBytesPerChannel, 0);
-    decoded_audio_data += dropped_size;
-    decoded_audio_size -= dropped_size;
-    output_bytes_to_drop_ -= dropped_size;
-    samples_decoded = decoded_audio_size /
-                      demuxer_stream_->audio_decoder_config().bytes_per_frame();
-  }
-
   if (decoded_audio_size > 0) {
     // Copy the audio samples into an output buffer.
     uint8* data[] = { decoded_audio_data };
@@ -560,8 +556,23 @@ bool OpusAudioDecoder::Decode(const scoped_refptr<DecoderBuffer>& input,
         output_timestamp_helper_->GetTimestamp(),
         output_timestamp_helper_->GetFrameDuration(samples_decoded));
     output_timestamp_helper_->AddFrames(samples_decoded);
+    if (skip_samples_ > 0) {
+      int dropped_size = std::min(samples_decoded, skip_samples_);
+      output_buffer->get()->TrimStart(dropped_size);
+      skip_samples_ -= dropped_size;
+      samples_decoded -= dropped_size;
+    }
+    if (input->discard_padding() > 0) {
+      int discard_padding = input->discard_padding() * samples_per_second_ /
+                            1000000000;

[acolwell GONE FROM CHROMIUM, 2013/08/28 20:44:47]

nit: This may need overflow protection as well.

[vignesh, 2013/08/29 21:37:31]

Done.

+      output_buffer->get()->TrimEnd(std::min(samples_decoded, discard_padding));
+      samples_decoded -= discard_padding;
+    }
   }
 
+  decoded_audio_size =
+      samples_decoded *
+      demuxer_stream_->audio_decoder_config().bytes_per_frame();
   // Decoding finished successfully, update statistics.
   PipelineStatistics statistics;
   statistics.audio_bytes_decoded = decoded_audio_size;