From c4692a5b5f5abc048eb980a5884fc65d506703f2 Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Mon, 19 Feb 2018 00:16:32 -0800 Subject: [PATCH] Rework sequencing code and fix audio dropout across 64K boundary in the process --- src/Limelight-internal.h | 13 ++++++++++++- src/Misc.c | 17 ----------------- src/RtpFecQueue.c | 29 +++++++++++++---------------- src/RtpReorderQueue.c | 25 ++++++++++--------------- src/Video.h | 4 ++-- src/VideoDepacketizer.c | 24 +++++++++++------------- 6 files changed, 48 insertions(+), 64 deletions(-) diff --git a/src/Limelight-internal.h b/src/Limelight-internal.h index 3e5e6de..4ed1f02 100644 --- a/src/Limelight-internal.h +++ b/src/Limelight-internal.h @@ -21,7 +21,18 @@ extern AUDIO_RENDERER_CALLBACKS AudioCallbacks; extern int NegotiatedVideoFormat; extern volatile int ConnectionInterrupted; -int isBeforeSignedInt(int numA, int numB, int ambiguousCase); +#ifndef UINT24_MAX +#define UINT24_MAX 0xFFFFFF +#endif + +#define U16(x) ((unsigned short) ((x) & UINT16_MAX)) +#define U24(x) ((unsigned int) ((x) & UINT24_MAX)) +#define U32(x) ((unsigned int) ((x) & UINT32_MAX)) + +#define isBefore16(x, y) (U16((x) - (y)) > (UINT16_MAX/2)) +#define isBefore24(x, y) (U24((x) - (y)) > (UINT24_MAX/2)) +#define isBefore32(x, y) (U32((x) - (y)) > (UINT32_MAX/2)) + int serviceEnetHost(ENetHost* client, ENetEvent* event, enet_uint32 timeoutMs); int extractVersionQuadFromString(const char* string, int* quad); int isReferenceFrameInvalidationEnabled(void); diff --git a/src/Misc.c b/src/Misc.c index f9b7652..91c304f 100644 --- a/src/Misc.c +++ b/src/Misc.c @@ -31,23 +31,6 @@ int serviceEnetHost(ENetHost* client, ENetEvent* event, enet_uint32 timeoutMs) { return ret; } -int isBeforeSignedInt(int numA, int numB, int ambiguousCase) { - // This should be the common case for most callers - if (numA == numB) { - return 0; - } - - // If numA and numB have the same signs, - // we can just do a regular comparison. - if ((numA < 0 && numB < 0) || (numA >= 0 && numB >= 0)) { - return numA < numB; - } - else { - // The sign switch is ambiguous - return ambiguousCase; - } -} - int extractVersionQuadFromString(const char* string, int* quad) { char versionString[128]; char* nextDot; diff --git a/src/RtpFecQueue.c b/src/RtpFecQueue.c index d333c70..11d19df 100644 --- a/src/RtpFecQueue.c +++ b/src/RtpFecQueue.c @@ -2,9 +2,6 @@ #include "RtpFecQueue.h" #include "rs.h" -#define ushort(x) ((unsigned short) ((x) % (UINT16_MAX+1))) -#define isBefore(x, y) (ushort((x) - (y)) > (UINT16_MAX/2)) - void RtpfInitializeQueue(PRTP_FEC_QUEUE queue) { reed_solomon_init(); memset(queue, 0, sizeof(*queue)); @@ -31,7 +28,7 @@ void RtpfCleanupQueue(PRTP_FEC_QUEUE queue) { static int queuePacket(PRTP_FEC_QUEUE queue, PRTPFEC_QUEUE_ENTRY newEntry, int head, PRTP_PACKET packet, int length, int isParity) { PRTPFEC_QUEUE_ENTRY entry; - LC_ASSERT(!isBefore(packet->sequenceNumber, queue->nextRtpSequenceNumber)); + LC_ASSERT(!isBefore16(packet->sequenceNumber, queue->nextRtpSequenceNumber)); // Don't queue duplicates either entry = queue->bufferHead; @@ -77,7 +74,7 @@ static int queuePacket(PRTP_FEC_QUEUE queue, PRTPFEC_QUEUE_ENTRY newEntry, int h // Returns 0 if the frame is completely constructed static int reconstructFrame(PRTP_FEC_QUEUE queue) { - int totalPackets = ushort(queue->bufferHighestSequenceNumber - queue->bufferLowestSequenceNumber) + 1; + int totalPackets = U16(queue->bufferHighestSequenceNumber - queue->bufferLowestSequenceNumber) + 1; int totalParityPackets = (queue->bufferDataPackets * queue->fecPercentage + 99) / 100; int parityPackets = totalPackets - queue->bufferDataPackets; int missingPackets = totalPackets - queue->bufferSize; @@ -120,7 +117,7 @@ static int reconstructFrame(PRTP_FEC_QUEUE queue) { PRTPFEC_QUEUE_ENTRY entry = queue->bufferHead; while (entry != NULL) { - int index = ushort(entry->packet->sequenceNumber - queue->bufferLowestSequenceNumber); + int index = U16(entry->packet->sequenceNumber - queue->bufferLowestSequenceNumber); packets[index] = (unsigned char*) entry->packet; marks[index] = 0; @@ -156,7 +153,7 @@ cleanup_packets: if (ret == 0 && i < queue->bufferDataPackets) { PRTPFEC_QUEUE_ENTRY queueEntry = (PRTPFEC_QUEUE_ENTRY)&packets[i][receiveSize]; PRTP_PACKET rtpPacket = (PRTP_PACKET) packets[i]; - rtpPacket->sequenceNumber = ushort(i + queue->bufferLowestSequenceNumber); + rtpPacket->sequenceNumber = U16(i + queue->bufferLowestSequenceNumber); rtpPacket->header = queue->bufferHead->packet->header; int dataOffset = sizeof(*rtpPacket); @@ -172,7 +169,7 @@ cleanup_packets: // discarded by decoders. It's not safe to strip all zero padding because // it may be a legitimate part of the H.264 bytestream. - LC_ASSERT(isBefore(rtpPacket->sequenceNumber, queue->bufferFirstParitySequenceNumber)); + LC_ASSERT(isBefore16(rtpPacket->sequenceNumber, queue->bufferFirstParitySequenceNumber)); queuePacket(queue, queueEntry, 0, rtpPacket, StreamConfig.packetSize + dataOffset, 0); } else if (packets[i] != NULL) { free(packets[i]); @@ -215,7 +212,7 @@ static void removeEntry(PRTP_FEC_QUEUE queue, PRTPFEC_QUEUE_ENTRY entry) { } int RtpfAddPacket(PRTP_FEC_QUEUE queue, PRTP_PACKET packet, int length, PRTPFEC_QUEUE_ENTRY packetEntry) { - if (isBefore(packet->sequenceNumber, queue->nextRtpSequenceNumber)) { + if (isBefore16(packet->sequenceNumber, queue->nextRtpSequenceNumber)) { // Reject packets behind our current sequence number return RTPF_RET_REJECTED; } @@ -227,7 +224,7 @@ int RtpfAddPacket(PRTP_FEC_QUEUE queue, PRTP_PACKET packet, int length, PRTPFEC_ PNV_VIDEO_PACKET nvPacket = (PNV_VIDEO_PACKET)(((char*)packet) + dataOffset); - if (isBefore(nvPacket->frameIndex, queue->currentFrameNumber)) { + if (isBefore16(nvPacket->frameIndex, queue->currentFrameNumber)) { // Reject frames behind our current frame number return RTPF_RET_REJECTED; } @@ -257,21 +254,21 @@ int RtpfAddPacket(PRTP_FEC_QUEUE queue, PRTP_PACKET packet, int length, PRTPFEC_ queue->bufferSize = 0; int fecIndex = (nvPacket->fecInfo & 0xFF000) >> 12; - queue->bufferLowestSequenceNumber = ushort(packet->sequenceNumber - fecIndex); + queue->bufferLowestSequenceNumber = U16(packet->sequenceNumber - fecIndex); queue->receivedBufferDataPackets = 0; queue->bufferHighestSequenceNumber = packet->sequenceNumber; queue->bufferDataPackets = ((nvPacket->fecInfo & 0xFFF00000) >> 20) / 4; queue->fecPercentage = ((nvPacket->fecInfo & 0xFF0) >> 4); - queue->bufferFirstParitySequenceNumber = ushort(queue->bufferLowestSequenceNumber + queue->bufferDataPackets); - } else if (isBefore(queue->bufferHighestSequenceNumber, packet->sequenceNumber)) { + queue->bufferFirstParitySequenceNumber = U16(queue->bufferLowestSequenceNumber + queue->bufferDataPackets); + } else if (isBefore16(queue->bufferHighestSequenceNumber, packet->sequenceNumber)) { queue->bufferHighestSequenceNumber = packet->sequenceNumber; } - if (!queuePacket(queue, packetEntry, 0, packet, length, !isBefore(packet->sequenceNumber, queue->bufferFirstParitySequenceNumber))) { + if (!queuePacket(queue, packetEntry, 0, packet, length, !isBefore16(packet->sequenceNumber, queue->bufferFirstParitySequenceNumber))) { return RTPF_RET_REJECTED; } else { - if (isBefore(packet->sequenceNumber, queue->bufferFirstParitySequenceNumber)) { + if (isBefore16(packet->sequenceNumber, queue->bufferFirstParitySequenceNumber)) { queue->receivedBufferDataPackets++; } @@ -326,7 +323,7 @@ PRTPFEC_QUEUE_ENTRY RtpfGetQueuedPacket(PRTP_FEC_QUEUE queue) { continue; } - if (queuedEntry == NULL || isBefore(entry->packet->sequenceNumber, lowestRtpSequenceNumber)) { + if (queuedEntry == NULL || isBefore16(entry->packet->sequenceNumber, lowestRtpSequenceNumber)) { lowestRtpSequenceNumber = entry->packet->sequenceNumber; queuedEntry = entry; } diff --git a/src/RtpReorderQueue.c b/src/RtpReorderQueue.c index ff6fdc0..5bd0a8a 100644 --- a/src/RtpReorderQueue.c +++ b/src/RtpReorderQueue.c @@ -19,23 +19,18 @@ void RtpqCleanupQueue(PRTP_REORDER_QUEUE queue) { // newEntry is contained within the packet buffer so we free the whole entry by freeing entry->packet static int queuePacket(PRTP_REORDER_QUEUE queue, PRTP_QUEUE_ENTRY newEntry, int head, PRTP_PACKET packet) { - if (queue->nextRtpSequenceNumber != UINT16_MAX) { - PRTP_QUEUE_ENTRY entry; + PRTP_QUEUE_ENTRY entry; - // Don't queue packets we're already ahead of - if (isBeforeSignedInt(packet->sequenceNumber, queue->nextRtpSequenceNumber, 0)) { + LC_ASSERT(!isBefore16(packet->sequenceNumber, queue->nextRtpSequenceNumber)); + + // Don't queue duplicates + entry = queue->queueHead; + while (entry != NULL) { + if (entry->packet->sequenceNumber == packet->sequenceNumber) { return 0; } - // Don't queue duplicates either - entry = queue->queueHead; - while (entry != NULL) { - if (entry->packet->sequenceNumber == packet->sequenceNumber) { - return 0; - } - - entry = entry->next; - } + entry = entry->next; } newEntry->packet = packet; @@ -93,7 +88,7 @@ static PRTP_QUEUE_ENTRY getEntryByLowestSeq(PRTP_REORDER_QUEUE queue) { lowestSeqEntry = queue->queueHead; entry = queue->queueHead; while (entry != NULL) { - if (isBeforeSignedInt(entry->packet->sequenceNumber, lowestSeqEntry->packet->sequenceNumber, 1)) { + if (isBefore16(entry->packet->sequenceNumber, lowestSeqEntry->packet->sequenceNumber)) { lowestSeqEntry = entry; } @@ -163,7 +158,7 @@ static PRTP_QUEUE_ENTRY validateQueueConstraints(PRTP_REORDER_QUEUE queue) { int RtpqAddPacket(PRTP_REORDER_QUEUE queue, PRTP_PACKET packet, PRTP_QUEUE_ENTRY packetEntry) { if (queue->nextRtpSequenceNumber != UINT16_MAX && - isBeforeSignedInt(packet->sequenceNumber, queue->nextRtpSequenceNumber, 0)) { + isBefore16(packet->sequenceNumber, queue->nextRtpSequenceNumber)) { // Reject packets behind our current sequence number return RTPQ_RET_REJECTED; } diff --git a/src/Video.h b/src/Video.h index 6bce417..2195842 100644 --- a/src/Video.h +++ b/src/Video.h @@ -17,8 +17,8 @@ int getNextQueuedDecodeUnit(PQUEUED_DECODE_UNIT* qdu); #define FLAG_SOF 0x4 typedef struct _NV_VIDEO_PACKET { - int streamPacketIndex; - int frameIndex; + unsigned int streamPacketIndex; + unsigned int frameIndex; char flags; char reserved[3]; int fecInfo; diff --git a/src/VideoDepacketizer.c b/src/VideoDepacketizer.c index 9ba3ffa..1e1c81f 100644 --- a/src/VideoDepacketizer.c +++ b/src/VideoDepacketizer.c @@ -6,19 +6,17 @@ static PLENTRY nalChainHead; static int nalChainDataLength; -static int nextFrameNumber; -static int startFrameNumber; +static unsigned int nextFrameNumber; +static unsigned int startFrameNumber; static int waitingForNextSuccessfulFrame; static int waitingForIdrFrame; -static int lastPacketInStream; +static unsigned int lastPacketInStream; static int decodingFrame; static int strictIdrFrameWait; static unsigned long long firstPacketReceiveTime; -#define TRUNCATE_24BIT(x) ((x) & 0xFFFFFF) - #define CONSECUTIVE_DROP_LIMIT 120 -static int consecutiveFrameDrops; +static unsigned int consecutiveFrameDrops; static LINKED_BLOCKING_QUEUE decodeUnitQueue; @@ -38,7 +36,7 @@ void initializeVideoDepacketizer(int pktSize) { startFrameNumber = 0; waitingForNextSuccessfulFrame = 0; waitingForIdrFrame = 1; - lastPacketInStream = -1; + lastPacketInStream = UINT32_MAX; decodingFrame = 0; firstPacketReceiveTime = 0; strictIdrFrameWait = !isReferenceFrameInvalidationEnabled(); @@ -441,8 +439,8 @@ void processRtpPayload(PNV_VIDEO_PACKET videoPacket, int length, unsigned long l BUFFER_DESC currentPos; int frameIndex; char flags; - int firstPacket; - int streamPacketIndex; + unsigned int firstPacket; + unsigned int streamPacketIndex; // Mask the top 8 bits from the SPI videoPacket->streamPacketIndex >>= 8; @@ -459,8 +457,8 @@ 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(!isBeforeSignedInt(streamPacketIndex, TRUNCATE_24BIT(lastPacketInStream + 1), 0)); - LC_ASSERT(!isBeforeSignedInt(frameIndex, nextFrameNumber, 0)); + LC_ASSERT(!isBefore24(streamPacketIndex, U24(lastPacketInStream + 1))); + LC_ASSERT(!isBefore32(frameIndex, nextFrameNumber)); // Notify the listener of the latest frame we've seen from the PC connectionSawFrame(frameIndex); @@ -472,7 +470,7 @@ void processRtpPayload(PNV_VIDEO_PACKET videoPacket, int length, unsigned long l // miss one in between if (firstPacket) { // Make sure this is the next consecutive frame - if (isBeforeSignedInt(nextFrameNumber, frameIndex, 1)) { + if (isBefore32(nextFrameNumber, frameIndex)) { Limelog("Network dropped an entire frame\n"); nextFrameNumber = frameIndex; @@ -491,7 +489,7 @@ void processRtpPayload(PNV_VIDEO_PACKET videoPacket, int length, unsigned long l // This must be the first packet in a frame or be contiguous with the last // packet received. - LC_ASSERT(firstPacket || streamPacketIndex == TRUNCATE_24BIT(lastPacketInStream + 1)); + LC_ASSERT(firstPacket || streamPacketIndex == U24(lastPacketInStream + 1)); lastPacketInStream = streamPacketIndex;