Skip to content

Commit 53a26bb

Browse files
committed
Consider start time for frame painting and fix naming in test.
These changes were accidentally dropped from the final commit after approval in previous review: https://codereview.chromium.org/2007463005 BUG=614099 TBR=xhwang Review-Url: https://codereview.chromium.org/2034903002 Cr-Commit-Position: refs/heads/master@{#397614} (cherry picked from commit 094201d) Review URL: https://codereview.chromium.org/2038053003 . Cr-Commit-Position: refs/branch-heads/2743@{crosswalk-project#217} Cr-Branched-From: 2b3ae3b-refs/heads/master@{#394939}
1 parent c3567d7 commit 53a26bb

File tree

2 files changed

+29
-24
lines changed

2 files changed

+29
-24
lines changed

media/renderers/video_renderer_impl.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,12 @@ void VideoRendererImpl::FrameReady(uint32_t sequence_token,
404404
// We want to paint the first frame under two conditions: Either (1) we have
405405
// enough frames to know it's definitely the first frame or (2) there may be
406406
// no more frames coming (sometimes unless we paint one of them).
407+
//
408+
// For the first condition, we need at least two frames or the first frame
409+
// must have a timestamp >= |start_timestamp_|, since otherwise we may be
410+
// prerolling frames before the actual start time that will be dropped.
407411
if (algorithm_->frames_queued() > 1 || received_end_of_stream_ ||
412+
algorithm_->first_frame()->timestamp() >= start_timestamp_ ||
408413
low_delay_ || !video_frame_stream_->CanReadWithoutStalling()) {
409414
scoped_refptr<VideoFrame> frame = algorithm_->first_frame();
410415
CheckForMetadataChanges(frame->format(), frame->natural_size());

media/renderers/video_renderer_impl_unittest.cc

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,8 @@ class VideoRendererImplTest : public testing::Test {
216216
event.RunAndWait();
217217
}
218218

219-
void WaitForPendingRead() {
220-
SCOPED_TRACE("WaitForPendingRead()");
219+
void WaitForPendingDecode() {
220+
SCOPED_TRACE("WaitForPendingDecode()");
221221
if (!decode_cb_.is_null())
222222
return;
223223

@@ -231,7 +231,7 @@ class VideoRendererImplTest : public testing::Test {
231231
DCHECK(wait_for_pending_decode_cb_.is_null());
232232
}
233233

234-
void SatisfyPendingRead() {
234+
void SatisfyPendingDecode() {
235235
CHECK(!decode_cb_.is_null());
236236
CHECK(!decode_results_.empty());
237237

@@ -245,7 +245,7 @@ class VideoRendererImplTest : public testing::Test {
245245
decode_results_.pop_front();
246246
}
247247

248-
void SatisfyPendingReadWithEndOfStream() {
248+
void SatisfyPendingDecodeWithEndOfStream() {
249249
DCHECK(!decode_cb_.is_null());
250250

251251
// Return EOS buffer to trigger EOS frame.
@@ -258,7 +258,7 @@ class VideoRendererImplTest : public testing::Test {
258258
FROM_HERE,
259259
base::Bind(base::ResetAndReturn(&decode_cb_), DecodeStatus::OK));
260260

261-
WaitForPendingRead();
261+
WaitForPendingDecode();
262262

263263
message_loop_.PostTask(
264264
FROM_HERE,
@@ -353,7 +353,7 @@ class VideoRendererImplTest : public testing::Test {
353353
EXPECT_CALL(mock_cb_, OnBufferingStateChange(BUFFERING_HAVE_ENOUGH))
354354
.WillOnce(RunClosure(event.GetClosure()));
355355
EXPECT_CALL(mock_cb_, OnEnded());
356-
SatisfyPendingReadWithEndOfStream();
356+
SatisfyPendingDecodeWithEndOfStream();
357357
event.RunAndWait();
358358
}
359359

@@ -414,7 +414,7 @@ class VideoRendererImplTest : public testing::Test {
414414
QueueFrames("80 100 120 140 160");
415415
else
416416
QueueFrames("40 60 80 90");
417-
SatisfyPendingRead();
417+
SatisfyPendingDecode();
418418
event.RunAndWait();
419419
}
420420

@@ -449,22 +449,22 @@ class VideoRendererImplTest : public testing::Test {
449449
CHECK(decode_cb_.is_null());
450450
decode_cb_ = decode_cb;
451451

452-
// Wake up WaitForPendingRead() if needed.
452+
// Wake up WaitForPendingDecode() if needed.
453453
if (!wait_for_pending_decode_cb_.is_null())
454454
base::ResetAndReturn(&wait_for_pending_decode_cb_).Run();
455455

456456
if (decode_results_.empty())
457457
return;
458458

459-
SatisfyPendingRead();
459+
SatisfyPendingDecode();
460460
}
461461

462462
void FlushRequested(const base::Closure& callback) {
463463
DCHECK_EQ(&message_loop_, base::MessageLoop::current());
464464
decode_results_.clear();
465465
if (!decode_cb_.is_null()) {
466466
QueueFrames("abort");
467-
SatisfyPendingRead();
467+
SatisfyPendingDecode();
468468
}
469469

470470
message_loop_.PostTask(FROM_HERE, callback);
@@ -479,7 +479,7 @@ class VideoRendererImplTest : public testing::Test {
479479
VideoDecoder::DecodeCB decode_cb_;
480480
base::TimeDelta next_frame_timestamp_;
481481

482-
// Run during DecodeRequested() to unblock WaitForPendingRead().
482+
// Run during DecodeRequested() to unblock WaitForPendingDecode().
483483
base::Closure wait_for_pending_decode_cb_;
484484

485485
std::deque<std::pair<DecodeStatus, scoped_refptr<VideoFrame>>>
@@ -517,14 +517,14 @@ TEST_F(VideoRendererImplTest, InitializeAndStartPlayingFrom) {
517517
TEST_F(VideoRendererImplTest, InitializeAndEndOfStream) {
518518
Initialize();
519519
StartPlayingFrom(0);
520-
WaitForPendingRead();
520+
WaitForPendingDecode();
521521
{
522522
SCOPED_TRACE("Waiting for BUFFERING_HAVE_ENOUGH");
523523
WaitableMessageLoopEvent event;
524524
EXPECT_CALL(mock_cb_, OnBufferingStateChange(BUFFERING_HAVE_ENOUGH))
525525
.WillOnce(RunClosure(event.GetClosure()));
526526
EXPECT_CALL(mock_cb_, OnEnded());
527-
SatisfyPendingReadWithEndOfStream();
527+
SatisfyPendingDecodeWithEndOfStream();
528528
event.RunAndWait();
529529
}
530530
// Firing a time state changed to true should be ignored...
@@ -594,7 +594,7 @@ TEST_F(VideoRendererImplTest, DecodeError_Playing) {
594594
AdvanceTimeInMs(10);
595595

596596
QueueFrames("error");
597-
SatisfyPendingRead();
597+
SatisfyPendingDecode();
598598
WaitForError(PIPELINE_ERROR_DECODE);
599599
Destroy();
600600
}
@@ -664,7 +664,7 @@ TEST_F(VideoRendererImplTest, StartPlayingFrom_LowDelay) {
664664
StartPlayingFrom(10);
665665

666666
QueueFrames("20");
667-
SatisfyPendingRead();
667+
SatisfyPendingDecode();
668668

669669
renderer_->OnTimeStateChanged(true);
670670
time_source_.StartTicking();
@@ -744,7 +744,7 @@ TEST_F(VideoRendererImplTest, RenderingStopsAfterFirstFrame) {
744744
StartPlayingFrom(0);
745745

746746
EXPECT_TRUE(IsReadPending());
747-
SatisfyPendingReadWithEndOfStream();
747+
SatisfyPendingDecodeWithEndOfStream();
748748

749749
event.RunAndWait();
750750
}
@@ -773,7 +773,7 @@ TEST_F(VideoRendererImplTest, RenderingStopsAfterOneFrameWithEOS) {
773773
renderer_->OnTimeStateChanged(true);
774774

775775
EXPECT_TRUE(IsReadPending());
776-
SatisfyPendingReadWithEndOfStream();
776+
SatisfyPendingDecodeWithEndOfStream();
777777
WaitForEnded();
778778

779779
renderer_->OnTimeStateChanged(false);
@@ -807,7 +807,7 @@ TEST_F(VideoRendererImplTest, RenderingStartedThenStopped) {
807807

808808
// Consider the case that rendering is faster than we setup the test event.
809809
// In that case, when we run out of the frames, BUFFERING_HAVE_NOTHING will
810-
// be called. And then during SatisfyPendingReadWithEndOfStream,
810+
// be called. And then during SatisfyPendingDecodeWithEndOfStream,
811811
// BUFFER_HAVE_ENOUGH will be called again.
812812
EXPECT_CALL(mock_cb_, OnBufferingStateChange(BUFFERING_HAVE_ENOUGH))
813813
.Times(testing::AtMost(1));
@@ -824,8 +824,8 @@ TEST_F(VideoRendererImplTest, RenderingStartedThenStopped) {
824824
null_video_sink_->set_background_render(true);
825825
AdvanceTimeInMs(91);
826826
EXPECT_CALL(mock_cb_, FrameReceived(HasTimestamp(90)));
827-
WaitForPendingRead();
828-
SatisfyPendingReadWithEndOfStream();
827+
WaitForPendingDecode();
828+
SatisfyPendingDecodeWithEndOfStream();
829829

830830
// If this wasn't background rendering mode, this would result in two frames
831831
// being dropped, but since we set background render to true, none should be
@@ -867,15 +867,15 @@ TEST_F(VideoRendererImplTest, UnderflowEvictionBeforeEOS) {
867867
event.RunAndWait();
868868
}
869869

870-
WaitForPendingRead();
870+
WaitForPendingDecode();
871871

872872
// Jump time far enough forward that no frames are valid.
873873
renderer_->OnTimeStateChanged(false);
874874
AdvanceTimeInMs(1000);
875875
time_source_.StopTicking();
876876

877877
// Providing the end of stream packet should remove all frames and exit.
878-
SatisfyPendingReadWithEndOfStream();
878+
SatisfyPendingDecodeWithEndOfStream();
879879
EXPECT_CALL(mock_cb_, OnBufferingStateChange(BUFFERING_HAVE_ENOUGH));
880880
WaitForEnded();
881881
Destroy();
@@ -905,8 +905,8 @@ TEST_F(VideoRendererImplTest, StartPlayingFromThenFlushThenEOS) {
905905
Flush();
906906

907907
StartPlayingFrom(200);
908-
WaitForPendingRead();
909-
SatisfyPendingReadWithEndOfStream();
908+
WaitForPendingDecode();
909+
SatisfyPendingDecodeWithEndOfStream();
910910
EXPECT_CALL(mock_cb_, OnBufferingStateChange(BUFFERING_HAVE_ENOUGH));
911911
WaitForEnded();
912912
Destroy();

0 commit comments

Comments
 (0)