From b70a36e770f6ea382a5e98b4959ba233a56996b5 Mon Sep 17 00:00:00 2001 From: Jakob Ivarsson Date: Tue, 11 Apr 2023 16:36:02 +0200 Subject: [PATCH] Require exact timestamp to be available when extracting multiple packets for decoding. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the only remaining dependency on sequence number in NetEq except for the NackTracker (which arguably doesn't belong in NetEq). This fixes a potential issue where FEC is not perfectly aligned with the original packet boundaries, causing both the FEC and the original packet to be decoded. Bug: webrtc:13322 Change-Id: I3abec9ebfc194976fca42d5f4f4ed4ee136f44ef Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300560 Reviewed-by: Henrik Lundin Commit-Queue: Jakob Ivarsson‎ Cr-Commit-Position: refs/heads/main@{#39815} --- modules/audio_coding/neteq/neteq_impl.cc | 30 ++++++------------------ 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/modules/audio_coding/neteq/neteq_impl.cc b/modules/audio_coding/neteq/neteq_impl.cc index 328bdb104c..113215db8e 100644 --- a/modules/audio_coding/neteq/neteq_impl.cc +++ b/modules/audio_coding/neteq/neteq_impl.cc @@ -1940,9 +1940,6 @@ int NetEqImpl::DtmfOverdub(const DtmfEvent& dtmf_event, int NetEqImpl::ExtractPackets(size_t required_samples, PacketList* packet_list) { bool first_packet = true; - uint8_t prev_payload_type = 0; - uint32_t prev_timestamp = 0; - uint16_t prev_sequence_number = 0; bool next_packet_available = false; const Packet* next_packet = packet_buffer_->PeekNextPacket(); @@ -1978,9 +1975,6 @@ int NetEqImpl::ExtractPackets(size_t required_samples, nack_->UpdateLastDecodedPacket(packet->sequence_number, packet->timestamp); } - prev_sequence_number = packet->sequence_number; - prev_timestamp = packet->timestamp; - prev_payload_type = packet->payload_type; } const bool has_cng_packet = @@ -2012,25 +2006,15 @@ int NetEqImpl::ExtractPackets(size_t required_samples, controller_->TargetLevelMs(), controller_->UnlimitedTargetLevelMs()); - packet_list->push_back(std::move(*packet)); // Store packet in list. - packet = absl::nullopt; // Ensure it's never used after the move. - // Check what packet is available next. next_packet = packet_buffer_->PeekNextPacket(); - next_packet_available = false; - if (next_packet && prev_payload_type == next_packet->payload_type && - !has_cng_packet) { - int16_t seq_no_diff = next_packet->sequence_number - prev_sequence_number; - size_t ts_diff = next_packet->timestamp - prev_timestamp; - if ((seq_no_diff == 1 || seq_no_diff == 0) && - ts_diff <= packet_duration) { - // The next sequence number is available, or the next part of a packet - // that was split into pieces upon insertion. - next_packet_available = true; - } - prev_sequence_number = next_packet->sequence_number; - prev_timestamp = next_packet->timestamp; - } + next_packet_available = + next_packet && next_packet->payload_type == packet->payload_type && + next_packet->timestamp == packet->timestamp + packet_duration && + !has_cng_packet; + + packet_list->push_back(std::move(*packet)); // Store packet in list. + packet = absl::nullopt; // Ensure it's never used after the move. } while (extracted_samples < required_samples && next_packet_available); if (extracted_samples > 0) {