From 018cd3d6fc86245d5023af54a24706e7090b9adc Mon Sep 17 00:00:00 2001 From: Jakob Ivarsson Date: Mon, 13 Sep 2021 16:06:28 +0200 Subject: [PATCH] Avoid NACKing after DTX. This is done by not adding missing packets to the NACK list if the number of samples per packet is too large. Bug: webrtc:10178 Change-Id: If46398d6d05ea35f30d7028040d3b808559e950b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/231841 Reviewed-by: Ivo Creusen Commit-Queue: Jakob Ivarsson Cr-Commit-Position: refs/heads/main@{#34984} --- modules/audio_coding/neteq/nack_tracker.cc | 42 ++++++++++++------- modules/audio_coding/neteq/nack_tracker.h | 23 +++++----- .../neteq/nack_tracker_unittest.cc | 15 +++++++ 3 files changed, 53 insertions(+), 27 deletions(-) diff --git a/modules/audio_coding/neteq/nack_tracker.cc b/modules/audio_coding/neteq/nack_tracker.cc index c95679ac17..1df07c970f 100644 --- a/modules/audio_coding/neteq/nack_tracker.cc +++ b/modules/audio_coding/neteq/nack_tracker.cc @@ -22,7 +22,7 @@ namespace webrtc { namespace { const int kDefaultSampleRateKhz = 48; -const int kDefaultPacketSizeMs = 20; +const int kMaxPacketSizeMs = 120; constexpr char kNackTrackerConfigFieldTrial[] = "WebRTC-Audio-NetEqNackTrackerConfig"; @@ -52,7 +52,6 @@ NackTracker::NackTracker(int nack_threshold_packets) timestamp_last_decoded_rtp_(0), any_rtp_decoded_(false), sample_rate_khz_(kDefaultSampleRateKhz), - samples_per_packet_(sample_rate_khz_ * kDefaultPacketSizeMs), max_nack_list_size_(kNackListSizeLimit) {} NackTracker::~NackTracker() = default; @@ -95,33 +94,39 @@ void NackTracker::UpdateLastReceivedPacket(uint16_t sequence_number, UpdatePacketLossRate(sequence_number - sequence_num_last_received_rtp_ - 1); - UpdateSamplesPerPacket(sequence_number, timestamp); - - UpdateList(sequence_number); + UpdateList(sequence_number, timestamp); sequence_num_last_received_rtp_ = sequence_number; timestamp_last_received_rtp_ = timestamp; LimitNackListSize(); } -void NackTracker::UpdateSamplesPerPacket( +absl::optional NackTracker::GetSamplesPerPacket( uint16_t sequence_number_current_received_rtp, - uint32_t timestamp_current_received_rtp) { + uint32_t timestamp_current_received_rtp) const { uint32_t timestamp_increase = timestamp_current_received_rtp - timestamp_last_received_rtp_; uint16_t sequence_num_increase = sequence_number_current_received_rtp - sequence_num_last_received_rtp_; - samples_per_packet_ = timestamp_increase / sequence_num_increase; + int samples_per_packet = timestamp_increase / sequence_num_increase; + if (samples_per_packet == 0 || + samples_per_packet > kMaxPacketSizeMs * sample_rate_khz_) { + // Not a valid samples per packet. + return absl::nullopt; + } + return samples_per_packet; } -void NackTracker::UpdateList(uint16_t sequence_number_current_received_rtp) { +void NackTracker::UpdateList(uint16_t sequence_number_current_received_rtp, + uint32_t timestamp_current_received_rtp) { // Some of the packets which were considered late, now are considered missing. ChangeFromLateToMissing(sequence_number_current_received_rtp); if (IsNewerSequenceNumber(sequence_number_current_received_rtp, sequence_num_last_received_rtp_ + 1)) - AddToList(sequence_number_current_received_rtp); + AddToList(sequence_number_current_received_rtp, + timestamp_current_received_rtp); } void NackTracker::ChangeFromLateToMissing( @@ -134,16 +139,24 @@ void NackTracker::ChangeFromLateToMissing( it->second.is_missing = true; } -uint32_t NackTracker::EstimateTimestamp(uint16_t sequence_num) { +uint32_t NackTracker::EstimateTimestamp(uint16_t sequence_num, + int samples_per_packet) { uint16_t sequence_num_diff = sequence_num - sequence_num_last_received_rtp_; - return sequence_num_diff * samples_per_packet_ + timestamp_last_received_rtp_; + return sequence_num_diff * samples_per_packet + timestamp_last_received_rtp_; } -void NackTracker::AddToList(uint16_t sequence_number_current_received_rtp) { +void NackTracker::AddToList(uint16_t sequence_number_current_received_rtp, + uint32_t timestamp_current_received_rtp) { RTC_DCHECK(!any_rtp_decoded_ || IsNewerSequenceNumber(sequence_number_current_received_rtp, sequence_num_last_decoded_rtp_)); + absl::optional samples_per_packet = GetSamplesPerPacket( + sequence_number_current_received_rtp, timestamp_current_received_rtp); + if (!samples_per_packet) { + return; + } + // Packets with sequence numbers older than `upper_bound_missing` are // considered missing, and the rest are considered late. uint16_t upper_bound_missing = @@ -152,7 +165,7 @@ void NackTracker::AddToList(uint16_t sequence_number_current_received_rtp) { for (uint16_t n = sequence_num_last_received_rtp_ + 1; IsNewerSequenceNumber(sequence_number_current_received_rtp, n); ++n) { bool is_missing = IsNewerSequenceNumber(upper_bound_missing, n); - uint32_t timestamp = EstimateTimestamp(n); + uint32_t timestamp = EstimateTimestamp(n, *samples_per_packet); NackElement nack_element(TimeToPlay(timestamp), timestamp, is_missing); nack_list_.insert(nack_list_.end(), std::make_pair(n, nack_element)); } @@ -211,7 +224,6 @@ void NackTracker::Reset() { timestamp_last_decoded_rtp_ = 0; any_rtp_decoded_ = false; sample_rate_khz_ = kDefaultSampleRateKhz; - samples_per_packet_ = sample_rate_khz_ * kDefaultPacketSizeMs; } void NackTracker::SetMaxNackListSize(size_t max_nack_list_size) { diff --git a/modules/audio_coding/neteq/nack_tracker.h b/modules/audio_coding/neteq/nack_tracker.h index f3cce74560..34e4de6ae6 100644 --- a/modules/audio_coding/neteq/nack_tracker.h +++ b/modules/audio_coding/neteq/nack_tracker.h @@ -17,6 +17,7 @@ #include #include +#include "absl/types/optional.h" #include "modules/include/module_common_types_public.h" #include "rtc_base/gtest_prod_util.h" @@ -159,22 +160,24 @@ class NackTracker { // Given the `sequence_number_current_received_rtp` of currently received RTP, // recognize packets which are not arrive and add to the list. - void AddToList(uint16_t sequence_number_current_received_rtp); + void AddToList(uint16_t sequence_number_current_received_rtp, + uint32_t timestamp_current_received_rtp); // This function subtracts 10 ms of time-to-play for all packets in NACK list. // This is called when 10 ms elapsed with no new RTP packet decoded. void UpdateEstimatedPlayoutTimeBy10ms(); - // Given the `sequence_number_current_received_rtp` and - // `timestamp_current_received_rtp` of currently received RTP update number - // of samples per packet. - void UpdateSamplesPerPacket(uint16_t sequence_number_current_received_rtp, - uint32_t timestamp_current_received_rtp); + // Returns a valid number of samples per packet given the current received + // sequence number and timestamp or nullopt of none could be computed. + absl::optional GetSamplesPerPacket( + uint16_t sequence_number_current_received_rtp, + uint32_t timestamp_current_received_rtp) const; // Given the `sequence_number_current_received_rtp` of currently received RTP // update the list. That is; some packets will change from late to missing, // some packets are inserted as missing and some inserted as late. - void UpdateList(uint16_t sequence_number_current_received_rtp); + void UpdateList(uint16_t sequence_number_current_received_rtp, + uint32_t timestamp_current_received_rtp); // Packets which are considered late for too long (according to // `nack_threshold_packets_`) are flagged as missing. @@ -186,7 +189,7 @@ class NackTracker { void LimitNackListSize(); // Estimate timestamp of a missing packet given its sequence number. - uint32_t EstimateTimestamp(uint16_t sequence_number); + uint32_t EstimateTimestamp(uint16_t sequence_number, int samples_per_packet); // Compute time-to-play given a timestamp. int64_t TimeToPlay(uint32_t timestamp) const; @@ -215,10 +218,6 @@ class NackTracker { int sample_rate_khz_; // Sample rate in kHz. - // Number of samples per packet. We update this every time we receive a - // packet, not only for consecutive packets. - int samples_per_packet_; - // A list of missing packets to be retransmitted. Components of the list // contain the sequence number of missing packets and the estimated time that // each pack is going to be played out. diff --git a/modules/audio_coding/neteq/nack_tracker_unittest.cc b/modules/audio_coding/neteq/nack_tracker_unittest.cc index 6ee8f2af0a..9fc3ae224f 100644 --- a/modules/audio_coding/neteq/nack_tracker_unittest.cc +++ b/modules/audio_coding/neteq/nack_tracker_unittest.cc @@ -539,4 +539,19 @@ TEST(NackTrackerTest, PacketLossRateCorrect) { EXPECT_NEAR(nack->GetPacketLossRateForTest(), 1 << 28, (1 << 30) / 100); } +TEST(NackTrackerTest, DoNotNackAfterDtx) { + const int kNackListSize = 200; + std::unique_ptr nack(NackTracker::Create(0)); + nack->UpdateSampleRate(kSampleRateHz); + nack->SetMaxNackListSize(kNackListSize); + uint16_t seq_num = 0; + uint32_t timestamp = 0x87654321; + nack->UpdateLastReceivedPacket(seq_num, timestamp); + EXPECT_TRUE(nack->GetNackList(0).empty()); + constexpr int kDtxPeriod = 400; + nack->UpdateLastReceivedPacket(seq_num + 2, + timestamp + kDtxPeriod * kSampleRateHz / 1000); + EXPECT_TRUE(nack->GetNackList(0).empty()); +} + } // namespace webrtc