From 423032797f1503aaf81b153fcc660337586f406d Mon Sep 17 00:00:00 2001
From: James Delaplane <delaplan@amazon.com>
Date: Sun, 26 Mar 2023 21:01:24 -0700
Subject: [PATCH 1/8] Modified jitterbuffer to include checks for timestamp
 overflow

---
 .../kinesis/video/webrtcclient/Include.h      |   5 +
 src/source/PeerConnection/JitterBuffer.c      | 356 +++++++++++++++---
 src/source/PeerConnection/JitterBuffer.h      |   7 +-
 src/source/PeerConnection/PeerConnection.c    |   1 -
 4 files changed, 323 insertions(+), 46 deletions(-)

diff --git a/src/include/com/amazonaws/kinesis/video/webrtcclient/Include.h b/src/include/com/amazonaws/kinesis/video/webrtcclient/Include.h
index 1015e37d3b..a6810c0865 100644
--- a/src/include/com/amazonaws/kinesis/video/webrtcclient/Include.h
+++ b/src/include/com/amazonaws/kinesis/video/webrtcclient/Include.h
@@ -621,6 +621,11 @@ extern "C" {
  */
 #define MAX_SEQUENCE_NUM ((UINT32) MAX_UINT16)
 
+/**
+ * Maximum timestamp in rtp packet/jitter buffer
+ */
+#define MAX_TIMESTAMP ((UINT32) MAX_UINT32)
+
 /**
  * Parameterized string for KVS STUN Server
  */
diff --git a/src/source/PeerConnection/JitterBuffer.c b/src/source/PeerConnection/JitterBuffer.c
index aea092edc0..81637cc99e 100644
--- a/src/source/PeerConnection/JitterBuffer.c
+++ b/src/source/PeerConnection/JitterBuffer.c
@@ -32,11 +32,14 @@ STATUS createJitterBuffer(FrameReadyFunc onFrameReadyFunc, FrameDroppedFunc onFr
     }
     pJitterBuffer->maxLatency = pJitterBuffer->maxLatency * pJitterBuffer->clockRate / HUNDREDS_OF_NANOS_IN_A_SECOND;
 
-    pJitterBuffer->lastPushTimestamp = 0;
+    pJitterBuffer->tailTimestamp = 0;
     pJitterBuffer->headTimestamp = MAX_UINT32;
     pJitterBuffer->headSequenceNumber = MAX_SEQUENCE_NUM;
+    pJitterBuffer->tailSequenceNumber = MAX_SEQUENCE_NUM;
     pJitterBuffer->started = FALSE;
     pJitterBuffer->firstFrameProcessed = FALSE;
+    pJitterBuffer->timestampOverFlowState = FALSE;
+    pJitterBuffer->sequenceNumberOverflowState = FALSE;
 
     pJitterBuffer->customData = customData;
     CHK_STATUS(hashTableCreateWithParams(JITTER_BUFFER_HASH_TABLE_BUCKET_COUNT, JITTER_BUFFER_HASH_TABLE_BUCKET_LENGTH,
@@ -82,12 +85,291 @@ STATUS freeJitterBuffer(PJitterBuffer* ppJitterBuffer)
     return retStatus;
 }
 
+//return true if sequence numbers are now overflowing
+BOOL enterSequenceNumberOverflowCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+    BOOL overflow = FALSE;
+    BOOL underflow = FALSE;
+    UINT16 packetsUntilOverflow = MAX_SEQUENCE_NUM - pJitterBuffer->tailSequenceNumber;
+    UINT16 underflowFromMaxRange = 0;
+
+    if(!pJitterBuffer->sequenceNumberOverflowState) {
+
+        //overflow case
+        if(MAX_OUT_OF_ORDER_PACKET_DIFFERENCE >= packetsUntilOverflow) {
+            //It's possible sequence numbers and timestamps are both overflowing.
+            if(pRtpPacket->header.sequenceNumber < pJitterBuffer->tailSequenceNumber &&
+               pRtpPacket->header.sequenceNumber <= MAX_OUT_OF_ORDER_PACKET_DIFFERENCE - packetsUntilOverflow) {
+                //Sequence number overflow detected
+                overflow = TRUE;
+            }
+        }
+        //underflow case
+        else if(headCheckingAllowed(pJitterBuffer, pRtpPacket)) { 
+            if (pJitterBuffer->headSequenceNumber < MAX_OUT_OF_ORDER_PACKET_DIFFERENCE) {
+                if ((pRtpPacket->header.sequenceNumber >=
+                            (MAX_UINT16 - (MAX_OUT_OF_ORDER_PACKET_DIFFERENCE - pJitterBuffer->headSequenceNumber))) ||
+                        (pJitterBuffer->headSequenceNumber > pRtpPacket->header.sequenceNumber)) {
+                    //Sequence number underflow detected
+                    underflow = TRUE;
+                }
+            }
+        }
+    }
+    if(overflow && underflow) {
+        //This shouldn't be possible. 
+        DLOGE("Critical underflow/overflow error in jitterbuffer");
+    }
+    if(overflow) {
+        pJitterBuffer->sequenceNumberOverflowState = TRUE;
+        pJitterBuffer->tailSequenceNumber = pRtpPacket->header.sequenceNumber;
+    }
+    if(underflow) {
+        pJitterBuffer->sequenceNumberOverflowState = TRUE;
+        pJitterBuffer->headSequenceNumber = pRtpPacket->header.sequenceNumber;
+    }
+    return (overflow || underflow);
+}
+
+BOOL enterTimestampOverflowCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+    BOOL underflow = FALSE;
+    BOOL overflow = FALSE;
+    if(!pJitterBuffer->timestampOverFlowState) {
+        //overflow check
+        if(pJitterBuffer->headTimestamp > pRtpPacket->header.timestamp &&
+           pJitterBuffer->tailTimestamp > pRtpPacket->header.timestamp) {
+            //Check to see if this could be a timestamp overflow case
+            //We always check sequence number first, so the 'or equal to' checks if we just set the tail.
+            //That would be a corner case of sequence number and timestamp both overflowing
+            //in this one packet.
+            if(pRtpPacket->header.sequenceNumber >= pJitterBuffer->tailSequenceNumber) {
+                //RTP timestamp overflow detected!
+                overflow = TRUE;
+            }
+        }
+        //underflow check
+        else if (pJitterBuffer->headTimestamp < pRtpPacket->header.timestamp &&
+                pJitterBuffer->tailTimestamp < pRtpPacket->header.timestamp) {
+            if(headSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
+                underflow = TRUE;
+            }
+        }
+
+    }
+    if(overflow && underflow) {
+        //This shouldn't be possible. 
+        DLOGE("Critical underflow/overflow error in jitterbuffer");
+    }
+    if(overflow) {
+        pJitterBuffer->timestampOverFlowState = TRUE;
+        pJitterBuffer->tailTimestamp = pRtpPacket->header.timestamp;
+    }
+    else if(underflow) {
+        pJitterBuffer->timestampOverFlowState = TRUE;
+        pJitterBuffer->headTimestamp = pRtpPacket->header.timestamp;
+    }
+    return (underflow || overflow);
+}
+
+BOOL exitSequenceNumberOverflowCheck(PJitterBuffer pJitterBuffer) {
+    BOOL retVal = FALSE;
+
+    //can't exit if you're not in it
+    if(pJitterBuffer->sequenceNumberOverflowState) {
+        if(pJitterBuffer->headSequenceNumber <= pJitterBuffer->tailSequenceNumber) {
+            retVal = TRUE;
+        }
+    }
+
+    return retVal;
+}
+
+BOOL exitTimestampOverflowCheck(PJitterBuffer pJitterBuffer) {
+    BOOL retVal = FALSE;
+
+    //can't exit if you're not in it
+    if(pJitterBuffer->timestampOverFlowState) {
+        if(pJitterBuffer->headTimestamp <= pJitterBuffer->tailTimestamp) {
+            retVal = TRUE;
+        }
+    }
+
+    return retVal;
+}
+
+BOOL headCheckingAllowed(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+    BOOL retVal = FALSE;
+    /*If we haven't yet processed a frame yet, then we don't have a definitive way of knowing if
+     *the first packet we receive is actually the earliest packet we'll ever receive. Since sequence numbers
+     *can start anywhere from 0 - 65535, we need to incorporate some checks to determine if a newly received packet
+     *should be considered the new head. Part of how we determine this is by setting a limit to how many packets off we allow
+     *this out of order case to be. Without setting a limit, then we could run into an odd scenario.
+     * Example:
+     * Push->Packet->SeqNumber == 0. //FIRST PACKET! new head of buffer!
+     * Push->Packet->SeqNumber == 3. //... new head of 65532 packet sized frame? maybe? was 0 the tail?
+     *
+     * To resolve that insanity we set a MAX, and will use that MAX for the range.
+     * This logic is present in headSequenceNumberCheck()
+     *
+     *After the first frame has been processed we don't need or want to make this consideration, since if our parser has
+     *dropped a frame for a good reason then we want to ignore any packets from that dropped frame that may come later.
+     *
+     *However if the packet's timestamp is the same as the head timestamp, then it's possible this is simply an earlier
+     *sequence number of the same packet.
+     */
+    if(!(pJitterBuffer->firstFrameProcessed) || 
+         pJitterBuffer->headTimestamp == pRtpPacket->header.timestamp) {
+        retVal = TRUE;
+    }
+    return retVal;
+}
+
+//return true if pRtpPacket contains a new head timestamp
+BOOL headTimestampCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+    BOOL retVal = FALSE;
+
+    if(headCheckingAllowed(pJitterBuffer, pRtpPacket)) {
+        if(pJitterBuffer->timestampOverFlowState) {
+            if(pJitterBuffer->headTimestamp > pRtpPacket->header.timestamp &&
+               pJitterBuffer->tailTimestamp < pRtpPacket->header.timestamp) {
+                //in the correct range to be a new head or new tail.
+                //if it's also the head sequence number then it's the new headtimestamp
+                if(headSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
+                    pJitterBuffer->headTimestamp = pRtpPacket->header.timestamp;
+                    retVal = TRUE;
+                }
+            }
+        }
+        else {
+            if(pJitterBuffer->headTimestamp > pRtpPacket->header.timestamp ||
+               pJitterBuffer->tailTimestamp < pRtpPacket->header.timestamp) {
+                //in the correct range to be a new head or new tail.
+                //if it's also the head sequence number then it's the new headtimestamp
+                if(headSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
+                    pJitterBuffer->headTimestamp = pRtpPacket->header.timestamp;
+                    retVal = TRUE;
+                }
+            }
+        }
+    }
+    return retVal;
+}
+
+//return true if pRtpPacket contains a new tail timestamp
+BOOL tailTimestampCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+    BOOL retVal = FALSE;
+
+    if(pJitterBuffer->tailTimestamp < pRtpPacket->header.timestamp) {
+        if(!pJitterBuffer->timestampOverFlowState ||
+            pJitterBuffer->headTimestamp > pRtpPacket->header.timestamp) {
+            //in the correct range to be a new head or new tail.
+            //if it's also the tail sequence number then it's the new tail timestamp
+            if(tailSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
+                pJitterBuffer->tailTimestamp = pRtpPacket->header.timestamp;
+                retVal = TRUE;
+            }
+        }
+    }
+    return retVal;
+}
+
+//return true if pRtpPacket contains the head sequence number
+BOOL headSequenceNumberCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+    BOOL retVal = FALSE;
+    UINT16 minimumHead = 0;
+    if(pJitterBuffer->headSequenceNumber >= MAX_OUT_OF_ORDER_PACKET_DIFFERENCE) {
+        minimumHead = pJitterBuffer->headSequenceNumber - MAX_OUT_OF_ORDER_PACKET_DIFFERENCE;
+    }
+
+    //If we've already done this check and it was true
+    if(pJitterBuffer->headSequenceNumber == pRtpPacket->header.sequenceNumber) {
+        retVal = TRUE;
+    }
+    else if(headCheckingAllowed(pJitterBuffer, pRtpPacket)){
+        if(pJitterBuffer->sequenceNumberOverflowState) {
+            if(pJitterBuffer->tailSequenceNumber < pRtpPacket->header.sequenceNumber &&
+               pJitterBuffer->headSequenceNumber > pRtpPacket->header.sequenceNumber &&
+               pRtpPacket->header.sequenceNumber >= minimumHead) {
+                //This purposefully misses the usecase where the buffer has >65000 entries.
+                //Our buffer is not designed for that use case, and it becomes far too ambiguous
+                //as to which packets are new tails or new heads without adding epoch checks.
+                pJitterBuffer->headSequenceNumber = pRtpPacket->header.sequenceNumber;
+                retVal = TRUE;
+            }
+        }
+        else {
+            if(pRtpPacket->header.sequenceNumber < pJitterBuffer->headSequenceNumber) {
+                if(pRtpPacket->header.sequenceNumber >= minimumHead) {
+                    pJitterBuffer->headSequenceNumber = pRtpPacket->header.sequenceNumber;
+                    retVal = TRUE;
+                }
+            }
+        }
+
+    }
+    return retVal;
+}
+
+//return true if pRtpPacket contains a new tail sequence number
+BOOL tailSequenceNumberCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+    BOOL retVal = FALSE;
+    //If we've already done this check and it was true
+    if(pJitterBuffer->tailSequenceNumber == pRtpPacket->header.sequenceNumber) {
+        retVal = TRUE;
+    }
+    else if(pRtpPacket->header.sequenceNumber > pJitterBuffer->tailSequenceNumber &&
+            (!pJitterBuffer->sequenceNumberOverflowState ||
+            pJitterBuffer->headSequenceNumber > pRtpPacket->header.sequenceNumber)) {
+        retVal = TRUE;
+        pJitterBuffer->tailSequenceNumber = pRtpPacket->header.sequenceNumber;            
+    }
+    return retVal;
+}
+
+//return true if pRtpPacket is within the latency tolerance (not much earlier than current head)
+BOOL withinLatencyTolerance(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+    BOOL retVal = FALSE;
+    UINT32 minimumTimestamp = 0;
+
+    //Simple check, if we're at or past the tail timestamp then we're always within latency tolerance.
+    //overflow is checked earlier
+    if(tailTimestampCheck(pJitterBuffer, pRtpPacket) || pJitterBuffer->tailTimestamp == pRtpPacket->header.timestamp) {
+        retVal = TRUE;
+    }
+    else {
+        //Is our tail current less than our head due to timestamp overflow?
+        if(pJitterBuffer->timestampOverFlowState) {
+            //calculate max-latency across the overflow boundry without triggering underflow
+            if(pJitterBuffer->tailTimestamp < pJitterBuffer->maxLatency) {
+                minimumTimestamp = MAX_TIMESTAMP - (pJitterBuffer->maxLatency - pJitterBuffer->tailTimestamp);
+            }
+            //Is the packet within the current range or is it a new head/tail
+            if(pRtpPacket->header.timestamp < pJitterBuffer->tailTimestamp ||
+               pRtpPacket->header.timestamp > pJitterBuffer->headTimestamp) {
+                //The packet is within the current range
+                retVal = TRUE;
+            }
+            //The only remaining option is that timestamp must be before headTimestamp
+            else if(pRtpPacket->header.timestamp >= minimumTimestamp) {
+                retVal = TRUE;                
+            }
+        }
+        else {
+            if ((pRtpPacket->header.timestamp < pJitterBuffer->maxLatency && pJitterBuffer->tailTimestamp <= pJitterBuffer->maxLatency) ||
+                pRtpPacket->header.timestamp >= pJitterBuffer->tailTimestamp - pJitterBuffer->maxLatency) {
+                retVal = TRUE;
+            }
+        }
+    }
+    return retVal;
+}
+
 STATUS jitterBufferPush(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket, PBOOL pPacketDiscarded)
 {
     ENTERS();
     STATUS retStatus = STATUS_SUCCESS, status = STATUS_SUCCESS;
     UINT64 hashValue = 0;
     PRtpPacket pCurPacket = NULL;
+    UINT16 packetsUntilOverflow = 0;
 
     CHK(pJitterBuffer != NULL && pRtpPacket != NULL, STATUS_NULL_ARG);
 
@@ -95,16 +377,24 @@ STATUS jitterBufferPush(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket, PBOO
         // Set to started and initialize the sequence number
         pJitterBuffer->started = TRUE;
         pJitterBuffer->headSequenceNumber = pRtpPacket->header.sequenceNumber;
+        pJitterBuffer->tailSequenceNumber = pRtpPacket->header.sequenceNumber;
         pJitterBuffer->headTimestamp = pRtpPacket->header.timestamp;
     }
 
-    if (pJitterBuffer->lastPushTimestamp < pRtpPacket->header.timestamp) {
-        pJitterBuffer->lastPushTimestamp = pRtpPacket->header.timestamp;
+    //We'll check sequence numbers first, with our MAX Out of Order packet count to avoid
+    //defining a timestamp window for overflow
+    //Returning true means this packet is a new tail AND we've entered overflow state.
+    if(!enterSequenceNumberOverflowCheck(pJitterBuffer, pRtpPacket)) {
+        DLOGS("Entered sequenceNumber overflow state");
+        tailSequenceNumberCheck(pJitterBuffer, pRtpPacket);
+    }
+    if(!enterTimestampOverflowCheck(pJitterBuffer, pRtpPacket)) {
+        DLOGS("Entered timestamp overflow state");
+        tailTimestampCheck(pJitterBuffer, pRtpPacket);
     }
 
     // is the packet within the accepted latency range, if so, add it to the hashtable
-    if ((pRtpPacket->header.timestamp < pJitterBuffer->maxLatency && pJitterBuffer->lastPushTimestamp <= pJitterBuffer->maxLatency) ||
-        pRtpPacket->header.timestamp >= pJitterBuffer->lastPushTimestamp - pJitterBuffer->maxLatency) {
+    if (withinLatencyTolerance(pJitterBuffer, pRtpPacket)) {
         status = hashTableGet(pJitterBuffer->pPkgBufferHashTable, pRtpPacket->header.sequenceNumber, &hashValue);
         pCurPacket = (PRtpPacket) hashValue;
         if (STATUS_SUCCEEDED(status) && pCurPacket != NULL) {
@@ -114,38 +404,13 @@ STATUS jitterBufferPush(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket, PBOO
 
         CHK_STATUS(hashTablePut(pJitterBuffer->pPkgBufferHashTable, pRtpPacket->header.sequenceNumber, (UINT64) pRtpPacket));
 
-        /*If we haven't yet processed a frame yet, then we don't have a definitive way of knowing if
-         *the first packet we receive is actually the earliest packet we'll ever receive. Since sequence numbers
-         *can start anywhere from 0 - 65535, we need to incorporate some checks to determine if a newly received packet
-         *should be considered the new head. Part of how we determine this is by setting a limit to how many packets off we allow
-         *this out of order case to be. Without setting a limit, then we could run into an odd scenario.
-         * Example:
-         * Push->Packet->SeqNumber == 0. //FIRST PACKET! new head of buffer!
-         * Push->Packet->SeqNumber == 3. //... new head of 65532 packet sized frame? maybe? was 0 the tail?
-         *
-         * To resolve that insanity we set a MAX, and will use that MAX for the range.
-         *
-         *After the first frame has been processed we don't need or want to make this consideration, since if our parser has
-         *dropped a frame for a good reason then we want to ignore any packets from that dropped frame that may come later.
-         */
-        if (!(pJitterBuffer->firstFrameProcessed)) {
+        if (headCheckingAllowed(pJitterBuffer, pRtpPacket)) {
             // if the timestamp is less, we'll accept it as a new head, since it must be an earlier frame.
-            if (pRtpPacket->header.timestamp < pJitterBuffer->headTimestamp) {
-                pJitterBuffer->headSequenceNumber = pRtpPacket->header.sequenceNumber;
-                pJitterBuffer->headTimestamp = pRtpPacket->header.timestamp;
+            if(headTimestampCheck(pJitterBuffer, pRtpPacket)) {
+                DLOGS("New jitterbuffer head timestamp");
             }
-            // timestamp is equal, we're in the same frame.
-            else if (pRtpPacket->header.timestamp == pJitterBuffer->headTimestamp) {
-                if (pJitterBuffer->headSequenceNumber < MAX_OUT_OF_ORDER_PACKET_DIFFERENCE) {
-                    if ((pRtpPacket->header.sequenceNumber >=
-                         (MAX_UINT16 - (MAX_OUT_OF_ORDER_PACKET_DIFFERENCE - pJitterBuffer->headSequenceNumber))) ||
-                        (pJitterBuffer->headSequenceNumber > pRtpPacket->header.sequenceNumber)) {
-                        pJitterBuffer->headSequenceNumber = pRtpPacket->header.sequenceNumber;
-                    }
-                } else if ((pRtpPacket->header.sequenceNumber >= pJitterBuffer->headSequenceNumber - MAX_OUT_OF_ORDER_PACKET_DIFFERENCE) &&
-                           (pRtpPacket->header.sequenceNumber < pJitterBuffer->headSequenceNumber)) {
-                    pJitterBuffer->headSequenceNumber = pRtpPacket->header.sequenceNumber;
-                }
+            if(headSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
+                DLOGS("New jitterbuffer head sequenceNumber");
             }
         }
         // DONE with considering the head.
@@ -187,13 +452,13 @@ STATUS jitterBufferInternalParse(PJitterBuffer pJitterBuffer, BOOL bufferClosed)
     PRtpPacket pCurPacket = NULL;
 
     CHK(pJitterBuffer != NULL && pJitterBuffer->onFrameDroppedFn != NULL && pJitterBuffer->onFrameReadyFn != NULL, STATUS_NULL_ARG);
-    CHK(pJitterBuffer->lastPushTimestamp != 0, retStatus);
+    CHK(pJitterBuffer->tailTimestamp != 0, retStatus);
 
-    if (pJitterBuffer->lastPushTimestamp > pJitterBuffer->maxLatency) {
-        earliestAllowedTimestamp = pJitterBuffer->lastPushTimestamp - pJitterBuffer->maxLatency;
+    if (pJitterBuffer->tailTimestamp > pJitterBuffer->maxLatency) {
+        earliestAllowedTimestamp = pJitterBuffer->tailTimestamp - pJitterBuffer->maxLatency;
     }
 
-    lastIndex = pJitterBuffer->headSequenceNumber - 1;
+    lastIndex = pJitterBuffer->tailSequenceNumber;
     index = pJitterBuffer->headSequenceNumber;
     startDropIndex = index;
     // Loop through entire buffer to find complete frames.
@@ -210,7 +475,6 @@ STATUS jitterBufferInternalParse(PJitterBuffer pJitterBuffer, BOOL bufferClosed)
      *
      *The buffer is parsed in order of sequence numbers. It is important to note that if the Frame ready
      *conditions have been met from dropping an earlier frame, then it will be processed.
-     *
      */
     for (; index != lastIndex; index++) {
         CHK_STATUS(hashTableContains(pJitterBuffer->pPkgBufferHashTable, index, &hasEntry));
@@ -242,10 +506,10 @@ STATUS jitterBufferInternalParse(PJitterBuffer pJitterBuffer, BOOL bufferClosed)
                     startDropIndex = index;
                     containStartForEarliestFrame = FALSE;
                 }
-                // are we force clearing out the buffer? if so drop the contents of incomplete frame
+                // are we forcibly clearing out the buffer? if so drop the contents of incomplete frame
                 else if (pJitterBuffer->headTimestamp < earliestAllowedTimestamp || bufferClosed) {
-                    CHK_STATUS(
-                        pJitterBuffer->onFrameDroppedFn(pJitterBuffer->customData, startDropIndex, UINT16_DEC(index), pJitterBuffer->headTimestamp));
+                    //do not CHK_STATUS of onFrameDropped because we need to clear the jitter buffer no matter what else happens.
+                    pJitterBuffer->onFrameDroppedFn(pJitterBuffer->customData, startDropIndex, UINT16_DEC(index), pJitterBuffer->headTimestamp);
                     CHK_STATUS(jitterBufferDropBufferData(pJitterBuffer, startDropIndex, UINT16_DEC(index), curTimestamp));
                     pJitterBuffer->firstFrameProcessed = TRUE;
                     isFrameDataContinuous = TRUE;
@@ -325,6 +589,12 @@ STATUS jitterBufferDropBufferData(PJitterBuffer pJitterBuffer, UINT16 startIndex
     }
     pJitterBuffer->headTimestamp = nextTimestamp;
     pJitterBuffer->headSequenceNumber = endIndex + 1;
+    if(exitTimestampOverflowCheck(pJitterBuffer)) {
+        DLOGS("Exited timestamp overflow state");
+    }
+    if(exitSequenceNumberOverflowCheck(pJitterBuffer)) {
+        DLOGS("Exited sequenceNumber overflow state");
+    }
 
 CleanUp:
     CHK_LOG_ERR(retStatus);
diff --git a/src/source/PeerConnection/JitterBuffer.h b/src/source/PeerConnection/JitterBuffer.h
index a5a0ca59be..59afcf1d25 100644
--- a/src/source/PeerConnection/JitterBuffer.h
+++ b/src/source/PeerConnection/JitterBuffer.h
@@ -28,14 +28,17 @@ typedef struct {
     UINT64 transit;
     // holds estimated jitter, in clockRate units
     DOUBLE jitter;
-    UINT32 lastPushTimestamp;
     UINT16 headSequenceNumber;
+    UINT16 tailSequenceNumber;
     UINT32 headTimestamp;
-    UINT64 maxLatency;
+    UINT32 tailTimestamp;
+    UINT32 maxLatency;
     UINT64 customData;
     UINT32 clockRate;
     BOOL started;
     BOOL firstFrameProcessed;
+    BOOL sequenceNumberOverflowState;
+    BOOL timestampOverFlowState;
     PHashTable pPkgBufferHashTable;
 } JitterBuffer, *PJitterBuffer;
 
diff --git a/src/source/PeerConnection/PeerConnection.c b/src/source/PeerConnection/PeerConnection.c
index 584880ac69..836f1157f4 100644
--- a/src/source/PeerConnection/PeerConnection.c
+++ b/src/source/PeerConnection/PeerConnection.c
@@ -367,7 +367,6 @@ STATUS onFrameDroppedFunc(UINT64 customData, UINT16 startIndex, UINT16 endIndex,
     } else {
         CHK(FALSE, retStatus);
     }
-    // TODO: handle multi-packet frames
     CHK(pPacket != NULL, STATUS_NULL_ARG);
     MUTEX_LOCK(pTransceiver->statsLock);
     // https://www.w3.org/TR/webrtc-stats/#dom-rtcinboundrtpstreamstats-jitterbufferdelay

From d8eff57e40e3f406fc55b9421c2e40438e4b7315 Mon Sep 17 00:00:00 2001
From: James Delaplane <delaplan@amazon.com>
Date: Tue, 28 Mar 2023 21:26:57 -0700
Subject: [PATCH 2/8] All tests passing, some new tests still need to be added

---
 src/source/PeerConnection/JitterBuffer.c |  46 +++++++-
 src/source/PeerConnection/JitterBuffer.h |   4 +-
 tst/JitterBufferFunctionalityTest.cpp    | 134 +++++++++++++++++++++++
 3 files changed, 177 insertions(+), 7 deletions(-)

diff --git a/src/source/PeerConnection/JitterBuffer.c b/src/source/PeerConnection/JitterBuffer.c
index 81637cc99e..517e76eb99 100644
--- a/src/source/PeerConnection/JitterBuffer.c
+++ b/src/source/PeerConnection/JitterBuffer.c
@@ -32,6 +32,8 @@ STATUS createJitterBuffer(FrameReadyFunc onFrameReadyFunc, FrameDroppedFunc onFr
     }
     pJitterBuffer->maxLatency = pJitterBuffer->maxLatency * pJitterBuffer->clockRate / HUNDREDS_OF_NANOS_IN_A_SECOND;
 
+    CHK(pJitterBuffer->maxLatency < MAX_TIMESTAMP, STATUS_INVALID_ARG);
+
     pJitterBuffer->tailTimestamp = 0;
     pJitterBuffer->headTimestamp = MAX_UINT32;
     pJitterBuffer->headSequenceNumber = MAX_SEQUENCE_NUM;
@@ -85,6 +87,34 @@ STATUS freeJitterBuffer(PJitterBuffer* ppJitterBuffer)
     return retStatus;
 }
 
+BOOL underflowPossible(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+    BOOL retVal = FALSE;
+    UINT16 seqNoDifference = 0;
+    UINT64 timestampDifference = 0;
+    UINT64 maxTimePassed = 0;
+    if(pJitterBuffer->headTimestamp == pRtpPacket->header.timestamp) {
+        retVal = TRUE;
+    }
+    else {
+        seqNoDifference = (MAX_SEQUENCE_NUM - pRtpPacket->header.sequenceNumber) + pJitterBuffer->headSequenceNumber;
+        if(pJitterBuffer->headTimestamp > pRtpPacket->header.timestamp) {
+            timestampDifference = pJitterBuffer->headTimestamp - pRtpPacket->header.timestamp;
+        }
+        else {
+            timestampDifference = (MAX_TIMESTAMP - pRtpPacket->header.timestamp) + pJitterBuffer->headTimestamp;
+        }
+
+        //1 frame per second, and 1 packet per frame, the most charitable case we can consider
+        //TODO track most recent FPS to improve this metric
+        maxTimePassed = pJitterBuffer->clockRate * seqNoDifference;
+
+        if(maxTimePassed >= timestampDifference) {
+            retVal = TRUE;
+        }
+    }
+    return retVal;
+}
+
 //return true if sequence numbers are now overflowing
 BOOL enterSequenceNumberOverflowCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
     BOOL overflow = FALSE;
@@ -106,11 +136,13 @@ BOOL enterSequenceNumberOverflowCheck(PJitterBuffer pJitterBuffer, PRtpPacket pR
         //underflow case
         else if(headCheckingAllowed(pJitterBuffer, pRtpPacket)) { 
             if (pJitterBuffer->headSequenceNumber < MAX_OUT_OF_ORDER_PACKET_DIFFERENCE) {
-                if ((pRtpPacket->header.sequenceNumber >=
-                            (MAX_UINT16 - (MAX_OUT_OF_ORDER_PACKET_DIFFERENCE - pJitterBuffer->headSequenceNumber))) ||
-                        (pJitterBuffer->headSequenceNumber > pRtpPacket->header.sequenceNumber)) {
-                    //Sequence number underflow detected
-                    underflow = TRUE;
+                if (pRtpPacket->header.sequenceNumber >=
+                            (MAX_UINT16 - (MAX_OUT_OF_ORDER_PACKET_DIFFERENCE - pJitterBuffer->headSequenceNumber))) {
+                    //Possible sequence number underflow detected, now lets check the timestamps to be certain
+                    //this is an earlier value, and not a much later.
+                    if(underflowPossible(pJitterBuffer, pRtpPacket)) {
+                        underflow = TRUE;
+                    }
                 }
             }
         }
@@ -176,6 +208,7 @@ BOOL exitSequenceNumberOverflowCheck(PJitterBuffer pJitterBuffer) {
     //can't exit if you're not in it
     if(pJitterBuffer->sequenceNumberOverflowState) {
         if(pJitterBuffer->headSequenceNumber <= pJitterBuffer->tailSequenceNumber) {
+            pJitterBuffer->sequenceNumberOverflowState = FALSE;
             retVal = TRUE;
         }
     }
@@ -189,6 +222,7 @@ BOOL exitTimestampOverflowCheck(PJitterBuffer pJitterBuffer) {
     //can't exit if you're not in it
     if(pJitterBuffer->timestampOverFlowState) {
         if(pJitterBuffer->headTimestamp <= pJitterBuffer->tailTimestamp) {
+            pJitterBuffer->timestampOverFlowState = FALSE;
             retVal = TRUE;
         }
     }
@@ -458,7 +492,7 @@ STATUS jitterBufferInternalParse(PJitterBuffer pJitterBuffer, BOOL bufferClosed)
         earliestAllowedTimestamp = pJitterBuffer->tailTimestamp - pJitterBuffer->maxLatency;
     }
 
-    lastIndex = pJitterBuffer->tailSequenceNumber;
+    lastIndex = pJitterBuffer->tailSequenceNumber + 1;
     index = pJitterBuffer->headSequenceNumber;
     startDropIndex = index;
     // Loop through entire buffer to find complete frames.
diff --git a/src/source/PeerConnection/JitterBuffer.h b/src/source/PeerConnection/JitterBuffer.h
index 59afcf1d25..36560d3ad5 100644
--- a/src/source/PeerConnection/JitterBuffer.h
+++ b/src/source/PeerConnection/JitterBuffer.h
@@ -32,7 +32,9 @@ typedef struct {
     UINT16 tailSequenceNumber;
     UINT32 headTimestamp;
     UINT32 tailTimestamp;
-    UINT32 maxLatency;
+    //this is set to U64 even though rtp timestamps are U32
+    //in order to allow calculations to not cause overflow
+    UINT64 maxLatency;
     UINT64 customData;
     UINT32 clockRate;
     BOOL started;
diff --git a/tst/JitterBufferFunctionalityTest.cpp b/tst/JitterBufferFunctionalityTest.cpp
index c857bea78a..906f6f191d 100644
--- a/tst/JitterBufferFunctionalityTest.cpp
+++ b/tst/JitterBufferFunctionalityTest.cpp
@@ -14,7 +14,11 @@ TEST_F(JitterBufferFunctionalityTest, continousPacketsComeInOrder)
 {
     UINT32 i;
     UINT32 pktCount = 5;
+    UINT32 startingSequenceNumber = 0;
+
     initializeJitterBuffer(3, 0, pktCount);
+    srand(time(0));
+    startingSequenceNumber = rand()%UINT16_MAX;
 
     // First frame "1" at timestamp 100 - rtp packet #0
     mPRtpPackets[0]->payloadLength = 1;
@@ -22,6 +26,7 @@ TEST_F(JitterBufferFunctionalityTest, continousPacketsComeInOrder)
     mPRtpPackets[0]->payload[0] = 1;
     mPRtpPackets[0]->payload[1] = 1; // First packet of a frame
     mPRtpPackets[0]->header.timestamp = 100;
+    mPRtpPackets[0]->header.sequenceNumber = startingSequenceNumber++;
 
     // Expected to get frame "1"
     mPExpectedFrameArr[0] = (PBYTE) MEMALLOC(1);
@@ -34,12 +39,15 @@ TEST_F(JitterBufferFunctionalityTest, continousPacketsComeInOrder)
     mPRtpPackets[1]->payload[0] = 2;
     mPRtpPackets[1]->payload[1] = 1; // First packet of a frame
     mPRtpPackets[1]->header.timestamp = 200;
+    mPRtpPackets[1]->header.sequenceNumber = startingSequenceNumber++;
+
     mPRtpPackets[2]->payloadLength = 2;
     mPRtpPackets[2]->payload = (PBYTE) MEMALLOC(mPRtpPackets[2]->payloadLength + 1);
     mPRtpPackets[2]->payload[0] = 3;
     mPRtpPackets[2]->payload[1] = 4;
     mPRtpPackets[2]->payload[2] = 0; // following packet of a frame
     mPRtpPackets[2]->header.timestamp = 200;
+    mPRtpPackets[2]->header.sequenceNumber = startingSequenceNumber++;
 
     // Expected to get frame "234"
     mPExpectedFrameArr[1] = (PBYTE) MEMALLOC(3);
@@ -55,11 +63,13 @@ TEST_F(JitterBufferFunctionalityTest, continousPacketsComeInOrder)
     mPRtpPackets[3]->payload[1] = 6;
     mPRtpPackets[3]->payload[2] = 1; // First packet of a frame
     mPRtpPackets[3]->header.timestamp = 300;
+    mPRtpPackets[3]->header.sequenceNumber = startingSequenceNumber++;
     mPRtpPackets[4]->payloadLength = 1;
     mPRtpPackets[4]->payload = (PBYTE) MEMALLOC(mPRtpPackets[4]->payloadLength + 1);
     mPRtpPackets[4]->payload[0] = 7;
     mPRtpPackets[4]->payload[1] = 0; // Following packet of a frame
     mPRtpPackets[4]->header.timestamp = 300;
+    mPRtpPackets[4]->header.sequenceNumber = startingSequenceNumber++;
 
     // Expected to get frame "567" at close
     mPExpectedFrameArr[2] = (PBYTE) MEMALLOC(3);
@@ -1120,6 +1130,130 @@ TEST_F(JitterBufferFunctionalityTest, latePacketsOfAlreadyDroppedFrame)
 
     clearJitterBufferForTest();
 }
+
+TEST_F(JitterBufferFunctionalityTest, timestampOverflowTest)
+{
+}
+
+TEST_F(JitterBufferFunctionalityTest, timestampUnderflowTest)
+{
+}
+
+TEST_F(JitterBufferFunctionalityTest, SequenceNumberOverflowTest)
+{
+    UINT32 i;
+    UINT32 pktCount = 7;
+    initializeJitterBuffer(4, 0, pktCount);
+
+    // First frame "1" at timestamp 100 - rtp packet #65534
+    mPRtpPackets[0]->payloadLength = 1;
+    mPRtpPackets[0]->payload = (PBYTE) MEMALLOC(mPRtpPackets[0]->payloadLength + 1);
+    mPRtpPackets[0]->payload[0] = 1;
+    mPRtpPackets[0]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[0]->header.timestamp = 100;
+    mPRtpPackets[0]->header.sequenceNumber = 65534;
+
+    mPRtpPackets[1]->payloadLength = 1;
+    mPRtpPackets[1]->payload = (PBYTE) MEMALLOC(mPRtpPackets[1]->payloadLength + 1);
+    mPRtpPackets[1]->payload[0] = 2;
+    mPRtpPackets[1]->payload[1] = 0;
+    mPRtpPackets[1]->header.timestamp = 100;
+    mPRtpPackets[1]->header.sequenceNumber = 65535;
+
+    mPRtpPackets[2]->payloadLength = 1;
+    mPRtpPackets[2]->payload = (PBYTE) MEMALLOC(mPRtpPackets[2]->payloadLength + 1);
+    mPRtpPackets[2]->payload[0] = 3;
+    mPRtpPackets[2]->payload[1] = 0;
+    mPRtpPackets[2]->header.timestamp = 100;
+    mPRtpPackets[2]->header.sequenceNumber = 0;
+
+    // Expected to get frame "1234"
+    mPExpectedFrameArr[0] = (PBYTE) MEMALLOC(4);
+    mPExpectedFrameArr[0][0] = 1;
+    mPExpectedFrameArr[0][1] = 2;
+    mPExpectedFrameArr[0][2] = 3;
+    mPExpectedFrameArr[0][3] = 4;
+    mExpectedFrameSizeArr[0] = 4;
+
+    // Second frame "2" at timestamp 200 - rtp packet #65535
+    mPRtpPackets[3]->payloadLength = 1;
+    mPRtpPackets[3]->payload = (PBYTE) MEMALLOC(mPRtpPackets[3]->payloadLength + 1);
+    mPRtpPackets[3]->payload[0] = 2;
+    mPRtpPackets[3]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[3]->header.timestamp = 200;
+    mPRtpPackets[3]->header.sequenceNumber = 2;
+
+    // Expected to get frame "2"
+    mPExpectedFrameArr[1] = (PBYTE) MEMALLOC(1);
+    mPExpectedFrameArr[1][0] = 2;
+    mExpectedFrameSizeArr[1] = 1;
+
+    // Third frame "3" at timestamp 300 - rtp packet #0
+    mPRtpPackets[4]->payloadLength = 1;
+    mPRtpPackets[4]->payload = (PBYTE) MEMALLOC(mPRtpPackets[4]->payloadLength + 1);
+    mPRtpPackets[4]->payload[0] = 3;
+    mPRtpPackets[4]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[4]->header.timestamp = 300;
+    mPRtpPackets[4]->header.sequenceNumber = 3;
+
+    // Expected to get frame "3" at close
+    mPExpectedFrameArr[2] = (PBYTE) MEMALLOC(1);
+    mPExpectedFrameArr[2][0] = 3;
+    mExpectedFrameSizeArr[2] = 1;
+
+    // Third frame "4" at timestamp 400 - rtp packet #1
+    mPRtpPackets[5]->payloadLength = 1;
+    mPRtpPackets[5]->payload = (PBYTE) MEMALLOC(mPRtpPackets[5]->payloadLength + 1);
+    mPRtpPackets[5]->payload[0] = 4;
+    mPRtpPackets[5]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[5]->header.timestamp = 400;
+    mPRtpPackets[5]->header.sequenceNumber = 4;
+
+    // Expected to get frame "4" at close
+    mPExpectedFrameArr[3] = (PBYTE) MEMALLOC(1);
+    mPExpectedFrameArr[3][0] = 4;
+    mExpectedFrameSizeArr[3] = 1;
+
+    mPRtpPackets[6]->payloadLength = 1;
+    mPRtpPackets[6]->payload = (PBYTE) MEMALLOC(mPRtpPackets[6]->payloadLength + 1);
+    mPRtpPackets[6]->payload[0] = 4;
+    mPRtpPackets[6]->payload[1] = 0; // First packet of a frame
+    mPRtpPackets[6]->header.timestamp = 100;
+    mPRtpPackets[6]->header.sequenceNumber = 1;
+
+    setPayloadToFree();
+
+    for (i = 0; i < pktCount; i++) {
+        EXPECT_EQ(STATUS_SUCCESS, jitterBufferPush(mJitterBuffer, mPRtpPackets[i], nullptr));
+        switch (i) {
+            case 0:
+            case 1:
+            case 2:
+            case 3:
+            case 4:
+            case 5:
+                EXPECT_EQ(0, mReadyFrameIndex);
+                break;
+            case 6:
+                EXPECT_EQ(3, mReadyFrameIndex);
+                break;
+            default:
+                ASSERT_TRUE(FALSE);
+        }
+        EXPECT_EQ(0, mDroppedFrameIndex);
+    }
+
+    clearJitterBufferForTest();
+}
+
+TEST_F(JitterBufferFunctionalityTest, SequenceNumberUnderflowTest)
+{
+}
+
+TEST_F(JitterBufferFunctionalityTest, DoubleOverflowTest)
+{
+}
+
 } // namespace webrtcclient
 } // namespace video
 } // namespace kinesis

From 7131159a8ba8702b2de703dad30ddbed1962bd2c Mon Sep 17 00:00:00 2001
From: James Delaplane <delaplan@amazon.com>
Date: Wed, 29 Mar 2023 10:15:02 -0700
Subject: [PATCH 3/8] More tests and accompanying fixes

---
 .gitignore                                    |   1 +
 .../kinesis/video/webrtcclient/Include.h      |   4 +-
 src/source/PeerConnection/JitterBuffer.c      |  20 +-
 tst/JitterBufferFunctionalityTest.cpp         | 375 +++++++++++++++++-
 4 files changed, 387 insertions(+), 13 deletions(-)

diff --git a/.gitignore b/.gitignore
index ab8c440fa0..6cd9353247 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,3 +10,4 @@ open-source/
 outputs
 tags
 CMakeLists.txt.user
+*.swp
diff --git a/src/include/com/amazonaws/kinesis/video/webrtcclient/Include.h b/src/include/com/amazonaws/kinesis/video/webrtcclient/Include.h
index a6810c0865..eb558a74e7 100644
--- a/src/include/com/amazonaws/kinesis/video/webrtcclient/Include.h
+++ b/src/include/com/amazonaws/kinesis/video/webrtcclient/Include.h
@@ -619,12 +619,12 @@ extern "C" {
 /**
  * Maximum sequence number in rtp packet/jitter buffer
  */
-#define MAX_SEQUENCE_NUM ((UINT32) MAX_UINT16)
+#define MAX_RTP_SEQUENCE_NUM ((UINT32) MAX_UINT16)
 
 /**
  * Maximum timestamp in rtp packet/jitter buffer
  */
-#define MAX_TIMESTAMP ((UINT32) MAX_UINT32)
+#define MAX_RTP_TIMESTAMP ((UINT32) MAX_UINT32)
 
 /**
  * Parameterized string for KVS STUN Server
diff --git a/src/source/PeerConnection/JitterBuffer.c b/src/source/PeerConnection/JitterBuffer.c
index 517e76eb99..116fd7788c 100644
--- a/src/source/PeerConnection/JitterBuffer.c
+++ b/src/source/PeerConnection/JitterBuffer.c
@@ -32,12 +32,12 @@ STATUS createJitterBuffer(FrameReadyFunc onFrameReadyFunc, FrameDroppedFunc onFr
     }
     pJitterBuffer->maxLatency = pJitterBuffer->maxLatency * pJitterBuffer->clockRate / HUNDREDS_OF_NANOS_IN_A_SECOND;
 
-    CHK(pJitterBuffer->maxLatency < MAX_TIMESTAMP, STATUS_INVALID_ARG);
+    CHK(pJitterBuffer->maxLatency < MAX_RTP_TIMESTAMP, STATUS_INVALID_ARG);
 
     pJitterBuffer->tailTimestamp = 0;
     pJitterBuffer->headTimestamp = MAX_UINT32;
-    pJitterBuffer->headSequenceNumber = MAX_SEQUENCE_NUM;
-    pJitterBuffer->tailSequenceNumber = MAX_SEQUENCE_NUM;
+    pJitterBuffer->headSequenceNumber = MAX_RTP_SEQUENCE_NUM;
+    pJitterBuffer->tailSequenceNumber = MAX_RTP_SEQUENCE_NUM;
     pJitterBuffer->started = FALSE;
     pJitterBuffer->firstFrameProcessed = FALSE;
     pJitterBuffer->timestampOverFlowState = FALSE;
@@ -75,7 +75,7 @@ STATUS freeJitterBuffer(PJitterBuffer* ppJitterBuffer)
     pJitterBuffer = *ppJitterBuffer;
 
     jitterBufferInternalParse(pJitterBuffer, TRUE);
-    jitterBufferDropBufferData(pJitterBuffer, 0, MAX_SEQUENCE_NUM, 0);
+    jitterBufferDropBufferData(pJitterBuffer, 0, MAX_RTP_SEQUENCE_NUM, 0);
     hashTableFree(pJitterBuffer->pPkgBufferHashTable);
 
     SAFE_MEMFREE(*ppJitterBuffer);
@@ -96,12 +96,12 @@ BOOL underflowPossible(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
         retVal = TRUE;
     }
     else {
-        seqNoDifference = (MAX_SEQUENCE_NUM - pRtpPacket->header.sequenceNumber) + pJitterBuffer->headSequenceNumber;
+        seqNoDifference = (MAX_RTP_SEQUENCE_NUM - pRtpPacket->header.sequenceNumber) + pJitterBuffer->headSequenceNumber;
         if(pJitterBuffer->headTimestamp > pRtpPacket->header.timestamp) {
             timestampDifference = pJitterBuffer->headTimestamp - pRtpPacket->header.timestamp;
         }
         else {
-            timestampDifference = (MAX_TIMESTAMP - pRtpPacket->header.timestamp) + pJitterBuffer->headTimestamp;
+            timestampDifference = (MAX_RTP_TIMESTAMP - pRtpPacket->header.timestamp) + pJitterBuffer->headTimestamp;
         }
 
         //1 frame per second, and 1 packet per frame, the most charitable case we can consider
@@ -119,7 +119,7 @@ BOOL underflowPossible(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
 BOOL enterSequenceNumberOverflowCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
     BOOL overflow = FALSE;
     BOOL underflow = FALSE;
-    UINT16 packetsUntilOverflow = MAX_SEQUENCE_NUM - pJitterBuffer->tailSequenceNumber;
+    UINT16 packetsUntilOverflow = MAX_RTP_SEQUENCE_NUM - pJitterBuffer->tailSequenceNumber;
     UINT16 underflowFromMaxRange = 0;
 
     if(!pJitterBuffer->sequenceNumberOverflowState) {
@@ -154,10 +154,12 @@ BOOL enterSequenceNumberOverflowCheck(PJitterBuffer pJitterBuffer, PRtpPacket pR
     if(overflow) {
         pJitterBuffer->sequenceNumberOverflowState = TRUE;
         pJitterBuffer->tailSequenceNumber = pRtpPacket->header.sequenceNumber;
+        pJitterBuffer->tailTimestamp = pRtpPacket->header.timestamp;
     }
     if(underflow) {
         pJitterBuffer->sequenceNumberOverflowState = TRUE;
         pJitterBuffer->headSequenceNumber = pRtpPacket->header.sequenceNumber;
+        pJitterBuffer->headTimestamp = pRtpPacket->header.timestamp;
     }
     return (overflow || underflow);
 }
@@ -173,7 +175,7 @@ BOOL enterTimestampOverflowCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPac
             //We always check sequence number first, so the 'or equal to' checks if we just set the tail.
             //That would be a corner case of sequence number and timestamp both overflowing
             //in this one packet.
-            if(pRtpPacket->header.sequenceNumber >= pJitterBuffer->tailSequenceNumber) {
+            if(tailSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
                 //RTP timestamp overflow detected!
                 overflow = TRUE;
             }
@@ -374,7 +376,7 @@ BOOL withinLatencyTolerance(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket)
         if(pJitterBuffer->timestampOverFlowState) {
             //calculate max-latency across the overflow boundry without triggering underflow
             if(pJitterBuffer->tailTimestamp < pJitterBuffer->maxLatency) {
-                minimumTimestamp = MAX_TIMESTAMP - (pJitterBuffer->maxLatency - pJitterBuffer->tailTimestamp);
+                minimumTimestamp = MAX_RTP_TIMESTAMP - (pJitterBuffer->maxLatency - pJitterBuffer->tailTimestamp);
             }
             //Is the packet within the current range or is it a new head/tail
             if(pRtpPacket->header.timestamp < pJitterBuffer->tailTimestamp ||
diff --git a/tst/JitterBufferFunctionalityTest.cpp b/tst/JitterBufferFunctionalityTest.cpp
index 906f6f191d..5bbc67b8e0 100644
--- a/tst/JitterBufferFunctionalityTest.cpp
+++ b/tst/JitterBufferFunctionalityTest.cpp
@@ -15,11 +15,11 @@ TEST_F(JitterBufferFunctionalityTest, continousPacketsComeInOrder)
     UINT32 i;
     UINT32 pktCount = 5;
     UINT32 startingSequenceNumber = 0;
-
-    initializeJitterBuffer(3, 0, pktCount);
     srand(time(0));
     startingSequenceNumber = rand()%UINT16_MAX;
 
+    initializeJitterBuffer(3, 0, pktCount);
+
     // First frame "1" at timestamp 100 - rtp packet #0
     mPRtpPackets[0]->payloadLength = 1;
     mPRtpPackets[0]->payload = (PBYTE) MEMALLOC(mPRtpPackets[0]->payloadLength + 1);
@@ -1137,6 +1137,137 @@ TEST_F(JitterBufferFunctionalityTest, timestampOverflowTest)
 
 TEST_F(JitterBufferFunctionalityTest, timestampUnderflowTest)
 {
+    UINT32 i;
+    UINT32 pktCount = 8;
+    UINT32 startingSequenceNumber = 0;
+    UINT32 missingSequenceNumber = 0;
+    UINT32 firstSequenceNumber = 0;
+    srand(time(0));
+    startingSequenceNumber = rand()%UINT16_MAX;
+    firstSequenceNumber = startingSequenceNumber - 1;
+
+    initializeJitterBuffer(5, 0, pktCount);
+
+    // Second frame "1234" at timestamp 0 -- first frame comes later
+    // The "4" will come late
+    mPRtpPackets[0]->payloadLength = 1;
+    mPRtpPackets[0]->payload = (PBYTE) MEMALLOC(mPRtpPackets[0]->payloadLength + 1);
+    mPRtpPackets[0]->payload[0] = 1;
+    mPRtpPackets[0]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[0]->header.timestamp = 0;
+    mPRtpPackets[0]->header.sequenceNumber = startingSequenceNumber++;
+
+    mPRtpPackets[1]->payloadLength = 1;
+    mPRtpPackets[1]->payload = (PBYTE) MEMALLOC(mPRtpPackets[1]->payloadLength + 1);
+    mPRtpPackets[1]->payload[0] = 2;
+    mPRtpPackets[1]->payload[1] = 0;
+    mPRtpPackets[1]->header.timestamp = 0;
+    mPRtpPackets[1]->header.sequenceNumber = startingSequenceNumber++;
+
+    mPRtpPackets[2]->payloadLength = 1;
+    mPRtpPackets[2]->payload = (PBYTE) MEMALLOC(mPRtpPackets[2]->payloadLength + 1);
+    mPRtpPackets[2]->payload[0] = 3;
+    mPRtpPackets[2]->payload[1] = 0;
+    mPRtpPackets[2]->header.timestamp = 0;
+    mPRtpPackets[2]->header.sequenceNumber = startingSequenceNumber++;
+
+    //sequence number of 4th packet of this frame
+    missingSequenceNumber = startingSequenceNumber++;
+
+    // Expected to get frame "1234"
+    mPExpectedFrameArr[1] = (PBYTE) MEMALLOC(4);
+    mPExpectedFrameArr[1][0] = 1;
+    mPExpectedFrameArr[1][1] = 2;
+    mPExpectedFrameArr[1][2] = 3;
+    mPExpectedFrameArr[1][3] = 4;
+    mExpectedFrameSizeArr[1] = 4;
+
+    // Third frame "2" at timestamp 300
+    mPRtpPackets[3]->payloadLength = 1;
+    mPRtpPackets[3]->payload = (PBYTE) MEMALLOC(mPRtpPackets[3]->payloadLength + 1);
+    mPRtpPackets[3]->payload[0] = 2;
+    mPRtpPackets[3]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[3]->header.timestamp = 300;
+    mPRtpPackets[3]->header.sequenceNumber = startingSequenceNumber++;
+
+    // Expected to get frame "2"
+    mPExpectedFrameArr[2] = (PBYTE) MEMALLOC(1);
+    mPExpectedFrameArr[2][0] = 2;
+    mExpectedFrameSizeArr[2] = 1;
+
+    // Fourth frame "3" at timestamp 600
+    mPRtpPackets[4]->payloadLength = 1;
+    mPRtpPackets[4]->payload = (PBYTE) MEMALLOC(mPRtpPackets[4]->payloadLength + 1);
+    mPRtpPackets[4]->payload[0] = 3;
+    mPRtpPackets[4]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[4]->header.timestamp = 600;
+    mPRtpPackets[4]->header.sequenceNumber = startingSequenceNumber++;
+
+    // Expected to get frame "3" at close
+    mPExpectedFrameArr[3] = (PBYTE) MEMALLOC(1);
+    mPExpectedFrameArr[3][0] = 3;
+    mExpectedFrameSizeArr[3] = 1;
+
+    // Fifth frame "4" at timestamp 400 - rtp packet #1
+    mPRtpPackets[5]->payloadLength = 1;
+    mPRtpPackets[5]->payload = (PBYTE) MEMALLOC(mPRtpPackets[5]->payloadLength + 1);
+    mPRtpPackets[5]->payload[0] = 4;
+    mPRtpPackets[5]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[5]->header.timestamp = 900;
+    mPRtpPackets[5]->header.sequenceNumber = startingSequenceNumber++;
+
+    // Expected to get frame "4" at close
+    mPExpectedFrameArr[4] = (PBYTE) MEMALLOC(1);
+    mPExpectedFrameArr[4][0] = 4;
+    mExpectedFrameSizeArr[4] = 1;
+
+    // Actual first frame, underflow timestamp
+    mPRtpPackets[6]->payloadLength = 1;
+    mPRtpPackets[6]->payload = (PBYTE) MEMALLOC(mPRtpPackets[6]->payloadLength + 1);
+    mPRtpPackets[6]->payload[0] = 4;
+    mPRtpPackets[6]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[6]->header.timestamp = MAX_RTP_TIMESTAMP - 300;
+    mPRtpPackets[6]->header.sequenceNumber = firstSequenceNumber;
+
+    // Expected to get frame "4" at close
+    mPExpectedFrameArr[0] = (PBYTE) MEMALLOC(1);
+    mPExpectedFrameArr[0][0] = 4;
+    mExpectedFrameSizeArr[0] = 1;
+
+    // Missing 4th packet in 2nd frame
+    mPRtpPackets[7]->payloadLength = 1;
+    mPRtpPackets[7]->payload = (PBYTE) MEMALLOC(mPRtpPackets[7]->payloadLength + 1);
+    mPRtpPackets[7]->payload[0] = 4;
+    mPRtpPackets[7]->payload[1] = 0; 
+    mPRtpPackets[7]->header.timestamp = 0;
+    mPRtpPackets[7]->header.sequenceNumber = missingSequenceNumber;
+
+    setPayloadToFree();
+
+    for (i = 0; i < pktCount; i++) {
+        EXPECT_EQ(STATUS_SUCCESS, jitterBufferPush(mJitterBuffer, mPRtpPackets[i], nullptr));
+        switch (i) {
+            case 0:
+            case 1:
+            case 2:
+            case 3:
+            case 4:
+            case 5:
+                EXPECT_EQ(0, mReadyFrameIndex);
+                break;
+            case 6:
+                EXPECT_EQ(1, mReadyFrameIndex);
+                break;
+            case 7:
+                EXPECT_EQ(4, mReadyFrameIndex);
+                break;
+            default:
+                ASSERT_TRUE(FALSE);
+        }
+        EXPECT_EQ(0, mDroppedFrameIndex);
+    }
+
+    clearJitterBufferForTest();
 }
 
 TEST_F(JitterBufferFunctionalityTest, SequenceNumberOverflowTest)
@@ -1248,9 +1379,249 @@ TEST_F(JitterBufferFunctionalityTest, SequenceNumberOverflowTest)
 
 TEST_F(JitterBufferFunctionalityTest, SequenceNumberUnderflowTest)
 {
+    UINT32 i;
+    UINT32 pktCount = 8;
+    UINT32 startingSequenceNumber = 0;
+    UINT32 missingSequenceNumber = 0;
+    UINT32 firstSequenceNumber = MAX_RTP_SEQUENCE_NUM-2;
+
+    srand(time(0));
+    UINT32 startingTimestamp = rand()%UINT32_MAX;
+
+    initializeJitterBuffer(5, 0, pktCount);
+
+    // Fourth frame "1234", first frame comes later
+    // The "4" will come late
+    mPRtpPackets[0]->payloadLength = 1;
+    mPRtpPackets[0]->payload = (PBYTE) MEMALLOC(mPRtpPackets[0]->payloadLength + 1);
+    mPRtpPackets[0]->payload[0] = 1;
+    mPRtpPackets[0]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[0]->header.timestamp = startingTimestamp + 300;
+    mPRtpPackets[0]->header.sequenceNumber = startingSequenceNumber++;
+
+    mPRtpPackets[1]->payloadLength = 1;
+    mPRtpPackets[1]->payload = (PBYTE) MEMALLOC(mPRtpPackets[1]->payloadLength + 1);
+    mPRtpPackets[1]->payload[0] = 2;
+    mPRtpPackets[1]->payload[1] = 0;
+    mPRtpPackets[1]->header.timestamp = startingTimestamp + 300;
+    mPRtpPackets[1]->header.sequenceNumber = startingSequenceNumber++;
+
+    mPRtpPackets[2]->payloadLength = 1;
+    mPRtpPackets[2]->payload = (PBYTE) MEMALLOC(mPRtpPackets[2]->payloadLength + 1);
+    mPRtpPackets[2]->payload[0] = 3;
+    mPRtpPackets[2]->payload[1] = 0;
+    mPRtpPackets[2]->header.timestamp = startingTimestamp + 300;
+    mPRtpPackets[2]->header.sequenceNumber = startingSequenceNumber++;
+
+    //sequence number of 4th packet of this frame
+    missingSequenceNumber = startingSequenceNumber++;
+
+    // Expected to get frame "1234"
+    mPExpectedFrameArr[3] = (PBYTE) MEMALLOC(4);
+    mPExpectedFrameArr[3][0] = 1;
+    mPExpectedFrameArr[3][1] = 2;
+    mPExpectedFrameArr[3][2] = 3;
+    mPExpectedFrameArr[3][3] = 4;
+    mExpectedFrameSizeArr[3] = 4;
+
+    // First frame "2" at timestamp 300
+    mPRtpPackets[3]->payloadLength = 1;
+    mPRtpPackets[3]->payload = (PBYTE) MEMALLOC(mPRtpPackets[3]->payloadLength + 1);
+    mPRtpPackets[3]->payload[0] = 2;
+    mPRtpPackets[3]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[3]->header.timestamp = startingTimestamp;
+    mPRtpPackets[3]->header.sequenceNumber = firstSequenceNumber++;
+
+    // Expected to get frame "2"
+    mPExpectedFrameArr[0] = (PBYTE) MEMALLOC(1);
+    mPExpectedFrameArr[0][0] = 2;
+    mExpectedFrameSizeArr[0] = 1;
+
+    // Second frame "3" at timestamp 600
+    mPRtpPackets[4]->payloadLength = 1;
+    mPRtpPackets[4]->payload = (PBYTE) MEMALLOC(mPRtpPackets[4]->payloadLength + 1);
+    mPRtpPackets[4]->payload[0] = 3;
+    mPRtpPackets[4]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[4]->header.timestamp = startingTimestamp+100;
+    mPRtpPackets[4]->header.sequenceNumber = firstSequenceNumber++;
+
+    // Expected to get frame "3" at close
+    mPExpectedFrameArr[1] = (PBYTE) MEMALLOC(1);
+    mPExpectedFrameArr[1][0] = 3;
+    mExpectedFrameSizeArr[1] = 1;
+
+    // Third frame "4" 
+    mPRtpPackets[5]->payloadLength = 1;
+    mPRtpPackets[5]->payload = (PBYTE) MEMALLOC(mPRtpPackets[5]->payloadLength + 1);
+    mPRtpPackets[5]->payload[0] = 4;
+    mPRtpPackets[5]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[5]->header.timestamp = startingTimestamp+200;
+    mPRtpPackets[5]->header.sequenceNumber = firstSequenceNumber++;
+
+    // Expected to get frame "4" at close
+    mPExpectedFrameArr[2] = (PBYTE) MEMALLOC(1);
+    mPExpectedFrameArr[2][0] = 4;
+    mExpectedFrameSizeArr[2] = 1;
+
+    // Fifth frame
+    mPRtpPackets[6]->payloadLength = 1;
+    mPRtpPackets[6]->payload = (PBYTE) MEMALLOC(mPRtpPackets[6]->payloadLength + 1);
+    mPRtpPackets[6]->payload[0] = 4;
+    mPRtpPackets[6]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[6]->header.timestamp = startingTimestamp+400;
+    mPRtpPackets[6]->header.sequenceNumber = startingSequenceNumber++;
+
+    // Expected to get frame "4" at close
+    mPExpectedFrameArr[4] = (PBYTE) MEMALLOC(1);
+    mPExpectedFrameArr[4][0] = 4;
+    mExpectedFrameSizeArr[4] = 1;
+
+    // Missing 4th packet in 4th frame
+    mPRtpPackets[7]->payloadLength = 1;
+    mPRtpPackets[7]->payload = (PBYTE) MEMALLOC(mPRtpPackets[7]->payloadLength + 1);
+    mPRtpPackets[7]->payload[0] = 4;
+    mPRtpPackets[7]->payload[1] = 0; 
+    mPRtpPackets[7]->header.timestamp = startingTimestamp+300;
+    mPRtpPackets[7]->header.sequenceNumber = missingSequenceNumber;
+
+    setPayloadToFree();
+
+    for (i = 0; i < pktCount; i++) {
+        EXPECT_EQ(STATUS_SUCCESS, jitterBufferPush(mJitterBuffer, mPRtpPackets[i], nullptr));
+        switch (i) {
+            case 0:
+            case 1:
+            case 2:
+            case 3:
+                EXPECT_EQ(0, mReadyFrameIndex);
+                break;
+            case 4:
+                EXPECT_EQ(1, mReadyFrameIndex);
+                break;
+            case 5:
+            case 6:
+                EXPECT_EQ(3, mReadyFrameIndex);
+                break;
+            case 7:
+                EXPECT_EQ(4, mReadyFrameIndex);
+                break;
+            default:
+                ASSERT_TRUE(FALSE);
+        }
+        EXPECT_EQ(0, mDroppedFrameIndex);
+    }
+
+    clearJitterBufferForTest();
 }
 
 TEST_F(JitterBufferFunctionalityTest, DoubleOverflowTest)
+{
+    UINT32 i;
+    UINT32 pktCount = 7;
+    initializeJitterBuffer(4, 0, pktCount);
+
+    // First frame "1" at timestamp 100 - rtp packet #65534
+    mPRtpPackets[0]->payloadLength = 1;
+    mPRtpPackets[0]->payload = (PBYTE) MEMALLOC(mPRtpPackets[0]->payloadLength + 1);
+    mPRtpPackets[0]->payload[0] = 1;
+    mPRtpPackets[0]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[0]->header.timestamp = MAX_RTP_TIMESTAMP - 500;
+    mPRtpPackets[0]->header.sequenceNumber = 65534;
+
+    mPRtpPackets[1]->payloadLength = 1;
+    mPRtpPackets[1]->payload = (PBYTE) MEMALLOC(mPRtpPackets[1]->payloadLength + 1);
+    mPRtpPackets[1]->payload[0] = 2;
+    mPRtpPackets[1]->payload[1] = 0;
+    mPRtpPackets[1]->header.timestamp = MAX_RTP_TIMESTAMP - 500;
+    mPRtpPackets[1]->header.sequenceNumber = 65535;
+
+    mPRtpPackets[2]->payloadLength = 1;
+    mPRtpPackets[2]->payload = (PBYTE) MEMALLOC(mPRtpPackets[2]->payloadLength + 1);
+    mPRtpPackets[2]->payload[0] = 3;
+    mPRtpPackets[2]->payload[1] = 0;
+    mPRtpPackets[2]->header.timestamp = MAX_RTP_TIMESTAMP - 500;
+    mPRtpPackets[2]->header.sequenceNumber = 0;
+
+    // Expected to get frame "1234"
+    mPExpectedFrameArr[0] = (PBYTE) MEMALLOC(4);
+    mPExpectedFrameArr[0][0] = 1;
+    mPExpectedFrameArr[0][1] = 2;
+    mPExpectedFrameArr[0][2] = 3;
+    mPExpectedFrameArr[0][3] = 4;
+    mExpectedFrameSizeArr[0] = 4;
+
+    // Second frame "2" at timestamp 200 - rtp packet #65535
+    mPRtpPackets[3]->payloadLength = 1;
+    mPRtpPackets[3]->payload = (PBYTE) MEMALLOC(mPRtpPackets[3]->payloadLength + 1);
+    mPRtpPackets[3]->payload[0] = 2;
+    mPRtpPackets[3]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[3]->header.timestamp = MAX_RTP_TIMESTAMP - 100;
+    mPRtpPackets[3]->header.sequenceNumber = 2;
+
+    // Expected to get frame "2"
+    mPExpectedFrameArr[1] = (PBYTE) MEMALLOC(1);
+    mPExpectedFrameArr[1][0] = 2;
+    mExpectedFrameSizeArr[1] = 1;
+
+    // Third frame "3" at timestamp 300 - rtp packet #0
+    mPRtpPackets[4]->payloadLength = 1;
+    mPRtpPackets[4]->payload = (PBYTE) MEMALLOC(mPRtpPackets[4]->payloadLength + 1);
+    mPRtpPackets[4]->payload[0] = 3;
+    mPRtpPackets[4]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[4]->header.timestamp = 300;
+    mPRtpPackets[4]->header.sequenceNumber = 3;
+
+    // Expected to get frame "3" at close
+    mPExpectedFrameArr[2] = (PBYTE) MEMALLOC(1);
+    mPExpectedFrameArr[2][0] = 3;
+    mExpectedFrameSizeArr[2] = 1;
+
+    // Third frame "4" at timestamp 400 - rtp packet #1
+    mPRtpPackets[5]->payloadLength = 1;
+    mPRtpPackets[5]->payload = (PBYTE) MEMALLOC(mPRtpPackets[5]->payloadLength + 1);
+    mPRtpPackets[5]->payload[0] = 4;
+    mPRtpPackets[5]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[5]->header.timestamp = 600;
+    mPRtpPackets[5]->header.sequenceNumber = 4;
+
+    // Expected to get frame "4" at close
+    mPExpectedFrameArr[3] = (PBYTE) MEMALLOC(1);
+    mPExpectedFrameArr[3][0] = 4;
+    mExpectedFrameSizeArr[3] = 1;
+
+    mPRtpPackets[6]->payloadLength = 1;
+    mPRtpPackets[6]->payload = (PBYTE) MEMALLOC(mPRtpPackets[6]->payloadLength + 1);
+    mPRtpPackets[6]->payload[0] = 4;
+    mPRtpPackets[6]->payload[1] = 0; // First packet of a frame
+    mPRtpPackets[6]->header.timestamp = MAX_RTP_TIMESTAMP - 500;
+    mPRtpPackets[6]->header.sequenceNumber = 1;
+
+    setPayloadToFree();
+
+    for (i = 0; i < pktCount; i++) {
+        EXPECT_EQ(STATUS_SUCCESS, jitterBufferPush(mJitterBuffer, mPRtpPackets[i], nullptr));
+        switch (i) {
+            case 0:
+            case 1:
+            case 2:
+            case 3:
+            case 4:
+            case 5:
+                EXPECT_EQ(0, mReadyFrameIndex);
+                break;
+            case 6:
+                EXPECT_EQ(3, mReadyFrameIndex);
+                break;
+            default:
+                ASSERT_TRUE(FALSE);
+        }
+        EXPECT_EQ(0, mDroppedFrameIndex);
+    }
+
+    clearJitterBufferForTest();
+}
+
+TEST_F(JitterBufferFunctionalityTest, LongRunningWithDroppedPacketsTest)
 {
 }
 

From a601400b514b6623185222c9d50a33c72ddb9a4c Mon Sep 17 00:00:00 2001
From: James Delaplane <delaplan@amazon.com>
Date: Wed, 29 Mar 2023 13:07:54 -0700
Subject: [PATCH 4/8] Resolve coverity finding of multiplication causing type
 overflow

---
 src/source/PeerConnection/JitterBuffer.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/source/PeerConnection/JitterBuffer.c b/src/source/PeerConnection/JitterBuffer.c
index 116fd7788c..37e11c5dad 100644
--- a/src/source/PeerConnection/JitterBuffer.c
+++ b/src/source/PeerConnection/JitterBuffer.c
@@ -89,7 +89,7 @@ STATUS freeJitterBuffer(PJitterBuffer* ppJitterBuffer)
 
 BOOL underflowPossible(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
     BOOL retVal = FALSE;
-    UINT16 seqNoDifference = 0;
+    UINT32 seqNoDifference = 0;
     UINT64 timestampDifference = 0;
     UINT64 maxTimePassed = 0;
     if(pJitterBuffer->headTimestamp == pRtpPacket->header.timestamp) {
@@ -106,7 +106,12 @@ BOOL underflowPossible(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
 
         //1 frame per second, and 1 packet per frame, the most charitable case we can consider
         //TODO track most recent FPS to improve this metric
-        maxTimePassed = pJitterBuffer->clockRate * seqNoDifference;
+        if((MAX_RTP_TIMESTAMP / pJitterBuffer->clockRate) <= seqNoDifference) {
+            maxTimePassed = MAX_RTP_TIMESTAMP;
+        }
+        else {
+            maxTimePassed = pJitterBuffer->clockRate * seqNoDifference;
+        }
 
         if(maxTimePassed >= timestampDifference) {
             retVal = TRUE;

From 555e4fd0a09c34e299852bbdf55bee014e5ccec1 Mon Sep 17 00:00:00 2001
From: James Delaplane <delaplan@amazon.com>
Date: Tue, 4 Apr 2023 10:22:11 -0700
Subject: [PATCH 5/8] Change function order

---
 src/source/PeerConnection/JitterBuffer.c | 169 ++++++++++++-----------
 1 file changed, 87 insertions(+), 82 deletions(-)

diff --git a/src/source/PeerConnection/JitterBuffer.c b/src/source/PeerConnection/JitterBuffer.c
index 37e11c5dad..bf45b3bab1 100644
--- a/src/source/PeerConnection/JitterBuffer.c
+++ b/src/source/PeerConnection/JitterBuffer.c
@@ -120,6 +120,86 @@ BOOL underflowPossible(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
     return retVal;
 }
 
+BOOL headCheckingAllowed(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+    BOOL retVal = FALSE;
+    /*If we haven't yet processed a frame yet, then we don't have a definitive way of knowing if
+     *the first packet we receive is actually the earliest packet we'll ever receive. Since sequence numbers
+     *can start anywhere from 0 - 65535, we need to incorporate some checks to determine if a newly received packet
+     *should be considered the new head. Part of how we determine this is by setting a limit to how many packets off we allow
+     *this out of order case to be. Without setting a limit, then we could run into an odd scenario.
+     * Example:
+     * Push->Packet->SeqNumber == 0. //FIRST PACKET! new head of buffer!
+     * Push->Packet->SeqNumber == 3. //... new head of 65532 packet sized frame? maybe? was 0 the tail?
+     *
+     * To resolve that insanity we set a MAX, and will use that MAX for the range.
+     * This logic is present in headSequenceNumberCheck()
+     *
+     *After the first frame has been processed we don't need or want to make this consideration, since if our parser has
+     *dropped a frame for a good reason then we want to ignore any packets from that dropped frame that may come later.
+     *
+     *However if the packet's timestamp is the same as the head timestamp, then it's possible this is simply an earlier
+     *sequence number of the same packet.
+     */
+    if(!(pJitterBuffer->firstFrameProcessed) || 
+         pJitterBuffer->headTimestamp == pRtpPacket->header.timestamp) {
+        retVal = TRUE;
+    }
+    return retVal;
+}
+
+//return true if pRtpPacket contains the head sequence number
+BOOL headSequenceNumberCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+    BOOL retVal = FALSE;
+    UINT16 minimumHead = 0;
+    if(pJitterBuffer->headSequenceNumber >= MAX_OUT_OF_ORDER_PACKET_DIFFERENCE) {
+        minimumHead = pJitterBuffer->headSequenceNumber - MAX_OUT_OF_ORDER_PACKET_DIFFERENCE;
+    }
+
+    //If we've already done this check and it was true
+    if(pJitterBuffer->headSequenceNumber == pRtpPacket->header.sequenceNumber) {
+        retVal = TRUE;
+    }
+    else if(headCheckingAllowed(pJitterBuffer, pRtpPacket)){
+        if(pJitterBuffer->sequenceNumberOverflowState) {
+            if(pJitterBuffer->tailSequenceNumber < pRtpPacket->header.sequenceNumber &&
+               pJitterBuffer->headSequenceNumber > pRtpPacket->header.sequenceNumber &&
+               pRtpPacket->header.sequenceNumber >= minimumHead) {
+                //This purposefully misses the usecase where the buffer has >65000 entries.
+                //Our buffer is not designed for that use case, and it becomes far too ambiguous
+                //as to which packets are new tails or new heads without adding epoch checks.
+                pJitterBuffer->headSequenceNumber = pRtpPacket->header.sequenceNumber;
+                retVal = TRUE;
+            }
+        }
+        else {
+            if(pRtpPacket->header.sequenceNumber < pJitterBuffer->headSequenceNumber) {
+                if(pRtpPacket->header.sequenceNumber >= minimumHead) {
+                    pJitterBuffer->headSequenceNumber = pRtpPacket->header.sequenceNumber;
+                    retVal = TRUE;
+                }
+            }
+        }
+
+    }
+    return retVal;
+}
+
+//return true if pRtpPacket contains a new tail sequence number
+BOOL tailSequenceNumberCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+    BOOL retVal = FALSE;
+    //If we've already done this check and it was true
+    if(pJitterBuffer->tailSequenceNumber == pRtpPacket->header.sequenceNumber) {
+        retVal = TRUE;
+    }
+    else if(pRtpPacket->header.sequenceNumber > pJitterBuffer->tailSequenceNumber &&
+            (!pJitterBuffer->sequenceNumberOverflowState ||
+            pJitterBuffer->headSequenceNumber > pRtpPacket->header.sequenceNumber)) {
+        retVal = TRUE;
+        pJitterBuffer->tailSequenceNumber = pRtpPacket->header.sequenceNumber;            
+    }
+    return retVal;
+}
+
 //return true if sequence numbers are now overflowing
 BOOL enterSequenceNumberOverflowCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
     BOOL overflow = FALSE;
@@ -237,33 +317,6 @@ BOOL exitTimestampOverflowCheck(PJitterBuffer pJitterBuffer) {
     return retVal;
 }
 
-BOOL headCheckingAllowed(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
-    BOOL retVal = FALSE;
-    /*If we haven't yet processed a frame yet, then we don't have a definitive way of knowing if
-     *the first packet we receive is actually the earliest packet we'll ever receive. Since sequence numbers
-     *can start anywhere from 0 - 65535, we need to incorporate some checks to determine if a newly received packet
-     *should be considered the new head. Part of how we determine this is by setting a limit to how many packets off we allow
-     *this out of order case to be. Without setting a limit, then we could run into an odd scenario.
-     * Example:
-     * Push->Packet->SeqNumber == 0. //FIRST PACKET! new head of buffer!
-     * Push->Packet->SeqNumber == 3. //... new head of 65532 packet sized frame? maybe? was 0 the tail?
-     *
-     * To resolve that insanity we set a MAX, and will use that MAX for the range.
-     * This logic is present in headSequenceNumberCheck()
-     *
-     *After the first frame has been processed we don't need or want to make this consideration, since if our parser has
-     *dropped a frame for a good reason then we want to ignore any packets from that dropped frame that may come later.
-     *
-     *However if the packet's timestamp is the same as the head timestamp, then it's possible this is simply an earlier
-     *sequence number of the same packet.
-     */
-    if(!(pJitterBuffer->firstFrameProcessed) || 
-         pJitterBuffer->headTimestamp == pRtpPacket->header.timestamp) {
-        retVal = TRUE;
-    }
-    return retVal;
-}
-
 //return true if pRtpPacket contains a new head timestamp
 BOOL headTimestampCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
     BOOL retVal = FALSE;
@@ -313,59 +366,6 @@ BOOL tailTimestampCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
     return retVal;
 }
 
-//return true if pRtpPacket contains the head sequence number
-BOOL headSequenceNumberCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
-    BOOL retVal = FALSE;
-    UINT16 minimumHead = 0;
-    if(pJitterBuffer->headSequenceNumber >= MAX_OUT_OF_ORDER_PACKET_DIFFERENCE) {
-        minimumHead = pJitterBuffer->headSequenceNumber - MAX_OUT_OF_ORDER_PACKET_DIFFERENCE;
-    }
-
-    //If we've already done this check and it was true
-    if(pJitterBuffer->headSequenceNumber == pRtpPacket->header.sequenceNumber) {
-        retVal = TRUE;
-    }
-    else if(headCheckingAllowed(pJitterBuffer, pRtpPacket)){
-        if(pJitterBuffer->sequenceNumberOverflowState) {
-            if(pJitterBuffer->tailSequenceNumber < pRtpPacket->header.sequenceNumber &&
-               pJitterBuffer->headSequenceNumber > pRtpPacket->header.sequenceNumber &&
-               pRtpPacket->header.sequenceNumber >= minimumHead) {
-                //This purposefully misses the usecase where the buffer has >65000 entries.
-                //Our buffer is not designed for that use case, and it becomes far too ambiguous
-                //as to which packets are new tails or new heads without adding epoch checks.
-                pJitterBuffer->headSequenceNumber = pRtpPacket->header.sequenceNumber;
-                retVal = TRUE;
-            }
-        }
-        else {
-            if(pRtpPacket->header.sequenceNumber < pJitterBuffer->headSequenceNumber) {
-                if(pRtpPacket->header.sequenceNumber >= minimumHead) {
-                    pJitterBuffer->headSequenceNumber = pRtpPacket->header.sequenceNumber;
-                    retVal = TRUE;
-                }
-            }
-        }
-
-    }
-    return retVal;
-}
-
-//return true if pRtpPacket contains a new tail sequence number
-BOOL tailSequenceNumberCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
-    BOOL retVal = FALSE;
-    //If we've already done this check and it was true
-    if(pJitterBuffer->tailSequenceNumber == pRtpPacket->header.sequenceNumber) {
-        retVal = TRUE;
-    }
-    else if(pRtpPacket->header.sequenceNumber > pJitterBuffer->tailSequenceNumber &&
-            (!pJitterBuffer->sequenceNumberOverflowState ||
-            pJitterBuffer->headSequenceNumber > pRtpPacket->header.sequenceNumber)) {
-        retVal = TRUE;
-        pJitterBuffer->tailSequenceNumber = pRtpPacket->header.sequenceNumber;            
-    }
-    return retVal;
-}
-
 //return true if pRtpPacket is within the latency tolerance (not much earlier than current head)
 BOOL withinLatencyTolerance(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
     BOOL retVal = FALSE;
@@ -426,13 +426,18 @@ STATUS jitterBufferPush(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket, PBOO
     //defining a timestamp window for overflow
     //Returning true means this packet is a new tail AND we've entered overflow state.
     if(!enterSequenceNumberOverflowCheck(pJitterBuffer, pRtpPacket)) {
-        DLOGS("Entered sequenceNumber overflow state");
         tailSequenceNumberCheck(pJitterBuffer, pRtpPacket);
     }
+    else {
+        DLOGS("Entered sequenceNumber overflow state");
+    }
+
     if(!enterTimestampOverflowCheck(pJitterBuffer, pRtpPacket)) {
-        DLOGS("Entered timestamp overflow state");
         tailTimestampCheck(pJitterBuffer, pRtpPacket);
     }
+    else {
+        DLOGS("Entered timestamp overflow state");
+    }
 
     // is the packet within the accepted latency range, if so, add it to the hashtable
     if (withinLatencyTolerance(pJitterBuffer, pRtpPacket)) {

From fc84c7a7a169432a32ea5b8cb2d86d2e1ecb21b4 Mon Sep 17 00:00:00 2001
From: James Delaplane <delaplan@amazon.com>
Date: Tue, 4 Apr 2023 10:26:12 -0700
Subject: [PATCH 6/8] Clang format

---
 src/source/PeerConnection/JitterBuffer.c | 290 +++++++++++------------
 src/source/PeerConnection/JitterBuffer.h |   4 +-
 2 files changed, 140 insertions(+), 154 deletions(-)

diff --git a/src/source/PeerConnection/JitterBuffer.c b/src/source/PeerConnection/JitterBuffer.c
index bf45b3bab1..db68b3114a 100644
--- a/src/source/PeerConnection/JitterBuffer.c
+++ b/src/source/PeerConnection/JitterBuffer.c
@@ -87,40 +87,39 @@ STATUS freeJitterBuffer(PJitterBuffer* ppJitterBuffer)
     return retStatus;
 }
 
-BOOL underflowPossible(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+BOOL underflowPossible(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket)
+{
     BOOL retVal = FALSE;
     UINT32 seqNoDifference = 0;
     UINT64 timestampDifference = 0;
     UINT64 maxTimePassed = 0;
-    if(pJitterBuffer->headTimestamp == pRtpPacket->header.timestamp) {
+    if (pJitterBuffer->headTimestamp == pRtpPacket->header.timestamp) {
         retVal = TRUE;
-    }
-    else {
+    } else {
         seqNoDifference = (MAX_RTP_SEQUENCE_NUM - pRtpPacket->header.sequenceNumber) + pJitterBuffer->headSequenceNumber;
-        if(pJitterBuffer->headTimestamp > pRtpPacket->header.timestamp) {
+        if (pJitterBuffer->headTimestamp > pRtpPacket->header.timestamp) {
             timestampDifference = pJitterBuffer->headTimestamp - pRtpPacket->header.timestamp;
-        }
-        else {
+        } else {
             timestampDifference = (MAX_RTP_TIMESTAMP - pRtpPacket->header.timestamp) + pJitterBuffer->headTimestamp;
         }
 
-        //1 frame per second, and 1 packet per frame, the most charitable case we can consider
-        //TODO track most recent FPS to improve this metric
-        if((MAX_RTP_TIMESTAMP / pJitterBuffer->clockRate) <= seqNoDifference) {
+        // 1 frame per second, and 1 packet per frame, the most charitable case we can consider
+        // TODO track most recent FPS to improve this metric
+        if ((MAX_RTP_TIMESTAMP / pJitterBuffer->clockRate) <= seqNoDifference) {
             maxTimePassed = MAX_RTP_TIMESTAMP;
-        }
-        else {
+        } else {
             maxTimePassed = pJitterBuffer->clockRate * seqNoDifference;
         }
 
-        if(maxTimePassed >= timestampDifference) {
+        if (maxTimePassed >= timestampDifference) {
             retVal = TRUE;
         }
     }
     return retVal;
 }
 
-BOOL headCheckingAllowed(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+BOOL headCheckingAllowed(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket)
+{
     BOOL retVal = FALSE;
     /*If we haven't yet processed a frame yet, then we don't have a definitive way of knowing if
      *the first packet we receive is actually the earliest packet we'll ever receive. Since sequence numbers
@@ -140,108 +139,102 @@ BOOL headCheckingAllowed(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
      *However if the packet's timestamp is the same as the head timestamp, then it's possible this is simply an earlier
      *sequence number of the same packet.
      */
-    if(!(pJitterBuffer->firstFrameProcessed) || 
-         pJitterBuffer->headTimestamp == pRtpPacket->header.timestamp) {
+    if (!(pJitterBuffer->firstFrameProcessed) || pJitterBuffer->headTimestamp == pRtpPacket->header.timestamp) {
         retVal = TRUE;
     }
     return retVal;
 }
 
-//return true if pRtpPacket contains the head sequence number
-BOOL headSequenceNumberCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+// return true if pRtpPacket contains the head sequence number
+BOOL headSequenceNumberCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket)
+{
     BOOL retVal = FALSE;
     UINT16 minimumHead = 0;
-    if(pJitterBuffer->headSequenceNumber >= MAX_OUT_OF_ORDER_PACKET_DIFFERENCE) {
+    if (pJitterBuffer->headSequenceNumber >= MAX_OUT_OF_ORDER_PACKET_DIFFERENCE) {
         minimumHead = pJitterBuffer->headSequenceNumber - MAX_OUT_OF_ORDER_PACKET_DIFFERENCE;
     }
 
-    //If we've already done this check and it was true
-    if(pJitterBuffer->headSequenceNumber == pRtpPacket->header.sequenceNumber) {
+    // If we've already done this check and it was true
+    if (pJitterBuffer->headSequenceNumber == pRtpPacket->header.sequenceNumber) {
         retVal = TRUE;
-    }
-    else if(headCheckingAllowed(pJitterBuffer, pRtpPacket)){
-        if(pJitterBuffer->sequenceNumberOverflowState) {
-            if(pJitterBuffer->tailSequenceNumber < pRtpPacket->header.sequenceNumber &&
-               pJitterBuffer->headSequenceNumber > pRtpPacket->header.sequenceNumber &&
-               pRtpPacket->header.sequenceNumber >= minimumHead) {
-                //This purposefully misses the usecase where the buffer has >65000 entries.
-                //Our buffer is not designed for that use case, and it becomes far too ambiguous
-                //as to which packets are new tails or new heads without adding epoch checks.
+    } else if (headCheckingAllowed(pJitterBuffer, pRtpPacket)) {
+        if (pJitterBuffer->sequenceNumberOverflowState) {
+            if (pJitterBuffer->tailSequenceNumber < pRtpPacket->header.sequenceNumber &&
+                pJitterBuffer->headSequenceNumber > pRtpPacket->header.sequenceNumber && pRtpPacket->header.sequenceNumber >= minimumHead) {
+                // This purposefully misses the usecase where the buffer has >65000 entries.
+                // Our buffer is not designed for that use case, and it becomes far too ambiguous
+                // as to which packets are new tails or new heads without adding epoch checks.
                 pJitterBuffer->headSequenceNumber = pRtpPacket->header.sequenceNumber;
                 retVal = TRUE;
             }
-        }
-        else {
-            if(pRtpPacket->header.sequenceNumber < pJitterBuffer->headSequenceNumber) {
-                if(pRtpPacket->header.sequenceNumber >= minimumHead) {
+        } else {
+            if (pRtpPacket->header.sequenceNumber < pJitterBuffer->headSequenceNumber) {
+                if (pRtpPacket->header.sequenceNumber >= minimumHead) {
                     pJitterBuffer->headSequenceNumber = pRtpPacket->header.sequenceNumber;
                     retVal = TRUE;
                 }
             }
         }
-
     }
     return retVal;
 }
 
-//return true if pRtpPacket contains a new tail sequence number
-BOOL tailSequenceNumberCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+// return true if pRtpPacket contains a new tail sequence number
+BOOL tailSequenceNumberCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket)
+{
     BOOL retVal = FALSE;
-    //If we've already done this check and it was true
-    if(pJitterBuffer->tailSequenceNumber == pRtpPacket->header.sequenceNumber) {
+    // If we've already done this check and it was true
+    if (pJitterBuffer->tailSequenceNumber == pRtpPacket->header.sequenceNumber) {
         retVal = TRUE;
-    }
-    else if(pRtpPacket->header.sequenceNumber > pJitterBuffer->tailSequenceNumber &&
-            (!pJitterBuffer->sequenceNumberOverflowState ||
-            pJitterBuffer->headSequenceNumber > pRtpPacket->header.sequenceNumber)) {
+    } else if (pRtpPacket->header.sequenceNumber > pJitterBuffer->tailSequenceNumber &&
+               (!pJitterBuffer->sequenceNumberOverflowState || pJitterBuffer->headSequenceNumber > pRtpPacket->header.sequenceNumber)) {
         retVal = TRUE;
-        pJitterBuffer->tailSequenceNumber = pRtpPacket->header.sequenceNumber;            
+        pJitterBuffer->tailSequenceNumber = pRtpPacket->header.sequenceNumber;
     }
     return retVal;
 }
 
-//return true if sequence numbers are now overflowing
-BOOL enterSequenceNumberOverflowCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+// return true if sequence numbers are now overflowing
+BOOL enterSequenceNumberOverflowCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket)
+{
     BOOL overflow = FALSE;
     BOOL underflow = FALSE;
     UINT16 packetsUntilOverflow = MAX_RTP_SEQUENCE_NUM - pJitterBuffer->tailSequenceNumber;
     UINT16 underflowFromMaxRange = 0;
 
-    if(!pJitterBuffer->sequenceNumberOverflowState) {
-
-        //overflow case
-        if(MAX_OUT_OF_ORDER_PACKET_DIFFERENCE >= packetsUntilOverflow) {
-            //It's possible sequence numbers and timestamps are both overflowing.
-            if(pRtpPacket->header.sequenceNumber < pJitterBuffer->tailSequenceNumber &&
-               pRtpPacket->header.sequenceNumber <= MAX_OUT_OF_ORDER_PACKET_DIFFERENCE - packetsUntilOverflow) {
-                //Sequence number overflow detected
+    if (!pJitterBuffer->sequenceNumberOverflowState) {
+        // overflow case
+        if (MAX_OUT_OF_ORDER_PACKET_DIFFERENCE >= packetsUntilOverflow) {
+            // It's possible sequence numbers and timestamps are both overflowing.
+            if (pRtpPacket->header.sequenceNumber < pJitterBuffer->tailSequenceNumber &&
+                pRtpPacket->header.sequenceNumber <= MAX_OUT_OF_ORDER_PACKET_DIFFERENCE - packetsUntilOverflow) {
+                // Sequence number overflow detected
                 overflow = TRUE;
             }
         }
-        //underflow case
-        else if(headCheckingAllowed(pJitterBuffer, pRtpPacket)) { 
+        // underflow case
+        else if (headCheckingAllowed(pJitterBuffer, pRtpPacket)) {
             if (pJitterBuffer->headSequenceNumber < MAX_OUT_OF_ORDER_PACKET_DIFFERENCE) {
-                if (pRtpPacket->header.sequenceNumber >=
-                            (MAX_UINT16 - (MAX_OUT_OF_ORDER_PACKET_DIFFERENCE - pJitterBuffer->headSequenceNumber))) {
-                    //Possible sequence number underflow detected, now lets check the timestamps to be certain
-                    //this is an earlier value, and not a much later.
-                    if(underflowPossible(pJitterBuffer, pRtpPacket)) {
+                if (pRtpPacket->header.sequenceNumber >= (MAX_UINT16 - (MAX_OUT_OF_ORDER_PACKET_DIFFERENCE - pJitterBuffer->headSequenceNumber))) {
+                    // Possible sequence number underflow detected, now lets check the timestamps to be certain
+                    // this is an earlier value, and not a much later.
+                    if (underflowPossible(pJitterBuffer, pRtpPacket)) {
                         underflow = TRUE;
                     }
                 }
             }
         }
     }
-    if(overflow && underflow) {
-        //This shouldn't be possible. 
+    if (overflow && underflow) {
+        // This shouldn't be possible.
         DLOGE("Critical underflow/overflow error in jitterbuffer");
     }
-    if(overflow) {
+    if (overflow) {
         pJitterBuffer->sequenceNumberOverflowState = TRUE;
         pJitterBuffer->tailSequenceNumber = pRtpPacket->header.sequenceNumber;
         pJitterBuffer->tailTimestamp = pRtpPacket->header.timestamp;
     }
-    if(underflow) {
+    if (underflow) {
         pJitterBuffer->sequenceNumberOverflowState = TRUE;
         pJitterBuffer->headSequenceNumber = pRtpPacket->header.sequenceNumber;
         pJitterBuffer->headTimestamp = pRtpPacket->header.timestamp;
@@ -249,52 +242,50 @@ BOOL enterSequenceNumberOverflowCheck(PJitterBuffer pJitterBuffer, PRtpPacket pR
     return (overflow || underflow);
 }
 
-BOOL enterTimestampOverflowCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+BOOL enterTimestampOverflowCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket)
+{
     BOOL underflow = FALSE;
     BOOL overflow = FALSE;
-    if(!pJitterBuffer->timestampOverFlowState) {
-        //overflow check
-        if(pJitterBuffer->headTimestamp > pRtpPacket->header.timestamp &&
-           pJitterBuffer->tailTimestamp > pRtpPacket->header.timestamp) {
-            //Check to see if this could be a timestamp overflow case
-            //We always check sequence number first, so the 'or equal to' checks if we just set the tail.
-            //That would be a corner case of sequence number and timestamp both overflowing
-            //in this one packet.
-            if(tailSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
-                //RTP timestamp overflow detected!
+    if (!pJitterBuffer->timestampOverFlowState) {
+        // overflow check
+        if (pJitterBuffer->headTimestamp > pRtpPacket->header.timestamp && pJitterBuffer->tailTimestamp > pRtpPacket->header.timestamp) {
+            // Check to see if this could be a timestamp overflow case
+            // We always check sequence number first, so the 'or equal to' checks if we just set the tail.
+            // That would be a corner case of sequence number and timestamp both overflowing
+            // in this one packet.
+            if (tailSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
+                // RTP timestamp overflow detected!
                 overflow = TRUE;
             }
         }
-        //underflow check
-        else if (pJitterBuffer->headTimestamp < pRtpPacket->header.timestamp &&
-                pJitterBuffer->tailTimestamp < pRtpPacket->header.timestamp) {
-            if(headSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
+        // underflow check
+        else if (pJitterBuffer->headTimestamp < pRtpPacket->header.timestamp && pJitterBuffer->tailTimestamp < pRtpPacket->header.timestamp) {
+            if (headSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
                 underflow = TRUE;
             }
         }
-
     }
-    if(overflow && underflow) {
-        //This shouldn't be possible. 
+    if (overflow && underflow) {
+        // This shouldn't be possible.
         DLOGE("Critical underflow/overflow error in jitterbuffer");
     }
-    if(overflow) {
+    if (overflow) {
         pJitterBuffer->timestampOverFlowState = TRUE;
         pJitterBuffer->tailTimestamp = pRtpPacket->header.timestamp;
-    }
-    else if(underflow) {
+    } else if (underflow) {
         pJitterBuffer->timestampOverFlowState = TRUE;
         pJitterBuffer->headTimestamp = pRtpPacket->header.timestamp;
     }
     return (underflow || overflow);
 }
 
-BOOL exitSequenceNumberOverflowCheck(PJitterBuffer pJitterBuffer) {
+BOOL exitSequenceNumberOverflowCheck(PJitterBuffer pJitterBuffer)
+{
     BOOL retVal = FALSE;
 
-    //can't exit if you're not in it
-    if(pJitterBuffer->sequenceNumberOverflowState) {
-        if(pJitterBuffer->headSequenceNumber <= pJitterBuffer->tailSequenceNumber) {
+    // can't exit if you're not in it
+    if (pJitterBuffer->sequenceNumberOverflowState) {
+        if (pJitterBuffer->headSequenceNumber <= pJitterBuffer->tailSequenceNumber) {
             pJitterBuffer->sequenceNumberOverflowState = FALSE;
             retVal = TRUE;
         }
@@ -303,12 +294,13 @@ BOOL exitSequenceNumberOverflowCheck(PJitterBuffer pJitterBuffer) {
     return retVal;
 }
 
-BOOL exitTimestampOverflowCheck(PJitterBuffer pJitterBuffer) {
+BOOL exitTimestampOverflowCheck(PJitterBuffer pJitterBuffer)
+{
     BOOL retVal = FALSE;
 
-    //can't exit if you're not in it
-    if(pJitterBuffer->timestampOverFlowState) {
-        if(pJitterBuffer->headTimestamp <= pJitterBuffer->tailTimestamp) {
+    // can't exit if you're not in it
+    if (pJitterBuffer->timestampOverFlowState) {
+        if (pJitterBuffer->headTimestamp <= pJitterBuffer->tailTimestamp) {
             pJitterBuffer->timestampOverFlowState = FALSE;
             retVal = TRUE;
         }
@@ -317,28 +309,26 @@ BOOL exitTimestampOverflowCheck(PJitterBuffer pJitterBuffer) {
     return retVal;
 }
 
-//return true if pRtpPacket contains a new head timestamp
-BOOL headTimestampCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+// return true if pRtpPacket contains a new head timestamp
+BOOL headTimestampCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket)
+{
     BOOL retVal = FALSE;
 
-    if(headCheckingAllowed(pJitterBuffer, pRtpPacket)) {
-        if(pJitterBuffer->timestampOverFlowState) {
-            if(pJitterBuffer->headTimestamp > pRtpPacket->header.timestamp &&
-               pJitterBuffer->tailTimestamp < pRtpPacket->header.timestamp) {
-                //in the correct range to be a new head or new tail.
-                //if it's also the head sequence number then it's the new headtimestamp
-                if(headSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
+    if (headCheckingAllowed(pJitterBuffer, pRtpPacket)) {
+        if (pJitterBuffer->timestampOverFlowState) {
+            if (pJitterBuffer->headTimestamp > pRtpPacket->header.timestamp && pJitterBuffer->tailTimestamp < pRtpPacket->header.timestamp) {
+                // in the correct range to be a new head or new tail.
+                // if it's also the head sequence number then it's the new headtimestamp
+                if (headSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
                     pJitterBuffer->headTimestamp = pRtpPacket->header.timestamp;
                     retVal = TRUE;
                 }
             }
-        }
-        else {
-            if(pJitterBuffer->headTimestamp > pRtpPacket->header.timestamp ||
-               pJitterBuffer->tailTimestamp < pRtpPacket->header.timestamp) {
-                //in the correct range to be a new head or new tail.
-                //if it's also the head sequence number then it's the new headtimestamp
-                if(headSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
+        } else {
+            if (pJitterBuffer->headTimestamp > pRtpPacket->header.timestamp || pJitterBuffer->tailTimestamp < pRtpPacket->header.timestamp) {
+                // in the correct range to be a new head or new tail.
+                // if it's also the head sequence number then it's the new headtimestamp
+                if (headSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
                     pJitterBuffer->headTimestamp = pRtpPacket->header.timestamp;
                     retVal = TRUE;
                 }
@@ -348,16 +338,16 @@ BOOL headTimestampCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
     return retVal;
 }
 
-//return true if pRtpPacket contains a new tail timestamp
-BOOL tailTimestampCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+// return true if pRtpPacket contains a new tail timestamp
+BOOL tailTimestampCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket)
+{
     BOOL retVal = FALSE;
 
-    if(pJitterBuffer->tailTimestamp < pRtpPacket->header.timestamp) {
-        if(!pJitterBuffer->timestampOverFlowState ||
-            pJitterBuffer->headTimestamp > pRtpPacket->header.timestamp) {
-            //in the correct range to be a new head or new tail.
-            //if it's also the tail sequence number then it's the new tail timestamp
-            if(tailSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
+    if (pJitterBuffer->tailTimestamp < pRtpPacket->header.timestamp) {
+        if (!pJitterBuffer->timestampOverFlowState || pJitterBuffer->headTimestamp > pRtpPacket->header.timestamp) {
+            // in the correct range to be a new head or new tail.
+            // if it's also the tail sequence number then it's the new tail timestamp
+            if (tailSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
                 pJitterBuffer->tailTimestamp = pRtpPacket->header.timestamp;
                 retVal = TRUE;
             }
@@ -366,35 +356,33 @@ BOOL tailTimestampCheck(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
     return retVal;
 }
 
-//return true if pRtpPacket is within the latency tolerance (not much earlier than current head)
-BOOL withinLatencyTolerance(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket) {
+// return true if pRtpPacket is within the latency tolerance (not much earlier than current head)
+BOOL withinLatencyTolerance(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket)
+{
     BOOL retVal = FALSE;
     UINT32 minimumTimestamp = 0;
 
-    //Simple check, if we're at or past the tail timestamp then we're always within latency tolerance.
-    //overflow is checked earlier
-    if(tailTimestampCheck(pJitterBuffer, pRtpPacket) || pJitterBuffer->tailTimestamp == pRtpPacket->header.timestamp) {
+    // Simple check, if we're at or past the tail timestamp then we're always within latency tolerance.
+    // overflow is checked earlier
+    if (tailTimestampCheck(pJitterBuffer, pRtpPacket) || pJitterBuffer->tailTimestamp == pRtpPacket->header.timestamp) {
         retVal = TRUE;
-    }
-    else {
-        //Is our tail current less than our head due to timestamp overflow?
-        if(pJitterBuffer->timestampOverFlowState) {
-            //calculate max-latency across the overflow boundry without triggering underflow
-            if(pJitterBuffer->tailTimestamp < pJitterBuffer->maxLatency) {
+    } else {
+        // Is our tail current less than our head due to timestamp overflow?
+        if (pJitterBuffer->timestampOverFlowState) {
+            // calculate max-latency across the overflow boundry without triggering underflow
+            if (pJitterBuffer->tailTimestamp < pJitterBuffer->maxLatency) {
                 minimumTimestamp = MAX_RTP_TIMESTAMP - (pJitterBuffer->maxLatency - pJitterBuffer->tailTimestamp);
             }
-            //Is the packet within the current range or is it a new head/tail
-            if(pRtpPacket->header.timestamp < pJitterBuffer->tailTimestamp ||
-               pRtpPacket->header.timestamp > pJitterBuffer->headTimestamp) {
-                //The packet is within the current range
+            // Is the packet within the current range or is it a new head/tail
+            if (pRtpPacket->header.timestamp < pJitterBuffer->tailTimestamp || pRtpPacket->header.timestamp > pJitterBuffer->headTimestamp) {
+                // The packet is within the current range
                 retVal = TRUE;
             }
-            //The only remaining option is that timestamp must be before headTimestamp
-            else if(pRtpPacket->header.timestamp >= minimumTimestamp) {
-                retVal = TRUE;                
+            // The only remaining option is that timestamp must be before headTimestamp
+            else if (pRtpPacket->header.timestamp >= minimumTimestamp) {
+                retVal = TRUE;
             }
-        }
-        else {
+        } else {
             if ((pRtpPacket->header.timestamp < pJitterBuffer->maxLatency && pJitterBuffer->tailTimestamp <= pJitterBuffer->maxLatency) ||
                 pRtpPacket->header.timestamp >= pJitterBuffer->tailTimestamp - pJitterBuffer->maxLatency) {
                 retVal = TRUE;
@@ -422,20 +410,18 @@ STATUS jitterBufferPush(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket, PBOO
         pJitterBuffer->headTimestamp = pRtpPacket->header.timestamp;
     }
 
-    //We'll check sequence numbers first, with our MAX Out of Order packet count to avoid
-    //defining a timestamp window for overflow
-    //Returning true means this packet is a new tail AND we've entered overflow state.
-    if(!enterSequenceNumberOverflowCheck(pJitterBuffer, pRtpPacket)) {
+    // We'll check sequence numbers first, with our MAX Out of Order packet count to avoid
+    // defining a timestamp window for overflow
+    // Returning true means this packet is a new tail AND we've entered overflow state.
+    if (!enterSequenceNumberOverflowCheck(pJitterBuffer, pRtpPacket)) {
         tailSequenceNumberCheck(pJitterBuffer, pRtpPacket);
-    }
-    else {
+    } else {
         DLOGS("Entered sequenceNumber overflow state");
     }
 
-    if(!enterTimestampOverflowCheck(pJitterBuffer, pRtpPacket)) {
+    if (!enterTimestampOverflowCheck(pJitterBuffer, pRtpPacket)) {
         tailTimestampCheck(pJitterBuffer, pRtpPacket);
-    }
-    else {
+    } else {
         DLOGS("Entered timestamp overflow state");
     }
 
@@ -452,10 +438,10 @@ STATUS jitterBufferPush(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket, PBOO
 
         if (headCheckingAllowed(pJitterBuffer, pRtpPacket)) {
             // if the timestamp is less, we'll accept it as a new head, since it must be an earlier frame.
-            if(headTimestampCheck(pJitterBuffer, pRtpPacket)) {
+            if (headTimestampCheck(pJitterBuffer, pRtpPacket)) {
                 DLOGS("New jitterbuffer head timestamp");
             }
-            if(headSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
+            if (headSequenceNumberCheck(pJitterBuffer, pRtpPacket)) {
                 DLOGS("New jitterbuffer head sequenceNumber");
             }
         }
@@ -554,7 +540,7 @@ STATUS jitterBufferInternalParse(PJitterBuffer pJitterBuffer, BOOL bufferClosed)
                 }
                 // are we forcibly clearing out the buffer? if so drop the contents of incomplete frame
                 else if (pJitterBuffer->headTimestamp < earliestAllowedTimestamp || bufferClosed) {
-                    //do not CHK_STATUS of onFrameDropped because we need to clear the jitter buffer no matter what else happens.
+                    // do not CHK_STATUS of onFrameDropped because we need to clear the jitter buffer no matter what else happens.
                     pJitterBuffer->onFrameDroppedFn(pJitterBuffer->customData, startDropIndex, UINT16_DEC(index), pJitterBuffer->headTimestamp);
                     CHK_STATUS(jitterBufferDropBufferData(pJitterBuffer, startDropIndex, UINT16_DEC(index), curTimestamp));
                     pJitterBuffer->firstFrameProcessed = TRUE;
@@ -635,10 +621,10 @@ STATUS jitterBufferDropBufferData(PJitterBuffer pJitterBuffer, UINT16 startIndex
     }
     pJitterBuffer->headTimestamp = nextTimestamp;
     pJitterBuffer->headSequenceNumber = endIndex + 1;
-    if(exitTimestampOverflowCheck(pJitterBuffer)) {
+    if (exitTimestampOverflowCheck(pJitterBuffer)) {
         DLOGS("Exited timestamp overflow state");
     }
-    if(exitSequenceNumberOverflowCheck(pJitterBuffer)) {
+    if (exitSequenceNumberOverflowCheck(pJitterBuffer)) {
         DLOGS("Exited sequenceNumber overflow state");
     }
 
diff --git a/src/source/PeerConnection/JitterBuffer.h b/src/source/PeerConnection/JitterBuffer.h
index 36560d3ad5..543b05c78c 100644
--- a/src/source/PeerConnection/JitterBuffer.h
+++ b/src/source/PeerConnection/JitterBuffer.h
@@ -32,8 +32,8 @@ typedef struct {
     UINT16 tailSequenceNumber;
     UINT32 headTimestamp;
     UINT32 tailTimestamp;
-    //this is set to U64 even though rtp timestamps are U32
-    //in order to allow calculations to not cause overflow
+    // this is set to U64 even though rtp timestamps are U32
+    // in order to allow calculations to not cause overflow
     UINT64 maxLatency;
     UINT64 customData;
     UINT32 clockRate;

From c6bf339189970a2565e017864d8df930aa85987b Mon Sep 17 00:00:00 2001
From: James Delaplane <delaplan@amazon.com>
Date: Tue, 4 Apr 2023 13:05:53 -0700
Subject: [PATCH 7/8] Mac compile errors fixed

---
 src/source/PeerConnection/JitterBuffer.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/source/PeerConnection/JitterBuffer.c b/src/source/PeerConnection/JitterBuffer.c
index db68b3114a..300c3fe07c 100644
--- a/src/source/PeerConnection/JitterBuffer.c
+++ b/src/source/PeerConnection/JitterBuffer.c
@@ -200,7 +200,6 @@ BOOL enterSequenceNumberOverflowCheck(PJitterBuffer pJitterBuffer, PRtpPacket pR
     BOOL overflow = FALSE;
     BOOL underflow = FALSE;
     UINT16 packetsUntilOverflow = MAX_RTP_SEQUENCE_NUM - pJitterBuffer->tailSequenceNumber;
-    UINT16 underflowFromMaxRange = 0;
 
     if (!pJitterBuffer->sequenceNumberOverflowState) {
         // overflow case
@@ -398,7 +397,6 @@ STATUS jitterBufferPush(PJitterBuffer pJitterBuffer, PRtpPacket pRtpPacket, PBOO
     STATUS retStatus = STATUS_SUCCESS, status = STATUS_SUCCESS;
     UINT64 hashValue = 0;
     PRtpPacket pCurPacket = NULL;
-    UINT16 packetsUntilOverflow = 0;
 
     CHK(pJitterBuffer != NULL && pRtpPacket != NULL, STATUS_NULL_ARG);
 

From 98a061e0695509e04287e1c2ec2682031506403d Mon Sep 17 00:00:00 2001
From: James Delaplane <delaplan@amazon.com>
Date: Wed, 5 Apr 2023 10:17:49 -0700
Subject: [PATCH 8/8] Commenting out long-running test, adding timestamp
 overflow test

---
 tst/JitterBufferFunctionalityTest.cpp | 230 +++++++++++++++++++++++++-
 1 file changed, 229 insertions(+), 1 deletion(-)

diff --git a/tst/JitterBufferFunctionalityTest.cpp b/tst/JitterBufferFunctionalityTest.cpp
index 5bbc67b8e0..8408629df5 100644
--- a/tst/JitterBufferFunctionalityTest.cpp
+++ b/tst/JitterBufferFunctionalityTest.cpp
@@ -234,7 +234,7 @@ TEST_F(JitterBufferFunctionalityTest, gapBetweenTwoContinousPackets)
     mPRtpPackets[3]->header.timestamp = 400;
     mPRtpPackets[3]->header.sequenceNumber = 5;
 
-    // Expected to dropped frames when "2" "5" is not received
+    // Expected to dropped frames when "1" "4" is not received
     mExpectedDroppedFrameTimestampArr[0] = 100;
     mExpectedDroppedFrameTimestampArr[1] = 200;
 
@@ -1133,6 +1133,116 @@ TEST_F(JitterBufferFunctionalityTest, latePacketsOfAlreadyDroppedFrame)
 
 TEST_F(JitterBufferFunctionalityTest, timestampOverflowTest)
 {
+    UINT32 i;
+    UINT32 pktCount = 7;
+    UINT32 startingSequenceNumber = 0;
+    UINT32 missingSequenceNumber = 0;
+    UINT32 firstSequenceNumber = 0;
+    initializeJitterBuffer(4, 0, pktCount);
+    srand(time(0));
+    startingSequenceNumber = rand()%UINT16_MAX;
+    firstSequenceNumber = startingSequenceNumber - 1;
+
+    // First frame "1"
+    mPRtpPackets[0]->payloadLength = 1;
+    mPRtpPackets[0]->payload = (PBYTE) MEMALLOC(mPRtpPackets[0]->payloadLength + 1);
+    mPRtpPackets[0]->payload[0] = 1;
+    mPRtpPackets[0]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[0]->header.timestamp = MAX_RTP_TIMESTAMP - 500;
+    mPRtpPackets[0]->header.sequenceNumber = startingSequenceNumber++;
+
+    mPRtpPackets[1]->payloadLength = 1;
+    mPRtpPackets[1]->payload = (PBYTE) MEMALLOC(mPRtpPackets[1]->payloadLength + 1);
+    mPRtpPackets[1]->payload[0] = 2;
+    mPRtpPackets[1]->payload[1] = 0;
+    mPRtpPackets[1]->header.timestamp = MAX_RTP_TIMESTAMP - 500;
+    mPRtpPackets[1]->header.sequenceNumber = startingSequenceNumber++;
+
+    mPRtpPackets[2]->payloadLength = 1;
+    mPRtpPackets[2]->payload = (PBYTE) MEMALLOC(mPRtpPackets[2]->payloadLength + 1);
+    mPRtpPackets[2]->payload[0] = 3;
+    mPRtpPackets[2]->payload[1] = 0;
+    mPRtpPackets[2]->header.timestamp = MAX_RTP_TIMESTAMP - 500;
+    mPRtpPackets[2]->header.sequenceNumber = startingSequenceNumber++;
+    missingSequenceNumber = startingSequenceNumber++;
+
+    // Expected to get frame "1234"
+    mPExpectedFrameArr[0] = (PBYTE) MEMALLOC(4);
+    mPExpectedFrameArr[0][0] = 1;
+    mPExpectedFrameArr[0][1] = 2;
+    mPExpectedFrameArr[0][2] = 3;
+    mPExpectedFrameArr[0][3] = 4;
+    mExpectedFrameSizeArr[0] = 4;
+
+    // Second frame "2"
+    mPRtpPackets[3]->payloadLength = 1;
+    mPRtpPackets[3]->payload = (PBYTE) MEMALLOC(mPRtpPackets[3]->payloadLength + 1);
+    mPRtpPackets[3]->payload[0] = 2;
+    mPRtpPackets[3]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[3]->header.timestamp = MAX_RTP_TIMESTAMP - 100;
+    mPRtpPackets[3]->header.sequenceNumber = startingSequenceNumber++;
+
+    // Expected to get frame "2"
+    mPExpectedFrameArr[1] = (PBYTE) MEMALLOC(1);
+    mPExpectedFrameArr[1][0] = 2;
+    mExpectedFrameSizeArr[1] = 1;
+
+    // Third frame "3"
+    mPRtpPackets[4]->payloadLength = 1;
+    mPRtpPackets[4]->payload = (PBYTE) MEMALLOC(mPRtpPackets[4]->payloadLength + 1);
+    mPRtpPackets[4]->payload[0] = 3;
+    mPRtpPackets[4]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[4]->header.timestamp = 300;
+    mPRtpPackets[4]->header.sequenceNumber = startingSequenceNumber++;
+
+    // Expected to get frame "3" at close
+    mPExpectedFrameArr[2] = (PBYTE) MEMALLOC(1);
+    mPExpectedFrameArr[2][0] = 3;
+    mExpectedFrameSizeArr[2] = 1;
+
+    // Third frame "4"
+    mPRtpPackets[5]->payloadLength = 1;
+    mPRtpPackets[5]->payload = (PBYTE) MEMALLOC(mPRtpPackets[5]->payloadLength + 1);
+    mPRtpPackets[5]->payload[0] = 4;
+    mPRtpPackets[5]->payload[1] = 1; // First packet of a frame
+    mPRtpPackets[5]->header.timestamp = 600;
+    mPRtpPackets[5]->header.sequenceNumber = startingSequenceNumber++;
+
+    // Expected to get frame "4" at close
+    mPExpectedFrameArr[3] = (PBYTE) MEMALLOC(1);
+    mPExpectedFrameArr[3][0] = 4;
+    mExpectedFrameSizeArr[3] = 1;
+
+    mPRtpPackets[6]->payloadLength = 1;
+    mPRtpPackets[6]->payload = (PBYTE) MEMALLOC(mPRtpPackets[6]->payloadLength + 1);
+    mPRtpPackets[6]->payload[0] = 4;
+    mPRtpPackets[6]->payload[1] = 0; // First packet of a frame
+    mPRtpPackets[6]->header.timestamp = MAX_RTP_TIMESTAMP - 500;
+    mPRtpPackets[6]->header.sequenceNumber = missingSequenceNumber;
+
+    setPayloadToFree();
+
+    for (i = 0; i < pktCount; i++) {
+        EXPECT_EQ(STATUS_SUCCESS, jitterBufferPush(mJitterBuffer, mPRtpPackets[i], nullptr));
+        switch (i) {
+            case 0:
+            case 1:
+            case 2:
+            case 3:
+            case 4:
+            case 5:
+                EXPECT_EQ(0, mReadyFrameIndex);
+                break;
+            case 6:
+                EXPECT_EQ(3, mReadyFrameIndex);
+                break;
+            default:
+                ASSERT_TRUE(FALSE);
+        }
+        EXPECT_EQ(0, mDroppedFrameIndex);
+    }
+
+    clearJitterBufferForTest();
 }
 
 TEST_F(JitterBufferFunctionalityTest, timestampUnderflowTest)
@@ -1621,9 +1731,127 @@ TEST_F(JitterBufferFunctionalityTest, DoubleOverflowTest)
     clearJitterBufferForTest();
 }
 
+#if 0
+//TODO complete this test
 TEST_F(JitterBufferFunctionalityTest, LongRunningWithDroppedPacketsTest)
 {
+    UINT32 timeStep = 100;
+    UINT16 startingSequenceNumber = 0;
+    UINT32 startingTimestamp = 0;
+    UINT32 totalPackets = 0;
+    UINT32 droppedPackets = 0;
+    UINT32 rangeForNextDrop = 0;
+    UINT32 nextDrop = 0;
+    UINT32 dropSection = 0;
+    UINT32 packetCount = 0;
+    UINT32 frameCount = 0;
+    UINT32 readyFrameCount = 0;
+    UINT32 droppedFrameCount = 0;
+    UINT32 currentDroppedPacket = 0;
+    BOOL dropped = FALSE;
+    std::vector<UINT32> dropNextPacket;
+    std::vector<UINT32> packetsPerFrame;
+    std::vector<UINT32> packetsOfFrameToDrop;
+  
+    srand(time(0));
+
+    //between 1-3 UINT16 cycles
+    totalPackets = rand()%(UINT16_MAX*2) + UINT16_MAX;
+    //100-200 dropped OR very late packets
+    droppedPackets = rand()%(100) + 100;
+    startingSequenceNumber = rand()%UINT16_MAX;
+    startingTimestamp = rand()%UINT32_MAX;
+
+    dropSection = totalPackets/droppedPackets;
+    rangeForNextDrop = dropSection;
+    for(auto i = 0; i < droppedPackets; i++) {
+        nextDrop = rand()%rangeForNextDrop;
+        rangeForNextDrop += std::abs(dropSection-nextDrop);
+        if(i == 0) {
+            dropNextPacket.push_back(nextDrop);
+        }
+        else {
+            dropNextPacket.push_back(dropNextPacket.back() + nextDrop);
+        }
+    }
+
+    while(packetCount < totalPackets) {
+        if((totalPackets - packetCount) <= 30) {
+            packetsPerFrame.push_back(totalPackets-packetCount);
+        }
+        else {
+            packetsPerFrame.push_back(rand()%29 + 2);
+        }
+        packetCount += packetsPerFrame.back();
+        frameCount++;
+        while(packetCount > dropNextPacket[currentDroppedPacket]) {
+            dropped = TRUE;
+            currentDroppedPacket++;
+        }
+        if(dropped) {
+            droppedFrameCount++;
+        }
+        else {
+            readyFrameCount++;
+        }
+        dropped = FALSE;
+    }
+
+    initializeJitterBuffer(readyFrameCount, droppedFrameCount, totalPackets);
+
+    dropped = FALSE;
+    packetCount = 0;
+    droppedFrameCount = 0;
+    currentDroppedPacket = 0;
+
+    for(auto frame = 0; frame < frameCount; frame++) {
+        //is frame missing a packet
+        while((packetsPerFrame[frame] + packetCount ) > dropNextPacket[currentDroppedPacket]){
+            dropped = TRUE;
+            packetsOfFrameToDrop.push_back(dropNextPacket[currentDroppedPacket] - packetCount);
+            currentDroppedPacket++;
+        }
+        if(dropped) {
+            droppedFrameCount++;
+        }
+        else {
+            mPExpectedFrameArr[frame-droppedFrameCount] = (PBYTE) MEMALLOC(packetsPerFrame[frame]);
+            mExpectedFrameSizeArr[frame-droppedFrameCount] = packetsPerFrame[frame];
+        }
+        for(auto packet = 0; packet < packetsPerFrame[frame]; packet++) {
+            if(!dropped) {
+                mPExpectedFrameArr[frame-droppedFrameCount][packet] = packet;
+            }
+
+            mPRtpPackets[packetCount]->payloadLength = 1;
+            mPRtpPackets[packetCount]->payload = (PBYTE) MEMALLOC(mPRtpPackets[packetCount]->payloadLength + 1);
+            mPRtpPackets[packetCount]->payload[0] = packet;
+            if(packet == 0) {
+                mPRtpPackets[packetCount]->payload[1] = 1; // First packet of a frame
+            }
+            else {
+                mPRtpPackets[packetCount]->payload[1] = 0;
+            }
+            mPRtpPackets[packetCount]->header.timestamp = startingTimestamp;
+            mPRtpPackets[packetCount]->header.sequenceNumber = startingSequenceNumber++;
+
+            packetCount++;
+        }
+        if(dropped) {
+            mExpectedDroppedFrameTimestampArr[droppedFrameCount-1] = startingTimestamp;
+        }
+        startingTimestamp += timeStep;
+
+    }
+
+    setPayloadToFree();
+
+    for(auto i = 0; i < totalPackets; i++) {
+        EXPECT_EQ(STATUS_SUCCESS, jitterBufferPush(mJitterBuffer, mPRtpPackets[i], nullptr));
+    }
+    clearJitterBufferForTest();
 }
+#endif
 
 } // namespace webrtcclient
 } // namespace video