Skip to content

Commit 021c789

Browse files
authored
Fix the uint16 promotion case (#2101)
1 parent da2b12b commit 021c789

File tree

3 files changed

+40
-2
lines changed

3 files changed

+40
-2
lines changed

src/source/PeerConnection/PeerConnection.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1909,7 +1909,7 @@ static STATUS twccRollingWindowDeletion(PKvsPeerConnection pKvsPeerConnection, P
19091909
}
19101910
// reset before next iteration
19111911
tempTwccRtpPktInfo = NULL;
1912-
} while (!isCheckComplete && updatedSeqNum != (endingSeqNum + 1));
1912+
} while (!isCheckComplete && updatedSeqNum != (UINT16) (endingSeqNum + 1));
19131913

19141914
// Update regardless. The loop checks until current RTP packets seq number irrespective of the failure
19151915
pKvsPeerConnection->pTwccManager->firstSeqNumInRollingWindow = updatedSeqNum;

src/source/PeerConnection/Rtcp.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ STATUS updateTwccHashTable(PTwccManager pTwccManager, PINT64 duration, PUINT64 r
344344
// We also check for twcc->lastReportedSeqNum + 1 to include the last seq number in the
345345
// report. Without this, we do not check for the seqNum that could cause it to not be cleared
346346
// from memory
347-
for (seqNum = baseSeqNum; seqNum != (pTwccManager->lastReportedSeqNum + 1); seqNum++) {
347+
for (seqNum = baseSeqNum; seqNum != (UINT16) (pTwccManager->lastReportedSeqNum + 1); seqNum++) {
348348
if (!localStartTimeRecorded) {
349349
// This could happen if the prev packet was deleted as part of rolling window or if there
350350
// is an overlap of RTP packet statuses between TWCC packets. This could also fail if it is

tst/RtcpFunctionalityTest.cpp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,44 @@ TEST_F(RtcpFunctionalityTest, updateTwccHashTableTest)
471471
EXPECT_EQ(STATUS_SUCCESS, freePeerConnection(&pRtcPeerConnection));
472472
}
473473

474+
TEST_F(RtcpFunctionalityTest, updateTwccHashTableIntPromotionCase) {
475+
PRtcPeerConnection pRtcPeerConnection = NULL;
476+
PKvsPeerConnection pKvsPeerConnection = NULL;
477+
RtcConfiguration config{};
478+
EXPECT_EQ(STATUS_SUCCESS, createPeerConnection(&config, &pRtcPeerConnection));
479+
pKvsPeerConnection = reinterpret_cast<PKvsPeerConnection>(pRtcPeerConnection);
480+
PTwccRtpPacketInfo pTwccRtpPacketInfo = NULL;
481+
INT64 duration = 0;
482+
UINT64 receivedBytes = 0, receivedPackets = 0, sentBytes = 0, sentPackets = 0;
483+
UINT16 i;
484+
485+
// Grab the hash table
486+
PHashTable pTwccRtpPktInfosHashTable = pKvsPeerConnection->pTwccManager->pTwccRtpPktInfosHashTable;
487+
UINT16 hashTableInsertionCount = 0;
488+
489+
// Set up the hash table
490+
pKvsPeerConnection->pTwccManager->prevReportedBaseSeqNum = UINT16_MAX;
491+
pKvsPeerConnection->pTwccManager->lastReportedSeqNum = UINT16_MAX;
492+
493+
// Add packet at UINT16_MAX
494+
pTwccRtpPacketInfo = (PTwccRtpPacketInfo) MEMCALLOC(1, SIZEOF(TwccRtpPacketInfo));
495+
EXPECT_EQ(STATUS_SUCCESS, hashTableUpsert(pTwccRtpPktInfosHashTable, UINT16_MAX, (UINT64) pTwccRtpPacketInfo));
496+
hashTableInsertionCount++;
497+
498+
// Even though pTwccManager->lastReportedSeqNum is a UINT16, (pTwccManager->lastReportedSeqNum + 1) can get
499+
// promoted to an int (32) when pTwccManager->lastReportedSeqNum == UINT16_MAX
500+
EXPECT_EQ(STATUS_SUCCESS, updateTwccHashTable(pKvsPeerConnection->pTwccManager, &duration,
501+
&receivedBytes, &receivedPackets,
502+
&sentBytes, &sentPackets));
503+
504+
EXPECT_EQ(0, pTwccRtpPktInfosHashTable->itemCount); // Ensure the table is cleared again
505+
506+
MUTEX_LOCK(pKvsPeerConnection->twccLock);
507+
MUTEX_UNLOCK(pKvsPeerConnection->twccLock);
508+
509+
EXPECT_EQ(STATUS_SUCCESS, freePeerConnection(&pRtcPeerConnection));
510+
}
511+
474512
} // namespace webrtcclient
475513
} // namespace video
476514
} // namespace kinesis

0 commit comments

Comments
 (0)