Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory Optimization - ICE Agent Stats (#1947) #2074

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions samples/Common.c
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,9 @@ STATUS initializePeerConnection(PSampleConfiguration pSampleConfiguration, PRtcP
// Set the ICE mode explicitly
configuration.iceTransportPolicy = ICE_TRANSPORT_POLICY_ALL;

#ifdef ENABLE_STATS_CALCULATION_CONTROL
configuration.kvsRtcConfiguration.enableIceStats = pSampleConfiguration->enableIceStats;
#endif

// Set the STUN server
PCHAR pKinesisVideoStunUrlPostFix = KINESIS_VIDEO_STUN_URL_POSTFIX;
Expand Down Expand Up @@ -505,9 +507,7 @@ STATUS createSampleStreamingSession(PSampleConfiguration pSampleConfiguration, P

// Flag to enable/disable SDK calculations of selected ice server, local, remote and candidate pair stats.
// Note: enableIceStats only has an effect if compiler flag ENABLE_STATS_CALCULATION_CONTROL is defined.
#ifdef ENABLE_STATS_CALCULATION_CONTROL
pSampleConfiguration->enableIceStats = FALSE;
#endif

CHK_STATUS(initializePeerConnection(pSampleConfiguration, &pSampleStreamingSession->pPeerConnection));
CHK_STATUS(peerConnectionOnIceCandidate(pSampleStreamingSession->pPeerConnection, (UINT64) pSampleStreamingSession, onIceCandidateHandler));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1199,8 +1199,10 @@ typedef struct {
BOOL disableSenderSideBandwidthEstimation; //!< Disable TWCC feedback based sender bandwidth estimation, enabled by default.
//!< You want to set this to TRUE if you are on a very stable connection and want to save 1.2MB of
//!< memory
#ifdef ENABLE_STATS_CALCULATION_CONTROL
BOOL enableIceStats; //!< Control whether ICE agent stats are to be calculated. ENABLE_STATS_CALCULATION_CONTROL compiler flag must be defined
stefankiesz marked this conversation as resolved.
Show resolved Hide resolved
//!< to use this member, else stats are enabled by default.
#endif
} KvsRtcConfiguration, *PKvsRtcConfiguration;

/**
Expand Down
19 changes: 9 additions & 10 deletions src/source/Ice/IceAgent.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ STATUS createIceAgent(PCHAR username, PCHAR password, PIceAgentCallbacks pIceAge
PIceAgent pIceAgent = NULL;
UINT32 i;
UINT64 startTimeInMacro = 0;
BOOL statsControlEnabled = FALSE;
BOOL disableStats = FALSE;

// Ice agent stats calculations are on by default.
// Runtime control for turning stats calculations on/off can be activated with this compiler flag.
#ifdef ENABLE_STATS_CALCULATION_CONTROL
statsControlEnabled = TRUE;
disableStats = !pIceAgent->kvsRtcConfiguration.enableIceStats;
#endif

CHK(ppIceAgent != NULL && username != NULL && password != NULL && pConnectionListener != NULL, STATUS_NULL_ARG);
Expand Down Expand Up @@ -113,7 +113,7 @@ STATUS createIceAgent(PCHAR username, PCHAR password, PIceAgentCallbacks pIceAge
(PCHAR) pRtcConfiguration->iceServers[i].username, (PCHAR) pRtcConfiguration->iceServers[i].credential),
pIceAgent->iceAgentProfileDiagnostics.iceServerParsingTime[i], "ICE server parsing");
if (STATUS_SUCCEEDED(retStatus)) {
if (!statsControlEnabled || (statsControlEnabled && pIceAgent->kvsRtcConfiguration.enableIceStats)) {
if (!disableStats) {
CHK(NULL != (pIceAgent->pRtcIceServerDiagnostics[i] = (PRtcIceServerDiagnostics) MEMCALLOC(1, SIZEOF(RtcIceServerDiagnostics))),
STATUS_NOT_ENOUGH_MEMORY);
pIceAgent->pRtcIceServerDiagnostics[i]->port = (INT32) getInt16(pIceAgent->iceServers[i].ipAddress.port);
Expand All @@ -136,8 +136,7 @@ STATUS createIceAgent(PCHAR username, PCHAR password, PIceAgentCallbacks pIceAge
}
}

// Note: The conditional is emphasizing that statsControlEnabled must be TRUE in order to use kvsRtcConfiguration.enableIceStats.
if (!statsControlEnabled || (statsControlEnabled && pIceAgent->kvsRtcConfiguration.enableIceStats)) {
if (!disableStats) {
CHK(NULL !=
(pIceAgent->pRtcSelectedRemoteIceCandidateDiagnostics =
(PRtcIceCandidateDiagnostics) MEMCALLOC(1, SIZEOF(RtcIceCandidateDiagnostics))),
Expand Down Expand Up @@ -1084,12 +1083,12 @@ STATUS createIceCandidatePairs(PIceAgent pIceAgent, PIceCandidate pIceCandidate,
PIceCandidatePair pIceCandidatePair = NULL;
BOOL freeObjOnFailure = TRUE;
PIceCandidate pCurrentIceCandidate = NULL;
BOOL statsControlEnabled = FALSE;
BOOL disableStats = FALSE;
stefankiesz marked this conversation as resolved.
Show resolved Hide resolved

// Ice agent stats calculations are on by default.
// Runtime control for turning stats calculations on/off can be activated with this compiler flag.
#ifdef ENABLE_STATS_CALCULATION_CONTROL
statsControlEnabled = TRUE;
disableStats = !pIceAgent->kvsRtcConfiguration.enableIceStats;
#endif

CHK(pIceAgent != NULL && pIceCandidate != NULL, STATUS_NULL_ARG);
Expand All @@ -1110,7 +1109,7 @@ STATUS createIceCandidatePairs(PIceAgent pIceAgent, PIceCandidate pIceCandidate,
if (pCurrentIceCandidate->state == ICE_CANDIDATE_STATE_VALID && pCurrentIceCandidate->ipAddress.family == pIceCandidate->ipAddress.family) {
pIceCandidatePair = (PIceCandidatePair) MEMCALLOC(1, SIZEOF(IceCandidatePair));
CHK(pIceCandidatePair != NULL, STATUS_NOT_ENOUGH_MEMORY);
if (!statsControlEnabled || (statsControlEnabled && pIceAgent->kvsRtcConfiguration.enableIceStats)) {
if (!disableStats) {
CHK(NULL !=
(pIceCandidatePair->pRtcIceCandidatePairDiagnostics =
(PRtcIceCandidatePairDiagnostics) MEMCALLOC(1, SIZEOF(RtcIceCandidatePairDiagnostics))),
Expand Down Expand Up @@ -2587,10 +2586,10 @@ STATUS handleStunPacket(PIceAgent pIceAgent, PBYTE pBuffer, UINT32 bufferLen, PS
pIceAgent->pRtcIceServerDiagnostics[pIceCandidate->iceServerIndex]->totalResponsesReceived++;
// Transaction ID count be same for candidates coming from same interface, which means there would only
// be one entry. It is not necessary to update a return sttaus since it is not indicative of a failure
if ((hashTableGet(pIceAgent->requestTimestampDiagnostics, checkSum, &requestSentTime)) == STATUS_SUCCESS) {
if (STATUS_SUCCEEDED(hashTableGet(pIceAgent->requestTimestampDiagnostics, checkSum, &requestSentTime))) {
pIceAgent->pRtcIceServerDiagnostics[pIceCandidate->iceServerIndex]->totalRoundTripTime += GETTIME() - requestSentTime;
CHK_STATUS(hashTableRemove(pIceAgent->requestTimestampDiagnostics, checkSum));
hashTableGetCount(pIceAgent->requestTimestampDiagnostics, &count);
CHK_STATUS(hashTableGetCount(pIceAgent->requestTimestampDiagnostics, &count));
}
}

Expand Down
32 changes: 15 additions & 17 deletions src/source/Metrics/Metrics.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ STATUS getIceCandidatePairStats(PRtcPeerConnection pRtcPeerConnection, PRtcIceCa
pIceAgent = ((PKvsPeerConnection) pRtcPeerConnection)->pIceAgent;
MUTEX_LOCK(pIceAgent->lock);
locked = TRUE;
CHK_WARN(!statsControlEnabled || pIceAgent->kvsRtcConfiguration.enableIceStats, STATUS_SUCCESS, "ICE stats are not enabled.");
#ifdef ENABLE_STATS_CALCULATION_CONTROL
CHK_WARN(pIceAgent->kvsRtcConfiguration.enableIceStats, STATUS_INVALID_OPERATION, "ICE stats not enabled");
stefankiesz marked this conversation as resolved.
Show resolved Hide resolved
#endif
CHK(pIceAgent->pDataSendingIceCandidatePair != NULL, STATUS_SUCCESS);
PRtcIceCandidatePairDiagnostics pRtcIceCandidatePairDiagnostics = pIceAgent->pDataSendingIceCandidatePair->pRtcIceCandidatePairDiagnostics;
if (pRtcIceCandidatePairDiagnostics != NULL) {
Expand Down Expand Up @@ -64,18 +66,16 @@ STATUS getIceCandidateStats(PRtcPeerConnection pRtcPeerConnection, BOOL isRemote
{
STATUS retStatus = STATUS_SUCCESS;
BOOL locked = FALSE;
PIceAgent pIceAgent = ((PKvsPeerConnection) pRtcPeerConnection)->pIceAgent;
BOOL statsControlEnabled = FALSE;

#ifdef ENABLE_STATS_CALCULATION_CONTROL
statsControlEnabled = TRUE;
#endif

PIceAgent pIceAgent = NULL;
PRtcIceCandidateDiagnostics pRtcIceCandidateDiagnostics = NULL;
CHK((pRtcPeerConnection != NULL || pRtcIceCandidateStats != NULL), STATUS_NULL_ARG);
pIceAgent = ((PKvsPeerConnection) pRtcPeerConnection)->pIceAgent;
MUTEX_LOCK(pIceAgent->lock);
locked = TRUE;
CHK_WARN(!statsControlEnabled || pIceAgent->kvsRtcConfiguration.enableIceStats, STATUS_SUCCESS, "ICE stats not enabled");
PRtcIceCandidateDiagnostics pRtcIceCandidateDiagnostics = pIceAgent->pRtcSelectedRemoteIceCandidateDiagnostics;
#ifdef ENABLE_STATS_CALCULATION_CONTROL
CHK_WARN(pIceAgent->kvsRtcConfiguration.enableIceStats, STATUS_INVALID_OPERATION, "ICE stats not enabled");
#endif
pRtcIceCandidateDiagnostics = pIceAgent->pRtcSelectedRemoteIceCandidateDiagnostics;
if (pRtcIceCandidateDiagnostics != NULL) {
if (!isRemote) {
pRtcIceCandidateDiagnostics = pIceAgent->pRtcSelectedLocalIceCandidateDiagnostics;
Expand All @@ -100,17 +100,15 @@ STATUS getIceServerStats(PRtcPeerConnection pRtcPeerConnection, PRtcIceServerSta
STATUS retStatus = STATUS_SUCCESS;
BOOL locked = FALSE;
PIceAgent pIceAgent = ((PKvsPeerConnection) pRtcPeerConnection)->pIceAgent;
BOOL statsControlEnabled = FALSE;

#ifdef ENABLE_STATS_CALCULATION_CONTROL
statsControlEnabled = TRUE;
#endif

CHK((pRtcPeerConnection != NULL && pRtcIceServerStats != NULL), STATUS_NULL_ARG);

MUTEX_LOCK(pIceAgent->lock);
locked = TRUE;
CHK_WARN(!statsControlEnabled || pIceAgent->kvsRtcConfiguration.enableIceStats, STATUS_SUCCESS, "ICE stats not enabled");

#ifdef ENABLE_STATS_CALCULATION_CONTROL
CHK_WARN(pIceAgent->kvsRtcConfiguration.enableIceStats, STATUS_INVALID_OPERATION, "ICE stats not enabled");
#endif

CHK(pRtcIceServerStats->iceServerIndex < pIceAgent->iceServersCount, STATUS_ICE_SERVER_INDEX_INVALID);

if (pIceAgent->pRtcIceServerDiagnostics[pRtcIceServerStats->iceServerIndex] != NULL) {
Expand Down
2 changes: 0 additions & 2 deletions tst/MetricsApiTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ TEST_F(MetricsApiTest, webRtcIceServerGetMetrics)
STRNCPY(configuration.iceServers[1].urls, (PCHAR) "turns:54.202.170.151:443?transport=tcp", MAX_ICE_CONFIG_URI_LEN);
STRNCPY(configuration.iceServers[1].credential, (PCHAR) "username", MAX_ICE_CONFIG_CREDENTIAL_LEN);
STRNCPY(configuration.iceServers[1].username, (PCHAR) "password", MAX_ICE_CONFIG_USER_NAME_LEN);
configuration.kvsRtcConfiguration.enableIceStats = TRUE;
ASSERT_EQ(STATUS_SUCCESS, createPeerConnection(&configuration, &pRtcPeerConnection));

EXPECT_EQ(STATUS_ICE_SERVER_INDEX_INVALID, rtcPeerConnectionGetMetrics(pRtcPeerConnection, NULL, &rtcIceMetrics));
Expand Down Expand Up @@ -89,7 +88,6 @@ TEST_F(MetricsApiTest, webRtcIceCandidateGetMetrics)
STRNCPY(configuration.iceServers[0].urls, (PCHAR) "stun:stun.kinesisvideo.us-west-2.amazonaws.com:443", MAX_ICE_CONFIG_URI_LEN);
STRNCPY(configuration.iceServers[0].credential, (PCHAR) "", MAX_ICE_CONFIG_CREDENTIAL_LEN);
STRNCPY(configuration.iceServers[0].username, (PCHAR) "", MAX_ICE_CONFIG_USER_NAME_LEN);
configuration.kvsRtcConfiguration.enableIceStats = TRUE;
ASSERT_EQ(STATUS_SUCCESS, createPeerConnection(&configuration, &pRtcPeerConnection));

pIceAgent = ((PKvsPeerConnection) pRtcPeerConnection)->pIceAgent;
Expand Down
Loading