Add plumbing and support for crossfading StreamParserBuffers.

media/filters/chunk_demuxer.cc

Index: media/filters/chunk_demuxer.cc
diff --git a/media/filters/chunk_demuxer.cc b/media/filters/chunk_demuxer.cc
index f5fcf7dac7c7ca3995256ac9e2b01c4194a6cac5..939284200dc3d670e867b93010c95b245d7b46da 100644
--- a/media/filters/chunk_demuxer.cc
+++ b/media/filters/chunk_demuxer.cc
@@ -301,11 +301,12 @@ class ChunkDemuxerStream : public DemuxerStream {
 
   void CompletePendingReadIfPossible_Locked();
 
-  // Gets the value to pass to the next Read() callback. Returns true if
-  // |status| and |buffer| should be passed to the callback. False indicates
-  // that |status| and |buffer| were not set and more data is needed.
-  bool GetNextBuffer_Locked(DemuxerStream::Status* status,
-                            scoped_refptr<StreamParserBuffer>* buffer);
+  // Retrieves a new buffer from SourceBufferStream::GetNextBuffer() and handles
+  // splice frame buffers.  For normal buffers, the status is simply passed
+  // through.  Splice frame buffers will return kSuccess for buffer within the
+  // crossfade along with a kConfigChange in between fade out and fade in.
+  SourceBufferStream::Status GetNextBuffer_Locked(
+      scoped_refptr<StreamParserBuffer>* buffer);
 
   // Specifies the type of the stream.
   Type type_;
@@ -316,6 +317,15 @@ class ChunkDemuxerStream : public DemuxerStream {
   State state_;
   ReadCB read_cb_;
 
+  // Used by GetNextBuffer_Locked() when a buffer with fade out is returned from
+  // the SourceBufferStream.  Will be set to the returned buffer and will be
+  // consumed after the fade out section has been exhausted.
+  scoped_refptr<StreamParserBuffer> fade_in_buffer_;
+
+  // Indicates which of the fade out preroll buffers in |fade_in_buffer_| should
+  // be handled out next.
+  size_t fade_out_preroll_index_;
+
   DISALLOW_IMPLICIT_CONSTRUCTORS(ChunkDemuxerStream);
 };
 
@@ -749,7 +759,8 @@ void SourceState::FilterWithAppendWindow(
 
 ChunkDemuxerStream::ChunkDemuxerStream(Type type)
     : type_(type),
-      state_(UNINITIALIZED) {
+      state_(UNINITIALIZED),
+      fade_out_preroll_index_(0) {
 }
 
 void ChunkDemuxerStream::StartReturningData() {
@@ -757,6 +768,8 @@ void ChunkDemuxerStream::StartReturningData() {
   base::AutoLock auto_lock(lock_);
   DCHECK(read_cb_.is_null());
   ChangeState_Locked(RETURNING_DATA_FOR_READS);
+  fade_in_buffer_ = NULL;
+  fade_out_preroll_index_ = 0;
 }
 
 void ChunkDemuxerStream::AbortReads() {
@@ -967,7 +980,7 @@ void ChunkDemuxerStream::CompletePendingReadIfPossible_Locked() {
       NOTREACHED();
       return;
     case RETURNING_DATA_FOR_READS:
-      switch (stream_->GetNextBuffer(&buffer)) {
+      switch (GetNextBuffer_Locked(&buffer)) {
         case SourceBufferStream::kSuccess:
           status = DemuxerStream::kOk;
           break;
@@ -1002,6 +1015,51 @@ void ChunkDemuxerStream::CompletePendingReadIfPossible_Locked() {
   base::ResetAndReturn(&read_cb_).Run(status, buffer);
 }
 
+SourceBufferStream::Status ChunkDemuxerStream::GetNextBuffer_Locked(
+    scoped_refptr<StreamParserBuffer>* out_buffer) {
+  lock_.AssertAcquired();
+
+  if (!fade_in_buffer_) {
+    const SourceBufferStream::Status status =
+        stream_->GetNextBuffer(out_buffer);
+
+    // Just return if GetNextBuffer() failed or there's no fade out preroll,
+    // there's nothing else to do.
+    if (status != SourceBufferStream::kSuccess ||
+        (*out_buffer)->GetFadeOutPreroll().empty()) {
+      return status;
+    }
+
+    // Setup fade in buffer and fall through into splice frame buffer handling.
+    fade_out_preroll_index_ = 0;
+    fade_in_buffer_ = *out_buffer;
+  }
+
+  DCHECK(fade_in_buffer_);
+  const size_t fade_out_size = fade_in_buffer_->GetFadeOutPreroll().size();
+
+  // Are there any fade out buffers left to hand out?
+  if (fade_out_preroll_index_ < fade_out_size) {
+    *out_buffer =

[acolwell GONE FROM CHROMIUM, 2014/01/08 18:13:26]

I think you need to be careful here because I believe it is possible for buffers
in GetFadeOutPreroll() to have different config_ids so you'll need to properly
detect & signal config changes while you are iterating over these buffers. This
may require this logic to be pushed back into SourceBufferStream because that is
where the id -> config mappings are. I didn't realize this until I saw this
code. Sorry. :/

Also I believe this code will emit an extra config change if the splice point
also happens to be a config change location. SourceBufferStream would return
kConfigChange before this code even gets the buffer with a non-empty
GetFadeOutPreroll(). Since the buffers in FadeOutPreroll would likely have the
config of the previous buffer before the splice, this would trigger signalling
another kConfigChange to restore the old config and decoder state would have
been destroyed during that config toggle.

[DaleCurtis, 2014/01/08 18:45:55]

Ah you're right on both counts. Crap; this will probably need to be in
SourceBufferStream... I'll play around with it some more and see if I can come
up with something clean.

[DaleCurtis, 2014/01/08 22:23:52]

Actually, we could keep the code here if we extend the config id checks in
SourceBufferStream to account for fade-out buffers. Specifically this means
extending SourceBufferRange::GetNextConfigId() and the track_buffer_ config id
checks in SourceBufferStream::GetNextBuffer().

With those changes, this code just needs an added check for config id changes
within a buffer, which is doable here since we'd know the current_config_id ==
fade_out_preroll_buffer[0].configId.

It's a little messy to have the logic split across classes, but rolling this all
into SourceBufferStream::GetNextBuffer() was starting to look hideous. WDYT?

[DaleCurtis, 2014/01/08 23:00:38]

Ahgh, an issue with this idea is when the next buffer after a splice frame is
processed... |current_config_index_| will become stale if config changes occur
within the fade-out or between the fade-out and the overlapping buffer.  As such
we'd get an extra config change after each splice frame :(

I'll keep trying to find a clean SourceBufferStream approach.

+        fade_in_buffer_->GetFadeOutPreroll()[fade_out_preroll_index_++];
+    return SourceBufferStream::kSuccess;
+  }
+
+  // Did we hand out the last fade out buffer on the last call?
+  if (fade_out_preroll_index_ == fade_out_size) {
+    fade_out_preroll_index_++;
+    return SourceBufferStream::kConfigChange;
+  }
+
+  // All fade out buffers have been handed out and a config change completed, so
+  // hand out the final buffer for fade in.
+  DCHECK_GT(fade_out_preroll_index_, fade_out_size);
+  *out_buffer = fade_in_buffer_;
+  fade_in_buffer_ = NULL;
+  fade_out_preroll_index_ = 0;
+  return SourceBufferStream::kSuccess;
+}
+
 ChunkDemuxer::ChunkDemuxer(const base::Closure& open_cb,
                            const NeedKeyCB& need_key_cb,
                            const LogCB& log_cb)