Commit 141cb9d8 authored by acolwell@chromium.org's avatar acolwell@chromium.org

Fix SourceBufferStream::Remove() for buffers before the current play position.

BUG=286442
TEST=SourceBufferStreamTest.Remove_BeforeCurrentPosition

Review URL: https://chromiumcodereview.appspot.com/23703007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@221896 0039d316-1c4b-4281-b951-d872f2087c98
parent a6739bcd
...@@ -471,6 +471,9 @@ bool SourceBufferStream::Append( ...@@ -471,6 +471,9 @@ bool SourceBufferStream::Append(
void SourceBufferStream::Remove(base::TimeDelta start, base::TimeDelta end, void SourceBufferStream::Remove(base::TimeDelta start, base::TimeDelta end,
base::TimeDelta duration) { base::TimeDelta duration) {
DVLOG(1) << __FUNCTION__ << "(" << start.InSecondsF()
<< ", " << end.InSecondsF()
<< ", " << duration.InSecondsF() << ")";
DCHECK(start >= base::TimeDelta()) << start.InSecondsF(); DCHECK(start >= base::TimeDelta()) << start.InSecondsF();
DCHECK(start < end) << "start " << start.InSecondsF() DCHECK(start < end) << "start " << start.InSecondsF()
<< " end " << end.InSecondsF(); << " end " << end.InSecondsF();
...@@ -497,17 +500,22 @@ void SourceBufferStream::Remove(base::TimeDelta start, base::TimeDelta end, ...@@ -497,17 +500,22 @@ void SourceBufferStream::Remove(base::TimeDelta start, base::TimeDelta end,
if (new_range) { if (new_range) {
itr = ranges_.insert(++itr, new_range); itr = ranges_.insert(++itr, new_range);
--itr; --itr;
// Update the selected range if the next buffer position was transferred
// to |new_range|.
if (new_range->HasNextBufferPosition())
SetSelectedRange(new_range);
} }
// If the current range now is completely covered by the removal // If the current range now is completely covered by the removal
// range then delete it and move on. // range then delete it and move on.
if (start <= range->GetStartTimestamp()) { if (start <= range->GetStartTimestamp()) {
if (selected_range_ == range) if (selected_range_ == range)
SetSelectedRange(NULL); SetSelectedRange(NULL);
delete range; delete range;
itr = ranges_.erase(itr); itr = ranges_.erase(itr);
continue; continue;
} }
// Truncate the current range so that it only contains data before // Truncate the current range so that it only contains data before
...@@ -518,6 +526,7 @@ void SourceBufferStream::Remove(base::TimeDelta start, base::TimeDelta end, ...@@ -518,6 +526,7 @@ void SourceBufferStream::Remove(base::TimeDelta start, base::TimeDelta end,
// Check to see if the current playback position was removed and // Check to see if the current playback position was removed and
// update the selected range appropriately. // update the selected range appropriately.
if (!saved_buffers.empty()) { if (!saved_buffers.empty()) {
DCHECK(!range->HasNextBufferPosition());
SetSelectedRange(NULL); SetSelectedRange(NULL);
SetSelectedRangeIfNeeded(saved_buffers.front()->GetDecodeTimestamp()); SetSelectedRangeIfNeeded(saved_buffers.front()->GetDecodeTimestamp());
} }
...@@ -525,6 +534,9 @@ void SourceBufferStream::Remove(base::TimeDelta start, base::TimeDelta end, ...@@ -525,6 +534,9 @@ void SourceBufferStream::Remove(base::TimeDelta start, base::TimeDelta end,
// Move on to the next range. // Move on to the next range.
++itr; ++itr;
} }
DCHECK(IsRangeListSorted(ranges_));
DCHECK(OnlySelectedRangeIsSeeked());
} }
void SourceBufferStream::ResetSeekState() { void SourceBufferStream::ResetSeekState() {
...@@ -1253,6 +1265,8 @@ void SourceBufferStream::CompleteConfigChange() { ...@@ -1253,6 +1265,8 @@ void SourceBufferStream::CompleteConfigChange() {
void SourceBufferStream::SetSelectedRangeIfNeeded( void SourceBufferStream::SetSelectedRangeIfNeeded(
const base::TimeDelta timestamp) { const base::TimeDelta timestamp) {
DVLOG(1) << __FUNCTION__ << "(" << timestamp.InSecondsF() << ")";
if (selected_range_) { if (selected_range_) {
DCHECK(track_buffer_.empty()); DCHECK(track_buffer_.empty());
return; return;
......
...@@ -2940,7 +2940,7 @@ TEST_F(SourceBufferStreamTest, Remove_Partial4) { ...@@ -2940,7 +2940,7 @@ TEST_F(SourceBufferStreamTest, Remove_Partial4) {
CheckExpectedRangesByTimestamp("{ [10,40) [2060,2150) }"); CheckExpectedRangesByTimestamp("{ [10,40) [2060,2150) }");
} }
// Test behavior when the current positing is removed and new buffers // Test behavior when the current position is removed and new buffers
// are appended over the removal range. // are appended over the removal range.
TEST_F(SourceBufferStreamTest, Remove_CurrentPosition) { TEST_F(SourceBufferStreamTest, Remove_CurrentPosition) {
Seek(0); Seek(0);
...@@ -2964,6 +2964,21 @@ TEST_F(SourceBufferStreamTest, Remove_CurrentPosition) { ...@@ -2964,6 +2964,21 @@ TEST_F(SourceBufferStreamTest, Remove_CurrentPosition) {
CheckExpectedBuffers("210K 240 270K 300 330"); CheckExpectedBuffers("210K 240 270K 300 330");
} }
// Test behavior when buffers in the selected range before the current position
// are removed.
TEST_F(SourceBufferStreamTest, Remove_BeforeCurrentPosition) {
Seek(0);
NewSegmentAppend("0K 30 60 90K 120 150 180K 210 240 270K 300 330");
CheckExpectedRangesByTimestamp("{ [0,360) }");
CheckExpectedBuffers("0K 30 60 90K 120");
// Remove a range that is before the current playback position.
RemoveInMs(0, 90, 360);
CheckExpectedRangesByTimestamp("{ [90,360) }");
CheckExpectedBuffers("150 180K 210 240 270K 300 330");
}
// TODO(vrk): Add unit tests where keyframes are unaligned between streams. // TODO(vrk): Add unit tests where keyframes are unaligned between streams.
// (crbug.com/133557) // (crbug.com/133557)
......
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