Reset RtpFrameReferenceFinder on long pause
Bug: webrtc:11074
Change-Id: I4c9a8761e9039d32885ccf9ac0eebdffdf67f48d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/159240
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29747}
diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc
index 5902886..ea33eb5 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 @@
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 @@
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 @@
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<RtpGenericFrameDescriptor> descriptor =
frame->GetGenericFrameDescriptor();
@@ -629,14 +645,7 @@
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<video_coding::RtpFrameReferenceFinder>(
- this, last_completed_picture_id_ +
- std::numeric_limits<uint16_t>::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 @@
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<video_coding::RtpFrameReferenceFinder>(
+ this,
+ last_completed_picture_id_ + std::numeric_limits<uint16_t>::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 7021c3c..b7f38f8 100644
--- a/video/rtp_video_stream_receiver.h
+++ b/video/rtp_video_stream_receiver.h
@@ -319,7 +319,8 @@
std::atomic<bool> frames_decryptable_;
absl::optional<ColorSpace> 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 a60bb07..aef1c51 100644
--- a/video/video_receive_stream.cc
+++ b/video/video_receive_stream.cc
@@ -159,10 +159,9 @@
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::OnCompleteFrame(
std::unique_ptr<video_coding::EncodedFrame> 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) {
