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 <ivoc@webrtc.org>
Commit-Queue: Jakob Ivarsson <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#34984}
diff --git a/modules/audio_coding/neteq/nack_tracker.cc b/modules/audio_coding/neteq/nack_tracker.cc
index c95679a..1df07c9 100644
--- a/modules/audio_coding/neteq/nack_tracker.cc
+++ b/modules/audio_coding/neteq/nack_tracker.cc
@@ -22,7 +22,7 @@
namespace {
const int kDefaultSampleRateKhz = 48;
-const int kDefaultPacketSizeMs = 20;
+const int kMaxPacketSizeMs = 120;
constexpr char kNackTrackerConfigFieldTrial[] =
"WebRTC-Audio-NetEqNackTrackerConfig";
@@ -52,7 +52,6 @@
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 @@
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<int> 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 @@
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<int> 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 @@
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 @@
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 f3cce74..34e4de6 100644
--- a/modules/audio_coding/neteq/nack_tracker.h
+++ b/modules/audio_coding/neteq/nack_tracker.h
@@ -17,6 +17,7 @@
#include <map>
#include <vector>
+#include "absl/types/optional.h"
#include "modules/include/module_common_types_public.h"
#include "rtc_base/gtest_prod_util.h"
@@ -159,22 +160,24 @@
// 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<int> 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 @@
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 @@
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 6ee8f2a..9fc3ae2 100644
--- a/modules/audio_coding/neteq/nack_tracker_unittest.cc
+++ b/modules/audio_coding/neteq/nack_tracker_unittest.cc
@@ -539,4 +539,19 @@
EXPECT_NEAR(nack->GetPacketLossRateForTest(), 1 << 28, (1 << 30) / 100);
}
+TEST(NackTrackerTest, DoNotNackAfterDtx) {
+ const int kNackListSize = 200;
+ std::unique_ptr<NackTracker> 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
