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..741d10088cbe06605bdff7d4e93056b40a33a9a8 100644
--- a/media/base/audio_splicer.cc
+++ b/media/base/audio_splicer.cc
@@ -20,14 +20,21 @@ namespace media {
 // roughly represents the duration of 2 compressed AAC or MP3 frames.
 static const int kMaxTimeDeltaInMilliseconds = 50;
 
+// 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;
+
 AudioSplicer::AudioSplicer(int samples_per_second)
     : output_timestamp_helper_(samples_per_second),
-      min_gap_size_(2),
-      received_end_of_stream_(false) {
-}
+      received_end_of_stream_(false),
+      splice_frame_splicer_(samples_per_second) {}
 
-AudioSplicer::~AudioSplicer() {
-}
+AudioSplicer::~AudioSplicer() {}
 
 void AudioSplicer::Reset() {
   output_timestamp_helper_.SetBaseTimestamp(kNoTimestamp());
@@ -35,10 +42,13 @@ void AudioSplicer::Reset() {
   received_end_of_stream_ = false;
 }
 
-bool AudioSplicer::AddInput(const scoped_refptr<AudioBuffer>& input) {
+bool AudioSplicer::AddInput(const scoped_refptr<DecoderBuffer>& origin_buffer,

[acolwell GONE FROM CHROMIUM, 2014/02/10 20:19:57]

Passing origin_buffer here doesn't feel right to me. It feels like code outside
this class should be setting "splice preroll" state on this class instead of
using origin_buffer since audio codec delay can make it difficult to determine
which output audio is actually part of the preroll. For example Vorbis have a 1
frame delay so eventhough the second DecoderBuffer may have splice_preroll()
set, the first AudioBuffer that comes out of the decoder actually might not be
part of the splice. There needs to be code that associates DecoderBuffer
timestamp with the timestamps on the AudioBuffer and makes sure the AudioBuffers
are marked appropriately.

[DaleCurtis, 2014/02/15 01:20:02]

I've reworked this so that the future AudioBufferStream will call
SetSpliceTimestamp() when decoder buffers marked as such are received. 

+                            const scoped_refptr<AudioBuffer>& input) {
   DCHECK(!received_end_of_stream_ || input->end_of_stream());
 
   if (input->end_of_stream()) {
+    DCHECK(!origin_buffer->splice_preroll());
+    DCHECK(splice_buffers_.empty());
     output_buffers_.push_back(input);
     received_end_of_stream_ = true;
     return true;
@@ -60,16 +70,36 @@ bool AudioSplicer::AddInput(const scoped_refptr<AudioBuffer>& input) {
   base::TimeDelta expected_timestamp = output_timestamp_helper_.GetTimestamp();
   base::TimeDelta delta = timestamp - expected_timestamp;
 
+  // TODO(dalecurtis): Does this need to be removed to handle overlaps?
   if (std::abs(delta.InMilliseconds()) > kMaxTimeDeltaInMilliseconds) {
     DVLOG(1) << "Timestamp delta too large: " << delta.InMicroseconds() << "us";
     return false;
   }
 
+  // It's splicers all the way down!
+  if (origin_buffer->splice_preroll()) {
+    origin_buffer->set_splice_preroll(false);  // Pass NULL to inner instead?
+    return splice_frame_splicer_.AddInput(origin_buffer, input);
+  } else if (splice_frame_splicer_->HasNextBuffer()) {
+    origin_buffer->set_splice_preroll(false);  // Pass NULL to inner instead?
+    if (!splice_frame_splicer_.AddInput(origin_buffer, input))
+      return false;
+
+    // The inner splicer has now trimmed and crossfaded buffers appropriately,
+    // so we simply need to transfer them into the outer splicer.
+    while (splice_frame_splicer_->HasNextBuffer())
+      AddOutputBuffer(splice_frame_splicer_->GetNextBuffer());
+
+    // The splice frame is complete, so can reset for the next splice frame.
+    splice_frame_splicer_.Reset();
+    return true;
+  }
+
   int frames_to_fill = 0;
   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;
   }
@@ -93,23 +123,41 @@ bool AudioSplicer::AddInput(const scoped_refptr<AudioBuffer>& input) {
   }
 
   int frames_to_skip = -frames_to_fill;
-
   DVLOG(1) << "Overlap detected @ " << expected_timestamp.InMicroseconds()
            << " us: "  << -delta.InMicroseconds() << " us";
 
-  if (input->frame_count() <= frames_to_skip) {
-    DVLOG(1) << "Dropping whole buffer";
+  // If we have no buffers with which to crossfade, drop overlapping frames.
+  if (output_buffers_.empty()) {
+    if (input->frame_count() <= frames_to_skip) {
+      DVLOG(1) << "Dropping whole buffer";
+      return true;
+    }
+
+    // Copy the trailing samples that do not overlap samples already output
+    // into a new buffer. Add this new buffer to the output queue.
+    input->TrimStart(frames_to_skip);
+    AddOutputBuffer(input);
     return true;
   }
 
-  // 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;
+  const int five_ms_of_frames = output_timestamp_helper_->GetFramesToTarget(
+      output_timestamp_helper_->GetTimestamp() +
+      base::TimeDelta::FromMilliseconds(kCrossfadeDurationInMilliseconds));
+  const int frames_to_crossfade = std::min(frames_to_skip, five_ms_of_frames);
+
+  // Discard frames not used in the crossfade.
+  frames_to_skip = frames_to_skip - frames_to_crossfade;
+  for (int i = output_buffers_.size() - 1; i >= 0 && frames_to_skip > 0; --i) {
+    const int frame_count = output_buffers_.back()->frame_count();
+    if (frame_count <= frames_to_skip)
+      output_buffers_.pop_back();
+    else
+      output_buffers_.back()->TrimEnd(frames_to_skip);
+    frames_to_skip -= frame_count;
+  }
+
+  // TODO(dalecurtis): Crossfade |frames_to_crossfade| of frames from
+  // output_buffers_.  See https://codereview.chromium.org/20876002/
 }
 
 bool AudioSplicer::HasNextBuffer() const {