Commit f4713c13 authored by acolwell@chromium.org's avatar acolwell@chromium.org

Prevent incorrect start time from being used during new range creation.

If a SourceBuffer::Remove() call removed the GOP that is currently being
appended, an incorrect start timestamp would be used when the next GOP was
appended. This ultimately led to a crash if a followup Remove() call
ended up deleting the range. This change fixes the code that determines
the start timestamp for the range created for the second GOP.

BUG=382815
TEST=SourceBufferStreamTest, Remove_WholeGOPBeingAppended

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@276471 0039d316-1c4b-4281-b951-d872f2087c98
parent 8547213e
...@@ -503,7 +503,7 @@ bool SourceBufferStream::Append(const BufferQueue& buffers) { ...@@ -503,7 +503,7 @@ bool SourceBufferStream::Append(const BufferQueue& buffers) {
// segment, then we must make sure that we start with a keyframe. // segment, then we must make sure that we start with a keyframe.
// This can happen if the GOP in the previous append gets destroyed // This can happen if the GOP in the previous append gets destroyed
// by a Remove() call. // by a Remove() call.
if (!new_media_segment_ && !buffers.front()->IsKeyframe()) { if (!new_media_segment_) {
BufferQueue::const_iterator itr = buffers.begin(); BufferQueue::const_iterator itr = buffers.begin();
// Scan past all the non-keyframes. // Scan past all the non-keyframes.
...@@ -517,15 +517,17 @@ bool SourceBufferStream::Append(const BufferQueue& buffers) { ...@@ -517,15 +517,17 @@ bool SourceBufferStream::Append(const BufferQueue& buffers) {
last_appended_buffer_timestamp_ = buffers.back()->GetDecodeTimestamp(); last_appended_buffer_timestamp_ = buffers.back()->GetDecodeTimestamp();
last_appended_buffer_is_keyframe_ = buffers.back()->IsKeyframe(); last_appended_buffer_is_keyframe_ = buffers.back()->IsKeyframe();
return true; return true;
} } else if (itr != buffers.begin()) {
// Copy the first keyframe and everything after it into
// Copy the first keyframe and everything after it into |trimmed_buffers|. // |trimmed_buffers|.
trimmed_buffers.assign(itr, buffers.end()); trimmed_buffers.assign(itr, buffers.end());
new_range_start_time = trimmed_buffers.front()->GetDecodeTimestamp();
buffers_for_new_range = &trimmed_buffers; buffers_for_new_range = &trimmed_buffers;
} }
new_range_start_time =
buffers_for_new_range->front()->GetDecodeTimestamp();
}
range_for_next_append_ = range_for_next_append_ =
AddToRanges(new SourceBufferRange( AddToRanges(new SourceBufferRange(
GetType(), *buffers_for_new_range, new_range_start_time, GetType(), *buffers_for_new_range, new_range_start_time,
...@@ -1619,6 +1621,8 @@ void SourceBufferStream::DeleteAndRemoveRange(RangeList::iterator* itr) { ...@@ -1619,6 +1621,8 @@ void SourceBufferStream::DeleteAndRemoveRange(RangeList::iterator* itr) {
if (*itr == range_for_next_append_) { if (*itr == range_for_next_append_) {
DVLOG(1) << __FUNCTION__ << " deleting range_for_next_append_."; DVLOG(1) << __FUNCTION__ << " deleting range_for_next_append_.";
range_for_next_append_ = ranges_.end(); range_for_next_append_ = ranges_.end();
last_appended_buffer_timestamp_ = kNoTimestamp();
last_appended_buffer_is_keyframe_ = false;
} }
delete **itr; delete **itr;
......
...@@ -3511,6 +3511,33 @@ TEST_F(SourceBufferStreamTest, Remove_GOPBeingAppended) { ...@@ -3511,6 +3511,33 @@ TEST_F(SourceBufferStreamTest, Remove_GOPBeingAppended) {
CheckExpectedBuffers("240K 270 300"); CheckExpectedBuffers("240K 270 300");
} }
TEST_F(SourceBufferStreamTest, Remove_WholeGOPBeingAppended) {
Seek(0);
NewSegmentAppend("0K 30 60 90");
CheckExpectedRangesByTimestamp("{ [0,120) }");
// Remove the keyframe of the current GOP being appended.
RemoveInMs(0, 30, 120);
CheckExpectedRangesByTimestamp("{ }");
// Continue appending the current GOP.
AppendBuffers("210 240");
CheckExpectedRangesByTimestamp("{ }");
// Append the beginning of the next GOP.
AppendBuffers("270K 300");
// Verify that the new range is started at the
// beginning of the next GOP.
CheckExpectedRangesByTimestamp("{ [270,330) }");
// Verify the buffers in the ranges.
CheckNoNextBuffer();
SeekToTimestamp(base::TimeDelta::FromMilliseconds(270));
CheckExpectedBuffers("270K 300");
}
TEST_F(SourceBufferStreamTest, TEST_F(SourceBufferStreamTest,
Remove_PreviousAppendDestroyedAndOverwriteExistingRange) { Remove_PreviousAppendDestroyedAndOverwriteExistingRange) {
SeekToTimestamp(base::TimeDelta::FromMilliseconds(90)); SeekToTimestamp(base::TimeDelta::FromMilliseconds(90));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment