Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(626)

Unified Diff: media/filters/chunk_demuxer_unittest.cc

Issue 1008463002: Fix MSE GC, make it less aggressive, more spec-compliant (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Added new test case: GarbageCollection_SaveDataAtPlaybackPosition Created 5 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: media/filters/chunk_demuxer_unittest.cc
diff --git a/media/filters/chunk_demuxer_unittest.cc b/media/filters/chunk_demuxer_unittest.cc
index 16ddff5b50529e8060164a486728d159bf5472e6..7b495061f349f254d92e08b625545c55d377a734 100644
--- a/media/filters/chunk_demuxer_unittest.cc
+++ b/media/filters/chunk_demuxer_unittest.cc
@@ -176,8 +176,11 @@ class ChunkDemuxerTest : public ::testing::Test {
base::Bind(&ChunkDemuxerTest::DemuxerOpened, base::Unretained(this));
Demuxer::EncryptedMediaInitDataCB encrypted_media_init_data_cb = base::Bind(
&ChunkDemuxerTest::OnEncryptedMediaInitData, base::Unretained(this));
+ current_playback_position_ = kNoTimestamp();
demuxer_.reset(new ChunkDemuxer(
open_cb, encrypted_media_init_data_cb, base::Bind(&AddLogEntryForTest),
+ base::Bind(&ChunkDemuxerTest::GetCurrentPlaybackPosition,
+ base::Unretained(this)),
scoped_refptr<MediaLog>(new MediaLog()), true));
}
@@ -1183,12 +1186,24 @@ class ChunkDemuxerTest : public ::testing::Test {
return true;
}
+ void SetCurrentPlaybackPosition(base::TimeDelta media_time) {
+ current_playback_position_ = media_time;
+ }
+
+ base::TimeDelta GetCurrentPlaybackPosition() {
+ return current_playback_position_;
+ }
+
base::MessageLoop message_loop_;
MockDemuxerHost host_;
scoped_ptr<ChunkDemuxer> demuxer_;
ChunkDemuxer::InitSegmentReceivedCB init_segment_received_cb_;
+ // Used to provide current playback position for ChunkDemuxer garbage
+ // collection algorithm.
+ base::TimeDelta current_playback_position_;
+
base::TimeDelta append_window_start_for_next_append_;
base::TimeDelta append_window_end_for_next_append_;
@@ -3334,7 +3349,7 @@ TEST_F(ChunkDemuxerTest, SetMemoryLimitType) {
// Set different memory limits for audio and video.
demuxer_->SetMemoryLimits(DemuxerStream::AUDIO, 10 * kBlockSize);
- demuxer_->SetMemoryLimits(DemuxerStream::VIDEO, 5 * kBlockSize);
+ demuxer_->SetMemoryLimits(DemuxerStream::VIDEO, 5 * kBlockSize + 1);
base::TimeDelta seek_time = base::TimeDelta::FromMilliseconds(1000);
@@ -3342,6 +3357,9 @@ TEST_F(ChunkDemuxerTest, SetMemoryLimitType) {
AppendSingleStreamCluster(kSourceId, kAudioTrackNum, 0, 10);
AppendSingleStreamCluster(kSourceId, kVideoTrackNum, 0, 5);
+ // We should be right at buffer limit, should pass
+ EXPECT_TRUE(demuxer_->EvictFrames());
+
CheckExpectedRanges(DemuxerStream::AUDIO, "{ [0,230) }");
CheckExpectedRanges(DemuxerStream::VIDEO, "{ [0,165) }");
@@ -3354,6 +3372,9 @@ TEST_F(ChunkDemuxerTest, SetMemoryLimitType) {
AppendSingleStreamCluster(kSourceId, kVideoTrackNum,
seek_time.InMilliseconds(), 5);
+ // We should delete first append, and be exactly at buffer limit
+ EXPECT_TRUE(demuxer_->EvictFrames());
+
// Verify that the old data, and nothing more, has been garbage collected.
CheckExpectedRanges(DemuxerStream::AUDIO, "{ [1000,1230) }");
CheckExpectedRanges(DemuxerStream::VIDEO, "{ [1000,1165) }");
@@ -3378,12 +3399,13 @@ TEST_F(ChunkDemuxerTest, GCDuringSeek) {
// Signal that the second seek is starting.
demuxer_->StartWaitingForSeek(seek_time2);
- // Append data to satisfy the second seek. This append triggers
- // the garbage collection logic since we set the memory limit to
- // 5 blocks.
+ // Append data to satisfy the second seek.
AppendSingleStreamCluster(kSourceId, kAudioTrackNum,
seek_time2.InMilliseconds(), 5);
+ // Should remove first append
+ EXPECT_TRUE(demuxer_->EvictFrames());
+
// Verify that the buffers that cover |seek_time2| do not get
// garbage collected.
CheckExpectedRanges(kSourceId, "{ [500,615) }");
@@ -3391,15 +3413,46 @@ TEST_F(ChunkDemuxerTest, GCDuringSeek) {
// Complete the seek.
demuxer_->Seek(seek_time2, NewExpectedStatusCB(PIPELINE_OK));
-
// Append more data and make sure that the blocks for |seek_time2|
// don't get removed.
- //
- // NOTE: The current GC algorithm tries to preserve the GOP at the
- // current position as well as the last appended GOP. This is
- // why there are 2 ranges in the expectations.
AppendSingleStreamCluster(kSourceId, kAudioTrackNum, 700, 5);
- CheckExpectedRanges(kSourceId, "{ [500,592) [792,815) }");
+
+ // We should not delete things between head and last append, so buffer
+ // remains overfull.
+ EXPECT_FALSE(demuxer_->EvictFrames());
+
+ CheckExpectedRanges(kSourceId, "{ [500,615) [700,815) }");
+}
+
+TEST_F(ChunkDemuxerTest, GCKeepPlayhead) {
+ ASSERT_TRUE(InitDemuxer(HAS_AUDIO));
+
+ // Set different memory limits for audio and video.
+ demuxer_->SetMemoryLimits(DemuxerStream::AUDIO, 5 * kBlockSize);
+
+ // Set media time to report playback head at 0ms
+ SetCurrentPlaybackPosition(base::TimeDelta::FromMilliseconds(0));
+
+ // Append data at the start that can be garbage collected:
+ AppendSingleStreamCluster(kSourceId, kAudioTrackNum, 0, 10);
+
+ // We expect garbage collection to fail, as we don't want to spontaneously
+ // create gaps in source buffer stream. This could break playback for many
wolenetz 2015/06/04 20:06:50 nit: s/This/Gaps/
servolk 2015/06/04 20:22:46 Done.
+ // clients, who don't bother to check ranges after append.
+ EXPECT_FALSE(demuxer_->EvictFrames());
+ CheckExpectedRanges(kSourceId, "{ [0,230) }");
+
+ // Seek, confirm that media time holds us at zero
+ base::TimeDelta seek_time = base::TimeDelta::FromMilliseconds(207);
+ Seek(seek_time);
+
+ EXPECT_FALSE(demuxer_->EvictFrames());
+ CheckExpectedRanges(kSourceId, "{ [0,230) }");
+
+ // Progress media time, show that we can collect up to that point
+ SetCurrentPlaybackPosition(base::TimeDelta::FromMilliseconds(69));
+ EXPECT_FALSE(demuxer_->EvictFrames());
+ CheckExpectedRanges(kSourceId, "{ [69,230) }");
}
TEST_F(ChunkDemuxerTest, AppendWindow_Video) {

Powered by Google App Engine
This is Rietveld 408576698