From 884b8d0a950a47496fb96cd5d9a32c6745ba590a Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Wed, 5 Sep 2018 12:04:39 -0700 Subject: [PATCH] Additional fixes for corrupt FEC recovery handling --- src/RtpFecQueue.c | 2 +- src/SdpGenerator.c | 4 ++++ src/VideoDepacketizer.c | 24 +++++++++++++++++------- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/RtpFecQueue.c b/src/RtpFecQueue.c index ff60d70..d3782ab 100644 --- a/src/RtpFecQueue.c +++ b/src/RtpFecQueue.c @@ -72,11 +72,11 @@ static int queuePacket(PRTP_FEC_QUEUE queue, PRTPFEC_QUEUE_ENTRY newEntry, int h } #define PACKET_RECOVERY_FAILURE() \ - free(packets[i]); \ ret = -1; \ Limelog("FEC recovery returned corrupt packet %d" \ " (frame %d)", rtpPacket->sequenceNumber, \ queue->currentFrameNumber); \ + free(packets[i]); \ continue // Returns 0 if the frame is completely constructed diff --git a/src/SdpGenerator.c b/src/SdpGenerator.c index 54b656b..f2d0b39 100644 --- a/src/SdpGenerator.c +++ b/src/SdpGenerator.c @@ -162,6 +162,10 @@ static int addGen5Options(PSDP_OPTION* head) { err |= addAttributeString(head, "x-nv-vqos[0].fec.repairPercent", "5"); } + // Recovery mode can cause the FEC percentage to change mid-frame, which + // breaks many assumptions in RTP FEC queue. + err |= addAttributeString(head, "x-nv-general.enableRecoveryMode", "0"); + return err; } diff --git a/src/VideoDepacketizer.c b/src/VideoDepacketizer.c index bafcd41..91899b0 100644 --- a/src/VideoDepacketizer.c +++ b/src/VideoDepacketizer.c @@ -471,9 +471,23 @@ void processRtpPayload(PNV_VIDEO_PACKET videoPacket, int length, unsigned long l streamPacketIndex = videoPacket->streamPacketIndex; - // The packets and frames must be in sequence from the FEC queue - LC_ASSERT(!isBefore24(streamPacketIndex, U24(lastPacketInStream + 1))); - LC_ASSERT(!isBefore32(frameIndex, nextFrameNumber)); + // Drop packets from a previously corrupt frame + if (isBefore32(frameIndex, nextFrameNumber)) { + return; + } + + // The FEC queue can sometimes recover corrupt frames (see comments in RtpFecQueue). + // It almost always detects them before they get to us, but in case it doesn't + // the streamPacketIndex not matching correctly should find nearly all of the rest. + if (isBefore24(streamPacketIndex, U24(lastPacketInStream + 1)) || + (!firstPacket && streamPacketIndex != U24(lastPacketInStream + 1))) { + Limelog("Depacketizer detected corrupt frame: %d", frameIndex); + decodingFrame = 0; + nextFrameNumber = frameIndex + 1; + waitingForNextSuccessfulFrame = 1; + dropFrameState(); + return; + } // Notify the listener of the latest frame we've seen from the PC connectionSawFrame(frameIndex); @@ -502,10 +516,6 @@ void processRtpPayload(PNV_VIDEO_PACKET videoPacket, int length, unsigned long l firstPacketReceiveTime = receiveTimeMs; } - // This must be the first packet in a frame or be contiguous with the last - // packet received. - LC_ASSERT(firstPacket || streamPacketIndex == U24(lastPacketInStream + 1)); - lastPacketInStream = streamPacketIndex; // If this is the first packet, skip the frame header (if one exists)