Make VCMJitterBuffer::SetNackMode(kNack, -1, -1) the only mode
Bug: webrtc:7408
Change-Id: I9d9e4f97c7705b42c9575167710a3e79781b83e8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/130220
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27568}diff --git a/modules/video_coding/jitter_buffer.cc b/modules/video_coding/jitter_buffer.cc
index bd9ffb1..933b976 100644
--- a/modules/video_coding/jitter_buffer.cc
+++ b/modules/video_coding/jitter_buffer.cc
@@ -36,10 +36,6 @@
// Use this rtt if no value has been reported.
static const int64_t kDefaultRtt = 200;
-// Request a keyframe if no continuous frame has been received for this
-// number of milliseconds and NACKs are disabled.
-static const int64_t kMaxDiscontinuousFramesTime = 1000;
-
typedef std::pair<uint32_t, VCMFrameBuffer*> FrameListPair;
bool IsKeyFrame(FrameListPair pair) {
@@ -234,9 +230,6 @@
jitter_estimate_(clock),
inter_frame_delay_(clock_->TimeInMilliseconds()),
rtt_ms_(kDefaultRtt),
- nack_mode_(kNoNack),
- low_rtt_nack_threshold_ms_(-1),
- high_rtt_nack_threshold_ms_(-1),
missing_sequence_numbers_(SequenceNumberLessThan()),
latest_received_sequence_number_(0),
max_nack_list_size_(0),
@@ -391,8 +384,7 @@
// Frame pulled out from jitter buffer, update the jitter estimate.
const bool retransmitted = (frame->GetNackCount() > 0);
if (retransmitted) {
- if (WaitForRetransmissions())
- jitter_estimate_.FrameNacked();
+ jitter_estimate_.FrameNacked();
} else if (frame->size() > 0) {
// Ignore retransmitted and empty frames.
if (waiting_for_completion_.latest_packet_time >= 0) {
@@ -589,11 +581,6 @@
FindAndInsertContinuousFrames(*frame);
} else {
incomplete_frames_.InsertFrame(frame);
- // If NACKs are enabled, keyframes are triggered by |GetNackList|.
- if (nack_mode_ == kNoNack && NonContinuousOrIncompleteDuration() >
- 90 * kMaxDiscontinuousFramesTime) {
- return kFlushIndicator;
- }
}
break;
}
@@ -604,11 +591,6 @@
return kNoError;
} else {
incomplete_frames_.InsertFrame(frame);
- // If NACKs are enabled, keyframes are triggered by |GetNackList|.
- if (nack_mode_ == kNoNack && NonContinuousOrIncompleteDuration() >
- 90 * kMaxDiscontinuousFramesTime) {
- return kFlushIndicator;
- }
}
break;
}
@@ -702,15 +684,7 @@
uint32_t VCMJitterBuffer::EstimatedJitterMs() {
rtc::CritScope cs(&crit_sect_);
- // Compute RTT multiplier for estimation.
- // low_rtt_nackThresholdMs_ == -1 means no FEC.
- double rtt_mult = 1.0f;
- if (low_rtt_nack_threshold_ms_ >= 0 &&
- rtt_ms_ >= low_rtt_nack_threshold_ms_) {
- // For RTTs above low_rtt_nack_threshold_ms_ we don't apply extra delay
- // when waiting for retransmissions.
- rtt_mult = 0.0f;
- }
+ const double rtt_mult = 1.0f;
return jitter_estimate_.GetJitterEstimate(rtt_mult);
}
@@ -718,32 +692,6 @@
rtc::CritScope cs(&crit_sect_);
rtt_ms_ = rtt_ms;
jitter_estimate_.UpdateRtt(rtt_ms);
- if (!WaitForRetransmissions())
- jitter_estimate_.ResetNackCount();
-}
-
-void VCMJitterBuffer::SetNackMode(VCMNackMode mode,
- int64_t low_rtt_nack_threshold_ms,
- int64_t high_rtt_nack_threshold_ms) {
- rtc::CritScope cs(&crit_sect_);
- nack_mode_ = mode;
- if (mode == kNoNack) {
- missing_sequence_numbers_.clear();
- }
- assert(low_rtt_nack_threshold_ms >= -1 && high_rtt_nack_threshold_ms >= -1);
- assert(high_rtt_nack_threshold_ms == -1 ||
- low_rtt_nack_threshold_ms <= high_rtt_nack_threshold_ms);
- assert(low_rtt_nack_threshold_ms > -1 || high_rtt_nack_threshold_ms == -1);
- low_rtt_nack_threshold_ms_ = low_rtt_nack_threshold_ms;
- high_rtt_nack_threshold_ms_ = high_rtt_nack_threshold_ms;
- // Don't set a high start rtt if high_rtt_nack_threshold_ms_ is used, to not
- // disable NACK in |kNack| mode.
- if (rtt_ms_ == kDefaultRtt && high_rtt_nack_threshold_ms_ != -1) {
- rtt_ms_ = 0;
- }
- if (!WaitForRetransmissions()) {
- jitter_estimate_.ResetNackCount();
- }
}
void VCMJitterBuffer::SetNackSettings(size_t max_nack_list_size,
@@ -757,11 +705,6 @@
max_incomplete_time_ms_ = max_incomplete_time_ms;
}
-VCMNackMode VCMJitterBuffer::nack_mode() const {
- rtc::CritScope cs(&crit_sect_);
- return nack_mode_;
-}
-
int VCMJitterBuffer::NonContinuousOrIncompleteDuration() {
if (incomplete_frames_.empty()) {
return 0;
@@ -787,9 +730,6 @@
std::vector<uint16_t> VCMJitterBuffer::GetNackList(bool* request_key_frame) {
rtc::CritScope cs(&crit_sect_);
*request_key_frame = false;
- if (nack_mode_ == kNoNack) {
- return std::vector<uint16_t>();
- }
if (last_decoded_state_.in_initial_state()) {
VCMFrameBuffer* next_frame = NextFrame();
const bool first_frame_is_key =
@@ -854,9 +794,6 @@
}
bool VCMJitterBuffer::UpdateNackList(uint16_t sequence_number) {
- if (nack_mode_ == kNoNack) {
- return true;
- }
// Make sure we don't add packets which are already too old to be decoded.
if (!last_decoded_state_.in_initial_state()) {
latest_received_sequence_number_ = LatestSequenceNumber(
@@ -1065,20 +1002,6 @@
}
}
-bool VCMJitterBuffer::WaitForRetransmissions() {
- if (nack_mode_ == kNoNack) {
- // NACK disabled -> don't wait for retransmissions.
- return false;
- }
- // Evaluate if the RTT is higher than |high_rtt_nack_threshold_ms_|, and in
- // that case we don't wait for retransmissions.
- if (high_rtt_nack_threshold_ms_ >= 0 &&
- rtt_ms_ >= high_rtt_nack_threshold_ms_) {
- return false;
- }
- return true;
-}
-
void VCMJitterBuffer::RecycleFrameBuffer(VCMFrameBuffer* frame) {
frame->Reset();
free_frames_.push_back(frame);
diff --git a/modules/video_coding/jitter_buffer.h b/modules/video_coding/jitter_buffer.h
index c0775f0..0d24776 100644
--- a/modules/video_coding/jitter_buffer.h
+++ b/modules/video_coding/jitter_buffer.h
@@ -32,8 +32,6 @@
namespace webrtc {
-enum VCMNackMode { kNack, kNoNack };
-
// forward declarations
class Clock;
class VCMFrameBuffer;
@@ -157,23 +155,10 @@
// Updates the round-trip time estimate.
void UpdateRtt(int64_t rtt_ms);
- // Set the NACK mode. |high_rtt_nack_threshold_ms| is an RTT threshold in ms
- // above which NACK will be disabled if the NACK mode is |kNack|, -1 meaning
- // that NACK is always enabled in the |kNack| mode.
- // |low_rtt_nack_threshold_ms| is an RTT threshold in ms below which we expect
- // to rely on NACK only, and therefore are using larger buffers to have time
- // to wait for retransmissions.
- void SetNackMode(VCMNackMode mode,
- int64_t low_rtt_nack_threshold_ms,
- int64_t high_rtt_nack_threshold_ms);
-
void SetNackSettings(size_t max_nack_list_size,
int max_packet_age_to_nack,
int max_incomplete_time_ms);
- // Returns the current NACK mode.
- VCMNackMode nack_mode() const;
-
// Returns a list of the sequence numbers currently missing.
std::vector<uint16_t> GetNackList(bool* request_key_frame);
@@ -268,9 +253,6 @@
unsigned int frame_size,
bool incomplete_frame);
- // Returns true if we should wait for retransmissions, false otherwise.
- bool WaitForRetransmissions();
-
int NonContinuousOrIncompleteDuration()
RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
@@ -309,10 +291,6 @@
VCMJitterSample waiting_for_completion_;
int64_t rtt_ms_;
- // NACK and retransmissions.
- VCMNackMode nack_mode_;
- int64_t low_rtt_nack_threshold_ms_;
- int64_t high_rtt_nack_threshold_ms_;
// Holds the internal NACK list (the missing sequence numbers).
SequenceNumberSet missing_sequence_numbers_;
uint16_t latest_received_sequence_number_;
diff --git a/modules/video_coding/jitter_buffer_unittest.cc b/modules/video_coding/jitter_buffer_unittest.cc
index 2ff66fe..f92c092 100644
--- a/modules/video_coding/jitter_buffer_unittest.cc
+++ b/modules/video_coding/jitter_buffer_unittest.cc
@@ -395,7 +395,6 @@
TestJitterBufferNack() {}
virtual void SetUp() {
TestRunningJitterBuffer::SetUp();
- jitter_buffer_->SetNackMode(kNack, -1, -1);
}
virtual void TearDown() { TestRunningJitterBuffer::TearDown(); }
@@ -661,6 +660,7 @@
}
TEST_F(TestBasicJitterBuffer, TestReorderingWithPadding) {
+ jitter_buffer_->SetNackSettings(kMaxNumberOfFrames, kMaxNumberOfFrames, 0);
packet_->frameType = VideoFrameType::kVideoFrameKey;
packet_->video_header.is_first_packet_in_frame = true;
packet_->markerBit = true;
@@ -828,6 +828,7 @@
// -------------------------------------------------
// |<----------tl0idx:200--------->|<---tl0idx:201---
+ jitter_buffer_->SetNackSettings(kMaxNumberOfFrames, kMaxNumberOfFrames, 0);
auto& vp9_header =
packet_->video_header.video_type_header.emplace<RTPVideoHeaderVP9>();
@@ -1507,7 +1508,6 @@
// Make sure the jitter doesn't request a keyframe after too much non-
// decodable frames.
- jitter_buffer_->SetNackMode(kNack, -1, -1);
jitter_buffer_->SetNackSettings(kMaxNumberOfFrames, kMaxNumberOfFrames, 0);
int loop = 0;
@@ -1580,6 +1580,7 @@
// received the marker bit, unless we have received a packet from a later
// timestamp.
// Start with a complete key frame - insert and decode.
+ jitter_buffer_->SetNackSettings(kMaxNumberOfFrames, kMaxNumberOfFrames, 0);
packet_->frameType = VideoFrameType::kVideoFrameKey;
packet_->video_header.is_first_packet_in_frame = true;
packet_->markerBit = true;
@@ -1611,7 +1612,6 @@
TEST_F(TestRunningJitterBuffer, Full) {
// Make sure the jitter doesn't request a keyframe after too much non-
// decodable frames.
- jitter_buffer_->SetNackMode(kNack, -1, -1);
jitter_buffer_->SetNackSettings(kMaxNumberOfFrames, kMaxNumberOfFrames, 0);
// Insert a key frame and decode it.
EXPECT_GE(InsertFrame(VideoFrameType::kVideoFrameKey), kNoError);
@@ -1704,7 +1704,6 @@
TEST_F(TestJitterBufferNack, EmptyPackets) {
// Make sure empty packets doesn't clog the jitter buffer.
- jitter_buffer_->SetNackMode(kNack, media_optimization::kLowRttNackMs, -1);
EXPECT_GE(InsertFrames(kMaxNumberOfFrames, VideoFrameType::kEmptyFrame),
kNoError);
InsertFrame(VideoFrameType::kVideoFrameKey);
@@ -1893,8 +1892,6 @@
}
TEST_F(TestJitterBufferNack, NormalOperation) {
- EXPECT_EQ(kNack, jitter_buffer_->nack_mode());
-
EXPECT_GE(InsertFrame(VideoFrameType::kVideoFrameKey), kNoError);
EXPECT_TRUE(DecodeCompleteFrame());
diff --git a/modules/video_coding/receiver.cc b/modules/video_coding/receiver.cc
index decaa0d..189e502 100644
--- a/modules/video_coding/receiver.cc
+++ b/modules/video_coding/receiver.cc
@@ -196,15 +196,6 @@
jitter_buffer_.ReleaseFrame(frame);
}
-void VCMReceiver::SetNackMode(VCMNackMode nackMode,
- int64_t low_rtt_nack_threshold_ms,
- int64_t high_rtt_nack_threshold_ms) {
- rtc::CritScope cs(&crit_sect_);
- // Default to always having NACK enabled in hybrid mode.
- jitter_buffer_.SetNackMode(nackMode, low_rtt_nack_threshold_ms,
- high_rtt_nack_threshold_ms);
-}
-
void VCMReceiver::SetNackSettings(size_t max_nack_list_size,
int max_packet_age_to_nack,
int max_incomplete_time_ms) {
@@ -212,11 +203,6 @@
max_incomplete_time_ms);
}
-VCMNackMode VCMReceiver::NackMode() const {
- rtc::CritScope cs(&crit_sect_);
- return jitter_buffer_.nack_mode();
-}
-
std::vector<uint16_t> VCMReceiver::NackList(bool* request_key_frame) {
return jitter_buffer_.GetNackList(request_key_frame);
}
diff --git a/modules/video_coding/receiver.h b/modules/video_coding/receiver.h
index 0094e4c..857ddfb 100644
--- a/modules/video_coding/receiver.h
+++ b/modules/video_coding/receiver.h
@@ -50,13 +50,9 @@
void ReleaseFrame(VCMEncodedFrame* frame);
// NACK.
- void SetNackMode(VCMNackMode nackMode,
- int64_t low_rtt_nack_threshold_ms,
- int64_t high_rtt_nack_threshold_ms);
void SetNackSettings(size_t max_nack_list_size,
int max_packet_age_to_nack,
int max_incomplete_time_ms);
- VCMNackMode NackMode() const;
std::vector<uint16_t> NackList(bool* request_key_frame);
void TriggerDecoderShutdown();
diff --git a/modules/video_coding/receiver_unittest.cc b/modules/video_coding/receiver_unittest.cc
index 4fc9c33..ae322f2 100644
--- a/modules/video_coding/receiver_unittest.cc
+++ b/modules/video_coding/receiver_unittest.cc
@@ -87,8 +87,6 @@
};
TEST_F(TestVCMReceiver, NonDecodableDuration_Empty) {
- // Enable NACK and with no RTT thresholds for disabling retransmission delay.
- receiver_.SetNackMode(kNack, -1, -1);
const size_t kMaxNackListSize = 1000;
const int kMaxPacketAgeToNack = 1000;
const int kMaxNonDecodableDuration = 500;
@@ -105,8 +103,6 @@
}
TEST_F(TestVCMReceiver, NonDecodableDuration_NoKeyFrame) {
- // Enable NACK and with no RTT thresholds for disabling retransmission delay.
- receiver_.SetNackMode(kNack, -1, -1);
const size_t kMaxNackListSize = 1000;
const int kMaxPacketAgeToNack = 1000;
const int kMaxNonDecodableDuration = 500;
@@ -122,8 +118,6 @@
}
TEST_F(TestVCMReceiver, NonDecodableDuration_OneIncomplete) {
- // Enable NACK and with no RTT thresholds for disabling retransmission delay.
- receiver_.SetNackMode(kNack, -1, -1);
const size_t kMaxNackListSize = 1000;
const int kMaxPacketAgeToNack = 1000;
const int kMaxNonDecodableDuration = 500;
@@ -152,8 +146,6 @@
}
TEST_F(TestVCMReceiver, NonDecodableDuration_NoTrigger) {
- // Enable NACK and with no RTT thresholds for disabling retransmission delay.
- receiver_.SetNackMode(kNack, -1, -1);
const size_t kMaxNackListSize = 1000;
const int kMaxPacketAgeToNack = 1000;
const int kMaxNonDecodableDuration = 500;
@@ -184,8 +176,6 @@
}
TEST_F(TestVCMReceiver, NonDecodableDuration_NoTrigger2) {
- // Enable NACK and with no RTT thresholds for disabling retransmission delay.
- receiver_.SetNackMode(kNack, -1, -1);
const size_t kMaxNackListSize = 1000;
const int kMaxPacketAgeToNack = 1000;
const int kMaxNonDecodableDuration = 500;
@@ -216,8 +206,6 @@
}
TEST_F(TestVCMReceiver, NonDecodableDuration_KeyFrameAfterIncompleteFrames) {
- // Enable NACK and with no RTT thresholds for disabling retransmission delay.
- receiver_.SetNackMode(kNack, -1, -1);
const size_t kMaxNackListSize = 1000;
const int kMaxPacketAgeToNack = 1000;
const int kMaxNonDecodableDuration = 500;
diff --git a/modules/video_coding/video_coding_impl.cc b/modules/video_coding/video_coding_impl.cc
index dc60541..41a8eac 100644
--- a/modules/video_coding/video_coding_impl.cc
+++ b/modules/video_coding/video_coding_impl.cc
@@ -97,7 +97,8 @@
}
int SetReceiverRobustnessMode(ReceiverRobustness robustnessMode) override {
- return receiver_.SetReceiverRobustnessMode(robustnessMode);
+ RTC_CHECK_EQ(robustnessMode, kHardNack);
+ return VCM_OK;
}
void SetNackSettings(size_t max_nack_list_size,
diff --git a/modules/video_coding/video_coding_impl.h b/modules/video_coding/video_coding_impl.h
index 4d83047..a77de5e 100644
--- a/modules/video_coding/video_coding_impl.h
+++ b/modules/video_coding/video_coding_impl.h
@@ -80,17 +80,11 @@
int32_t SetRenderDelay(uint32_t timeMS);
int32_t Delay() const;
- // DEPRECATED.
- int SetReceiverRobustnessMode(
- VideoCodingModule::ReceiverRobustness robustnessMode);
-
void SetNackSettings(size_t max_nack_list_size,
int max_packet_age_to_nack,
int max_incomplete_time_ms);
int32_t SetReceiveChannelParameters(int64_t rtt);
- int32_t SetVideoProtection(VCMVideoProtection videoProtection, bool enable);
-
int64_t TimeUntilNextProcess() override;
void Process() override;
void ProcessThreadAttached(ProcessThread* process_thread) override;
diff --git a/modules/video_coding/video_receiver.cc b/modules/video_coding/video_receiver.cc
index 36b7f0a..72a004b 100644
--- a/modules/video_coding/video_receiver.cc
+++ b/modules/video_coding/video_receiver.cc
@@ -124,12 +124,11 @@
int64_t VideoReceiver::TimeUntilNextProcess() {
RTC_DCHECK_RUN_ON(&module_thread_checker_);
int64_t timeUntilNextProcess = _receiveStatsTimer.TimeUntilProcess();
- if (_receiver.NackMode() != kNoNack) {
- // We need a Process call more often if we are relying on
- // retransmissions
- timeUntilNextProcess =
- VCM_MIN(timeUntilNextProcess, _retransmissionTimer.TimeUntilProcess());
- }
+ // We need a Process call more often if we are relying on
+ // retransmissions
+ timeUntilNextProcess =
+ VCM_MIN(timeUntilNextProcess, _retransmissionTimer.TimeUntilProcess());
+
timeUntilNextProcess =
VCM_MIN(timeUntilNextProcess, _keyRequestTimer.TimeUntilProcess());
@@ -142,34 +141,6 @@
return 0;
}
-// Enable or disable a video protection method.
-// Note: This API should be deprecated, as it does not offer a distinction
-// between the protection method and decoding with or without errors.
-int32_t VideoReceiver::SetVideoProtection(VCMVideoProtection videoProtection,
- bool enable) {
- switch (videoProtection) {
- case kProtectionNack: {
- RTC_DCHECK(enable);
- _receiver.SetNackMode(kNack, -1, -1);
- break;
- }
-
- case kProtectionNackFEC: {
- RTC_DCHECK(enable);
- _receiver.SetNackMode(kNack, media_optimization::kLowRttNackMs,
- media_optimization::kMaxRttDelayThreshold);
- break;
- }
- case kProtectionFEC:
- case kProtectionNone:
- // No receiver-side protection.
- RTC_DCHECK(enable);
- _receiver.SetNackMode(kNoNack, -1, -1);
- break;
- }
- return VCM_OK;
-}
-
// Register a receive callback. Will be called whenever there is a new frame
// ready for rendering.
int32_t VideoReceiver::RegisterReceiveCallback(
@@ -404,25 +375,6 @@
return _timing->TargetVideoDelay();
}
-int VideoReceiver::SetReceiverRobustnessMode(
- VideoCodingModule::ReceiverRobustness robustnessMode) {
- RTC_DCHECK_RUN_ON(&construction_thread_checker_);
- RTC_DCHECK(!IsDecoderThreadRunning());
- switch (robustnessMode) {
- case VideoCodingModule::kNone:
- _receiver.SetNackMode(kNoNack, -1, -1);
- break;
- case VideoCodingModule::kHardNack:
- // Always wait for retransmissions (except when decoding with errors).
- _receiver.SetNackMode(kNack, -1, -1);
- break;
- default:
- RTC_NOTREACHED();
- return VCM_PARAMETER_ERROR;
- }
- return VCM_OK;
-}
-
void VideoReceiver::SetNackSettings(size_t max_nack_list_size,
int max_packet_age_to_nack,
int max_incomplete_time_ms) {
diff --git a/modules/video_coding/video_receiver_unittest.cc b/modules/video_coding/video_receiver_unittest.cc
index 1ca8728..f0251d6 100644
--- a/modules/video_coding/video_receiver_unittest.cc
+++ b/modules/video_coding/video_receiver_unittest.cc
@@ -44,7 +44,6 @@
const size_t kMaxNackListSize = 250;
const int kMaxPacketAgeToNack = 450;
receiver_.SetNackSettings(kMaxNackListSize, kMaxPacketAgeToNack, 0);
- EXPECT_EQ(0, receiver_.SetVideoProtection(kProtectionNack, true));
EXPECT_EQ(
0, receiver_.RegisterPacketRequestCallback(&packet_request_callback_));
diff --git a/video/video_stream_decoder.cc b/video/video_stream_decoder.cc
index dd65a29..4faa3bc 100644
--- a/video/video_stream_decoder.cc
+++ b/video/video_stream_decoder.cc
@@ -21,7 +21,7 @@
vcm::VideoReceiver* video_receiver,
VCMPacketRequestCallback* vcm_packet_request_callback,
bool enable_nack,
- bool enable_fec,
+ bool /* enable_fec */,
ReceiveStatisticsProxy* receive_statistics_proxy,
rtc::VideoSinkInterface<VideoFrame>* incoming_video_stream)
: video_receiver_(video_receiver),
@@ -34,11 +34,6 @@
video_receiver_->SetNackSettings(kMaxNackListSize, kMaxPacketAgeToNack, 0);
video_receiver_->RegisterReceiveCallback(this);
- VCMVideoProtection video_protection =
- enable_nack ? (enable_fec ? kProtectionNackFEC : kProtectionNack)
- : kProtectionNone;
-
- video_receiver_->SetVideoProtection(video_protection, true);
VCMPacketRequestCallback* packet_request_callback =
enable_nack ? vcm_packet_request_callback : nullptr;
video_receiver_->RegisterPacketRequestCallback(packet_request_callback);
