From c762a5da3118e2db7cd4768b15ea20c3f46e8fa0 Mon Sep 17 00:00:00 2001 From: "dalecurtis@chromium.org" Date: Sat, 12 Apr 2014 03:26:31 +0000 Subject: 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 --- media/filters/source_buffer_stream.cc | 5 +++++ media/filters/source_buffer_stream_unittest.cc | 12 ++++++++++++ 2 files changed, 17 insertions(+) (limited to 'media/filters') 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, ' ', ×tamps); 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 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()); } -- cgit v1.1