Add support for midstream audio configuration changes.

media/base/audio_buffer_converter.cc

Index: media/base/audio_buffer_converter.cc
diff --git a/media/base/audio_buffer_converter.cc b/media/base/audio_buffer_converter.cc
new file mode 100644
index 0000000000000000000000000000000000000000..c5e4fb5c4e7d69ea0c8689875e5f764fd0c450a0
--- /dev/null
+++ b/media/base/audio_buffer_converter.cc
@@ -0,0 +1,228 @@
+// Copyright 2014 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/base/audio_buffer_converter.h"
+
+#include <cstdlib>
+#include <list>
+
+#include "base/logging.h"
+#include "media/base/audio_buffer.h"
+#include "media/base/audio_bus.h"
+#include "media/base/audio_decoder_config.h"
+#include "media/base/audio_timestamp_helper.h"
+#include "media/base/buffers.h"
+#include "media/base/sinc_resampler.h"
+#include "media/base/vector_math.h"
+
+namespace media {
+
+AudioBufferConverter::AudioBufferConverter(const AudioParameters& output_params)
+    : output_params_(output_params),
+      offset_into_queue_(0),
+      input_frames_(0),
+      timestamp_helper_(output_params_.sample_rate()),
+      is_flushing_(false) {
+  ResetConverter(output_params_);
+}
+
+AudioBufferConverter::~AudioBufferConverter() {}
+
+void AudioBufferConverter::AddInput(
+    const scoped_refptr<AudioBuffer>& buffer) {
+
+  // On EOS flush any remaining buffered data.
+  if (buffer->end_of_stream()) {
+    Flush();
+    queued_outputs_.push_back(buffer);
+    return;
+  }
+
+  // We'll need a new |audio_converter_| if there was a config change.
+  AudioParameters buffer_params = AudioBufferToAudioParameters(buffer);

[DaleCurtis, 2014/03/20 01:36:26]

I'm loathe to create a new AudioParameters object for every AddInput(). I'd
change IsConfigChange() to work on buffer and drop AudioBufferToAudioParamaters
method since it's only done here.

[rileya (GONE FROM CHROMIUM), 2014/03/21 20:58:49]

Good point, I removed this and instead check directly against the buffer for
config changes.

+  if (IsConfigChange(buffer_params))
+    ResetConverter(buffer_params);
+
+  // Pass straight through if there's no work to be done.
+  if (!audio_converter_) {
+    queued_outputs_.push_back(buffer);
+    return;
+  }
+
+  if (timestamp_helper_.base_timestamp() == kNoTimestamp()) {

[rileya (GONE FROM CHROMIUM), 2014/03/20 00:55:11]

I'm not very familiar with how timestamps are handled, is this an acceptable way
of setting the base timestamp?

[DaleCurtis, 2014/03/20 01:36:26]

Yeah, this is fine, this is what we do in the AudioSplicer.

+    timestamp_helper_.SetBaseTimestamp(buffer->timestamp());
+  }
+
+  queued_inputs_.push_back(buffer);
+  input_frames_ += buffer->frame_count();
+
+  scoped_refptr<AudioBuffer> output_buffer = Convert();

[DaleCurtis, 2014/03/20 01:36:26]

Every Convert() call seems to do the same thing, so perhaps move the append of
output_buffers into Convert() ? Rename it ConvertIfPossible() or something like
that?

[rileya (GONE FROM CHROMIUM), 2014/03/21 20:58:49]

Sounds good. Done.

+  if (output_buffer)
+    queued_outputs_.push_back(output_buffer);
+}
+
+bool AudioBufferConverter::HasNextBuffer() {
+  return !queued_outputs_.empty();
+}
+
+scoped_refptr<AudioBuffer> AudioBufferConverter::GetNextBuffer() {
+  DCHECK(!queued_outputs_.empty());
+  scoped_refptr<AudioBuffer> out = queued_outputs_.front();
+  queued_outputs_.pop_front();
+  return out;
+}
+
+double AudioBufferConverter::ProvideInput(AudioBus* audio_bus,
+                                          base::TimeDelta buffer_delay) {
+  DCHECK(is_flushing_ || input_frames_ >= audio_bus->frames());
+
+  int requested_frames_left = audio_bus->frames();
+  int dest_index = 0;
+
+  while (requested_frames_left > 0 && !queued_inputs_.empty()) {
+    scoped_refptr<AudioBuffer> input_buffer = queued_inputs_.front();
+    int frames_to_read = requested_frames_left;

[DaleCurtis, 2014/03/20 01:36:26]

I think the logic below can be simplified a bit if you switch to:

int frames_to_read = std::min(input_buffer->frames() - offset_into_queue_,
requested_frames_left);

Then you should only need one ReadFrames() call.

(I haven't thought it all the way through though)

[rileya (GONE FROM CHROMIUM), 2014/03/21 20:58:49]

Sounds good. A conditional is still necessary for popping off a buffer when it's
exhausted, but I simplified the rest a fair bit.

+
+    if (input_buffer->frame_count() - offset_into_queue_ <=
+        requested_frames_left) {
+      // Consume all remaining frames in the buffer at the front of
+      // |queued_inputs_| and pop it.
+      frames_to_read = input_buffer->frame_count() - offset_into_queue_;
+      queued_inputs_.pop_front();
+      input_buffer->ReadFrames(
+          frames_to_read, offset_into_queue_, dest_index, audio_bus);
+      offset_into_queue_ = 0;
+    } else {
+      // Consume part of the front buffer in |queued_inputs_| and store our
+      // progress into that buffer in |offset_into_queue_|.
+      input_buffer->ReadFrames(
+          frames_to_read, offset_into_queue_, dest_index, audio_bus);
+      offset_into_queue_ += frames_to_read;
+    }
+
+    requested_frames_left -= frames_to_read;
+    dest_index += frames_to_read;
+  }
+
+  // Unless we're flushing we should always have enough data to satsify the
+  // request.
+  if (!is_flushing_)
+    DCHECK_EQ(requested_frames_left, 0);
+
+  input_frames_ -= (audio_bus->frames() - requested_frames_left);

[DaleCurtis, 2014/03/20 01:36:26]

You'll want to Zero out any unfilled frames. See ZeroFramesPartial().

[rileya (GONE FROM CHROMIUM), 2014/03/21 20:58:49]

Done.

+  DCHECK_GE(input_frames_, 0);
+
+  // Assume full volume (is this correct?)

[DaleCurtis, 2014/03/20 19:29:01]

Yes.  Volume is applied at the output device.  If we applied it here then it
gets stored for rendering at that volume, even though the volume might change
later.

+  return 1.0;
+}
+
+void AudioBufferConverter::ResetConverter(const AudioParameters& input_params) {
+  Flush();
+  DCHECK_EQ(input_frames_, 0);
+  DCHECK(queued_inputs_.empty());
+
+  offset_into_queue_ = 0;
+  audio_converter_.reset(NULL);
+  input_params_ = input_params;
+
+  if (!IsConfigChange(output_params_))

[DaleCurtis, 2014/03/20 01:36:26]

Hmm, maybe make this a bool that's passed in?

+    return;
+
+  sample_rate_ratio_ = static_cast<double>(output_params_.sample_rate()) /

[rileya (GONE FROM CHROMIUM), 2014/03/20 00:55:11]

I just realized this is only used in one place and we still have all the info
necessary to compute it there -- I'll remove this.

+                       input_params_.sample_rate();
+
+  audio_converter_.reset(
+      new AudioConverter(input_params_, output_params_, true));
+  audio_converter_->AddInput(this);
+}
+
+AudioParameters AudioBufferConverter::AudioBufferToAudioParameters(

[DaleCurtis, 2014/03/20 19:29:01]

This can be a static .cc only method. Prefer that when possible.

[rileya (GONE FROM CHROMIUM), 2014/03/21 20:58:49]

Removed entirely. (IsConfigChange is now static though, and compares an
AudioParameters to an AudioBuffer).

+    const scoped_refptr<AudioBuffer>& buffer) {
+  return AudioParameters(
+      AudioParameters::AUDIO_PCM_LOW_LATENCY,
+      buffer->channel_layout(),
+      buffer->sample_rate(),
+      32,  // This is a dummy value.

[rileya (GONE FROM CHROMIUM), 2014/03/20 00:55:11]

bits_per_channel is unused by the AudioConverter. The alternative here is adding
a sample_format() getter to AudioBuffer.

+      buffer->frame_count());
+}
+
+scoped_refptr<AudioBuffer> AudioBufferConverter::Convert() {
+  DCHECK(audio_converter_);
+
+  // How many calls to ProvideInput() we can satisfy completely.
+  int chunks = input_frames_ / audio_converter_->RequestSize();

[DaleCurtis, 2014/03/20 19:29:01]

ChunkSize?

[rileya (GONE FROM CHROMIUM), 2014/03/21 20:58:49]

ChunkSize is in output frames, this is to determine how many ProvideInput()
calls worth of input frames we have. I did kill RequestSize though, so this can
just be input_params_.frames_per_buffer() now.

+
+  // If we're flushing we want to convert *everything* even if this means we'll
+  // have to pad some silence in ProvideInput(). Otherwise we want a multiple of
+  // |audio_converter_|'s ChunkSize().
+  int request_frames =
+      is_flushing_ ? floor(sample_rate_ratio_ * input_frames_)
+                   : chunks * audio_converter_->ChunkSize();
+
+  if (!request_frames)

[rileya (GONE FROM CHROMIUM), 2014/03/20 00:55:11]

General style question: is "== 0" preferable in cases like this?

+    return NULL;
+
+  scoped_refptr<AudioBuffer> output_buffer =
+      AudioBuffer::CreateBuffer(kSampleFormatPlanarF32,
+                                output_params_.channel_layout(),
+                                output_params_.sample_rate(),
+                                request_frames);
+
+  int frames_remaining = request_frames;
+
+  // The AudioConverter wants requests of a fixed size, so we'll slide an
+  // AudioBus of that size across the |output_buffer|.

[rileya (GONE FROM CHROMIUM), 2014/03/20 00:55:11]

Since AudioConverter now supports arbitrarily sized requests, we could just make
one request, but we'd run into issues with its temp buffers being reallocated a
lot to match our request sizes. Unless calling Convert() multiple times is a
significant performance concern (I don't think it is), I think this works fine.

+  while (frames_remaining != 0) {
+

[DaleCurtis, 2014/03/20 19:29:01]

No extra line.

[rileya (GONE FROM CHROMIUM), 2014/03/21 20:58:49]

Fixed.

+    int frames_this_iteration = output_params_.frames_per_buffer();

[DaleCurtis, 2014/03/20 19:29:01]

use std::min() to simplify this + conditional?

+    if (frames_remaining < frames_this_iteration)
+      frames_this_iteration = frames_remaining;
+
+    int offset_into_buffer = output_buffer->frame_count() - frames_remaining;
+
+    // Wrap the portion of the AudioBuffer in an AudioBus so the AudioConverter
+    // can fill it.
+    scoped_ptr<AudioBus> output_bus =

[DaleCurtis, 2014/03/20 19:29:01]

Create this outside of the while() loop and just set the channel pointers/frame
count inside.

[rileya (GONE FROM CHROMIUM), 2014/03/21 20:58:49]

Done.

+        AudioBus::CreateWrapper(output_buffer->channel_count());
+    output_bus->set_frames(frames_this_iteration);
+    for (int ch = 0; ch < output_buffer->channel_count(); ++ch) {
+      output_bus->SetChannelData(
+          ch,
+          reinterpret_cast<float*>(output_buffer->channel_data()[ch]) +
+              offset_into_buffer);
+    }
+
+    // Do the actual conversion.
+    audio_converter_->Convert(output_bus.get());
+    frames_remaining -= frames_this_iteration;
+  }
+

[DaleCurtis, 2014/03/20 19:29:01]

You need to now trim off silence from the output using AudioBuffer::TrimEnd(). 
See the AudioSplicer's AccurateTrimEnd() for reference too.

[rileya (GONE FROM CHROMIUM), 2014/03/21 20:58:49]

Is this actually necessary here? The |output_buffer| should be sized to exactly
as many non-silence frames as we're getting out of the converter (give or take
one due to rounding when we flush), so I don't think there will be any silence
to trim?

[DaleCurtis, 2014/03/21 23:18:56]

Hmm, yeah you're right, this should be correct.

+  // Compute the timestamp.
+  output_buffer->set_timestamp(timestamp_helper_.GetTimestamp());
+  output_buffer->set_duration(
+      timestamp_helper_.GetFrameDuration(request_frames));
+  timestamp_helper_.AddFrames(request_frames);
+
+  return output_buffer;
+}
+
+void AudioBufferConverter::Flush() {
+  if (!audio_converter_)
+    return;
+  is_flushing_ = true;
+  scoped_refptr<AudioBuffer> output_buffer = Convert();
+  if (output_buffer)
+    queued_outputs_.push_back(output_buffer);
+  is_flushing_ = false;
+}
+
+bool AudioBufferConverter::IsConfigChange(
+    const AudioParameters& new_params) {
+  return new_params.format() != input_params_.format() ||
+         new_params.sample_rate() != input_params_.sample_rate() ||
+         new_params.channels() != input_params_.channels() ||
+         new_params.channel_layout() != input_params_.channel_layout();
+}
+
+} // namespace media