Add gapless playback support for AAC playback.

media/base/stream_parser_buffer.cc

 // Copyright (c) 2012 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/stream_parser_buffer.h"
 
 #include "base/logging.h"
 #include "media/base/buffers.h"
 
 namespace media {
@@ skipping 100 matching lines @@
   const scoped_refptr<StreamParserBuffer>& first_splice_buffer =
       pre_splice_buffers.front();
 
   // Ensure the given buffers are actually before the splice point.
   DCHECK(first_splice_buffer->timestamp() <= overlapping_buffer->timestamp());
 
   // TODO(dalecurtis): We should also clear |data| and |side_data|, but since
   // 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());
   set_timestamp(first_splice_buffer->timestamp());
   is_keyframe_ = first_splice_buffer->IsKeyframe();
   type_ = first_splice_buffer->type();
   track_id_ = first_splice_buffer->track_id();
   set_splice_timestamp(overlapping_buffer->timestamp());
 
   // The splice duration is the duration of all buffers before the splice plus
   // the highest ending timestamp after the splice point.
   set_duration(
       std::max(overlapping_buffer->timestamp() + overlapping_buffer->duration(),
                pre_splice_buffers.back()->timestamp() +
                    pre_splice_buffers.back()->duration()) -
       first_splice_buffer->timestamp());
 
   // Copy all pre splice buffers into our wrapper buffer.
   for (BufferQueue::const_iterator it = pre_splice_buffers.begin();
        it != pre_splice_buffers.end();
        ++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());
   }
 
   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