diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index a52dac8ea0..d1b90a07b5 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -578,14 +578,12 @@ void VideoReceiveStream::OnCompleteFrame( last_complete_frame_time_ms_ = time_now_ms; const PlayoutDelay& playout_delay = frame->EncodedImage().playout_delay_; - if (playout_delay.min_ms >= 0) { + // Both |min_ms| and |max_ms| must be valid if PlayoutDelay is set. + RTC_DCHECK((playout_delay.min_ms >= 0 && playout_delay.max_ms >= 0) || + (playout_delay.min_ms < 0 && playout_delay.max_ms < 0)); + if (playout_delay.min_ms >= 0 && playout_delay.max_ms >= 0) { rtc::CritScope cs(&playout_delay_lock_); frame_minimum_playout_delay_ms_ = playout_delay.min_ms; - UpdatePlayoutDelays(); - } - - if (playout_delay.max_ms >= 0) { - rtc::CritScope cs(&playout_delay_lock_); frame_maximum_playout_delay_ms_ = playout_delay.max_ms; UpdatePlayoutDelays(); } @@ -763,17 +761,19 @@ void VideoReceiveStream::HandleFrameBufferTimeout() { } void VideoReceiveStream::UpdatePlayoutDelays() const { - const int minimum_delay_ms = + int minimum_delay_ms = std::max({frame_minimum_playout_delay_ms_, base_minimum_playout_delay_ms_, syncable_minimum_playout_delay_ms_}); - if (minimum_delay_ms >= 0) { - timing_->set_min_playout_delay(minimum_delay_ms); - } - const int maximum_delay_ms = frame_maximum_playout_delay_ms_; if (maximum_delay_ms >= 0) { + // Make sure that minimum_delay_ms <= maximum_delay_ms. + minimum_delay_ms = std::min(minimum_delay_ms, maximum_delay_ms); timing_->set_max_playout_delay(maximum_delay_ms); } + + if (minimum_delay_ms >= 0) { + timing_->set_min_playout_delay(minimum_delay_ms); + } } std::vector VideoReceiveStream::GetSources() const { diff --git a/video/video_receive_stream_unittest.cc b/video/video_receive_stream_unittest.cc index 6d88f67e92..9edaa2b704 100644 --- a/video/video_receive_stream_unittest.cc +++ b/video/video_receive_stream_unittest.cc @@ -168,7 +168,7 @@ TEST_F(VideoReceiveStreamTest, CreateFrameFromH264FmtpSpropAndIdr) { } TEST_F(VideoReceiveStreamTest, PlayoutDelay) { - const PlayoutDelay kPlayoutDelayMs = {123, 321}; + const PlayoutDelay kPlayoutDelayMs = {123, 621}; std::unique_ptr test_frame(new FrameObjectFake()); test_frame->id.picture_id = 0; test_frame->SetPlayoutDelay(kPlayoutDelayMs); @@ -196,9 +196,10 @@ TEST_F(VideoReceiveStreamTest, PlayoutDelay) { EXPECT_EQ(123, timing_->min_playout_delay()); } -TEST_F(VideoReceiveStreamTest, PlayoutDelayPreservesDefaultMaxValue) { +TEST_F(VideoReceiveStreamTest, PlayoutDelayPreservesDefaultValues) { + const int default_min_playout_latency = timing_->min_playout_delay(); const int default_max_playout_latency = timing_->max_playout_delay(); - const PlayoutDelay kPlayoutDelayMs = {123, -1}; + const PlayoutDelay kPlayoutDelayMs = {-1, -1}; std::unique_ptr test_frame(new FrameObjectFake()); test_frame->id.picture_id = 0; @@ -206,26 +207,64 @@ TEST_F(VideoReceiveStreamTest, PlayoutDelayPreservesDefaultMaxValue) { video_receive_stream_->OnCompleteFrame(std::move(test_frame)); - // Ensure that -1 preserves default maximum value from |timing_|. - EXPECT_EQ(kPlayoutDelayMs.min_ms, timing_->min_playout_delay()); - EXPECT_NE(kPlayoutDelayMs.max_ms, timing_->max_playout_delay()); + // Ensure that -1 preserves default minimum and maximum value from |timing_|. + EXPECT_EQ(default_min_playout_latency, timing_->min_playout_delay()); EXPECT_EQ(default_max_playout_latency, timing_->max_playout_delay()); } -TEST_F(VideoReceiveStreamTest, PlayoutDelayPreservesDefaultMinValue) { - const int default_min_playout_latency = timing_->min_playout_delay(); - const PlayoutDelay kPlayoutDelayMs = {-1, 321}; +TEST_F(VideoReceiveStreamTest, ZeroMinMaxPlayoutDelayOverridesSyncAndBase) { + const PlayoutDelay kPlayoutDelayMs = {0, 0}; std::unique_ptr test_frame(new FrameObjectFake()); test_frame->id.picture_id = 0; test_frame->SetPlayoutDelay(kPlayoutDelayMs); - video_receive_stream_->OnCompleteFrame(std::move(test_frame)); + video_receive_stream_->SetMinimumPlayoutDelay(400); + EXPECT_EQ(400, timing_->min_playout_delay()); - // Ensure that -1 preserves default minimum value from |timing_|. - EXPECT_NE(kPlayoutDelayMs.min_ms, timing_->min_playout_delay()); - EXPECT_EQ(kPlayoutDelayMs.max_ms, timing_->max_playout_delay()); - EXPECT_EQ(default_min_playout_latency, timing_->min_playout_delay()); + video_receive_stream_->OnCompleteFrame(std::move(test_frame)); + EXPECT_EQ(0, timing_->min_playout_delay()); + EXPECT_EQ(0, timing_->max_playout_delay()); + + video_receive_stream_->SetBaseMinimumPlayoutDelayMs(1234); + EXPECT_EQ(0, timing_->min_playout_delay()); + EXPECT_EQ(0, timing_->max_playout_delay()); +} + +TEST_F(VideoReceiveStreamTest, PlayoutDelayFromFrameIsCached) { + // Expect the playout delay from one frame to be used until there's a new + // frame with a valid value. + + const PlayoutDelay kPlayoutDelay1Ms = {100, 1000}; + const PlayoutDelay kPlayoutDelay2Ms = {120, 900}; + + // Frame 1 with playout delay set. + std::unique_ptr frame1(new FrameObjectFake()); + frame1->id.picture_id = 0; + frame1->SetPlayoutDelay(kPlayoutDelay1Ms); + + video_receive_stream_->OnCompleteFrame(std::move(frame1)); + EXPECT_EQ(kPlayoutDelay1Ms.min_ms, timing_->min_playout_delay()); + EXPECT_EQ(kPlayoutDelay1Ms.max_ms, timing_->max_playout_delay()); + + // Frame 2 without playout delay set. + std::unique_ptr frame2_without_playout_delay( + new FrameObjectFake()); + frame2_without_playout_delay->id.picture_id = 1; + video_receive_stream_->OnCompleteFrame( + std::move(frame2_without_playout_delay)); + video_receive_stream_->SetBaseMinimumPlayoutDelayMs(40); + video_receive_stream_->SetMinimumPlayoutDelay(50); + EXPECT_EQ(kPlayoutDelay1Ms.min_ms, timing_->min_playout_delay()); + EXPECT_EQ(kPlayoutDelay1Ms.max_ms, timing_->max_playout_delay()); + + // Frame 3 with tighter playout delay bounds. + std::unique_ptr frame3(new FrameObjectFake()); + frame3->id.picture_id = 2; + frame3->SetPlayoutDelay(kPlayoutDelay2Ms); + video_receive_stream_->OnCompleteFrame(std::move(frame3)); + EXPECT_EQ(kPlayoutDelay2Ms.min_ms, timing_->min_playout_delay()); + EXPECT_EQ(kPlayoutDelay2Ms.max_ms, timing_->max_playout_delay()); } class VideoReceiveStreamTestWithFakeDecoder : public ::testing::Test {