Add FrameBufferProxy test for low-latency renderer
Ensures that frames are decoded instantly when in low-latency render
mode. This also tests the max queue size behaviour. Adds a new test
suite for FrameBufferProxy that sets the appropriate field trials.
* Fixes FrameDecodeTiming to never use negative wait times for decode
timestamps.
R=kron@webrtc.org
Change-Id: I06cbec52e1e866e21aa964b24c4fd0163c26961b
Bug: webrtc:13658
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251601
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Johannes Kron <kron@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35999}
diff --git a/modules/video_coding/timing.cc b/modules/video_coding/timing.cc
index 99e525a..df98416 100644
--- a/modules/video_coding/timing.cc
+++ b/modules/video_coding/timing.cc
@@ -10,7 +10,6 @@
#include "modules/video_coding/timing.h"
-
#include <algorithm>
#include "rtc_base/experiments/field_trial_parser.h"
diff --git a/video/BUILD.gn b/video/BUILD.gn
index 1a247f4..caa4f6d 100644
--- a/video/BUILD.gn
+++ b/video/BUILD.gn
@@ -362,6 +362,7 @@
]
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 d294eeb..a2e5406 100644
--- a/video/frame_buffer_proxy.cc
+++ b/video/frame_buffer_proxy.cc
@@ -140,7 +140,7 @@
// 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 @@
std::unique_ptr<EncodedFrame> 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 fc266d2..073d30b 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 @@
constexpr auto kMaxWaitForKeyframe = TimeDelta::Millis(500);
constexpr auto kMaxWaitForFrame = TimeDelta::Millis(1500);
-class FrameBufferProxyTest : public ::testing::TestWithParam<std::string>,
- public FrameSchedulingReceiver {
+class FrameBufferProxyFixture
+ : public ::testing::WithParamInterface<std::string>,
+ public FrameSchedulingReceiver {
public:
- FrameBufferProxyTest()
+ FrameBufferProxyFixture()
: field_trials_(GetParam()),
time_controller_(kClockStart),
clock_(time_controller_.GetClock()),
@@ -232,7 +234,7 @@
[this](auto num_dropped) { dropped_frames_ += num_dropped; });
}
- ~FrameBufferProxyTest() override {
+ ~FrameBufferProxyFixture() override {
if (proxy_) {
proxy_->StopOnWorker();
}
@@ -320,6 +322,9 @@
absl::optional<WaitResult> wait_result_;
};
+class FrameBufferProxyTest : public ::testing::Test,
+ public FrameBufferProxyFixture {};
+
TEST_P(FrameBufferProxyTest, InitialTimeoutAfterKeyframeTimeoutPeriod) {
StartNextDecodeForceKeyframe();
// No frame insterted. Timeout expected.
@@ -700,4 +705,88 @@
"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 02567ba..ddc6030 100644
--- a/video/frame_decode_timing.cc
+++ b/video/frame_decode_timing.cc
@@ -10,7 +10,10 @@
#include "video/frame_decode_timing.h"
+#include <algorithm>
+
#include "absl/types/optional.h"
+#include "api/units/time_delta.h"
#include "rtc_base/logging.h"
namespace webrtc {
@@ -45,7 +48,9 @@
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 1932e85..ac5bf9e 100644
--- a/video/frame_decode_timing_unittest.cc
+++ b/video/frame_decode_timing_unittest.cc
@@ -89,7 +89,8 @@
}
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, 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)))));
}
