Add gapless playback support for AAC playback.

media/base/stream_parser_buffer.cc

Index: media/base/stream_parser_buffer.cc
diff --git a/media/base/stream_parser_buffer.cc b/media/base/stream_parser_buffer.cc
index e2c25ae6fc8b39b39014c225d6498d895bf62ea7..902c5550f7eb03a001f137be6507c14bdee5baf1 100644
--- a/media/base/stream_parser_buffer.cc
+++ b/media/base/stream_parser_buffer.cc
@@ -118,6 +118,12 @@ void StreamParserBuffer::ConvertToSpliceBuffer(
   // that implies EOS care must be taken to ensure there are no clients relying
   // on that behavior.
 
+  // Move over any preroll from this buffer.
+  if (preroll_buffer_) {
+    DCHECK(!overlapping_buffer->preroll_buffer_);
+    overlapping_buffer->preroll_buffer_.swap(preroll_buffer_);
+  }
+
   // Rewrite |this| buffer as a splice buffer.
   SetDecodeTimestamp(first_splice_buffer->GetDecodeTimestamp());
   SetConfigId(first_splice_buffer->GetConfigId());
@@ -141,6 +147,7 @@ void StreamParserBuffer::ConvertToSpliceBuffer(
        ++it) {
     const scoped_refptr<StreamParserBuffer>& buffer = *it;
     DCHECK(!buffer->end_of_stream());
+    DCHECK(!buffer->GetPrerollBuffer());
     DCHECK(buffer->get_splice_buffers().empty());
     splice_buffers_.push_back(CopyBuffer(*buffer));
     splice_buffers_.back()->set_splice_timestamp(splice_timestamp());
@@ -149,4 +156,38 @@ void StreamParserBuffer::ConvertToSpliceBuffer(
   splice_buffers_.push_back(overlapping_buffer);
 }
 
+void StreamParserBuffer::SetPrerollBuffer(
+    const scoped_refptr<StreamParserBuffer>& preroll_buffer) {
+  DCHECK(!preroll_buffer_);
+  DCHECK(!end_of_stream());
+  DCHECK(!preroll_buffer->end_of_stream());
+  DCHECK(!preroll_buffer->preroll_buffer_);
+  DCHECK(preroll_buffer->splice_timestamp() == kNoTimestamp());
+  DCHECK(preroll_buffer->get_splice_buffers().empty());
+  DCHECK(preroll_buffer->timestamp() < timestamp());

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

nit: Is this strictly always true, even for out-of-order decode stream types
that might use preroll, or should we s/</<=/ ?

[DaleCurtis, 2014/05/17 00:55:41]

Hmm, it might need to be <= since that's what the IsRangeListSorted() method
uses.  I didn't think it's possible to have buffers with the same timestamp
within a single append.

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

Done.

+  DCHECK(preroll_buffer->discard_padding() == DecoderBuffer::DiscardPadding());
+  DCHECK_EQ(preroll_buffer->type(), type());

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

Could the app cause decoder problems by causing a situation where the preroll
buffer really is meant to be decoded with a different config than *this? For
example, append(buffer_with_config1+buffer_with_config2) with append_window
clipping set to use decoded frames from somewhere in the middle of
buffer_with_config2?
This might be a special case similar to acolwell's comments, below, too.

[DaleCurtis, 2014/05/17 00:55:41]

Yes this would be a problem.  They must have the same config though or preroll
doesn't make sense since the decode would be flushed in between them.  I haven't
yet looked at Aaron's request to handle preroll across appends, but to do so we
would definitely need to invalidate the buffer kept across appends if
append_config_index changes.

+
+  // Mark the entire buffer for discard.
+  // TODO(dalecurtis): This relies on accurate durations, which is a dubious
+  // assumption when working with WebM...

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

suggestion (possibly for a different CL): could discard_padding use
kInfiniteDuration() to indicate this "discard all resulting decoded frames"
condition?

[DaleCurtis, 2014/05/17 00:55:41]

That's a good idea.  I'll work it into the next patch set.

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

Done here: https://codereview.chromium.org/293053005/

+  preroll_buffer_ = preroll_buffer;
+  preroll_buffer_->set_discard_padding(
+      std::make_pair(preroll_buffer_->duration(), base::TimeDelta()));
+}
+
+const scoped_refptr<StreamParserBuffer>&
+StreamParserBuffer::GetPrerollBuffer() {
+  if (preroll_buffer_) {

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

nit: reverse condition and return early.

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

Removed completely now per acolwell's request to move all this into the setters.

+    // Set the timestamps and configuration to match our current buffer.  It's
+    // original timestamp shouldn't be considered by any downstream components.
+    preroll_buffer_->set_timestamp(timestamp());

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

This seems hacky to do all this setting on a get operation. Why can't this all
be done on the set?

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

If something comes along later and calls any Set() methods on the buffer the
preroll buffer needs to be updated.  I found this preferable to changing every
setter to have an "if (preroll) preroll->set_xxx()" clause.

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

But at least the that is a more direct reflection of what is going on. Please
update the setters instead of doing this.

[DaleCurtis, 2014/05/17 00:14:10]

It's not that simple since set_timestamp() is a hacker_style() method on
DecoderBuffer which is not-virtual.  My preference is not to gum up the setters
with this complexity.

+    preroll_buffer_->SetDecodeTimestamp(GetDecodeTimestamp());
+    preroll_buffer_->SetConfigId(GetConfigId());
+    preroll_buffer_->track_id_ = track_id();
+  }
+
+  return preroll_buffer_;
+}
+
 }  // namespace media