From d6c4b1641d27f619f9f33ad2b36e8e5e326ac148 Mon Sep 17 00:00:00 2001 From: Jakob Ivarsson Date: Mon, 27 Mar 2023 16:39:26 +0200 Subject: [PATCH] Remove decoded timestamp extrapolation from NackTracker. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UpdateLastDecodedPacket is anyway only called when a new packet is decoded. Bug: webrtc:10178 Change-Id: I8cfcc5791e71079034a2d0806c44b3b071ac2ffb Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299180 Reviewed-by: Henrik Lundin Commit-Queue: Jakob Ivarsson‎ Cr-Commit-Position: refs/heads/main@{#39703} --- modules/audio_coding/neteq/nack_tracker.cc | 47 +++++-------------- modules/audio_coding/neteq/nack_tracker.h | 7 +-- .../neteq/nack_tracker_unittest.cc | 13 ----- 3 files changed, 14 insertions(+), 53 deletions(-) diff --git a/modules/audio_coding/neteq/nack_tracker.cc b/modules/audio_coding/neteq/nack_tracker.cc index 04cc5b52e8..71c6fc3048 100644 --- a/modules/audio_coding/neteq/nack_tracker.cc +++ b/modules/audio_coding/neteq/nack_tracker.cc @@ -145,43 +145,22 @@ uint32_t NackTracker::EstimateTimestamp(uint16_t sequence_num, return sequence_num_diff * samples_per_packet + timestamp_last_received_rtp_; } -void NackTracker::UpdateEstimatedPlayoutTimeBy10ms() { - while (!nack_list_.empty() && - nack_list_.begin()->second.time_to_play_ms <= 10) - nack_list_.erase(nack_list_.begin()); - - for (NackList::iterator it = nack_list_.begin(); it != nack_list_.end(); ++it) - it->second.time_to_play_ms -= 10; -} - void NackTracker::UpdateLastDecodedPacket(uint16_t sequence_number, uint32_t timestamp) { - if (IsNewerSequenceNumber(sequence_number, sequence_num_last_decoded_rtp_) || - !any_rtp_decoded_) { - sequence_num_last_decoded_rtp_ = sequence_number; - timestamp_last_decoded_rtp_ = timestamp; - // Packets in the list with sequence numbers less than the - // sequence number of the decoded RTP should be removed from the lists. - // They will be discarded by the jitter buffer if they arrive. - nack_list_.erase(nack_list_.begin(), - nack_list_.upper_bound(sequence_num_last_decoded_rtp_)); - - // Update estimated time-to-play. - for (NackList::iterator it = nack_list_.begin(); it != nack_list_.end(); - ++it) - it->second.time_to_play_ms = TimeToPlay(it->second.estimated_timestamp); - } else { - RTC_DCHECK_EQ(sequence_number, sequence_num_last_decoded_rtp_); - - // Same sequence number as before. 10 ms is elapsed, update estimations for - // time-to-play. - UpdateEstimatedPlayoutTimeBy10ms(); - - // Update timestamp for better estimate of time-to-play, for packets which - // are added to NACK list later on. - timestamp_last_decoded_rtp_ += sample_rate_khz_ * 10; - } any_rtp_decoded_ = true; + sequence_num_last_decoded_rtp_ = sequence_number; + timestamp_last_decoded_rtp_ = timestamp; + // Packets in the list with sequence numbers less than the + // sequence number of the decoded RTP should be removed from the lists. + // They will be discarded by the jitter buffer if they arrive. + nack_list_.erase(nack_list_.begin(), + nack_list_.upper_bound(sequence_num_last_decoded_rtp_)); + + // Update estimated time-to-play. + for (NackList::iterator it = nack_list_.begin(); it != nack_list_.end(); + ++it) { + it->second.time_to_play_ms = TimeToPlay(it->second.estimated_timestamp); + } } NackTracker::NackList NackTracker::GetNackList() const { diff --git a/modules/audio_coding/neteq/nack_tracker.h b/modules/audio_coding/neteq/nack_tracker.h index 14ba2166d1..d9005da085 100644 --- a/modules/audio_coding/neteq/nack_tracker.h +++ b/modules/audio_coding/neteq/nack_tracker.h @@ -72,8 +72,7 @@ class NackTracker { // After Reset() is called sampling rate has to be set. void UpdateSampleRate(int sample_rate_hz); - // Update the sequence number and the timestamp of the last decoded RTP. This - // API should be called every time 10 ms audio is pulled from NetEq. + // Update the sequence number and the timestamp of the last decoded RTP. void UpdateLastDecodedPacket(uint16_t sequence_number, uint32_t timestamp); // Update the sequence number and the timestamp of the last received RTP. This @@ -149,10 +148,6 @@ class NackTracker { // computed correctly. NackList GetNackList() const; - // 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(); - // 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( diff --git a/modules/audio_coding/neteq/nack_tracker_unittest.cc b/modules/audio_coding/neteq/nack_tracker_unittest.cc index bcc5120ff3..a843425b78 100644 --- a/modules/audio_coding/neteq/nack_tracker_unittest.cc +++ b/modules/audio_coding/neteq/nack_tracker_unittest.cc @@ -238,19 +238,6 @@ TEST(NackTrackerTest, EstimateTimestampAndTimeToPlay) { EXPECT_EQ((index + 2) * kPacketSizeMs, it->second.time_to_play_ms); ++it; } - - // Pretend 10 ms is passed, and we had pulled audio from NetEq, it still - // reports the same sequence number as decoded, time-to-play should be - // updated by 10 ms. - nack.UpdateLastDecodedPacket(first_seq_num, first_timestamp); - nack_list = nack.GetNackList(); - it = nack_list.begin(); - while (it != nack_list.end()) { - seq_num = it->first - seq_num_offset; - int index = seq_num - kLostPackets[0]; - EXPECT_EQ((index + 2) * kPacketSizeMs - 10, it->second.time_to_play_ms); - ++it; - } } }