Enhance AudioSplicer to crossfade marked splice frames.

media/base/audio_splicer.cc

Index: media/base/audio_splicer.cc
diff --git a/media/base/audio_splicer.cc b/media/base/audio_splicer.cc
index 14b4199e0e3389d8d32478fae511333915cfcf12..0879697f07e9f47a04f0480fa0b2a5c496775a6e 100644
--- a/media/base/audio_splicer.cc
+++ b/media/base/audio_splicer.cc
@@ -4,13 +4,17 @@
 
 #include "media/base/audio_splicer.h"
 
+#include <cmath>
 #include <cstdlib>
+#include <deque>
 
 #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/vector_math.h"
 
 namespace media {
 
@@ -20,22 +24,68 @@ namespace media {
 // roughly represents the duration of 2 compressed AAC or MP3 frames.
 static const int kMaxTimeDeltaInMilliseconds = 50;
 
-AudioSplicer::AudioSplicer(int samples_per_second)
+// Minimum gap size needed before the splicer will take action to
+// fill a gap. This avoids periodically inserting and then dropping samples
+// when the buffer timestamps are slightly off because of timestamp rounding
+// in the source content. Unit is frames.
+static const int kMinGapSize = 2;
+
+// The number of milliseconds to crossfade before trimming when buffers overlap.
+static const int kCrossfadeDurationInMilliseconds = 5;
+
+class AudioStreamSanitizer {
+ public:
+  explicit AudioStreamSanitizer(int samples_per_second);
+  ~AudioStreamSanitizer();
+
+  // Resets the sanitizer state by clearing the output buffers queue,
+  // and resetting the timestamp helper.
+  void Reset();
+
+  // Adds a new buffer full of samples or end of stream buffer to the splicer.
+  // Returns true if the buffer was accepted. False is returned if an error
+  // occurred.
+  bool AddInput(const scoped_refptr<AudioBuffer>& input);
+
+  // Returns true if the sanitizer has a buffer to return.
+  bool HasNextBuffer() const;
+
+  // Removes the next buffer from the output buffer queue and returns it; should
+  // only be called if HasNextBuffer() returns true.
+  scoped_refptr<AudioBuffer> GetNextBuffer();
+
+  // Returns a reference to the next buffer in the output buffer queue; should
+  // only be called if HasNextBuffer() returns true.
+  const scoped_refptr<AudioBuffer>& PeekNextBuffer() const;
+
+  // Get the current timestamp.  This value is computed from based on the first
+  // buffer's timestamp and the number of frames that have been added so far.
+  base::TimeDelta GetTimestamp() const;
+
+  // Returns the duration of all buffers added to the output queue thus far.
+  base::TimeDelta GetDuration() const;
+
+ private:
+  void AddOutputBuffer(const scoped_refptr<AudioBuffer>& buffer);
+
+  AudioTimestampHelper output_timestamp_helper_;
+  std::deque<scoped_refptr<AudioBuffer> > output_buffers_;
+  bool received_end_of_stream_;
+};
+
+AudioStreamSanitizer::AudioStreamSanitizer(int samples_per_second)
     : output_timestamp_helper_(samples_per_second),
-      min_gap_size_(2),
-      received_end_of_stream_(false) {
-}
+      received_end_of_stream_(false) {}
 
-AudioSplicer::~AudioSplicer() {
-}
+AudioStreamSanitizer::~AudioStreamSanitizer() {}
 
-void AudioSplicer::Reset() {
+void AudioStreamSanitizer::Reset() {
   output_timestamp_helper_.SetBaseTimestamp(kNoTimestamp());
   output_buffers_.clear();
   received_end_of_stream_ = false;
 }
 
-bool AudioSplicer::AddInput(const scoped_refptr<AudioBuffer>& input) {
+bool AudioStreamSanitizer::AddInput(const scoped_refptr<AudioBuffer>& input) {
   DCHECK(!received_end_of_stream_ || input->end_of_stream());
 
   if (input->end_of_stream()) {
@@ -69,7 +119,7 @@ bool AudioSplicer::AddInput(const scoped_refptr<AudioBuffer>& input) {
   if (delta != base::TimeDelta())
     frames_to_fill = output_timestamp_helper_.GetFramesToTarget(timestamp);
 
-  if (frames_to_fill == 0 || std::abs(frames_to_fill) < min_gap_size_) {
+  if (frames_to_fill == 0 || std::abs(frames_to_fill) < kMinGapSize) {
     AddOutputBuffer(input);
     return true;
   }
@@ -92,11 +142,16 @@ bool AudioSplicer::AddInput(const scoped_refptr<AudioBuffer>& input) {
     return true;
   }
 
-  int frames_to_skip = -frames_to_fill;
-
+  // Overlapping buffers marked as splice frames are handled by AudioSplicer,
+  // but decoder and demuxer quirks may sometimes produce overlapping samples
+  // which need to be sanitized.
+  //
+  // A crossfade can't be done here because only the current buffer is available
+  // at this point, not previous buffers.
   DVLOG(1) << "Overlap detected @ " << expected_timestamp.InMicroseconds()
-           << " us: "  << -delta.InMicroseconds() << " us";
+           << " us: " << -delta.InMicroseconds() << " us";
 
+  int frames_to_skip = -frames_to_fill;
   if (input->frame_count() <= frames_to_skip) {
     DVLOG(1) << "Dropping whole buffer";
     return true;
@@ -104,27 +159,241 @@ bool AudioSplicer::AddInput(const scoped_refptr<AudioBuffer>& input) {
 
   // Copy the trailing samples that do not overlap samples already output
   // into a new buffer. Add this new buffer to the output queue.
-  //
-  // TODO(acolwell): Implement a cross-fade here so the transition is less
-  // jarring.
   input->TrimStart(frames_to_skip);
   AddOutputBuffer(input);
   return true;
 }
 
-bool AudioSplicer::HasNextBuffer() const {
+bool AudioStreamSanitizer::HasNextBuffer() const {
   return !output_buffers_.empty();
 }
 
-scoped_refptr<AudioBuffer> AudioSplicer::GetNextBuffer() {
+scoped_refptr<AudioBuffer> AudioStreamSanitizer::GetNextBuffer() {
   scoped_refptr<AudioBuffer> ret = output_buffers_.front();
   output_buffers_.pop_front();
   return ret;
 }
 
-void AudioSplicer::AddOutputBuffer(const scoped_refptr<AudioBuffer>& buffer) {
+const scoped_refptr<AudioBuffer>& AudioStreamSanitizer::PeekNextBuffer() const {
+  return output_buffers_.front();
+}
+
+void AudioStreamSanitizer::AddOutputBuffer(
+    const scoped_refptr<AudioBuffer>& buffer) {
   output_timestamp_helper_.AddFrames(buffer->frame_count());
   output_buffers_.push_back(buffer);
 }
 
+base::TimeDelta AudioStreamSanitizer::GetTimestamp() const {
+  return output_timestamp_helper_.GetTimestamp();
+}
+
+base::TimeDelta AudioStreamSanitizer::GetDuration() const {
+  DCHECK(output_timestamp_helper_.base_timestamp() != kNoTimestamp());
+  return output_timestamp_helper_.GetTimestamp() -
+         output_timestamp_helper_.base_timestamp();
+}
+
+AudioSplicer::AudioSplicer(int samples_per_second)
+    : samples_per_second_(samples_per_second),
+      max_crossfade_duration_(
+          base::TimeDelta::FromMilliseconds(kCrossfadeDurationInMilliseconds)),
+      splice_timestamp_(kNoTimestamp()),
+      output_sanitizer_(new AudioStreamSanitizer(samples_per_second)),
+      pre_splice_sanitizer_(new AudioStreamSanitizer(samples_per_second)),
+      post_splice_sanitizer_(new AudioStreamSanitizer(samples_per_second)) {}
+
+AudioSplicer::~AudioSplicer() {}
+
+void AudioSplicer::Reset() {
+  output_sanitizer_->Reset();
+  pre_splice_sanitizer_->Reset();
+  post_splice_sanitizer_->Reset();
+  splice_timestamp_ = kNoTimestamp();
+}
+
+bool AudioSplicer::AddInput(const scoped_refptr<AudioBuffer>& input) {
+  // If we're not processing a splice, add the input to the output queue.
+  if (splice_timestamp_ == kNoTimestamp())
+    return output_sanitizer_->AddInput(input);
+
+  // If we're still receiving buffers before the splice point figure out which
+  // sanitizer (if any) to put them in.
+  if (!post_splice_sanitizer_->HasNextBuffer()) {
+    DCHECK(!input->end_of_stream());
+
+    // If the provided buffer is entirely before the splice point it can also be
+    // added to the output queue.
+    if (input->timestamp() + input->duration() < splice_timestamp_)
+      return output_sanitizer_->AddInput(input);
+
+    // If we're processing a splice and the input buffer does not overlap any of
+    // the existing buffers, append it to the splice queue for processing.
+    if (!pre_splice_sanitizer_->HasNextBuffer() ||
+        input->timestamp() >= pre_splice_sanitizer_->GetTimestamp()) {
+      return pre_splice_sanitizer_->AddInput(input);
+    }
+
+    // We've received the first overlapping buffer.
+  }
+
+  // At this point we have all the fade out preroll buffers from the decoder.
+  // We now need to wait until we have enough data to perform the crossfade (or
+  // we receive an end of stream).
+  if (!post_splice_sanitizer_->AddInput(input))
+    return false;
+
+  if (!input->end_of_stream() &&
+      post_splice_sanitizer_->GetDuration() < max_crossfade_duration_) {
+    return true;
+  }
+
+  // Determine crossfade duration based on available frames in each splicer and
+  // capping to the maximum crossfade duration.
+  const base::TimeDelta crossfade_duration = std::min(
+      max_crossfade_duration_,
+      std::min(pre_splice_sanitizer_->GetTimestamp() - splice_timestamp_,
+               post_splice_sanitizer_->GetDuration()));
+  const int frames_to_crossfade =
+      CalculateFrameCountFromDuration(crossfade_duration);

[acolwell GONE FROM CHROMIUM, 2014/02/24 21:46:11]

I think you use GetFramesToTarget() on the output splicer's timestamp helper
just to make sure we don't accidentally introduce error because the
CalculateFrameCountFromDuration() computation always rounds up.

[DaleCurtis, 2014/02/26 02:38:45]

Done.

+
+  const int channel_count =
+      pre_splice_sanitizer_->PeekNextBuffer()->channel_count();
+  DCHECK_EQ(channel_count,
+            post_splice_sanitizer_->PeekNextBuffer()->channel_count());
+
+  // Transfer out preroll buffers involved in the splice, drop those not.  Since
+  // we don't want to care what format the AudioBuffers are in, we need to use
+  // an intermediary AudioBus to convert the data to float.
+  scoped_ptr<AudioBus> pre_splice_bus =
+      AudioBus::Create(channel_count, frames_to_crossfade);
+  ExtractCrossfadeFromPreSplice(pre_splice_bus.get());

[acolwell GONE FROM CHROMIUM, 2014/02/24 21:46:11]

nit: Any reason to not have this method just return the scoped_ptr<AudioBus>? 
scoped_ptr<AudioBus> pre_splice_bus =
ExtractCrossfadeFromPreSplice(frames_to_crossfade);

[DaleCurtis, 2014/02/26 02:38:45]

Done.

+  DCHECK(!pre_splice_sanitizer_->HasNextBuffer());
+
+  // Allocate output buffer for crossfade.
+  scoped_refptr<AudioBuffer> crossfade_buffer = AudioBuffer::CreateBuffer(
+      kSampleFormatPlanarF32, channel_count, frames_to_crossfade);
+  crossfade_buffer->set_timestamp(splice_timestamp_);
+  crossfade_buffer->set_duration(crossfade_duration);
+
+  // AudioBuffer::ReadFrames() only allows output into an AudioBus, so wrap
+  // our AudioBuffer in one so we can avoid extra data copies.
+  scoped_ptr<AudioBus> crossfade_bus_wrapper =
+      AudioBus::CreateWrapper(crossfade_buffer->channel_count());

[acolwell GONE FROM CHROMIUM, 2014/02/24 21:46:11]

nit: Lines 282-287 look like they could be moved into a
AudioBus::CreateWrapper(const scoped_refptr<AudioBuffer>&) method. WDYT?

[DaleCurtis, 2014/02/24 23:04:09]

It's only used here, so I don't think it warrants adding to AudioBus, we expose
the SetChannelData() code for one offs like this.

+  crossfade_bus_wrapper->set_frames(frames_to_crossfade);
+  for (int ch = 0; ch < crossfade_buffer->channel_count(); ++ch) {
+    crossfade_bus_wrapper->SetChannelData(
+        ch, reinterpret_cast<float*>(crossfade_buffer->channel_data()[ch]));
+  }
+
+  // Insert the crossfade buffer into the output queue now so post splice
+  // buffers can be added in processing order.  We will still modify the buffer
+  // during the crossfade step.
+  CHECK(output_sanitizer_->AddInput(crossfade_buffer));

[acolwell GONE FROM CHROMIUM, 2014/02/24 21:46:11]

nit: This seems weird to add it to the output before we have actually rendered
the crossfade. Please move this to line 303?

[DaleCurtis, 2014/02/24 23:04:09]

Can't since then the buffers added during ExtractCrossfadeFromPostSplice will be
out of order.

+
+  ExtractCrossfadeFromPostSplice(crossfade_bus_wrapper.get());
+  DCHECK(!post_splice_sanitizer_->HasNextBuffer());
+
+  // Crossfade the audio into |crossfade_buffer|.
+  for (int ch = 0; ch < crossfade_bus_wrapper->channels(); ++ch) {
+    vector_math::Crossfade(pre_splice_bus->channel(ch),
+                           frames_to_crossfade,
+                           crossfade_bus_wrapper->channel(ch));
+  }
+
+  // Clear the splice timestamp and sanitizers so new splices can be accepted.
+  splice_timestamp_ = kNoTimestamp();
+  pre_splice_sanitizer_->Reset();
+  post_splice_sanitizer_->Reset();
+  return true;
+}
+
+bool AudioSplicer::HasNextBuffer() const {
+  return output_sanitizer_->HasNextBuffer();
+}
+
+scoped_refptr<AudioBuffer> AudioSplicer::GetNextBuffer() {
+  return output_sanitizer_->GetNextBuffer();
+}
+
+void AudioSplicer::SetSpliceTimestamp(base::TimeDelta splice_timestamp) {
+  DCHECK(splice_timestamp != kNoTimestamp());
+  if (splice_timestamp_ == splice_timestamp)
+    return;
+
+  // TODO(dalecurtis): We may need the concept of a future_splice_timestamp_ to
+  // handle cases where another splice comes in before we've received 5ms of
+  // data from the last one.  Leave this as a CHECK for now to figure out if
+  // this case is possible.
+  CHECK(splice_timestamp_ == kNoTimestamp());
+  splice_timestamp_ = splice_timestamp;
+}
+
+void AudioSplicer::ExtractCrossfadeFromPreSplice(AudioBus* output_bus) {
+  int frames_read = 0;
+  while (pre_splice_sanitizer_->HasNextBuffer() &&
+         frames_read < output_bus->frames()) {
+    scoped_refptr<AudioBuffer> preroll = pre_splice_sanitizer_->GetNextBuffer();
+
+    int read_offset = 0;
+    if (splice_timestamp_ > preroll->timestamp()) {
+      // This should only happen if the splice point is within the preroll

[acolwell GONE FROM CHROMIUM, 2014/02/24 21:46:11]

nit: s/preroll/|preroll|/

[DaleCurtis, 2014/02/26 02:38:45]

Removed.

+      // buffer somewhere.  Early code should have put it in |sanitizer_|

[acolwell GONE FROM CHROMIUM, 2014/02/24 21:46:11]

nit: s/sanitizer_/output_sanitizer_/?

[DaleCurtis, 2014/02/26 02:38:45]

Removed.

+      // otherwise.
+      DCHECK(preroll->timestamp() + preroll->duration() >= splice_timestamp_);

[acolwell GONE FROM CHROMIUM, 2014/02/24 21:46:11]

I wonder if we should create an end_timestamp() to avoid (preroll->timestamp() +
preroll->duration()) in a bunch of places?

[DaleCurtis, 2014/02/24 23:04:09]

Only two occurrences in this code.  Seems like something for a cross cutting CL
later if there are instances elsewhere.

+      read_offset =
+          ceil(preroll->frame_count() *

[acolwell GONE FROM CHROMIUM, 2014/02/24 21:46:11]

I think we should use GetFramesToTarget() on output_sanitizer_
AudioTimestampHelper to determine how many frames we need. Using a computation
like this can lead to errors that the output sanitizer would end up having to
fix.

In general, if you are having to do frame <-> timestamp conversions an
AudioTimestampHelper should be involved to avoid accidentally introducing
rounding errors. That is why I created it. :)

[DaleCurtis, 2014/02/26 02:38:45]

Done.

+               (splice_timestamp_ - preroll->timestamp()).InSecondsF() /
+               preroll->duration().InSecondsF());
+    }
+
+    const int frames_to_read = std::min(preroll->frame_count() - read_offset,
+                                        output_bus->frames() - frames_read);
+    preroll->ReadFrames(frames_to_read, read_offset, frames_read, output_bus);
+    frames_read += frames_to_read;
+
+    // If only part of the buffer was consumed, trim it appropriately and stick
+    // it into the output queue.
+    if (read_offset) {
+      preroll->TrimEnd(preroll->frame_count() - read_offset);
+      CHECK(output_sanitizer_->AddInput(preroll));
+    }
+  }
+
+  // All necessary buffers have been processed, it's safe to reset.
+  DCHECK_EQ(output_bus->frames(), frames_read);
+  pre_splice_sanitizer_->Reset();
+}
+
+void AudioSplicer::ExtractCrossfadeFromPostSplice(AudioBus* output_bus) {
+  int frames_read = 0;
+  while (post_splice_sanitizer_->HasNextBuffer() &&
+         frames_read < output_bus->frames()) {
+    scoped_refptr<AudioBuffer> postroll =
+        post_splice_sanitizer_->GetNextBuffer();
+    const int frames_to_read =
+        std::min(postroll->frame_count(), output_bus->frames() - frames_read);
+    postroll->ReadFrames(frames_to_read, 0, frames_read, output_bus);
+    frames_read += frames_to_read;
+
+    // If only part of the buffer was consumed, trim it appropriately and stick
+    // it into the output queue.
+    if (frames_to_read < postroll->frame_count()) {
+      postroll->TrimStart(frames_to_read);
+      CHECK(output_sanitizer_->AddInput(postroll));
+    }
+  }
+
+  DCHECK_EQ(output_bus->frames(), frames_read);
+
+  // Transfer all remaining buffers out.
+  while (post_splice_sanitizer_->HasNextBuffer())
+    CHECK(output_sanitizer_->AddInput(post_splice_sanitizer_->GetNextBuffer()));
+}
+
+int AudioSplicer::CalculateFrameCountFromDuration(base::TimeDelta duration) {
+  return ceil(samples_per_second_ * duration.InMicroseconds() /
+              base::Time::kMicrosecondsPerSecond);
+}
+
 }  // namespace media