Commit e834ecea authored by xhwang's avatar xhwang Committed by Commit bot

media: In low delay mode, do not accumulate frames earlier than the start time.

In low delay mode, we'll start playback when we have any frame. If we accumulate
frames earlier than the start time, we'll start playing that frame prematurely.
Instead, drop those frames immediately. Playback will start when we start to
have frames on or later than the start time.

BUG=512145
TEST=Updated the unittest to cover this case.

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

Cr-Commit-Position: refs/heads/master@{#339774}
parent eacd62ff
...@@ -490,6 +490,16 @@ void VideoRendererImpl::FrameReady(VideoFrameStream::Status status, ...@@ -490,6 +490,16 @@ void VideoRendererImpl::FrameReady(VideoFrameStream::Status status,
return; return;
} }
// In low delay mode, don't accumulate frames that's earlier than the start
// time. Otherwise we could declare HAVE_ENOUGH_DATA and start playback
// prematurely.
if (low_delay_ &&
!frame->metadata()->IsTrue(VideoFrameMetadata::END_OF_STREAM) &&
frame->timestamp() < start_timestamp_) {
AttemptRead_Locked();
return;
}
if (frame->metadata()->IsTrue(VideoFrameMetadata::END_OF_STREAM)) { if (frame->metadata()->IsTrue(VideoFrameMetadata::END_OF_STREAM)) {
DCHECK(!received_end_of_stream_); DCHECK(!received_end_of_stream_);
received_end_of_stream_ = true; received_end_of_stream_ = true;
......
...@@ -46,7 +46,8 @@ MATCHER_P(HasTimestamp, ms, "") { ...@@ -46,7 +46,8 @@ MATCHER_P(HasTimestamp, ms, "") {
return arg->timestamp().InMilliseconds() == ms; return arg->timestamp().InMilliseconds() == ms;
} }
class VideoRendererImplTest : public testing::TestWithParam<bool> { class VideoRendererImplTest
: public testing::TestWithParam<bool /* new_video_renderer */> {
public: public:
VideoRendererImplTest() VideoRendererImplTest()
: tick_clock_(new base::SimpleTestTickClock()), : tick_clock_(new base::SimpleTestTickClock()),
...@@ -487,28 +488,29 @@ TEST_P(VideoRendererImplTest, StartPlayingFrom_RightAfter) { ...@@ -487,28 +488,29 @@ TEST_P(VideoRendererImplTest, StartPlayingFrom_RightAfter) {
} }
TEST_P(VideoRendererImplTest, StartPlayingFrom_LowDelay) { TEST_P(VideoRendererImplTest, StartPlayingFrom_LowDelay) {
// In low-delay mode only one frame is required to finish preroll. // In low-delay mode only one frame is required to finish preroll. But frames
// prior to the start time will not be used.
InitializeWithLowDelay(true); InitializeWithLowDelay(true);
QueueFrames("0"); QueueFrames("0 10");
EXPECT_CALL(mock_cb_, FrameReceived(HasTimestamp(10)));
// Expect some amount of have enough/nothing due to only requiring one frame. // Expect some amount of have enough/nothing due to only requiring one frame.
EXPECT_CALL(mock_cb_, FrameReceived(HasTimestamp(0)));
EXPECT_CALL(mock_cb_, BufferingStateChange(BUFFERING_HAVE_ENOUGH)) EXPECT_CALL(mock_cb_, BufferingStateChange(BUFFERING_HAVE_ENOUGH))
.Times(AnyNumber()); .Times(AnyNumber());
EXPECT_CALL(mock_cb_, BufferingStateChange(BUFFERING_HAVE_NOTHING)) EXPECT_CALL(mock_cb_, BufferingStateChange(BUFFERING_HAVE_NOTHING))
.Times(AnyNumber()); .Times(AnyNumber());
StartPlayingFrom(0); StartPlayingFrom(10);
QueueFrames("10"); QueueFrames("20");
SatisfyPendingRead(); SatisfyPendingRead();
renderer_->OnTimeStateChanged(true); renderer_->OnTimeStateChanged(true);
time_source_.StartTicking(); time_source_.StartTicking();
WaitableMessageLoopEvent event; WaitableMessageLoopEvent event;
EXPECT_CALL(mock_cb_, FrameReceived(HasTimestamp(10))) EXPECT_CALL(mock_cb_, FrameReceived(HasTimestamp(20)))
.WillOnce(RunClosure(event.GetClosure())); .WillOnce(RunClosure(event.GetClosure()));
AdvanceTimeInMs(10); AdvanceTimeInMs(20);
event.RunAndWait(); event.RunAndWait();
Destroy(); Destroy();
......
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