Fix handling of duplicate packets when OOS data was also received

It's possible for a sequence of packets seqnums like 2, 0, 1, 2 to end up hitting the fast path
again after filling in gap of OOS packets. The fast path assumes duplicate packets are caught
by the next contiguous sequence number check, but this is only true if that value is kept updated.

Since the slow path doesn't update the next contiguous sequence number, it's no longer safe to
use the fast path for the remaining packets in the frame because duplicate packets would be
mishandled.

Queuing duplicate packets violates all sorts of queue assumptions and can cause us to return
corrupt data to the decoder or attempt an FEC recovery without enough shards.
This commit is contained in:
Cameron Gutman
2023-09-23 12:08:16 -04:00
parent 7a6d12fc4e
commit 1be56269a3
2 changed files with 18 additions and 6 deletions

View File

@@ -118,13 +118,14 @@ static bool queuePacket(PRTP_VIDEO_QUEUE queue, PRTPV_QUEUE_ENTRY newEntry, PRTP
// If the packet is in order, we can take the fast path and avoid having
// to loop through the whole list. If we get an out of order or missing
// packet, the fast path will stop working and we'll use the loop instead.
if (packet->sequenceNumber == queue->nextContiguousSequenceNumber) {
//
// NB: It's not enough to just check next contiguous sequence number because
// it's possible that we hit the OOS path earlier which doesn't update the
// next contiguous sequence number. If that happens, we need to use the slow
// path for this entire frame to avoid possibly mishandling a duplicate packet.
if (queue->useFastQueuePath && packet->sequenceNumber == queue->nextContiguousSequenceNumber) {
queue->nextContiguousSequenceNumber = U16(packet->sequenceNumber + 1);
// If we received the next contiguous sequence number but already have missing
// packets, that means we received some later packets before falling back into
// sequence with this one. By definition, that's OOS data so let's tag it.
outOfSequence = queue->missingPackets != 0;
outOfSequence = false;
}
else {
outOfSequence = false;
@@ -141,6 +142,11 @@ static bool queuePacket(PRTP_VIDEO_QUEUE queue, PRTPV_QUEUE_ENTRY newEntry, PRTP
entry = entry->next;
}
// If we make it here, we cannot use the fast queue path for this frame because
// we're about to queue a non-duplicate packet out of order. This will not update
// nextContiguousSequenceNumber which the fast path relies on.
queue->useFastQueuePath = false;
}
newEntry->packet = packet;
@@ -295,6 +301,10 @@ static int reconstructFrame(PRTP_VIDEO_QUEUE queue) {
}
#endif
// We should never have duplicate packets enqueued
LC_ASSERT(packets[index] == NULL);
LC_ASSERT(marks[index] != 0);
packets[index] = (unsigned char*) entry->packet;
marks[index] = 0;
@@ -681,6 +691,7 @@ int RtpvAddPacket(PRTP_VIDEO_QUEUE queue, PRTP_PACKET packet, int length, PRTPV_
queue->receivedParityPackets = 0;
queue->receivedHighestSequenceNumber = 0;
queue->missingPackets = 0;
queue->useFastQueuePath = true;
queue->reportedLostFrame = false;
queue->bufferDataPackets = (nvPacket->fecInfo & 0xFFC00000) >> 22;
queue->fecPercentage = (nvPacket->fecInfo & 0xFF0) >> 4;

View File

@@ -34,6 +34,7 @@ typedef struct _RTP_VIDEO_QUEUE {
uint32_t fecPercentage;
uint32_t nextContiguousSequenceNumber;
uint32_t missingPackets; // # of holes behind receivedHighestSequenceNumber
bool useFastQueuePath;
bool reportedLostFrame;
uint32_t currentFrameNumber;