Simplify speculative RFI tracking by looking at missing shards of any type

src/RtpVideoQueue.c

@@ -159,10 +159,12 @@ static bool queuePacket(PRTP_VIDEO_QUEUE queue, PRTPV_QUEUE_ENTRY newEntry, PRTP
 
 // Returns 0 if the frame is completely constructed
 static int reconstructFrame(PRTP_VIDEO_QUEUE queue) {
-    unsigned int totalPackets = U16(queue->bufferHighestSequenceNumber - queue->bufferLowestSequenceNumber) + 1;
+    unsigned int totalPackets = queue->bufferDataPackets + queue->bufferParityPackets;
     unsigned int neededPackets = queue->bufferDataPackets;
     int ret;
 
+    LC_ASSERT(totalPackets == U16(queue->bufferHighestSequenceNumber - queue->bufferLowestSequenceNumber) + 1);
+    
 #ifdef FEC_VALIDATION_MODE
     // We'll need an extra packet to run in FEC validation mode, because we will
     // be "dropping" one below and recovering it using parity. However, some frames
@@ -170,22 +172,23 @@ static int reconstructFrame(PRTP_VIDEO_QUEUE queue) {
     neededPackets += queue->fecPercentage ? 1 : 0;
 #endif
 
+    LC_ASSERT(totalPackets - neededPackets <= queue->bufferParityPackets);
+
     if (queue->pendingFecBlockList.count < neededPackets) {
         // If we've never received OOS data from this host, we can predict whether this frame will be recoverable
-        // based on the packets we've received (or not) so far.
+        // based on the packets we've received (or not) so far. If the number of missing shards exceeds the total
+        // needed shards, there is no hope of recovering the data. The only way we could recover this frame is by
+        // receiving OOS data, which is unlikely because we've not seen any recently from this host.
         if (!queue->reportedLostFrame && !queue->receivedOosData) {
-            if (queue->missingDataPackets > queue->bufferParityPackets) {
+            // NB: We use totalPackets - neededPackets instead of just bufferParityPackets here because we require
-                // If the number of missing data shards exceeds the total number of possible parity shards,
+            // one extra parity shard for recovery if we're in FEC validation mode.
-                // we will presume the frame is lost.
+            if (queue->missingPackets > totalPackets - neededPackets) {
                 notifyFrameLost(queue->currentFrameNumber, true);
                 queue->reportedLostFrame = true;
             }
-            else if (queue->receivedParityPackets != 0 &&
+            else {
-                     neededPackets - queue->pendingFecBlockList.count > U16(queue->bufferHighestSequenceNumber - queue->receivedParityHighestSequenceNumber)) {
+                // Assert that there are enough remaining packets to possibly recover this frame.
-                // If we've seen some parity shards but not enough parity shards remain to recover this frame
+                LC_ASSERT(neededPackets - queue->pendingFecBlockList.count <= U16(queue->bufferHighestSequenceNumber - queue->receivedHighestSequenceNumber));
-                // without additional data shards, we will presume the frame is lost.
-                notifyFrameLost(queue->currentFrameNumber, true);
-                queue->reportedLostFrame = true;
             }
         }
 
@@ -195,9 +198,9 @@ static int reconstructFrame(PRTP_VIDEO_QUEUE queue) {
 
     // If we make it here and reported a lost frame, we lied to the host. This can happen if we happen to get
     // unlucky and this particular frame happens to be the one with OOS data, but it should almost never happen.
-    LC_ASSERT(queue->receivedParityPackets >= queue->missingDataPackets);
+    LC_ASSERT(queue->missingPackets <= queue->bufferParityPackets);
-    LC_ASSERT(!queue->reportedLostFrame);
+    LC_ASSERT(!queue->reportedLostFrame || queue->receivedOosData);
-    if (queue->reportedLostFrame) {
+    if (queue->reportedLostFrame && !queue->receivedOosData) {
         // If it turns out that we lied to the host, stop further speculative RFI requests for a while.
         queue->receivedOosData = true;
         queue->lastOosFramePresentationTimestamp = queue->pendingFecBlockList.head->presentationTimeMs;
@@ -207,12 +210,11 @@ static int reconstructFrame(PRTP_VIDEO_QUEUE queue) {
 #ifdef FEC_VALIDATION_MODE
     // If FEC is disabled or unsupported for this frame, we must bail early here.
     if ((queue->fecPercentage == 0 || AppVersionQuad[0] < 5) &&
-            queue->receivedBufferDataPackets == queue->bufferDataPackets) {
+            queue->receivedDataPackets == queue->bufferDataPackets) {
 #else
-    if (queue->receivedBufferDataPackets == queue->bufferDataPackets) {
+    if (queue->receivedDataPackets == queue->bufferDataPackets) {
 #endif
         // We've received a full frame with no need for FEC.
-        LC_ASSERT(queue->missingDataPackets == 0);
         return 0;
     }
 
@@ -297,9 +299,9 @@ static int reconstructFrame(PRTP_VIDEO_QUEUE queue) {
     LC_ASSERT(ret == 0);
 
 #ifdef FEC_VERBOSE
-    if (queue->bufferDataPackets != queue->receivedBufferDataPackets) {
+    if (queue->bufferDataPackets != queue->receivedDataPackets) {
         Limelog("Recovered %d video data shards from frame %d\n",
-                queue->bufferDataPackets - queue->receivedBufferDataPackets,
+                queue->bufferDataPackets - queue->receivedDataPackets,
                 queue->currentFrameNumber);
     }
 #endif
@@ -545,7 +547,7 @@ int RtpvAddPacket(PRTP_VIDEO_QUEUE queue, PRTP_PACKET packet, int length, PRTPV_
                 Limelog("Unrecoverable frame %d (block %d of %d): %d+%d=%d received < %d needed\n",
                         queue->currentFrameNumber, queue->multiFecCurrentBlockNumber+1,
                         queue->multiFecLastBlockNumber+1,
-                        queue->receivedBufferDataPackets,
+                        queue->receivedDataPackets,
                         queue->receivedParityPackets,
                         queue->pendingFecBlockList.count,
                         queue->bufferDataPackets);
@@ -571,7 +573,7 @@ int RtpvAddPacket(PRTP_VIDEO_QUEUE queue, PRTP_PACKET packet, int length, PRTPV_
             }
             else {
                 Limelog("Unrecoverable frame %d: %d+%d=%d received < %d needed\n",
-                        queue->currentFrameNumber, queue->receivedBufferDataPackets,
+                        queue->currentFrameNumber, queue->receivedDataPackets,
                         queue->receivedParityPackets,
                         queue->pendingFecBlockList.count,
                         queue->bufferDataPackets);
@@ -636,11 +638,10 @@ int RtpvAddPacket(PRTP_VIDEO_QUEUE queue, PRTP_PACKET packet, int length, PRTPV_
         queue->bufferFirstRecvTimeMs = PltGetMillis();
         queue->bufferLowestSequenceNumber = U16(packet->sequenceNumber - fecIndex);
         queue->nextContiguousSequenceNumber = queue->bufferLowestSequenceNumber;
-        queue->receivedBufferDataPackets = 0;
+        queue->receivedDataPackets = 0;
         queue->receivedParityPackets = 0;
-        queue->receivedDataHighestSequenceNumber = 0;
+        queue->receivedHighestSequenceNumber = 0;
-        queue->receivedParityHighestSequenceNumber = 0;
+        queue->missingPackets = 0;
-        queue->missingDataPackets = 0;
         queue->reportedLostFrame = false;
         queue->bufferDataPackets = (nvPacket->fecInfo & 0xFFC00000) >> 22;
         queue->fecPercentage = (nvPacket->fecInfo & 0xFF0) >> 4;
@@ -672,32 +673,37 @@ int RtpvAddPacket(PRTP_VIDEO_QUEUE queue, PRTP_PACKET packet, int length, PRTPV_
         return RTPF_RET_REJECTED;
     }
     else {
-        if (isBefore16(packet->sequenceNumber, queue->bufferFirstParitySequenceNumber)) {
+        // Update total missing packet count
-            queue->receivedBufferDataPackets++;
+        if (queue->pendingFecBlockList.count == 1) {
-
+            // Initialize counts and highest seqnum on the first packet
-            if (queue->receivedBufferDataPackets == 1) {
+            LC_ASSERT(queue->missingPackets == 0);
-                queue->missingDataPackets += U16(packet->sequenceNumber - queue->bufferLowestSequenceNumber);
+            LC_ASSERT(queue->receivedHighestSequenceNumber == 0);
-                queue->receivedDataHighestSequenceNumber = packet->sequenceNumber;
+            queue->missingPackets += U16(packet->sequenceNumber - queue->bufferLowestSequenceNumber);
+            queue->receivedHighestSequenceNumber = packet->sequenceNumber;
         }
-            else if (isBefore16(queue->receivedDataHighestSequenceNumber, packet->sequenceNumber)) {
+        else if (isBefore16(queue->receivedHighestSequenceNumber, packet->sequenceNumber)) {
-                // If we receive a packet above the highest data sequence number,
+            // If we receive a packet above the highest sequence number,
             // adjust our missing packets count based on that new sequence number.
-                queue->missingDataPackets += U16(packet->sequenceNumber - queue->receivedDataHighestSequenceNumber - 1);
+            queue->missingPackets += U16(packet->sequenceNumber - queue->receivedHighestSequenceNumber - 1);
-                queue->receivedDataHighestSequenceNumber = packet->sequenceNumber;
+            queue->receivedHighestSequenceNumber = packet->sequenceNumber;
         }
         else {
-                // If we receive a packet behind the highest data sequence number and
+            // If we receive a packet behind the highest sequence number, but
             // queuePacket() accepted it, we must have received a missing packet.
-                queue->missingDataPackets--;
+            LC_ASSERT(queue->missingPackets > 0);
+            queue->missingPackets--;
         }
 
-            LC_ASSERT(queue->missingDataPackets < queue->bufferDataPackets);
+        // We explicitly assert less-than because we know we received at least one packet (this one)
+        LC_ASSERT(queue->missingPackets < queue->bufferDataPackets + queue->bufferParityPackets);
+
+        if (isBefore16(packet->sequenceNumber, queue->bufferFirstParitySequenceNumber)) {
+            queue->receivedDataPackets++;
+            LC_ASSERT(queue->receivedDataPackets <= queue->bufferDataPackets);
         }
         else {
             queue->receivedParityPackets++;
-            if (queue->receivedParityPackets == 1 || isBefore16(queue->receivedParityHighestSequenceNumber, packet->sequenceNumber)) {
+            LC_ASSERT(queue->receivedParityPackets <= queue->bufferParityPackets);
-                queue->receivedParityHighestSequenceNumber = packet->sequenceNumber;
-            }
         }
         
         // Try to submit this frame. If we haven't received enough packets,

src/RtpVideoQueue.h

@@ -28,13 +28,12 @@ typedef struct _RTP_VIDEO_QUEUE {
     uint32_t bufferFirstParitySequenceNumber;
     uint32_t bufferDataPackets;
     uint32_t bufferParityPackets;
-    uint32_t receivedBufferDataPackets;
+    uint32_t receivedDataPackets;
     uint32_t receivedParityPackets;
-    uint32_t receivedParityHighestSequenceNumber;
+    uint32_t receivedHighestSequenceNumber;
-    uint32_t receivedDataHighestSequenceNumber;
     uint32_t fecPercentage;
     uint32_t nextContiguousSequenceNumber;
-    uint32_t missingDataPackets; // # of holes behind receivedDataHighestSequenceNumber
+    uint32_t missingPackets; // # of holes behind receivedHighestSequenceNumber
     bool reportedLostFrame;
 
     uint32_t currentFrameNumber;