diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn index 0c964adc10..640d6626de 100644 --- a/modules/video_coding/BUILD.gn +++ b/modules/video_coding/BUILD.gn @@ -1136,6 +1136,7 @@ if (rtc_include_tests) { "fec_controller_unittest.cc", "frame_buffer2_unittest.cc", "frame_dependencies_calculator_unittest.cc", + "frame_helpers_unittest.cc", "generic_decoder_unittest.cc", "h264_packet_buffer_unittest.cc", "h264_sprop_parameter_sets_unittest.cc", @@ -1179,6 +1180,7 @@ if (rtc_include_tests) { ":encoded_frame", ":frame_buffer2", ":frame_dependencies_calculator", + ":frame_helpers", ":h264_packet_buffer", ":nack_requester", ":packet_buffer", diff --git a/modules/video_coding/frame_buffer2.cc b/modules/video_coding/frame_buffer2.cc index 9ecbf40bb8..b289663eec 100644 --- a/modules/video_coding/frame_buffer2.cc +++ b/modules/video_coding/frame_buffer2.cc @@ -254,8 +254,11 @@ std::unique_ptr FrameBuffer::GetNextFrame() { absl::optional render_time = first_frame.RenderTimestamp(); int64_t receive_time_ms = first_frame.ReceivedTime(); // Gracefully handle bad RTP timestamps and render time issues. - if (!render_time || - FrameHasBadRenderTiming(*render_time, now, timing_->TargetVideoDelay())) { + if (!render_time || FrameHasBadRenderTiming(*render_time, now) || + TargetVideoDelayIsTooLarge(timing_->TargetVideoDelay())) { + RTC_LOG(LS_WARNING) << "Resetting jitter estimator and timing module due " + "to bad render timing for rtp_timestamp=" + << first_frame.Timestamp(); jitter_estimator_.Reset(); timing_->Reset(); render_time = timing_->RenderTime(first_frame.Timestamp(), now); diff --git a/modules/video_coding/frame_helpers.cc b/modules/video_coding/frame_helpers.cc index 08b47ef547..e25eac8a18 100644 --- a/modules/video_coding/frame_helpers.cc +++ b/modules/video_coding/frame_helpers.cc @@ -16,9 +16,11 @@ namespace webrtc { -bool FrameHasBadRenderTiming(Timestamp render_time, - Timestamp now, - TimeDelta target_video_delay) { +namespace { +constexpr TimeDelta kMaxVideoDelay = TimeDelta::Millis(10000); +} + +bool FrameHasBadRenderTiming(Timestamp render_time, Timestamp now) { // Zero render time means render immediately. if (render_time.IsZero()) { return false; @@ -26,19 +28,23 @@ bool FrameHasBadRenderTiming(Timestamp render_time, if (render_time < Timestamp::Zero()) { return true; } - constexpr TimeDelta kMaxVideoDelay = TimeDelta::Millis(10000); - TimeDelta frame_delay = (render_time - now).Abs(); - if (frame_delay > kMaxVideoDelay) { - RTC_LOG(LS_WARNING) - << "A frame about to be decoded is out of the configured " - "delay bounds (" - << frame_delay.ms() << " > " << kMaxVideoDelay.ms() - << "). Resetting the video jitter buffer."; + TimeDelta frame_delay = render_time - now; + if (frame_delay.Abs() > kMaxVideoDelay) { + RTC_LOG(LS_WARNING) << "Frame has bad render timing because it is out of " + "the delay bounds (frame_delay_ms=" + << frame_delay.ms() + << ", kMaxVideoDelay_ms=" << kMaxVideoDelay.ms() << ")"; return true; } + return false; +} + +bool TargetVideoDelayIsTooLarge(TimeDelta target_video_delay) { if (target_video_delay > kMaxVideoDelay) { - RTC_LOG(LS_WARNING) << "The video target delay has grown larger than " - << kMaxVideoDelay.ms() << " ms."; + RTC_LOG(LS_WARNING) + << "Target video delay is too large. (target_video_delay_ms=" + << target_video_delay.ms() + << ", kMaxVideoDelay_ms=" << kMaxVideoDelay.ms() << ")"; return true; } return false; diff --git a/modules/video_coding/frame_helpers.h b/modules/video_coding/frame_helpers.h index b6d7b0f144..56ee593678 100644 --- a/modules/video_coding/frame_helpers.h +++ b/modules/video_coding/frame_helpers.h @@ -18,9 +18,9 @@ namespace webrtc { -bool FrameHasBadRenderTiming(Timestamp render_time, - Timestamp now, - TimeDelta target_video_delay); +bool FrameHasBadRenderTiming(Timestamp render_time, Timestamp now); + +bool TargetVideoDelayIsTooLarge(TimeDelta target_video_delay); std::unique_ptr CombineAndDeleteFrames( absl::InlinedVector, 4> frames); diff --git a/modules/video_coding/frame_helpers_unittest.cc b/modules/video_coding/frame_helpers_unittest.cc new file mode 100644 index 0000000000..1f73689c0a --- /dev/null +++ b/modules/video_coding/frame_helpers_unittest.cc @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2022 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "modules/video_coding/frame_helpers.h" + +#include "api/units/timestamp.h" +#include "test/gtest.h" + +namespace webrtc { +namespace { + +TEST(FrameHasBadRenderTimingTest, LargePositiveFrameDelayIsBad) { + Timestamp render_time = Timestamp::Seconds(12); + Timestamp now = Timestamp::Seconds(0); + + EXPECT_TRUE(FrameHasBadRenderTiming(render_time, now)); +} + +TEST(FrameHasBadRenderTimingTest, LargeNegativeFrameDelayIsBad) { + Timestamp render_time = Timestamp::Seconds(12); + Timestamp now = Timestamp::Seconds(24); + + EXPECT_TRUE(FrameHasBadRenderTiming(render_time, now)); +} + +} // namespace +} // namespace webrtc diff --git a/video/video_stream_buffer_controller.cc b/video/video_stream_buffer_controller.cc index 955cd43ea2..05810816b7 100644 --- a/video/video_stream_buffer_controller.cc +++ b/video/video_stream_buffer_controller.cc @@ -206,7 +206,11 @@ void VideoStreamBufferController::OnFrameReady( keyframe_required_ = false; // Gracefully handle bad RTP timestamps and render time issues. - if (FrameHasBadRenderTiming(render_time, now, timing_->TargetVideoDelay())) { + if (FrameHasBadRenderTiming(render_time, now) || + TargetVideoDelayIsTooLarge(timing_->TargetVideoDelay())) { + RTC_LOG(LS_WARNING) << "Resetting jitter estimator and timing module due " + "to bad render timing for rtp_timestamp=" + << first_frame.Timestamp(); jitter_estimator_.Reset(); timing_->Reset(); render_time = timing_->RenderTime(first_frame.Timestamp(), now);