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}

video/frame_buffer_proxy.cc

@@ -468,7 +468,8 @@ class FrameBuffer3Proxy : public FrameBufferProxy {
     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() !=

video/frame_decode_timing.cc

@@ -28,7 +28,9 @@ FrameDecodeTiming::FrameDecodeTiming(Clock* clock,
 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 @@ FrameDecodeTiming::OnFrameBufferUpdated(uint32_t next_temporal_unit_rtp,
                        << " 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};
 }

video/frame_decode_timing.h

@@ -41,6 +41,7 @@ class FrameDecodeTiming {
   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:

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 @@ TEST_F(FrameDecodeTimingTest, ReturnsWaitTimesWhenValid) {
   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 @@ TEST_F(FrameDecodeTimingTest, FastForwardsFrameTooFarInThePast) {
   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 @@ TEST_F(FrameDecodeTimingTest, NoFastForwardIfOnlyFrameToDecode) {
   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 @@ TEST_F(FrameDecodeTimingTest, NoFastForwardIfOnlyFrameToDecode) {
                         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

video/video_receive_stream2.cc

@@ -74,10 +74,8 @@ namespace internal {
 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.

video/video_receive_stream2.h

@@ -47,6 +47,9 @@ class RtpStreamReceiverControllerInterface;
 class RtxReceiveStream;
 class VCMTiming;
 
+constexpr TimeDelta kMaxWaitForKeyFrame = TimeDelta::Millis(200);
+constexpr TimeDelta kMaxWaitForFrame = TimeDelta::Seconds(3);
+
 namespace internal {
 
 class CallStats;