Skip to content

Commit

Permalink
Address comments, disable TWCC by default
Browse files Browse the repository at this point in the history
  • Loading branch information
@disa6302
disa6302 committed Mar 4, 2024
1 parent 37f0225 commit 33cc801
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 55 deletions.
4 changes: 2 additions & 2 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,11 +356,11 @@ In order to listen in on TWCC reports, the application must set up a callback us
sampleSenderBandwidthEstimationHandler));
```
Note that TWCC is enabled by default in the SDK. In order to disable it, set the `disableSenderSideBandwidthEstimation` flag to TRUE. For example,
Note that TWCC is disabled by default in the SDK samples. In order to enable it, set the `disableSenderSideBandwidthEstimation` flag to FALSE. For example,
```
RtcConfiguration configuration;
configuration.kvsRtcConfiguration.disableSenderSideBandwidthEstimation = TRUE;
configuration.kvsRtcConfiguration.disableSenderSideBandwidthEstimation = FALSE;
```
Expand Down
1 change: 1 addition & 0 deletions samples/Common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ STATUS initializePeerConnection(PSampleConfiguration pSampleConfiguration, PRtcP
// Set the ICE mode explicitly
configuration.iceTransportPolicy = ICE_TRANSPORT_POLICY_ALL;

configuration.kvsRtcConfiguration.disableSenderSideBandwidthEstimation = TRUE;
// Set the STUN server
PCHAR pKinesisVideoStunUrlPostFix = KINESIS_VIDEO_STUN_URL_POSTFIX;
// If region is in CN, add CN region uri postfix
Expand Down
31 changes: 19 additions & 12 deletions src/source/PeerConnection/PeerConnection.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -994,7 +994,8 @@ STATUS freePeerConnection(PRtcPeerConnection* ppPeerConnection)
PDoubleListNode pCurNode = NULL;
UINT64 item = 0;
UINT64 startTime;
UINT32 count;
UINT32 twccHashTableCount = 0;
BOOL twccLocked = FALSE;

CHK(ppPeerConnection != NULL, STATUS_NULL_ARG);

Expand Down Expand Up @@ -1069,12 +1070,16 @@ STATUS freePeerConnection(PRtcPeerConnection* ppPeerConnection)

if (pKvsPeerConnection->pTwccManager != NULL) {
MUTEX_LOCK(pKvsPeerConnection->twccLock);
hashTableGetCount(pKvsPeerConnection->pTwccManager->pTwccRtpPktInfosHashTable, &count);
DLOGI("Number of TWCC info packets in memory: %d", count);
twccLocked = TRUE;
hashTableGetCount(pKvsPeerConnection->pTwccManager->pTwccRtpPktInfosHashTable, &twccHashTableCount);
DLOGI("Number of TWCC info packets in memory: %d", twccHashTableCount);
hashTableIterateEntries(pKvsPeerConnection->pTwccManager->pTwccRtpPktInfosHashTable, 0, freeHashEntry);
hashTableFree(pKvsPeerConnection->pTwccManager->pTwccRtpPktInfosHashTable);
if (IS_VALID_MUTEX_VALUE(pKvsPeerConnection->twccLock)) {
MUTEX_UNLOCK(pKvsPeerConnection->twccLock);
if (twccLocked) {
MUTEX_UNLOCK(pKvsPeerConnection->twccLock);
twccLocked = FALSE;
}
MUTEX_FREE(pKvsPeerConnection->twccLock);
}
SAFE_MEMFREE(pKvsPeerConnection->pTwccManager);
Expand Down Expand Up @@ -1781,19 +1786,19 @@ static STATUS twccRollingWindowDeletion(PKvsPeerConnection pKvsPeerConnection, P
{
ENTERS();
STATUS retStatus = STATUS_SUCCESS;
UINT16 updatedSeqNum;
UINT16 updatedSeqNum = 0;
PTwccRtpPacketInfo tempTwccRtpPktInfo = NULL;
UINT64 ageOfOldest, firstRtpTime;
UINT64 value;
UINT64 ageOfOldest = 0, firstRtpTime = 0;
UINT64 twccPacketValue = 0;
BOOL isCheckComplete = FALSE;

CHK(pKvsPeerConnection != NULL && pRtpPacket != NULL, STATUS_NULL_ARG);
CHK(pKvsPeerConnection != NULL && pRtpPacket != NULL && pKvsPeerConnection->pTwccManager != NULL, STATUS_NULL_ARG);

updatedSeqNum = pKvsPeerConnection->pTwccManager->firstSeqNumInRollingWindow;
do {
// If the seqNum is not present in the hash table, it is ok. We move on to the next
if (STATUS_SUCCEEDED(hashTableGet(pKvsPeerConnection->pTwccManager->pTwccRtpPktInfosHashTable, updatedSeqNum, &value))) {
tempTwccRtpPktInfo = (PTwccRtpPacketInfo) value;
if (STATUS_SUCCEEDED(hashTableGet(pKvsPeerConnection->pTwccManager->pTwccRtpPktInfosHashTable, updatedSeqNum, &twccPacketValue))) {
tempTwccRtpPktInfo = (PTwccRtpPacketInfo) twccPacketValue;
}
if (tempTwccRtpPktInfo != NULL) {
firstRtpTime = tempTwccRtpPktInfo->localTimeKvs;
Expand All @@ -1819,6 +1824,8 @@ static STATUS twccRollingWindowDeletion(PKvsPeerConnection pKvsPeerConnection, P
} else {
updatedSeqNum++;
}
// reset before next iteration
tempTwccRtpPktInfo = NULL;
} while (!isCheckComplete && updatedSeqNum != (seqNum + 1));

// Update regardless. The loop checks until current RTP packets seq number irrespective of the failure
Expand All @@ -1833,8 +1840,8 @@ STATUS twccManagerOnPacketSent(PKvsPeerConnection pKvsPeerConnection, PRtpPacket
ENTERS();
STATUS retStatus = STATUS_SUCCESS;
BOOL locked = FALSE;
UINT16 seqNum;
PTwccRtpPacketInfo pTwccRtpPktInfo;
UINT16 seqNum = 0;
PTwccRtpPacketInfo pTwccRtpPktInfo = NULL;

CHK(pKvsPeerConnection != NULL && pRtpPacket != NULL, STATUS_NULL_ARG);
CHK(pKvsPeerConnection->onSenderBandwidthEstimation != NULL && pKvsPeerConnection->pTwccManager != NULL, STATUS_SUCCESS);
Expand Down
100 changes: 59 additions & 41 deletions src/source/PeerConnection/Rtcp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,8 @@ STATUS parseRtcpTwccPacket(PRtcpPacket pRtcpPacket, PTwccManager pTwccManager)
UINT32 statuses;
UINT32 i;
UINT64 referenceTime;
PTwccRtpPacketInfo pTwccPacket;
UINT64 value;
PTwccRtpPacketInfo pTwccPacket = NULL;
UINT64 twccPktValue = 0;
CHK(pTwccManager != NULL && pRtcpPacket != NULL, STATUS_NULL_ARG);

baseSeqNum = getUnalignedInt16BigEndian(pRtcpPacket->payload + 8);
Expand Down Expand Up @@ -220,10 +220,12 @@ STATUS parseRtcpTwccPacket(PRtcpPacket pRtcpPacket, PTwccManager pTwccManager)
case TWCC_STATUS_SYMBOL_NOTRECEIVED:
DLOGS("runLength packetSeqNum %u not received %lu", packetSeqNum, referenceTime);
// If it does not exist it means the packet was already visited
if (STATUS_SUCCEEDED(hashTableGet(pTwccManager->pTwccRtpPktInfosHashTable, packetSeqNum, &value))) {
pTwccPacket = (PTwccRtpPacketInfo) value;
pTwccPacket->remoteTimeKvs = TWCC_PACKET_LOST_TIME;
CHK_STATUS(hashTableUpsert(pTwccManager->pTwccRtpPktInfosHashTable, packetSeqNum, (UINT64) pTwccPacket));
if (STATUS_SUCCEEDED(hashTableGet(pTwccManager->pTwccRtpPktInfosHashTable, packetSeqNum, &twccPktValue))) {
pTwccPacket = (PTwccRtpPacketInfo) twccPktValue;
if (pTwccPacket != NULL) {
pTwccPacket->remoteTimeKvs = TWCC_PACKET_LOST_TIME;
CHK_STATUS(hashTableUpsert(pTwccManager->pTwccRtpPktInfosHashTable, packetSeqNum, (UINT64) pTwccPacket));
}
}
pTwccManager->lastReportedSeqNum = packetSeqNum;
break;
Expand All @@ -235,15 +237,19 @@ STATUS parseRtcpTwccPacket(PRtcpPacket pRtcpPacket, PTwccManager pTwccManager)
DLOGS("runLength packetSeqNum %u received %lu", packetSeqNum, referenceTime);

// If it does not exist it means the packet was already visited
if (STATUS_SUCCEEDED(hashTableGet(pTwccManager->pTwccRtpPktInfosHashTable, packetSeqNum, &value))) {
pTwccPacket = (PTwccRtpPacketInfo) value;
pTwccPacket->remoteTimeKvs = referenceTime;
CHK_STATUS(hashTableUpsert(pTwccManager->pTwccRtpPktInfosHashTable, packetSeqNum, (UINT64) pTwccPacket));
if (STATUS_SUCCEEDED(hashTableGet(pTwccManager->pTwccRtpPktInfosHashTable, packetSeqNum, &twccPktValue))) {
pTwccPacket = (PTwccRtpPacketInfo) twccPktValue;
if (pTwccPacket != NULL) {
pTwccPacket->remoteTimeKvs = referenceTime;
CHK_STATUS(hashTableUpsert(pTwccManager->pTwccRtpPktInfosHashTable, packetSeqNum, (UINT64) pTwccPacket));
}
}
pTwccManager->lastReportedSeqNum = packetSeqNum;
}
packetSeqNum++;
packetsRemaining--;
// Reset to NULL before next iteration
pTwccPacket = NULL;
}
} else {
statuses = MIN(TWCC_STATUSVECTOR_COUNT(packetChunk), packetsRemaining);
Expand All @@ -263,10 +269,12 @@ STATUS parseRtcpTwccPacket(PRtcpPacket pRtcpPacket, PTwccManager pTwccManager)
case TWCC_STATUS_SYMBOL_NOTRECEIVED:
DLOGS("statusVector packetSeqNum %u not received %lu", packetSeqNum, referenceTime);
// If it does not exist it means the packet was already visited
if (STATUS_SUCCEEDED(hashTableGet(pTwccManager->pTwccRtpPktInfosHashTable, packetSeqNum, &value))) {
pTwccPacket = (PTwccRtpPacketInfo) value;
pTwccPacket->remoteTimeKvs = TWCC_PACKET_LOST_TIME;
CHK_STATUS(hashTableUpsert(pTwccManager->pTwccRtpPktInfosHashTable, packetSeqNum, (UINT64) pTwccPacket));
if (STATUS_SUCCEEDED(hashTableGet(pTwccManager->pTwccRtpPktInfosHashTable, packetSeqNum, &twccPktValue))) {
pTwccPacket = (PTwccRtpPacketInfo) twccPktValue;
if (pTwccPacket != NULL) {
pTwccPacket->remoteTimeKvs = TWCC_PACKET_LOST_TIME;
CHK_STATUS(hashTableUpsert(pTwccManager->pTwccRtpPktInfosHashTable, packetSeqNum, (UINT64) pTwccPacket));
}
}
pTwccManager->lastReportedSeqNum = packetSeqNum;
break;
Expand All @@ -277,15 +285,19 @@ STATUS parseRtcpTwccPacket(PRtcpPacket pRtcpPacket, PTwccManager pTwccManager)
referenceTime += KVS_CONVERT_TIMESCALE(recvDelta, TWCC_TICKS_PER_SECOND, HUNDREDS_OF_NANOS_IN_A_SECOND);
DLOGS("statusVector packetSeqNum %u received %lu", packetSeqNum, referenceTime);
// If it does not exist it means the packet was already visited
if (STATUS_SUCCEEDED(hashTableGet(pTwccManager->pTwccRtpPktInfosHashTable, packetSeqNum, &value))) {
pTwccPacket = (PTwccRtpPacketInfo) value;
pTwccPacket->remoteTimeKvs = referenceTime;
CHK_STATUS(hashTableUpsert(pTwccManager->pTwccRtpPktInfosHashTable, packetSeqNum, (UINT64) pTwccPacket));
if (STATUS_SUCCEEDED(hashTableGet(pTwccManager->pTwccRtpPktInfosHashTable, packetSeqNum, &twccPktValue))) {
pTwccPacket = (PTwccRtpPacketInfo) twccPktValue;
if (pTwccPacket != NULL) {
pTwccPacket->remoteTimeKvs = referenceTime;
CHK_STATUS(hashTableUpsert(pTwccManager->pTwccRtpPktInfosHashTable, packetSeqNum, (UINT64) pTwccPacket));
}
}
pTwccManager->lastReportedSeqNum = packetSeqNum;
}
packetSeqNum++;
packetsRemaining--;
// Reset to NULL before next iteration
pTwccPacket = NULL;
}
}
chunkOffset += TWCC_FB_PACKETCHUNK_SIZE;
Expand All @@ -299,16 +311,16 @@ STATUS parseRtcpTwccPacket(PRtcpPacket pRtcpPacket, PTwccManager pTwccManager)
STATUS onRtcpTwccPacket(PRtcpPacket pRtcpPacket, PKvsPeerConnection pKvsPeerConnection)
{
STATUS retStatus = STATUS_SUCCESS;
PTwccManager pTwccManager;
PTwccManager pTwccManager = NULL;
BOOL locked = FALSE;
UINT64 sn = 0;
UINT64 localStartTimeKvs, localEndTimeKvs;
UINT64 sentBytes = 0, receivedBytes = 0;
UINT64 sentPackets = 0, receivedPackets = 0;
INT64 duration = 0;
UINT16 seqNum;
PTwccRtpPacketInfo pTwccPacket;
UINT64 value;
UINT16 seqNum = 0;
PTwccRtpPacketInfo pTwccPacket = NULL;
UINT64 twccPktValue = 0;
BOOL localStartTimeRecorded = FALSE;

CHK(pKvsPeerConnection != NULL && pRtcpPacket != NULL, STATUS_NULL_ARG);
Expand All @@ -330,36 +342,42 @@ STATUS onRtcpTwccPacket(PRtcpPacket pRtcpPacket, PKvsPeerConnection pKvsPeerConn
// This could happen if the prev packet was deleted as part of rolling window or if there
// is an overlap of RTP packet statuses between TWCC packets. This could also fail if it is
// the first ever packet (seqNum 0)
if (hashTableGet(pTwccManager->pTwccRtpPktInfosHashTable, seqNum - 1, &value) == STATUS_HASH_KEY_NOT_PRESENT) {
if (hashTableGet(pTwccManager->pTwccRtpPktInfosHashTable, seqNum - 1, &twccPktValue) == STATUS_HASH_KEY_NOT_PRESENT) {
localStartTimeKvs = TWCC_PACKET_UNITIALIZED_TIME;
} else {
pTwccPacket = (PTwccRtpPacketInfo) value;
localStartTimeKvs = pTwccPacket->localTimeKvs;
localStartTimeRecorded = TRUE;
pTwccPacket = (PTwccRtpPacketInfo) twccPktValue;
if (pTwccPacket != NULL) {
localStartTimeKvs = pTwccPacket->localTimeKvs;
localStartTimeRecorded = TRUE;
}
}
if (localStartTimeKvs == TWCC_PACKET_UNITIALIZED_TIME) {
// time not yet set. If prev seqNum was deleted
if (STATUS_SUCCEEDED(hashTableGet(pTwccManager->pTwccRtpPktInfosHashTable, seqNum, &value))) {
pTwccPacket = (PTwccRtpPacketInfo) value;
localStartTimeKvs = pTwccPacket->localTimeKvs;
localStartTimeRecorded = TRUE;
if (STATUS_SUCCEEDED(hashTableGet(pTwccManager->pTwccRtpPktInfosHashTable, seqNum, &twccPktValue))) {
pTwccPacket = (PTwccRtpPacketInfo) twccPktValue;
if (pTwccPacket != NULL) {
localStartTimeKvs = pTwccPacket->localTimeKvs;
localStartTimeRecorded = TRUE;
}
}
}
}

// The time it would not succeed is if there is an overlap in the RTP packet status between the TWCC
// packets
if (STATUS_SUCCEEDED(hashTableGet(pTwccManager->pTwccRtpPktInfosHashTable, seqNum, &value))) {
pTwccPacket = (PTwccRtpPacketInfo) value;
localEndTimeKvs = pTwccPacket->localTimeKvs;
duration = localEndTimeKvs - localStartTimeKvs;
sentBytes += pTwccPacket->packetSize;
sentPackets++;
if (pTwccPacket->remoteTimeKvs != TWCC_PACKET_LOST_TIME) {
receivedBytes += pTwccPacket->packetSize;
receivedPackets++;
if (STATUS_SUCCEEDED(hashTableRemove(pTwccManager->pTwccRtpPktInfosHashTable, seqNum))) {
SAFE_MEMFREE(pTwccPacket);
if (STATUS_SUCCEEDED(hashTableGet(pTwccManager->pTwccRtpPktInfosHashTable, seqNum, &twccPktValue))) {
pTwccPacket = (PTwccRtpPacketInfo) twccPktValue;
if (pTwccPacket != NULL) {
localEndTimeKvs = pTwccPacket->localTimeKvs;
duration = localEndTimeKvs - localStartTimeKvs;
sentBytes += pTwccPacket->packetSize;
sentPackets++;
if (pTwccPacket->remoteTimeKvs != TWCC_PACKET_LOST_TIME) {
receivedBytes += pTwccPacket->packetSize;
receivedPackets++;
if (STATUS_SUCCEEDED(hashTableRemove(pTwccManager->pTwccRtpPktInfosHashTable, seqNum))) {
SAFE_MEMFREE(pTwccPacket);
}
}
}
}
Expand Down

0 comments on commit 33cc801

Please sign in to comment.