Fixing AudioBuffer params and channel layout bugs

media/renderers/audio_renderer_impl.cc

Index: media/renderers/audio_renderer_impl.cc
diff --git a/media/renderers/audio_renderer_impl.cc b/media/renderers/audio_renderer_impl.cc
index 6689e36c7b4aa303f5da1651849cfe8f4cd92869..0912eeb55da3d16191936f552d35fd099b0545a3 100644
--- a/media/renderers/audio_renderer_impl.cc
+++ b/media/renderers/audio_renderer_impl.cc
@@ -535,6 +535,26 @@ void AudioRendererImpl::DecodedAudioReady(
       }
     }
   } else {
+    // TODO(chcunningham, tguilbert): Figure out if we want to support implicit

[chcunningham, 2016/04/19 00:28:56]

Implicit config changes for AAC actually work with today's src= code because
ffmpeg figures figures out the implicit AAC profile when it first opens it up.
We have src= tests for the various aac files here [0]. I'm not aware of other
containers that do implicit signalling nor am I aware of anyone asking for
additional src= support. Decode error LGTM for now :)


0:
https://code.google.com/p/chromium/codesearch#chromium/src/media/test/pipelin...

[tguilbert, 2016/04/19 18:17:57]

Discussed offline. Currently, the media used to test signaling doesn't work
properly on src= playback.

+    // config changes during src=. Doing so requires resampling each individual
+    // stream which is inefficient when there are many tags in a page.
+    //
+    // Check if the buffer we received matches the expected configuration.
+    // Note: We explicitly do not check channel layout here to avoid breaking
+    // weird behavior with multichannel wav files: http://crbug.com/600538.
+    if (!buffer->end_of_stream() &&
+        (buffer->sample_rate() != audio_parameters_.sample_rate() ||
+         buffer->channel_count() != audio_parameters_.channels())) {
+      MEDIA_LOG(ERROR, media_log_)
+          << "Unsupported midstream configuration change!"
+          << " Sample Rate: " << buffer->sample_rate() << " vs "
+          << audio_parameters_.sample_rate()
+          << ", Channels: " << buffer->channel_count() << " vs "
+          << audio_parameters_.channels();
+      HandleAbortedReadOrDecodeError(PIPELINE_ERROR_DECODE);
+      return;
+    }
+
     if (!splicer_->AddInput(buffer)) {

[chcunningham, 2016/04/19 00:28:56]

IIUC, prior to adding the decode error above, we were doing a buffer overflow in
AudioBuffer::ReadFrames. I notice that method has a few DCHECKS [0] for this
scenario... What do you guys think about making them CHECKS instead? 


0:
https://code.google.com/p/chromium/codesearch#chromium/src/media/base/audio_b...

[tguilbert, 2016/04/19 18:17:56]

SGTM, but please get a second opinion :)

       HandleAbortedReadOrDecodeError(AUDIO_RENDERER_ERROR_SPLICE_FAILED);
       return;