Fix missing splice_timestamp() on pre-splice buffers.

The original code was setting the splice_timestamp() on the buffer
before copying.  CopyBuffer() did not copy splice_timestamp() so
buffers which should have had a splice_timestamp() did not get one.

Now the splice_timestamp() is only set on the copied buffer and
CopyBuffer() is modified to include splice_timestamp().  This is
now enforced via DCHECK() when buffers are handed out.

What would happen is splices on top of splices would end up with
mixed splice_timestamp() values which cause crashes later in the
pipeline.

BUG=357385
TEST=new DCHECKs() added which fail existing tests.

Review URL: https://codereview.chromium.org/228663006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@263469 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 17 insertions, 0 deletions

diff --git a/media/filters/source_buffer_stream.cc b/media/filters/source_buffer_stream.cc
index 0ff0f3f..4b892d7 100644
--- a/media/filters/source_buffer_stream.cc
+++ b/media/filters/source_buffer_stream.cc
@@ -1152,6 +1152,10 @@ SourceBufferStream::Status SourceBufferStream::GetNextBuffer(
       return SourceBufferStream::kConfigChange;
     }
 
+    // Every pre splice buffer must have the same splice_timestamp().
+    DCHECK(splice_buffer_->splice_timestamp() ==
+           splice_buffers[splice_buffers_index_]->splice_timestamp());
+
     *out_buffer = splice_buffers[splice_buffers_index_++];
     return SourceBufferStream::kSuccess;
   }
@@ -1169,6 +1173,7 @@ SourceBufferStream::Status SourceBufferStream::GetNextBuffer(
   // always issued prior to handing out this buffer, any changes in config id
   // have been inherently handled.
   DCHECK_GE(splice_buffers_index_, splice_buffers.size());
+  DCHECK(splice_buffers.back()->splice_timestamp() == kNoTimestamp());
   *out_buffer = splice_buffers.back();
   splice_buffer_ = NULL;
   splice_buffers_index_ = 0;
diff --git a/media/filters/source_buffer_stream_unittest.cc b/media/filters/source_buffer_stream_unittest.cc
index 3a92732..db54a41 100644
--- a/media/filters/source_buffer_stream_unittest.cc
+++ b/media/filters/source_buffer_stream_unittest.cc
@@ -254,6 +254,7 @@ class SourceBufferStreamTest : public testing::Test {
     base::SplitString(expected, ' ', &timestamps);
     std::stringstream ss;
     const SourceBufferStream::Type type = stream_->GetType();
+    base::TimeDelta active_splice_timestamp = kNoTimestamp();
     for (size_t i = 0; i < timestamps.size(); i++) {
       scoped_refptr<StreamParserBuffer> buffer;
       SourceBufferStream::Status status = stream_->GetNextBuffer(&buffer);
@@ -288,6 +289,17 @@ class SourceBufferStreamTest : public testing::Test {
       ss << buffer->GetDecodeTimestamp().InMilliseconds();
       if (buffer->IsKeyframe())
         ss << "K";
+
+      // Until the last splice frame is seen, indicated by a matching timestamp,
+      // all buffers must have the same splice_timestamp().
+      if (buffer->timestamp() == active_splice_timestamp) {
+        ASSERT_EQ(buffer->splice_timestamp(), kNoTimestamp());
+      } else {
+        ASSERT_TRUE(active_splice_timestamp == kNoTimestamp() ||
+                    active_splice_timestamp == buffer->splice_timestamp());
+      }
+
+      active_splice_timestamp = buffer->splice_timestamp();
     }
     EXPECT_EQ(expected, ss.str());
   }