Revert "Reset RtpFrameReferenceFinder on long pause"

This reverts commit 7a4db6eb0ef5a998019f03428072f0cc6afae866.

Reason for revert: Caused regression on perf tests.

Original change's description:
> 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}

TBR=ilnik@webrtc.org,sprang@webrtc.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: webrtc:11074
Change-Id: Ic40779087bf8e6bd94f02d38161f6abb9ca395f1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/159690
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29775}
This commit is contained in:
Ilya Nikolaevskiy 2019-11-12 15:20:21 +00:00 committed by Commit Bot
parent 059daa48a9
commit 815e00c102
3 changed files with 14 additions and 35 deletions

View File

@ -40,7 +40,6 @@
#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"
@ -53,10 +52,6 @@ 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.
@ -219,9 +214,7 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver(
rtcp_feedback_buffer_(this, nack_sender, this),
packet_buffer_(clock_, kPacketBufferStartSize, PacketBufferMaxSize()),
has_received_frame_(false),
frames_decryptable_(false),
last_completed_picture_id_(0),
last_assembled_frame_time_(Timestamp::MinusInfinity()) {
frames_decryptable_(false) {
constexpr bool remb_candidate = true;
if (packet_router_)
packet_router_->AddReceiveRtpModule(rtp_rtcp_.get(), remb_candidate);
@ -604,15 +597,6 @@ 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<RtpGenericFrameDescriptor> descriptor =
frame->GetGenericFrameDescriptor();
@ -645,7 +629,14 @@ void RtpVideoStreamReceiver::OnAssembledFrame(
if (frame->codec_type() != current_codec_) {
if (frame_is_newer) {
recreate_reference_finder_requested = true;
// 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());
current_codec_ = frame->codec_type();
} else {
// Old frame from before the codec switch, discard it.
@ -661,16 +652,6 @@ 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<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 {

View File

@ -319,8 +319,7 @@ class RtpVideoStreamReceiver : public LossNotificationSender,
std::atomic<bool> frames_decryptable_;
absl::optional<ColorSpace> last_color_space_;
int64_t last_completed_picture_id_;
Timestamp last_assembled_frame_time_ RTC_GUARDED_BY(network_tc_);
int64_t last_completed_picture_id_ = 0;
};
} // namespace webrtc

View File

@ -159,9 +159,10 @@ 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 and picture IDs wraparounds to affect FrameBuffer.
constexpr int kInactiveStreamThresholdMs = 5000; // 5 seconds.
// timestamps wraparound to affect FrameBuffer.
constexpr int kInactiveStreamThresholdMs = 600000; // 10 minutes.
} // namespace
@ -545,9 +546,7 @@ void VideoReceiveStream::RequestKeyFrame() {
void VideoReceiveStream::OnCompleteFrame(
std::unique_ptr<video_coding::EncodedFrame> frame) {
RTC_DCHECK_RUN_ON(&network_sequence_checker_);
// Resetting of stream state if there was long enough pause in the stream.
// This is done to avoid undetected wraparounds in RTP fields.
// TODO(https://bugs.webrtc.org/9974): Consider removing this workaround.
int64_t time_now_ms = rtc::TimeMillis();
if (last_complete_frame_time_ms_ > 0 &&
time_now_ms - last_complete_frame_time_ms_ > kInactiveStreamThresholdMs) {