Fix race condition that could result in loss of NALUs in an IDR frame

Unfortunately, this was a design flaw in the way we handled IDR frames
NALUs. Because we split them all up and handled them as separate
"frames" in the system, some of them could be discarded due to a
video decode unit overflow, for example, and cause the decoder to get
confused due to receiving an SPS without a following PPS.

To address this, NALUs in the IDR frame are always delivered together
as one frame. The SPS, PPS, and VPS are now in separate buffers in
the IDR frame's buffer list and tagged as such with the new buffer
type field. The IDR frames themselves have a new frame type field
which marks them as special frames that clients may need to process
in a special way.

This will likely be a breaking change for your clients!

src/Limelight.h

@@ -61,6 +61,13 @@ typedef struct _STREAM_CONFIGURATION {
 // Use this function to zero the stream configuration when allocated on the stack or heap
 void LiInitializeStreamConfiguration(PSTREAM_CONFIGURATION streamConfig);
 
+// These identify codec configuration data in the buffer lists
+// of frames identified as IDR frames.
+#define BUFFER_TYPE_PICDATA  0x00
+#define BUFFER_TYPE_SPS      0x01
+#define BUFFER_TYPE_PPS      0x02
+#define BUFFER_TYPE_VPS      0x03
+
 typedef struct _LENTRY {
     // Pointer to the next entry or NULL if this is the last entry
     struct _LENTRY* next;
@@ -70,13 +77,29 @@ typedef struct _LENTRY {
 
     // Size of data in bytes (never <= 0)
     int length;
+
+    // Buffer type (listed above)
+    int bufferType;
 } LENTRY, *PLENTRY;
 
+// This is a standard frame which references the IDR frame and
+// previous P-frames.
+#define FRAME_TYPE_PFRAME 0x00
+
+// Indicates this frame contains SPS, PPS, and VPS (if applicable)
+// as the first buffers in the list. Each NALU will appear as a separate
+// buffer in the buffer list. The I-frame data follows immediately
+// after the codec configuration NALUs.
+#define FRAME_TYPE_IDR    0x01
+
 // A decode unit describes a buffer chain of video data from multiple packets
 typedef struct _DECODE_UNIT {
     // Frame number
     int frameNumber;
 
+    // Frame type
+    int frameType;
+
     // Receive time of first buffer
     // NOTE: This will be populated from gettimeofday() if !HAVE_CLOCK_GETTIME and
     // populated from clock_gettime(CLOCK_MONOTONIC) if HAVE_CLOCK_GETTIME

src/VideoDepacketizer.c

@@ -231,6 +231,14 @@ static void reassembleFrame(int frameNumber) {
             qdu->decodeUnit.frameNumber = frameNumber;
             qdu->decodeUnit.receiveTimeMs = firstPacketReceiveTime;
 
+            // IDR frames will have leading CSD buffers
+            if (nalChainHead->bufferType != BUFFER_TYPE_PICDATA) {
+                qdu->decodeUnit.frameType = FRAME_TYPE_IDR;
+            }
+            else {
+                qdu->decodeUnit.frameType = FRAME_TYPE_PFRAME;
+            }
+
             nalChainHead = NULL;
             nalChainDataLength = 0;
 
@@ -274,7 +282,43 @@ static void reassembleFrame(int frameNumber) {
     }
 }
 
-static void queueFragment(char*data, int offset, int length) {
+
+#define AVC_NAL_TYPE_SPS 0x67
+#define AVC_NAL_TYPE_PPS 0x68
+#define HEVC_NAL_TYPE_VPS 0x40
+#define HEVC_NAL_TYPE_SPS 0x42
+#define HEVC_NAL_TYPE_PPS 0x44
+
+static int getBufferFlags(char* data, int length) {
+    BUFFER_DESC buffer;
+    BUFFER_DESC candidate;
+
+    buffer.data = data;
+    buffer.length = (unsigned int)length;
+    buffer.offset = 0;
+
+    if (!getSpecialSeq(&buffer, &candidate) || !isSeqFrameStart(&candidate)) {
+        return BUFFER_TYPE_PICDATA;
+    }
+
+    switch (candidate.data[candidate.offset + candidate.length]) {
+        case AVC_NAL_TYPE_SPS:
+        case HEVC_NAL_TYPE_SPS:
+            return BUFFER_TYPE_SPS;
+
+        case AVC_NAL_TYPE_PPS:
+        case HEVC_NAL_TYPE_PPS:
+            return BUFFER_TYPE_PPS;
+
+        case HEVC_NAL_TYPE_VPS:
+            return BUFFER_TYPE_VPS;
+
+        default:
+            return BUFFER_TYPE_PICDATA;
+    }
+}
+
+static void queueFragment(char* data, int offset, int length) {
     PLENTRY entry = (PLENTRY)malloc(sizeof(*entry) + length);
     if (entry != NULL) {
         entry->next = NULL;
@@ -283,6 +327,8 @@ static void queueFragment(char*data, int offset, int length) {
 
         memcpy(entry->data, &data[offset], entry->length);
 
+        entry->bufferType = getBufferFlags(entry->data, entry->length);
+
         nalChainDataLength += entry->length;
 
         if (nalChainHead == NULL) {
@@ -305,6 +351,9 @@ static void processRtpPayloadSlow(PNV_VIDEO_PACKET videoPacket, PBUFFER_DESC cur
     BUFFER_DESC specialSeq;
     int decodingVideo = 0;
 
+    // We should not have any NALUs when processing the first packet in an IDR frame
+    LC_ASSERT(nalChainHead == NULL);
+
     while (currentPos->length != 0) {
         int start = currentPos->offset;
 
@@ -317,9 +366,6 @@ static void processRtpPayloadSlow(PNV_VIDEO_PACKET videoPacket, PBUFFER_DESC cur
                     // Now we're working on a frame
                     decodingFrame = 1;
 
-                    // Reassemble any pending frame
-                    reassembleFrame(videoPacket->frameIndex);
-
                     if (isSeqReferenceFrameStart(&specialSeq)) {
                         // No longer waiting for an IDR frame
                         waitingForIdrFrame = 0;
@@ -334,11 +380,6 @@ static void processRtpPayloadSlow(PNV_VIDEO_PACKET videoPacket, PBUFFER_DESC cur
                 currentPos->offset += specialSeq.length;
             }
             else {
-                // Check if this is padding after a full frame
-                if (decodingVideo && isSeqPadding(currentPos)) {
-                    reassembleFrame(videoPacket->frameIndex);
-                }
-
                 // Not decoding video
                 decodingVideo = 0;