Cap max decode delay for FrameBuffer3
When a large queue of frames builds up due to a lost frame, the decode
delay can sometimes become quite large. In this case the stream may
signal as timed out when in fact it is not. Instead, the delay should
be capped at the timeout limit.
Bug: webrtc:14168
Change-Id: I5b4e8851b2c6d7d27a698627dc1633931d7fc00e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265404
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37199}
diff --git a/video/frame_buffer_proxy.cc b/video/frame_buffer_proxy.cc
index 85ad274..c701122 100644
--- a/video/frame_buffer_proxy.cc
+++ b/video/frame_buffer_proxy.cc
@@ -468,7 +468,8 @@
while (decodable_tu_info) {
schedule = decode_timing_.OnFrameBufferUpdated(
decodable_tu_info->next_rtp_timestamp,
- decodable_tu_info->last_rtp_timestamp, IsTooManyFramesQueued());
+ decodable_tu_info->last_rtp_timestamp, MaxWait(),
+ IsTooManyFramesQueued());
if (schedule) {
// Don't schedule if already waiting for the same frame.
if (frame_decode_scheduler_->ScheduledRtpTimestamp() !=
diff --git a/video/frame_decode_timing.cc b/video/frame_decode_timing.cc
index c683834..15fcefb 100644
--- a/video/frame_decode_timing.cc
+++ b/video/frame_decode_timing.cc
@@ -28,7 +28,9 @@
absl::optional<FrameDecodeTiming::FrameSchedule>
FrameDecodeTiming::OnFrameBufferUpdated(uint32_t next_temporal_unit_rtp,
uint32_t last_temporal_unit_rtp,
+ TimeDelta max_wait_for_frame,
bool too_many_frames_queued) {
+ RTC_DCHECK_GT(max_wait_for_frame, TimeDelta::Zero());
const Timestamp now = clock_->CurrentTime();
Timestamp render_time = timing_->RenderTime(next_temporal_unit_rtp, now);
TimeDelta max_wait =
@@ -48,7 +50,8 @@
<< " render time " << render_time.ms()
<< " with a max wait of " << max_wait.ms() << "ms";
- Timestamp latest_decode_time = now + std::max(max_wait, TimeDelta::Zero());
+ max_wait.Clamp(TimeDelta::Zero(), max_wait_for_frame);
+ Timestamp latest_decode_time = now + max_wait;
return FrameSchedule{.latest_decode_time = latest_decode_time,
.render_time = render_time};
}
diff --git a/video/frame_decode_timing.h b/video/frame_decode_timing.h
index 991cfec..59247dc 100644
--- a/video/frame_decode_timing.h
+++ b/video/frame_decode_timing.h
@@ -41,6 +41,7 @@
absl::optional<FrameSchedule> OnFrameBufferUpdated(
uint32_t next_temporal_unit_rtp,
uint32_t last_temporal_unit_rtp,
+ TimeDelta max_wait_for_frame,
bool too_many_frames_queued);
private:
diff --git a/video/frame_decode_timing_unittest.cc b/video/frame_decode_timing_unittest.cc
index 51d115a..83ea916 100644
--- a/video/frame_decode_timing_unittest.cc
+++ b/video/frame_decode_timing_unittest.cc
@@ -19,6 +19,7 @@
#include "test/gmock.h"
#include "test/gtest.h"
#include "test/scoped_key_value_config.h"
+#include "video/video_receive_stream2.h"
namespace webrtc {
@@ -80,13 +81,13 @@
const Timestamp render_time = clock_.CurrentTime() + TimeDelta::Millis(60);
timing_.SetTimes(90000, render_time, decode_delay);
- EXPECT_THAT(
- frame_decode_scheduler_.OnFrameBufferUpdated(90000, 180000, false),
- Optional(
- AllOf(Field(&FrameDecodeTiming::FrameSchedule::latest_decode_time,
- Eq(clock_.CurrentTime() + decode_delay)),
- Field(&FrameDecodeTiming::FrameSchedule::render_time,
- Eq(render_time)))));
+ EXPECT_THAT(frame_decode_scheduler_.OnFrameBufferUpdated(
+ 90000, 180000, kMaxWaitForFrame, false),
+ Optional(AllOf(
+ Field(&FrameDecodeTiming::FrameSchedule::latest_decode_time,
+ Eq(clock_.CurrentTime() + decode_delay)),
+ Field(&FrameDecodeTiming::FrameSchedule::render_time,
+ Eq(render_time)))));
}
TEST_F(FrameDecodeTimingTest, FastForwardsFrameTooFarInThePast) {
@@ -95,9 +96,9 @@
const Timestamp render_time = clock_.CurrentTime();
timing_.SetTimes(90000, render_time, decode_delay);
- EXPECT_THAT(
- frame_decode_scheduler_.OnFrameBufferUpdated(90000, 180000, false),
- Eq(absl::nullopt));
+ EXPECT_THAT(frame_decode_scheduler_.OnFrameBufferUpdated(
+ 90000, 180000, kMaxWaitForFrame, false),
+ Eq(absl::nullopt));
}
TEST_F(FrameDecodeTimingTest, NoFastForwardIfOnlyFrameToDecode) {
@@ -107,7 +108,8 @@
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),
+ EXPECT_THAT(frame_decode_scheduler_.OnFrameBufferUpdated(
+ 90000, 90000, kMaxWaitForFrame, false),
Optional(AllOf(
Field(&FrameDecodeTiming::FrameSchedule::latest_decode_time,
Eq(clock_.CurrentTime())),
@@ -115,4 +117,31 @@
Eq(render_time)))));
}
+TEST_F(FrameDecodeTimingTest, MaxWaitCapped) {
+ TimeDelta frame_delay = TimeDelta::Millis(30);
+ const TimeDelta decode_delay = TimeDelta::Seconds(3);
+ const Timestamp render_time = clock_.CurrentTime() + TimeDelta::Seconds(3);
+ timing_.SetTimes(90000, render_time, decode_delay);
+ timing_.SetTimes(180000, render_time + frame_delay,
+ decode_delay + frame_delay);
+
+ EXPECT_THAT(frame_decode_scheduler_.OnFrameBufferUpdated(
+ 90000, 270000, kMaxWaitForFrame, false),
+ Optional(AllOf(
+ Field(&FrameDecodeTiming::FrameSchedule::latest_decode_time,
+ Eq(clock_.CurrentTime() + kMaxWaitForFrame)),
+ Field(&FrameDecodeTiming::FrameSchedule::render_time,
+ Eq(render_time)))));
+
+ // Test cap keyframe.
+ clock_.AdvanceTime(frame_delay);
+ EXPECT_THAT(frame_decode_scheduler_.OnFrameBufferUpdated(
+ 180000, 270000, kMaxWaitForKeyFrame, false),
+ Optional(AllOf(
+ Field(&FrameDecodeTiming::FrameSchedule::latest_decode_time,
+ Eq(clock_.CurrentTime() + kMaxWaitForKeyFrame)),
+ Field(&FrameDecodeTiming::FrameSchedule::render_time,
+ Eq(render_time + frame_delay)))));
+}
+
} // namespace webrtc
diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc
index 2cf4ba5..ee70869 100644
--- a/video/video_receive_stream2.cc
+++ b/video/video_receive_stream2.cc
@@ -74,10 +74,8 @@
namespace {
// The default delay before re-requesting a key frame to be sent.
-constexpr TimeDelta kMaxWaitForKeyFrame = TimeDelta::Millis(200);
constexpr TimeDelta kMinBaseMinimumDelay = TimeDelta::Zero();
constexpr TimeDelta kMaxBaseMinimumDelay = TimeDelta::Seconds(10);
-constexpr TimeDelta kMaxWaitForFrame = TimeDelta::Seconds(3);
// Create a decoder for the preferred codec before the stream starts and any
// other decoder lazily on demand.
diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h
index 9add6a1..71dbf41 100644
--- a/video/video_receive_stream2.h
+++ b/video/video_receive_stream2.h
@@ -47,6 +47,9 @@
class RtxReceiveStream;
class VCMTiming;
+constexpr TimeDelta kMaxWaitForKeyFrame = TimeDelta::Millis(200);
+constexpr TimeDelta kMaxWaitForFrame = TimeDelta::Seconds(3);
+
namespace internal {
class CallStats;
