diff --git a/modules/video_coding/timing.cc b/modules/video_coding/timing.cc index 99e525a5d7..df98416905 100644 --- a/modules/video_coding/timing.cc +++ b/modules/video_coding/timing.cc @@ -10,7 +10,6 @@ #include "modules/video_coding/timing.h" - #include #include "rtc_base/experiments/field_trial_parser.h" diff --git a/video/BUILD.gn b/video/BUILD.gn index 1a247f4b50..caa4f6d8dc 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn @@ -362,6 +362,7 @@ rtc_library("frame_decode_timing") { ] deps = [ "../api/task_queue", + "../api/units:time_delta", "../modules/video_coding:timing", "../rtc_base:logging", "../rtc_base/task_utils:pending_task_safety_flag", diff --git a/video/frame_buffer_proxy.cc b/video/frame_buffer_proxy.cc index d294eebc53..a2e5406ece 100644 --- a/video/frame_buffer_proxy.cc +++ b/video/frame_buffer_proxy.cc @@ -140,7 +140,7 @@ static constexpr size_t kZeroPlayoutDelayDefaultMaxDecodeQueueSize = 8; // Encapsulates use of the new frame buffer for use in VideoReceiveStream. This // behaves the same as the FrameBuffer2Proxy but uses frame_buffer3 instead. -// Responsiblities from frame_buffer2, like stats, jitter and frame timing +// Responsibilities from frame_buffer2, like stats, jitter and frame timing // accounting are moved into this pro class FrameBuffer3Proxy : public FrameBufferProxy { public: @@ -323,6 +323,8 @@ class FrameBuffer3Proxy : public FrameBufferProxy { std::unique_ptr frame = CombineAndDeleteFrames(std::move(frames)); + timing_->SetLastDecodeScheduledTimestamp(now_ms); + decoder_ready_for_new_frame_ = false; // VideoReceiveStream2 wants frames on the decoder thread. decode_queue_->PostTask(ToQueuedTask( diff --git a/video/frame_buffer_proxy_unittest.cc b/video/frame_buffer_proxy_unittest.cc index fc266d28bc..073d30b5de 100644 --- a/video/frame_buffer_proxy_unittest.cc +++ b/video/frame_buffer_proxy_unittest.cc @@ -25,6 +25,7 @@ #include "api/units/time_delta.h" #include "api/units/timestamp.h" #include "api/video/video_content_type.h" +#include "api/video/video_timing.h" #include "rtc_base/checks.h" #include "rtc_base/event.h" #include "system_wrappers/include/field_trial.h" @@ -201,10 +202,11 @@ class VCMReceiveStatisticsCallbackMock : public VCMReceiveStatisticsCallback { constexpr auto kMaxWaitForKeyframe = TimeDelta::Millis(500); constexpr auto kMaxWaitForFrame = TimeDelta::Millis(1500); -class FrameBufferProxyTest : public ::testing::TestWithParam, - public FrameSchedulingReceiver { +class FrameBufferProxyFixture + : public ::testing::WithParamInterface, + public FrameSchedulingReceiver { public: - FrameBufferProxyTest() + FrameBufferProxyFixture() : field_trials_(GetParam()), time_controller_(kClockStart), clock_(time_controller_.GetClock()), @@ -232,7 +234,7 @@ class FrameBufferProxyTest : public ::testing::TestWithParam, [this](auto num_dropped) { dropped_frames_ += num_dropped; }); } - ~FrameBufferProxyTest() override { + ~FrameBufferProxyFixture() override { if (proxy_) { proxy_->StopOnWorker(); } @@ -320,6 +322,9 @@ class FrameBufferProxyTest : public ::testing::TestWithParam, absl::optional wait_result_; }; +class FrameBufferProxyTest : public ::testing::Test, + public FrameBufferProxyFixture {}; + TEST_P(FrameBufferProxyTest, InitialTimeoutAfterKeyframeTimeoutPeriod) { StartNextDecodeForceKeyframe(); // No frame insterted. Timeout expected. @@ -700,4 +705,88 @@ INSTANTIATE_TEST_SUITE_P( "WebRTC-FrameBuffer3/arm:FrameBuffer3/", "WebRTC-FrameBuffer3/arm:SyncDecoding/")); +class LowLatencyFrameBufferProxyTest : public ::testing::Test, + public FrameBufferProxyFixture {}; + +TEST_P(LowLatencyFrameBufferProxyTest, + FramesDecodedInstantlyWithLowLatencyRendering) { + // Initial keyframe. + StartNextDecodeForceKeyframe(); + timing_.set_min_playout_delay(0); + timing_.set_max_playout_delay(10); + auto frame = Builder().Id(0).Time(0).AsLast().Build(); + // Playout delay of 0 implies low-latency rendering. + frame->SetPlayoutDelay({0, 10}); + proxy_->InsertFrame(std::move(frame)); + EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), Frame(WithId(0))); + + // Delta frame would normally wait here, but should decode at the pacing rate + // in low-latency mode. + StartNextDecode(); + frame = Builder().Id(1).Time(kFps30Rtp).AsLast().Build(); + frame->SetPlayoutDelay({0, 10}); + proxy_->InsertFrame(std::move(frame)); + // Pacing is set to 16ms in the field trial so we should not decode yet. + EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), Eq(absl::nullopt)); + time_controller_.AdvanceTime(TimeDelta::Millis(16)); + EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), Frame(WithId(1))); +} + +TEST_P(LowLatencyFrameBufferProxyTest, ZeroPlayoutDelayFullQueue) { + // Initial keyframe. + StartNextDecodeForceKeyframe(); + timing_.set_min_playout_delay(0); + timing_.set_max_playout_delay(10); + auto frame = Builder().Id(0).Time(0).AsLast().Build(); + // Playout delay of 0 implies low-latency rendering. + frame->SetPlayoutDelay({0, 10}); + proxy_->InsertFrame(std::move(frame)); + EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), Frame(WithId(0))); + + // Queue up 5 frames (configured max queue size for 0-playout delay pacing). + for (int id = 1; id <= 6; ++id) { + frame = Builder().Id(id).Time(kFps30Rtp * id).AsLast().Build(); + frame->SetPlayoutDelay({0, 10}); + proxy_->InsertFrame(std::move(frame)); + } + + // The queue is at its max size for zero playout delay pacing, so the pacing + // should be ignored and the next frame should be decoded instantly. + StartNextDecode(); + EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), Frame(WithId(1))); +} + +TEST_P(LowLatencyFrameBufferProxyTest, MinMaxDelayZeroLowLatencyMode) { + // Initial keyframe. + StartNextDecodeForceKeyframe(); + timing_.set_min_playout_delay(0); + timing_.set_max_playout_delay(0); + auto frame = Builder().Id(0).Time(0).AsLast().Build(); + // Playout delay of 0 implies low-latency rendering. + frame->SetPlayoutDelay({0, 0}); + proxy_->InsertFrame(std::move(frame)); + EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), Frame(WithId(0))); + + // Delta frame would normally wait here, but should decode at the pacing rate + // in low-latency mode. + StartNextDecode(); + frame = Builder().Id(1).Time(kFps30Rtp).AsLast().Build(); + frame->SetPlayoutDelay({0, 0}); + proxy_->InsertFrame(std::move(frame)); + // The min/max=0 version of low-latency rendering will result in a large + // negative decode wait time, so the frame should be ready right away. + EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), Frame(WithId(1))); +} + +INSTANTIATE_TEST_SUITE_P( + FrameBufferProxy, + LowLatencyFrameBufferProxyTest, + ::testing::Values( + "WebRTC-FrameBuffer3/arm:FrameBuffer2/" + "WebRTC-ZeroPlayoutDelay/min_pacing:16ms,max_decode_queue_size:5/", + "WebRTC-FrameBuffer3/arm:FrameBuffer3/" + "WebRTC-ZeroPlayoutDelay/min_pacing:16ms,max_decode_queue_size:5/", + "WebRTC-FrameBuffer3/arm:SyncDecoding/" + "WebRTC-ZeroPlayoutDelay/min_pacing:16ms,max_decode_queue_size:5/")); + } // namespace webrtc diff --git a/video/frame_decode_timing.cc b/video/frame_decode_timing.cc index 02567baa90..ddc60302ef 100644 --- a/video/frame_decode_timing.cc +++ b/video/frame_decode_timing.cc @@ -10,7 +10,10 @@ #include "video/frame_decode_timing.h" +#include + #include "absl/types/optional.h" +#include "api/units/time_delta.h" #include "rtc_base/logging.h" namespace webrtc { @@ -45,7 +48,9 @@ FrameDecodeTiming::OnFrameBufferUpdated(uint32_t next_temporal_unit_rtp, RTC_DLOG(LS_VERBOSE) << "Selected frame with rtp " << next_temporal_unit_rtp << " render time " << render_time.ms() << " with a max wait of " << max_wait.ms() << "ms"; - return FrameSchedule{.latest_decode_time = now + max_wait, + + Timestamp latest_decode_time = now + std::max(max_wait, TimeDelta::Zero()); + return FrameSchedule{.latest_decode_time = latest_decode_time, .render_time = render_time}; } diff --git a/video/frame_decode_timing_unittest.cc b/video/frame_decode_timing_unittest.cc index 1932e85246..ac5bf9ef10 100644 --- a/video/frame_decode_timing_unittest.cc +++ b/video/frame_decode_timing_unittest.cc @@ -89,7 +89,8 @@ TEST_F(FrameDecodeTimingTest, ReturnsWaitTimesWhenValid) { } TEST_F(FrameDecodeTimingTest, FastForwardsFrameTooFarInThePast) { - const TimeDelta decode_delay = TimeDelta::Millis(-6); + const TimeDelta decode_delay = + -FrameDecodeTiming::kMaxAllowedFrameDelay - TimeDelta::Millis(1); const Timestamp render_time = clock_.CurrentTime(); timing_.SetTimes(90000, render_time, decode_delay); @@ -99,14 +100,16 @@ TEST_F(FrameDecodeTimingTest, FastForwardsFrameTooFarInThePast) { } TEST_F(FrameDecodeTimingTest, NoFastForwardIfOnlyFrameToDecode) { - const TimeDelta decode_delay = TimeDelta::Millis(-6); + const TimeDelta decode_delay = + -FrameDecodeTiming::kMaxAllowedFrameDelay - TimeDelta::Millis(1); const Timestamp render_time = clock_.CurrentTime(); timing_.SetTimes(90000, render_time, decode_delay); + // Negative `decode_delay` means that `latest_decode_time` is now. EXPECT_THAT(frame_decode_scheduler_.OnFrameBufferUpdated(90000, 90000, false), Optional(AllOf( Field(&FrameDecodeTiming::FrameSchedule::latest_decode_time, - Eq(clock_.CurrentTime() + decode_delay)), + Eq(clock_.CurrentTime())), Field(&FrameDecodeTiming::FrameSchedule::render_time, Eq(render_time))))); }