Remove WebRTC-ZeroPlayoutDelay field trial Removes the WebRTC-ZeroPlayoutDelay field trial and promotes its default parameters to constants: - Min pacing: 8 ms in FrameDecodeTiming - Max decode queue size: 8 frames in VideoStreamBufferController Updates associated unit tests in FrameDecodeTiming and VideoStreamBufferController to test default behaviors. Bug: chromium:40228487 Change-Id: I69d5d80231a680b1d80f9c19d5a8ef9dca772b81 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/495780 Reviewed-by: Åsa Persson <asapersson@webrtc.org> Commit-Queue: Johannes Kron <kron@webrtc.org> Cr-Commit-Position: refs/heads/main@{#48348}
diff --git a/experiments/field_trials.py b/experiments/field_trials.py index 120a816..74d9ae4 100755 --- a/experiments/field_trials.py +++ b/experiments/field_trials.py
@@ -913,14 +913,11 @@ FieldTrial('WebRTC-Vp9IssueKeyFrameOnLayerDeactivation', 40595338, date(2024, 4, 1)), - FieldTrial('WebRTC-ZeroPlayoutDelay', - 40228487, - date(2024, 4, 1)), # keep-sorted end ]) # yapf: disable POLICY_EXEMPT_FIELD_TRIALS_DIGEST: str = \ - '2a90bac275f9d321f97614ea92d5e8e0cfa17af8' + 'c72a6c5ff92dac291461d264ebe2f7a234903131' REGISTERED_FIELD_TRIALS: FrozenSet[FieldTrial] = ACTIVE_FIELD_TRIALS.union( POLICY_EXEMPT_FIELD_TRIALS)
diff --git a/video/BUILD.gn b/video/BUILD.gn index 605e3e3..af577c7 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn
@@ -305,7 +305,6 @@ "../rtc_base:checks", "../rtc_base:logging", "../rtc_base:macromagic", - "../rtc_base/experiments:field_trial_parser", "../rtc_base/system:no_unique_address", "../system_wrappers", "//third_party/abseil-cpp/absl/base:core_headers", @@ -346,13 +345,11 @@ "frame_decode_timing.h", ] deps = [ - "../api:field_trials_view", "../api/units:time_delta", "../api/units:timestamp", "../modules/video_coding/timing:timing_module", "../rtc_base:checks", "../rtc_base:logging", - "../rtc_base/experiments:field_trial_parser", "../system_wrappers", ] }
diff --git a/video/frame_decode_timing.cc b/video/frame_decode_timing.cc index 2851502..2b76fba 100644 --- a/video/frame_decode_timing.cc +++ b/video/frame_decode_timing.cc
@@ -14,34 +14,19 @@ #include <cstdint> #include <optional> -#include "api/field_trials_view.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" #include "modules/video_coding/timing/timing.h" #include "rtc_base/checks.h" -#include "rtc_base/experiments/field_trial_parser.h" #include "rtc_base/logging.h" #include "system_wrappers/include/clock.h" namespace webrtc { -namespace { -// Default pacing that is used for the low-latency renderer path. -constexpr TimeDelta kZeroPlayoutDelayDefaultMinPacing = TimeDelta::Millis(8); - -} // namespace - -FrameDecodeTiming::FrameDecodeTiming(Clock* clock, - VCMTiming const* timing, - const FieldTrialsView& field_trials) - : clock_(clock), - timing_(timing), - zero_playout_delay_min_pacing_("min_pacing", - kZeroPlayoutDelayDefaultMinPacing) { +FrameDecodeTiming::FrameDecodeTiming(Clock* clock, VCMTiming const* timing) + : clock_(clock), timing_(timing) { RTC_DCHECK(clock_); RTC_DCHECK(timing_); - ParseFieldTrial({&zero_playout_delay_min_pacing_}, - field_trials.Lookup("WebRTC-ZeroPlayoutDelay")); } std::optional<FrameDecodeTiming::FrameSchedule> @@ -84,19 +69,18 @@ Timestamp now, bool too_many_frames_queued) const { const VCMTiming::VideoDelayTimings timings = timing_->GetTimings(); - if (render_time.IsZero() && zero_playout_delay_min_pacing_->us() > 0 && - timings.min_playout_delay.IsZero() && + if (render_time.IsZero() && timings.min_playout_delay.IsZero() && timings.max_playout_delay > TimeDelta::Zero()) { // `render_time` == 0 indicates that the frame should be decoded and // rendered as soon as possible. However, the decoder can be choked if too // many frames are sent at once. Therefore, limit the interframe delay to - // `zero_playout_delay_min_pacing_` unless too many frames are queued in + // `kZeroPlayoutDelayMinPacing` unless too many frames are queued in // which case the frames are sent to the decoder at once. if (too_many_frames_queued) { return TimeDelta::Zero(); } Timestamp earliest_next_decode_start_time = - last_decode_scheduled_ + zero_playout_delay_min_pacing_; + last_decode_scheduled_ + kZeroPlayoutDelayMinPacing; TimeDelta max_wait_time = now >= earliest_next_decode_start_time ? TimeDelta::Zero() : earliest_next_decode_start_time - now;
diff --git a/video/frame_decode_timing.h b/video/frame_decode_timing.h index ad0598b..90c88ac 100644 --- a/video/frame_decode_timing.h +++ b/video/frame_decode_timing.h
@@ -15,20 +15,16 @@ #include <optional> -#include "api/field_trials_view.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" #include "modules/video_coding/timing/timing.h" -#include "rtc_base/experiments/field_trial_parser.h" #include "system_wrappers/include/clock.h" namespace webrtc { class FrameDecodeTiming { public: - FrameDecodeTiming(Clock* clock, - VCMTiming const* timing, - const FieldTrialsView& field_trials); + FrameDecodeTiming(Clock* clock, VCMTiming const* timing); ~FrameDecodeTiming() = default; FrameDecodeTiming(const FrameDecodeTiming&) = delete; FrameDecodeTiming& operator=(const FrameDecodeTiming&) = delete; @@ -36,6 +32,7 @@ // Any frame that has decode delay more than this in the past can be // fast-forwarded. static constexpr TimeDelta kMaxAllowedFrameDelay = TimeDelta::Millis(5); + static constexpr TimeDelta kZeroPlayoutDelayMinPacing = TimeDelta::Millis(8); struct FrameSchedule { Timestamp latest_decode_time; @@ -65,11 +62,6 @@ Clock* const clock_; VCMTiming const* const timing_; - // Set by the field trial WebRTC-ZeroPlayoutDelay. The parameter min_pacing - // determines the minimum delay between frames scheduled for decoding that is - // used when min playout delay=0 and max playout delay>=0. - FieldTrialParameter<TimeDelta> zero_playout_delay_min_pacing_; - // Timestamp at which the last frame was scheduled to be sent to the decoder. // Used only when the RTP header extension playout delay is set to min=0 ms // which is indicated by a render time set to 0.
diff --git a/video/frame_decode_timing_unittest.cc b/video/frame_decode_timing_unittest.cc index 0767563..28a2ab9 100644 --- a/video/frame_decode_timing_unittest.cc +++ b/video/frame_decode_timing_unittest.cc
@@ -55,7 +55,7 @@ : clock_(Timestamp::Millis(1000)), env_(CreateTestEnvironment({.time = &clock_})), timing_(env_, /*render_delay=*/TimeDelta::Zero()), - frame_decode_scheduler_(&clock_, &timing_, env_.field_trials()) { + frame_decode_scheduler_(&clock_, &timing_) { timing_.OnCompleteFrame({.rtp_timestamp = kNextRtp, .time = clock_.CurrentTime(), .last_spatial_layer = true}); @@ -145,7 +145,7 @@ VCMTiming timing(env, kRenderDelay); timing.set_playout_delay({TimeDelta::Zero(), TimeDelta::Zero()}); - FrameDecodeTiming decode_timing(&clock, &timing, env.field_trials()); + FrameDecodeTiming decode_timing(&clock, &timing); for (int i = 0; i < 10; ++i) { clock.AdvanceTime(kTimeDelta); @@ -173,18 +173,17 @@ } TEST(FrameDecodeTimingMaxWaitingTimeTest, WithZeroDelayPacingActive) { - // The minimum pacing is enabled by a field trial and active if the RTP - // playout delay header extension is set to min==0. - constexpr TimeDelta kMinPacing = TimeDelta::Millis(3); + // The minimum pacing is active if the RTP playout delay header extension + // is set to min==0 and max>0. + constexpr TimeDelta kMinPacing = + FrameDecodeTiming::kZeroPlayoutDelayMinPacing; constexpr int64_t kStartTimeUs = 3.15e13; // About one year in us. constexpr TimeDelta kTimeDelta = 1 / Frequency::Hertz(60); constexpr Timestamp kZeroRenderTime = Timestamp::Zero(); SimulatedClock clock(kStartTimeUs); - Environment env = CreateTestEnvironment( - {.field_trials = "WebRTC-ZeroPlayoutDelay/min_pacing:3ms/", - .time = &clock}); + Environment env = CreateTestEnvironment({.time = &clock}); VCMTiming timing(env, kRenderDelay); - FrameDecodeTiming decode_timing(&clock, &timing, env.field_trials()); + FrameDecodeTiming decode_timing(&clock, &timing); // MaxWaitingTime() returns zero for evenly spaced video frames. for (int i = 0; i < 10; ++i) { @@ -195,8 +194,8 @@ TimeDelta::Zero()); decode_timing.SetLastDecodeScheduledTimestamp(now); } - // Another frame submitted at the same time is paced according to the field - // trial setting. + // Another frame submitted at the same time is paced according to the default + // pacing setting. Timestamp now = clock.CurrentTime(); EXPECT_EQ(decode_timing.MaxWaitingTime(kZeroRenderTime, now, /*too_many_frames_queued=*/false), @@ -225,17 +224,15 @@ } TEST(FrameDecodeTimingMaxWaitingTimeTest, - DefaultMaxWaitingTimeUnaffectedByPacingExperiment) { - // The minimum pacing is enabled by a field trial but should not have any - // effect if render_time is greater than 0; + DefaultMaxWaitingTimeUnaffectedByZeroPlayoutPacing) { + // The minimum pacing should not have any effect if render_time is greater + // than 0. constexpr int64_t kStartTimeUs = 3.15e13; // About one year in us. const TimeDelta kTimeDelta = TimeDelta::Millis(1000.0 / 60.0); SimulatedClock clock(kStartTimeUs); - Environment env = CreateTestEnvironment( - {.field_trials = "WebRTC-ZeroPlayoutDelay/min_pacing:3ms/", - .time = &clock}); + Environment env = CreateTestEnvironment({.time = &clock}); VCMTiming timing(env, kRenderDelay); - FrameDecodeTiming decode_timing(&clock, &timing, env.field_trials()); + FrameDecodeTiming decode_timing(&clock, &timing); clock.AdvanceTime(kTimeDelta); Timestamp now = clock.CurrentTime(); @@ -258,18 +255,17 @@ } TEST(FrameDecodeTimingMaxWaitingTimeTest, ReturnsZeroIfTooManyFramesAreQueued) { - // The minimum pacing is enabled by a field trial and active if the RTP - // playout delay header extension is set to min==0. - constexpr TimeDelta kMinPacing = TimeDelta::Millis(3); + // The minimum pacing is active if the RTP playout delay header extension is + // set to min==0 and max>0. + constexpr TimeDelta kMinPacing = + FrameDecodeTiming::kZeroPlayoutDelayMinPacing; constexpr int64_t kStartTimeUs = 3.15e13; // About one year in us. const TimeDelta kTimeDelta = TimeDelta::Millis(1000.0 / 60.0); constexpr Timestamp kZeroRenderTime = Timestamp::Zero(); SimulatedClock clock(kStartTimeUs); - Environment env = CreateTestEnvironment( - {.field_trials = "WebRTC-ZeroPlayoutDelay/min_pacing:3ms/", - .time = &clock}); + Environment env = CreateTestEnvironment({.time = &clock}); VCMTiming timing(env, kRenderDelay); - FrameDecodeTiming decode_timing(&clock, &timing, env.field_trials()); + FrameDecodeTiming decode_timing(&clock, &timing); // MaxWaitingTime() returns zero for evenly spaced video frames. for (int i = 0; i < 10; ++i) { @@ -280,8 +276,8 @@ TimeDelta::Zero()); decode_timing.SetLastDecodeScheduledTimestamp(now); } - // Another frame submitted at the same time is paced according to the field - // trial setting. + // Another frame submitted at the same time is paced according to the + // default pacing setting. Timestamp now_ms = clock.CurrentTime(); EXPECT_EQ(decode_timing.MaxWaitingTime(kZeroRenderTime, now_ms, /*too_many_frames_queued=*/false), @@ -301,7 +297,7 @@ Environment env = CreateTestEnvironment({.time = &clock}); VCMTiming timing(env, kRenderDelay); UpdateDecodeTimer(timing, clock, kDecodeTime); - FrameDecodeTiming decode_timing(&clock, &timing, env.field_trials()); + FrameDecodeTiming decode_timing(&clock, &timing); Timestamp on_time = clock.CurrentTime() + kDecodeTime + kRenderDelay;
diff --git a/video/video_stream_buffer_controller.cc b/video/video_stream_buffer_controller.cc index 033a9c5..ee42713 100644 --- a/video/video_stream_buffer_controller.cc +++ b/video/video_stream_buffer_controller.cc
@@ -32,7 +32,6 @@ #include "modules/video_coding/include/video_coding_defines.h" #include "modules/video_coding/timing/timing.h" #include "rtc_base/checks.h" -#include "rtc_base/experiments/field_trial_parser.h" #include "rtc_base/logging.h" #include "system_wrappers/include/clock.h" #include "video/frame_decode_scheduler.h" @@ -48,9 +47,11 @@ // Max number of decoded frame info that will be saved. constexpr int kMaxFramesHistory = 1 << 13; -// Default value for the maximum decode queue size that is used when the -// low-latency renderer is used. -constexpr size_t kZeroPlayoutDelayDefaultMaxDecodeQueueSize = 8; +// Maximum number of frames in the decode queue to allow pacing for the +// low-latency renderer path. If the queue grows beyond the max limit, +// pacing will be disabled and frames will be pushed to the decoder as +// soon as possible. +constexpr size_t kZeroPlayoutDelayMaxDecodeQueueSize = 8; struct FrameMetadata { explicit FrameMetadata(const EncodedFrame& frame) @@ -108,25 +109,19 @@ buffer_(std::make_unique<FrameBuffer>(kMaxFramesBuffered, kMaxFramesHistory, field_trials)), - decode_timing_(clock_, timing_, field_trials_), + decode_timing_(clock_, timing_), timeout_tracker_( clock_, worker_queue, VideoReceiveStreamTimeoutTracker::Timeouts{ .max_wait_for_keyframe = max_wait_for_keyframe, .max_wait_for_frame = max_wait_for_frame}, - absl::bind_front(&VideoStreamBufferController::OnTimeout, this)), - zero_playout_delay_max_decode_queue_size_( - "max_decode_queue_size", - kZeroPlayoutDelayDefaultMaxDecodeQueueSize) { + absl::bind_front(&VideoStreamBufferController::OnTimeout, this)) { RTC_DCHECK(stats_proxy_); RTC_DCHECK(receiver_); RTC_DCHECK(timing_); RTC_DCHECK(clock_); RTC_DCHECK(frame_decode_scheduler_); - - ParseFieldTrial({&zero_playout_delay_max_decode_queue_size_}, - field_trials.Lookup("WebRTC-ZeroPlayoutDelay")); } void VideoStreamBufferController::Stop() { @@ -352,7 +347,7 @@ bool VideoStreamBufferController::IsTooManyFramesQueued() const RTC_RUN_ON(&worker_sequence_checker_) { - return buffer_->CurrentSize() > zero_playout_delay_max_decode_queue_size_; + return buffer_->CurrentSize() > kZeroPlayoutDelayMaxDecodeQueueSize; } void VideoStreamBufferController::ForceKeyFrameReleaseImmediately()
diff --git a/video/video_stream_buffer_controller.h b/video/video_stream_buffer_controller.h index eea6fda..6086ccd 100644 --- a/video/video_stream_buffer_controller.h +++ b/video/video_stream_buffer_controller.h
@@ -28,7 +28,6 @@ #include "api/video/video_content_type.h" #include "modules/video_coding/include/video_coding_defines.h" #include "modules/video_coding/timing/timing.h" -#include "rtc_base/experiments/field_trial_parser.h" #include "rtc_base/system/no_unique_address.h" #include "rtc_base/thread_annotations.h" #include "system_wrappers/include/clock.h" @@ -135,13 +134,6 @@ bool decoder_ready_for_new_frame_ RTC_GUARDED_BY(&worker_sequence_checker_) = false; - // Maximum number of frames in the decode queue to allow pacing. If the - // queue grows beyond the max limit, pacing will be disabled and frames will - // be pushed to the decoder as soon as possible. This only has an effect - // when the low-latency rendering path is active, which is indicated by - // the frame's render time == 0. - FieldTrialParameter<unsigned> zero_playout_delay_max_decode_queue_size_; - ScopedTaskSafety worker_safety_; };
diff --git a/video/video_stream_buffer_controller_unittest.cc b/video/video_stream_buffer_controller_unittest.cc index 1a34d82..4985f2a 100644 --- a/video/video_stream_buffer_controller_unittest.cc +++ b/video/video_stream_buffer_controller_unittest.cc
@@ -16,7 +16,6 @@ #include <memory> #include <optional> #include <string> -#include <tuple> #include <utility> #include <variant> #include <vector> @@ -114,14 +113,13 @@ constexpr auto kMaxWaitForKeyframe = TimeDelta::Millis(500); constexpr auto kMaxWaitForFrame = TimeDelta::Millis(1500); class VideoStreamBufferControllerFixture - : public ::testing::WithParamInterface<std::tuple<bool, std::string>>, + : public ::testing::WithParamInterface<bool>, public FrameSchedulingReceiver { public: VideoStreamBufferControllerFixture() - : sync_decoding_(std::get<0>(GetParam())), + : sync_decoding_(GetParam()), time_controller_(kClockStart), - env_(CreateTestEnvironment({.field_trials = std::get<1>(GetParam()), - .time = &time_controller_})), + env_(CreateTestEnvironment({.time = &time_controller_})), fake_metronome_(TimeDelta::Millis(16)), decode_sync_(&env_.clock(), &fake_metronome_, @@ -822,11 +820,10 @@ INSTANTIATE_TEST_SUITE_P(VideoStreamBufferController, VideoStreamBufferControllerTest, - ::testing::Combine(::testing::Bool(), - ::testing::Values("")), + ::testing::Bool(), [](const auto& info) { - return std::get<0>(info.param) ? "SyncDecoding" - : "UnsyncedDecoding"; + return info.param ? "SyncDecoding" + : "UnsyncedDecoding"; }); class LowLatencyVideoStreamBufferControllerTest @@ -858,10 +855,19 @@ .AsLast() .Build(); buffer_->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(std::nullopt)); - time_controller_.AdvanceTime(TimeDelta::Millis(16)); - EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), Frame(test::WithId(1))); + if (sync_decoding_) { + // With synchronous decoding and default pacing (8ms), the latest decode + // time is before the next metronome tick minus allowed frame delay + // (16ms - 5ms = 11ms), so the frame is decoded right away. + EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), + Frame(test::WithId(1))); + } else { + // Pacing is 8ms so we should not decode yet. + EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), Eq(std::nullopt)); + time_controller_.AdvanceTime(TimeDelta::Millis(8)); + EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), + Frame(test::WithId(1))); + } } TEST_P(LowLatencyVideoStreamBufferControllerTest, ZeroPlayoutDelayFullQueue) { @@ -878,8 +884,9 @@ buffer_->InsertFrame(std::move(frame)); EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), Frame(test::WithId(0))); - // Queue up 5 frames (configured max queue size for 0-playout delay pacing). - for (int id = 1; id <= 6; ++id) { + // Queue up frames beyond the default max decode queue size (8) for + // zero-playout delay pacing. + for (int id = 1; id <= 9; ++id) { frame = test::FakeFrameBuilder() .Id(id) .Time(kFps30Rtp * id) @@ -925,14 +932,12 @@ EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), Frame(test::WithId(1))); } -INSTANTIATE_TEST_SUITE_P( - VideoStreamBufferController, - LowLatencyVideoStreamBufferControllerTest, - ::testing::Combine( - ::testing::Bool(), - ::testing::Values( - "WebRTC-ZeroPlayoutDelay/min_pacing:16ms,max_decode_queue_size:5/", - "WebRTC-ZeroPlayoutDelay/" - "min_pacing:16ms,max_decode_queue_size:5/"))); +INSTANTIATE_TEST_SUITE_P(VideoStreamBufferController, + LowLatencyVideoStreamBufferControllerTest, + ::testing::Bool(), + [](const auto& info) { + return info.param ? "SyncDecoding" + : "UnsyncedDecoding"; + }); } // namespace webrtc