From 7a4db6eb0ef5a998019f03428072f0cc6afae866 Mon Sep 17 00:00:00 2001 From: Ilya Nikolaevskiy Date: Fri, 8 Nov 2019 16:20:45 +0100 Subject: [PATCH] Reset RtpFrameReferenceFinder on long pause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:11074 Change-Id: I4c9a8761e9039d32885ccf9ac0eebdffdf67f48d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/159240 Commit-Queue: Ilya Nikolaevskiy Reviewed-by: Erik Språng Cr-Commit-Position: refs/heads/master@{#29747} --- video/rtp_video_stream_receiver.cc | 37 ++++++++++++++++++++++-------- video/rtp_video_stream_receiver.h | 3 ++- video/video_receive_stream.cc | 9 ++++---- 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index 5902886da4..ea33eb5449 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -40,6 +40,7 @@ #include "rtc_base/logging.h" #include "rtc_base/strings/string_builder.h" #include "rtc_base/system/fallthrough.h" +#include "rtc_base/time_utils.h" #include "system_wrappers/include/field_trial.h" #include "system_wrappers/include/metrics.h" #include "video/receive_statistics_proxy.h" @@ -52,6 +53,10 @@ namespace { constexpr int kPacketBufferStartSize = 512; constexpr int kPacketBufferMaxSize = 2048; +// Maximum time between frames before resetting reference_finder to avoid RTP +// fields wraparounds to affect FrameBuffer. +constexpr TimeDelta kInactiveStreamThreshold = TimeDelta::Seconds<5>(); + int PacketBufferMaxSize() { // The group here must be a positive power of 2, in which case that is used as // size. All other values shall result in the default value being used. @@ -214,7 +219,9 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver( rtcp_feedback_buffer_(this, nack_sender, this), packet_buffer_(clock_, kPacketBufferStartSize, PacketBufferMaxSize()), has_received_frame_(false), - frames_decryptable_(false) { + frames_decryptable_(false), + last_completed_picture_id_(0), + last_assembled_frame_time_(Timestamp::MinusInfinity()) { constexpr bool remb_candidate = true; if (packet_router_) packet_router_->AddReceiveRtpModule(rtp_rtcp_.get(), remb_candidate); @@ -597,6 +604,15 @@ void RtpVideoStreamReceiver::OnAssembledFrame( RTC_DCHECK_RUN_ON(&network_tc_); RTC_DCHECK(frame); + bool recreate_reference_finder_requested = false; + + Timestamp now = clock_->CurrentTime(); + if (last_assembled_frame_time_.IsFinite() && + now - last_assembled_frame_time_ > kInactiveStreamThreshold) { + recreate_reference_finder_requested = true; + } + last_assembled_frame_time_ = now; + absl::optional descriptor = frame->GetGenericFrameDescriptor(); @@ -629,14 +645,7 @@ void RtpVideoStreamReceiver::OnAssembledFrame( if (frame->codec_type() != current_codec_) { if (frame_is_newer) { - // When we reset the |reference_finder_| we don't want new picture ids - // to overlap with old picture ids. To ensure that doesn't happen we - // start from the |last_completed_picture_id_| and add an offset in case - // of reordering. - reference_finder_ = - std::make_unique( - this, last_completed_picture_id_ + - std::numeric_limits::max()); + recreate_reference_finder_requested = true; current_codec_ = frame->codec_type(); } else { // Old frame from before the codec switch, discard it. @@ -652,6 +661,16 @@ void RtpVideoStreamReceiver::OnAssembledFrame( last_assembled_frame_rtp_timestamp_ = frame->Timestamp(); } + if (recreate_reference_finder_requested) { + // When we reset the |reference_finder_| we don't want new picture ids + // to overlap with old picture ids. To ensure that doesn't happen we + // start from the |last_completed_picture_id_| and add an offset in case + // of reordering. + reference_finder_ = std::make_unique( + this, + last_completed_picture_id_ + std::numeric_limits::max()); + } + if (buffered_frame_decryptor_ == nullptr) { reference_finder_->ManageFrame(std::move(frame)); } else { diff --git a/video/rtp_video_stream_receiver.h b/video/rtp_video_stream_receiver.h index 7021c3c7dc..b7f38f84be 100644 --- a/video/rtp_video_stream_receiver.h +++ b/video/rtp_video_stream_receiver.h @@ -319,7 +319,8 @@ class RtpVideoStreamReceiver : public LossNotificationSender, std::atomic frames_decryptable_; absl::optional last_color_space_; - int64_t last_completed_picture_id_ = 0; + int64_t last_completed_picture_id_; + Timestamp last_assembled_frame_time_ RTC_GUARDED_BY(network_tc_); }; } // namespace webrtc diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index a60bb07911..aef1c518d7 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -159,10 +159,9 @@ class EncodedFrameForMediaTransport : public video_coding::EncodedFrame { int64_t RenderTime() const override { return 0; } }; -// TODO(https://bugs.webrtc.org/9974): Consider removing this workaround. // Maximum time between frames before resetting the FrameBuffer to avoid RTP -// timestamps wraparound to affect FrameBuffer. -constexpr int kInactiveStreamThresholdMs = 600000; // 10 minutes. +// timestamps and picture IDs wraparounds to affect FrameBuffer. +constexpr int kInactiveStreamThresholdMs = 5000; // 5 seconds. } // namespace @@ -546,7 +545,9 @@ void VideoReceiveStream::RequestKeyFrame() { void VideoReceiveStream::OnCompleteFrame( std::unique_ptr frame) { RTC_DCHECK_RUN_ON(&network_sequence_checker_); - // TODO(https://bugs.webrtc.org/9974): Consider removing this workaround. + + // Resetting of stream state if there was long enough pause in the stream. + // This is done to avoid undetected wraparounds in RTP fields. int64_t time_now_ms = rtc::TimeMillis(); if (last_complete_frame_time_ms_ > 0 && time_now_ms - last_complete_frame_time_ms_ > kInactiveStreamThresholdMs) {