diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index d1b90a07b5..a52dac8ea0 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -578,12 +578,14 @@ void VideoReceiveStream::OnCompleteFrame( last_complete_frame_time_ms_ = time_now_ms; const PlayoutDelay& playout_delay = frame->EncodedImage().playout_delay_; - // 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) { + if (playout_delay.min_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(); } @@ -761,19 +763,17 @@ void VideoReceiveStream::HandleFrameBufferTimeout() { } void VideoReceiveStream::UpdatePlayoutDelays() const { - int minimum_delay_ms = + const int minimum_delay_ms = std::max({frame_minimum_playout_delay_ms_, base_minimum_playout_delay_ms_, syncable_minimum_playout_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); } + + const int maximum_delay_ms = frame_maximum_playout_delay_ms_; + if (maximum_delay_ms >= 0) { + timing_->set_max_playout_delay(maximum_delay_ms); + } } std::vector VideoReceiveStream::GetSources() const { diff --git a/video/video_receive_stream_unittest.cc b/video/video_receive_stream_unittest.cc index 9edaa2b704..6d88f67e92 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, 621}; + const PlayoutDelay kPlayoutDelayMs = {123, 321}; std::unique_ptr test_frame(new FrameObjectFake()); test_frame->id.picture_id = 0; test_frame->SetPlayoutDelay(kPlayoutDelayMs); @@ -196,10 +196,9 @@ TEST_F(VideoReceiveStreamTest, PlayoutDelay) { EXPECT_EQ(123, timing_->min_playout_delay()); } -TEST_F(VideoReceiveStreamTest, PlayoutDelayPreservesDefaultValues) { - const int default_min_playout_latency = timing_->min_playout_delay(); +TEST_F(VideoReceiveStreamTest, PlayoutDelayPreservesDefaultMaxValue) { const int default_max_playout_latency = timing_->max_playout_delay(); - const PlayoutDelay kPlayoutDelayMs = {-1, -1}; + const PlayoutDelay kPlayoutDelayMs = {123, -1}; std::unique_ptr test_frame(new FrameObjectFake()); test_frame->id.picture_id = 0; @@ -207,64 +206,26 @@ TEST_F(VideoReceiveStreamTest, PlayoutDelayPreservesDefaultValues) { video_receive_stream_->OnCompleteFrame(std::move(test_frame)); - // Ensure that -1 preserves default minimum and maximum value from |timing_|. - EXPECT_EQ(default_min_playout_latency, timing_->min_playout_delay()); + // 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()); EXPECT_EQ(default_max_playout_latency, timing_->max_playout_delay()); } -TEST_F(VideoReceiveStreamTest, ZeroMinMaxPlayoutDelayOverridesSyncAndBase) { - const PlayoutDelay kPlayoutDelayMs = {0, 0}; +TEST_F(VideoReceiveStreamTest, PlayoutDelayPreservesDefaultMinValue) { + const int default_min_playout_latency = timing_->min_playout_delay(); + const PlayoutDelay kPlayoutDelayMs = {-1, 321}; std::unique_ptr test_frame(new FrameObjectFake()); test_frame->id.picture_id = 0; test_frame->SetPlayoutDelay(kPlayoutDelayMs); - video_receive_stream_->SetMinimumPlayoutDelay(400); - EXPECT_EQ(400, 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()); + // 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()); } class VideoReceiveStreamTestWithFakeDecoder : public ::testing::Test {