Add gapless playback support for AAC playback.

media/filters/source_buffer_stream.cc

Index: media/filters/source_buffer_stream.cc
diff --git a/media/filters/source_buffer_stream.cc b/media/filters/source_buffer_stream.cc
index 9247a25097fffe4d2442675f1cfdd43fa27cd9f6..e26b10feebaaf989841d84840a84e1125c80d02d 100644
--- a/media/filters/source_buffer_stream.cc
+++ b/media/filters/source_buffer_stream.cc
@@ -359,7 +359,7 @@ SourceBufferStream::SourceBufferStream(const AudioDecoderConfig& audio_config,
       memory_limit_(kDefaultAudioMemoryLimit),
       config_change_pending_(false),
       splice_buffers_index_(0),
-      pre_splice_complete_(false),
+      pending_buffers_complete_(false),
       splice_frames_enabled_(splice_frames_enabled) {
   DCHECK(audio_config.IsValidConfig());
   audio_configs_.push_back(audio_config);
@@ -385,7 +385,7 @@ SourceBufferStream::SourceBufferStream(const VideoDecoderConfig& video_config,
       memory_limit_(kDefaultVideoMemoryLimit),
       config_change_pending_(false),
       splice_buffers_index_(0),
-      pre_splice_complete_(false),
+      pending_buffers_complete_(false),
       splice_frames_enabled_(splice_frames_enabled) {
   DCHECK(video_config.IsValidConfig());
   video_configs_.push_back(video_config);
@@ -412,7 +412,7 @@ SourceBufferStream::SourceBufferStream(const TextTrackConfig& text_config,
       memory_limit_(kDefaultAudioMemoryLimit),
       config_change_pending_(false),
       splice_buffers_index_(0),
-      pre_splice_complete_(false),
+      pending_buffers_complete_(false),
       splice_frames_enabled_(splice_frames_enabled) {}
 
 SourceBufferStream::~SourceBufferStream() {
@@ -693,8 +693,8 @@ void SourceBufferStream::ResetSeekState() {
   config_change_pending_ = false;
   last_output_buffer_timestamp_ = kNoTimestamp();
   splice_buffers_index_ = 0;
-  splice_buffer_ = NULL;
-  pre_splice_complete_ = false;
+  pending_buffer_ = NULL;
+  pending_buffers_complete_ = false;
 }
 
 bool SourceBufferStream::ShouldSeekToStartOfBuffered(
@@ -1135,23 +1135,35 @@ void SourceBufferStream::OnSetDuration(base::TimeDelta duration) {
 
 SourceBufferStream::Status SourceBufferStream::GetNextBuffer(
     scoped_refptr<StreamParserBuffer>* out_buffer) {
-  if (!splice_buffer_) {
+  if (!pending_buffer_) {
     const SourceBufferStream::Status status = GetNextBufferInternal(out_buffer);
-
-    // Just return if GetNextBufferInternal() failed or there's no fade out
-    // preroll, there's nothing else to do.
-    if (status != SourceBufferStream::kSuccess ||
-        (*out_buffer)->get_splice_buffers().empty()) {
-      return status;
+    if (status == SourceBufferStream::kSuccess) {
+      if (!(*out_buffer)->get_splice_buffers().empty())
+        return HandleNextBufferWithSplice(out_buffer);
+      if ((*out_buffer)->GetPrerollBuffer())
+        return HandleNextBufferWithPreroll(out_buffer);
     }
+    return status;
+  }
+
+  if (!pending_buffer_->get_splice_buffers().empty())
+    return HandleNextBufferWithSplice(out_buffer);
 
-    // Fall through into splice buffer processing.
+  DCHECK(pending_buffer_->GetPrerollBuffer());
+  return HandleNextBufferWithPreroll(out_buffer);
+}
+
+SourceBufferStream::Status SourceBufferStream::HandleNextBufferWithSplice(
+    scoped_refptr<StreamParserBuffer>* out_buffer) {
+  if (!pending_buffer_) {

[acolwell GONE FROM CHROMIUM, 2014/05/16 17:01:11]

It seems weird to have this here. Why not put this code in GetNextBuffer() where
you detect a new splice buffer? That would at least make it more obvious that
pending_buffer_ gets set.

[DaleCurtis, 2014/05/16 17:48:59]

This is done for consistency with the HandleNextBufferWithPreroll() method
below.  It's done there since HandleNextBufferWithSplice() can flow into
HandleNextBufferWithPreroll() afterward.  I can extract a "set pending buffer"
method, but it might lose some of it's DCHECKs if I move it outside of each
Handle method.

[acolwell GONE FROM CHROMIUM, 2014/05/16 20:26:05]

I think the setting logic should be extracted into a helper and the helper
called at the relevant call sites instead of in this and
HandleNextBufferWithSplice(). I realize that might make the code at the bottom
of this func a little longer, but I think it will be a little clearer that
pending_buffers_ gets reset in the buffers with preroll case.

[DaleCurtis, 2014/05/22 23:58:57]

Done.

+    DCHECK(*out_buffer);
+    DCHECK(!(*out_buffer)->get_splice_buffers().empty());
     splice_buffers_index_ = 0;
-    splice_buffer_.swap(*out_buffer);
+    pending_buffer_.swap(*out_buffer);
+    pending_buffers_complete_ = false;
   }
 
-  DCHECK(splice_buffer_);
-  const BufferQueue& splice_buffers = splice_buffer_->get_splice_buffers();
+  const BufferQueue& splice_buffers = pending_buffer_->get_splice_buffers();
   const size_t last_splice_buffer_index = splice_buffers.size() - 1;
 
   // Are there any splice buffers left to hand out?  The last buffer should be
@@ -1166,17 +1178,20 @@ SourceBufferStream::Status SourceBufferStream::GetNextBuffer(
     }
 
     // Every pre splice buffer must have the same splice_timestamp().
-    DCHECK(splice_buffer_->splice_timestamp() ==
+    DCHECK(pending_buffer_->splice_timestamp() ==
            splice_buffers[splice_buffers_index_]->splice_timestamp());
 
+    // No pre splice buffers should have preroll.
+    DCHECK(!splice_buffers[splice_buffers_index_]->GetPrerollBuffer());
+
     *out_buffer = splice_buffers[splice_buffers_index_++];
     return SourceBufferStream::kSuccess;
   }
 
   // Did we hand out the last pre-splice buffer on the previous call?
-  if (!pre_splice_complete_) {
+  if (!pending_buffers_complete_) {
     DCHECK_EQ(splice_buffers_index_, last_splice_buffer_index);
-    pre_splice_complete_ = true;
+    pending_buffers_complete_ = true;
     config_change_pending_ = true;
     DVLOG(1) << "Config change (forced for fade in of splice frame).";
     return SourceBufferStream::kConfigChange;
@@ -1186,13 +1201,44 @@ SourceBufferStream::Status SourceBufferStream::GetNextBuffer(
   // so hand out the final buffer for fade in.  Because a config change is
   // always issued prior to handing out this buffer, any changes in config id
   // have been inherently handled.
-  DCHECK(pre_splice_complete_);
+  DCHECK(pending_buffers_complete_);
   DCHECK_EQ(splice_buffers_index_, splice_buffers.size() - 1);
   DCHECK(splice_buffers.back()->splice_timestamp() == kNoTimestamp());
   *out_buffer = splice_buffers.back();
-  splice_buffer_ = NULL;
+  pending_buffer_ = NULL;
   splice_buffers_index_ = 0;
-  pre_splice_complete_ = false;
+
+  // If the last splice buffer has preroll hand off to the preroll handler.
+  return (*out_buffer)->GetPrerollBuffer()
+             ? HandleNextBufferWithPreroll(out_buffer)
+             : SourceBufferStream::kSuccess;
+}
+
+SourceBufferStream::Status SourceBufferStream::HandleNextBufferWithPreroll(
+    scoped_refptr<StreamParserBuffer>* out_buffer) {
+  if (!pending_buffer_) {
+    DCHECK(*out_buffer);
+    DCHECK((*out_buffer)->GetPrerollBuffer());
+    pending_buffer_.swap(*out_buffer);

[acolwell GONE FROM CHROMIUM, 2014/05/16 17:01:11]

This seems odd as well. I think the setting operation should be split out so
that this is more explicit at the call sites.

[DaleCurtis, 2014/05/16 17:48:59]

As detailed above.

[DaleCurtis, 2014/05/22 23:58:57]

Done.

+    pending_buffers_complete_ = false;
+  }
+
+  // The preroll buffer should not be a splice buffer.
+  DCHECK(pending_buffer_->get_splice_buffers().empty());

[wolenetz, 2014/05/17 00:41:17]

nit: is it worth adding DCHECK_EQ(0, splice_buffers_index_); here too?

[DaleCurtis, 2014/05/22 23:58:57]

Moved into SetPendingBuffer().

[wolenetz, 2014/05/23 19:59:45]

The set is moved into SetPendingBuffer(), though the DCHECK_EQ I suggested would
still be good here to protect borders.

+
+  // Any config change should have already been handled.
+  DCHECK_EQ(current_config_index_, pending_buffer_->GetConfigId());
+
+  // Check if the preroll buffer has already been handed out.
+  if (!pending_buffers_complete_) {
+    pending_buffers_complete_ = true;
+    *out_buffer = pending_buffer_->GetPrerollBuffer();
+    return SourceBufferStream::kSuccess;
+  }
+
+  // Preroll complete, hand out the final buffer.
+  *out_buffer = pending_buffer_;
+  pending_buffer_ = NULL;
   return SourceBufferStream::kSuccess;
 }
 
@@ -1428,9 +1474,9 @@ bool SourceBufferStream::UpdateVideoConfig(const VideoDecoderConfig& config) {
 void SourceBufferStream::CompleteConfigChange() {
   config_change_pending_ = false;
 
-  if (splice_buffer_) {
+  if (pending_buffer_) {
     current_config_index_ =
-        GetConfigId(splice_buffer_, splice_buffers_index_);
+        GetConfigId(pending_buffer_, splice_buffers_index_);
     return;
   }
 
@@ -1637,7 +1683,8 @@ void SourceBufferStream::GenerateSpliceFrame(const BufferQueue& new_buffers) {
   if (pre_splice_buffers.front()->timestamp() >= splice_timestamp)
     return;
 
-  // If any |pre_splice_buffers| are already splices, do not generate a splice.
+  // If any |pre_splice_buffers| are already splices or preroll, do not generate
+  // a splice.
   for (size_t i = 0; i < pre_splice_buffers.size(); ++i) {
     const BufferQueue& original_splice_buffers =
         pre_splice_buffers[i]->get_splice_buffers();
@@ -1646,6 +1693,11 @@ void SourceBufferStream::GenerateSpliceFrame(const BufferQueue& new_buffers) {
                   "pre-existing splice.";
       return;
     }
+
+    if (pre_splice_buffers[i]->GetPrerollBuffer()) {
+      DVLOG(1) << "Can't generate splice: overlapped buffers contain preroll.";
+      return;
+    }
   }
 
   // Don't generate splice frames which represent less than two frames, since we