commit b70a36e770f6ea382a5e98b4959ba233a56996b5
author Jakob Ivarsson <jakobi@webrtc.org> Tue Apr 11 14:36:02 2023
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Apr 11 16:41:30 2023
tree 261c04b39b4facda449a1cad3bb2d9dd4683e7bb
parent 77c47947ad098e4182a6244cb998e4fa8c7bd37e

Require exact timestamp to be available when extracting multiple packets for decoding.

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 <henrik.lundin@webrtc.org>
Commit-Queue: Jakob Ivarsson‎ <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39815}

modules/audio_coding/neteq/neteq_impl.cc

diff --git a/modules/audio_coding/neteq/neteq_impl.cc b/modules/audio_coding/neteq/neteq_impl.cc
index 328bdb1..113215d 100644
--- a/modules/audio_coding/neteq/neteq_impl.cc
+++ b/modules/audio_coding/neteq/neteq_impl.cc
@@ -1940,9 +1940,6 @@
 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 @@
         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 @@
                               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) {