From 23bfff33837dce2d8d2381dfb7d3e72d40847232 Mon Sep 17 00:00:00 2001 From: Johannes Kron Date: Tue, 28 Sep 2021 21:31:46 +0200 Subject: [PATCH] Change default parameters for the low-latency video pipeline min_pacing:8ms, to avoid the situation where bursts of frames are sent to the decoder at once due to network jitter. The bursts of frames caused the queues further down in the processing to be full and therefore drop all frames. max_decode_queue_size:8, in the event that too many frames have piled up, do as before and send all frames to the decoder to avoid building up any latency. These setting only affect the low-latency video pipeline that is enabled by setting the playout RTP header extension to min=0ms, max>0ms. Bug: chromium:1138888 Change-Id: I8154bf3efe7450b770da8387f8fb6b23f6be26bd Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/233220 Commit-Queue: Johannes Kron Reviewed-by: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/main@{#35119} --- modules/video_coding/frame_buffer2.cc | 9 +++++++-- modules/video_coding/timing.cc | 7 ++++++- modules/video_coding/timing_unittest.cc | 3 ++- video/video_stream_decoder_impl_unittest.cc | 8 ++++++-- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/modules/video_coding/frame_buffer2.cc b/modules/video_coding/frame_buffer2.cc index 5250ea9d3e..6dae6f42f3 100644 --- a/modules/video_coding/frame_buffer2.cc +++ b/modules/video_coding/frame_buffer2.cc @@ -38,6 +38,10 @@ namespace { // Max number of frames the buffer will hold. constexpr size_t kMaxFramesBuffered = 800; +// Default value for the maximum decode queue size that is used when the +// low-latency renderer is used. +constexpr size_t kZeroPlayoutDelayDefaultMaxDecodeQueueSize = 8; + // Max number of decoded frame info that will be saved. constexpr int kMaxFramesHistory = 1 << 13; @@ -64,8 +68,9 @@ FrameBuffer::FrameBuffer(Clock* clock, add_rtt_to_playout_delay_( webrtc::field_trial::IsEnabled("WebRTC-AddRttToPlayoutDelay")), rtt_mult_settings_(RttMultExperiment::GetRttMultValue()), - zero_playout_delay_max_decode_queue_size_("max_decode_queue_size", - kMaxFramesBuffered) { + zero_playout_delay_max_decode_queue_size_( + "max_decode_queue_size", + kZeroPlayoutDelayDefaultMaxDecodeQueueSize) { ParseFieldTrial({&zero_playout_delay_max_decode_queue_size_}, field_trial::FindFullName("WebRTC-ZeroPlayoutDelay")); callback_checker_.Detach(); diff --git a/modules/video_coding/timing.cc b/modules/video_coding/timing.cc index b4a712b8b8..99e525a5d7 100644 --- a/modules/video_coding/timing.cc +++ b/modules/video_coding/timing.cc @@ -19,6 +19,10 @@ #include "system_wrappers/include/field_trial.h" namespace webrtc { +namespace { +// Default pacing that is used for the low-latency renderer path. +constexpr TimeDelta kZeroPlayoutDelayDefaultMinPacing = TimeDelta::Millis(8); +} // namespace VCMTiming::VCMTiming(Clock* clock) : clock_(clock), @@ -34,7 +38,8 @@ VCMTiming::VCMTiming(Clock* clock) timing_frame_info_(), num_decoded_frames_(0), low_latency_renderer_enabled_("enabled", true), - zero_playout_delay_min_pacing_("min_pacing", TimeDelta::Millis(0)), + zero_playout_delay_min_pacing_("min_pacing", + kZeroPlayoutDelayDefaultMinPacing), last_decode_scheduled_ts_(0) { ParseFieldTrial({&low_latency_renderer_enabled_}, field_trial::FindFullName("WebRTC-LowLatencyRenderer")); diff --git a/modules/video_coding/timing_unittest.cc b/modules/video_coding/timing_unittest.cc index cc87a3b4e0..71de1fe3da 100644 --- a/modules/video_coding/timing_unittest.cc +++ b/modules/video_coding/timing_unittest.cc @@ -130,13 +130,14 @@ TEST(ReceiverTimingTest, TimestampWrapAround) { TEST(ReceiverTimingTest, MaxWaitingTimeIsZeroForZeroRenderTime) { // This is the default path when the RTP playout delay header extension is set - // to min==0. + // to min==0 and max==0. constexpr int64_t kStartTimeUs = 3.15e13; // About one year in us. constexpr int64_t kTimeDeltaMs = 1000.0 / 60.0; constexpr int64_t kZeroRenderTimeMs = 0; SimulatedClock clock(kStartTimeUs); VCMTiming timing(&clock); timing.Reset(); + timing.set_max_playout_delay(0); for (int i = 0; i < 10; ++i) { clock.AdvanceTimeMilliseconds(kTimeDeltaMs); int64_t now_ms = clock.TimeInMilliseconds(); diff --git a/video/video_stream_decoder_impl_unittest.cc b/video/video_stream_decoder_impl_unittest.cc index f3fe3acf41..4cb434463b 100644 --- a/video/video_stream_decoder_impl_unittest.cc +++ b/video/video_stream_decoder_impl_unittest.cc @@ -164,6 +164,9 @@ class VideoStreamDecoderImplTest : public ::testing::Test { time_controller_.GetTaskQueueFactory(), {{1, std::make_pair(SdpVideoFormat("VP8"), 1)}, {2, std::make_pair(SdpVideoFormat("AV1"), 1)}}) { + // Set the min playout delay to a value greater than zero to not activate + // the low-latency renderer. + video_stream_decoder_.SetMinPlayoutDelay(TimeDelta::Millis(10)); } NiceMock callbacks_; @@ -217,17 +220,18 @@ TEST_F(VideoStreamDecoderImplTest, FailToDecodeFrame) { } TEST_F(VideoStreamDecoderImplTest, ChangeFramePayloadType) { + constexpr TimeDelta kFrameInterval = TimeDelta::Millis(1000 / 60); video_stream_decoder_.OnFrame( FrameBuilder().WithPayloadType(1).WithPictureId(0).Build()); EXPECT_CALL(decoder_factory_.Vp8Decoder(), DecodeCall); EXPECT_CALL(callbacks_, OnDecodedFrame); - time_controller_.AdvanceTime(TimeDelta::Millis(1)); + time_controller_.AdvanceTime(kFrameInterval); video_stream_decoder_.OnFrame( FrameBuilder().WithPayloadType(2).WithPictureId(1).Build()); EXPECT_CALL(decoder_factory_.Av1Decoder(), DecodeCall); EXPECT_CALL(callbacks_, OnDecodedFrame); - time_controller_.AdvanceTime(TimeDelta::Millis(1)); + time_controller_.AdvanceTime(kFrameInterval); } } // namespace