diff --git a/src/source/PeerConnection/PeerConnection.c b/src/source/PeerConnection/PeerConnection.c index 859f973bff..c23ad68cd3 100644 --- a/src/source/PeerConnection/PeerConnection.c +++ b/src/source/PeerConnection/PeerConnection.c @@ -998,6 +998,11 @@ STATUS freePeerConnection(PRtcPeerConnection* ppPeerConnection) CHK(pKvsPeerConnection != NULL, retStatus); + // This should be freed once answer is created, but unit tests might not invoke `createAnswer` + if (pKvsPeerConnection->pRemoteSessionDescription != NULL) { + DLOGW("Remote description field not deallocated"); + SAFE_MEMFREE(pKvsPeerConnection->pRemoteSessionDescription); + } startTime = GETTIME(); /* Shutdown IceAgent first so there is no more incoming packets which can cause * SCTP to be allocated again after SCTP is freed. */ @@ -1214,7 +1219,7 @@ STATUS peerConnectionGetLocalDescription(PRtcPeerConnection pRtcPeerConnection, pRtcSessionDescriptionInit->type = SDP_TYPE_ANSWER; } - CHK_STATUS(populateSessionDescription(pKvsPeerConnection, &(pKvsPeerConnection->remoteSessionDescription), pSessionDescription)); + CHK_STATUS(populateSessionDescription(pKvsPeerConnection, pKvsPeerConnection->pRemoteSessionDescription, pSessionDescription)); CHK_STATUS(serializeSessionDescription(pSessionDescription, NULL, &serializeLen)); CHK(serializeLen <= MAX_SESSION_DESCRIPTION_INIT_SDP_LEN, STATUS_NOT_ENOUGH_MEMORY); @@ -1238,11 +1243,11 @@ STATUS peerConnectionGetCurrentLocalDescription(PRtcPeerConnection pRtcPeerConne CHK(pRtcPeerConnection != NULL && pRtcSessionDescriptionInit != NULL, STATUS_NULL_ARG); // do nothing if remote session description hasn't been received - CHK(pKvsPeerConnection->remoteSessionDescription.sessionName[0] != '\0', retStatus); + CHK(pKvsPeerConnection->pRemoteSessionDescription != NULL, retStatus); CHK(NULL != (pSessionDescription = (PSessionDescription) MEMCALLOC(1, SIZEOF(SessionDescription))), STATUS_NOT_ENOUGH_MEMORY); - CHK_STATUS(populateSessionDescription(pKvsPeerConnection, &(pKvsPeerConnection->remoteSessionDescription), pSessionDescription)); + CHK_STATUS(populateSessionDescription(pKvsPeerConnection, pKvsPeerConnection->pRemoteSessionDescription, pSessionDescription)); CHK_STATUS(serializeSessionDescription(pSessionDescription, NULL, &serializeLen)); CHK(serializeLen <= MAX_SESSION_DESCRIPTION_INIT_SDP_LEN, STATUS_NOT_ENOUGH_MEMORY); @@ -1279,14 +1284,15 @@ STATUS setRemoteDescription(PRtcPeerConnection pPeerConnection, PRtcSessionDescr STATUS retStatus = STATUS_SUCCESS; PCHAR remoteIceUfrag = NULL, remoteIcePwd = NULL; UINT32 i, j; + PSessionDescription pSessionDescription; CHK(pPeerConnection != NULL, STATUS_NULL_ARG); PKvsPeerConnection pKvsPeerConnection = (PKvsPeerConnection) pPeerConnection; - PSessionDescription pSessionDescription = &pKvsPeerConnection->remoteSessionDescription; + pKvsPeerConnection->pRemoteSessionDescription = (PSessionDescription) MEMCALLOC(1, SIZEOF(SessionDescription)); + pSessionDescription = pKvsPeerConnection->pRemoteSessionDescription; CHK(pSessionDescriptionInit != NULL, STATUS_NULL_ARG); - MEMSET(pSessionDescription, 0x00, SIZEOF(SessionDescription)); pKvsPeerConnection->dtlsIsServer = FALSE; /* Assume cant trickle at first */ NULLABLE_SET_VALUE(pKvsPeerConnection->canTrickleIce, FALSE); @@ -1373,8 +1379,11 @@ STATUS setRemoteDescription(PRtcPeerConnection pPeerConnection, PRtcSessionDescr DLOGD("REMOTE_SDP:%s\n", pSessionDescriptionInit->sdp); } + // Remove remote description once required parameters are set from answer + if (pKvsPeerConnection->isOffer) { + SAFE_MEMFREE(pKvsPeerConnection->pRemoteSessionDescription); + } CleanUp: - LEAVES(); return retStatus; } @@ -1403,7 +1412,7 @@ STATUS createOffer(PRtcPeerConnection pPeerConnection, PRtcSessionDescriptionIni #endif CHK_STATUS(setPayloadTypesForOffer(pKvsPeerConnection->pCodecTable)); - CHK_STATUS(populateSessionDescription(pKvsPeerConnection, &(pKvsPeerConnection->remoteSessionDescription), pSessionDescription)); + CHK_STATUS(populateSessionDescription(pKvsPeerConnection, pKvsPeerConnection->pRemoteSessionDescription, pSessionDescription)); CHK_STATUS(serializeSessionDescription(pSessionDescription, NULL, &serializeLen)); CHK(serializeLen <= MAX_SESSION_DESCRIPTION_INIT_SDP_LEN, STATUS_NOT_ENOUGH_MEMORY); CHK_STATUS(serializeSessionDescription(pSessionDescription, pSessionDescriptionInit->sdp, &serializeLen)); @@ -1427,7 +1436,7 @@ STATUS createAnswer(PRtcPeerConnection pPeerConnection, PRtcSessionDescriptionIn PKvsPeerConnection pKvsPeerConnection = (PKvsPeerConnection) pPeerConnection; CHK(pKvsPeerConnection != NULL && pSessionDescriptionInit != NULL, STATUS_NULL_ARG); - CHK(pKvsPeerConnection->remoteSessionDescription.sessionName[0] != '\0', STATUS_PEERCONNECTION_CREATE_ANSWER_WITHOUT_REMOTE_DESCRIPTION); + CHK(pKvsPeerConnection->pRemoteSessionDescription != NULL, STATUS_PEERCONNECTION_CREATE_ANSWER_WITHOUT_REMOTE_DESCRIPTION); pSessionDescriptionInit->type = SDP_TYPE_ANSWER; pKvsPeerConnection->isOffer = FALSE; @@ -1437,6 +1446,10 @@ STATUS createAnswer(PRtcPeerConnection pPeerConnection, PRtcSessionDescriptionIn if (NULL != GETENV(DEBUG_LOG_SDP)) { DLOGD("LOCAL_SDP:%s", pSessionDescriptionInit->sdp); } + + // Once answer is created, remote SDP is not needed anymore. We also clear only if + // answer SDP is successfully created + SAFE_MEMFREE(pKvsPeerConnection->pRemoteSessionDescription); CleanUp: LEAVES(); diff --git a/src/source/PeerConnection/PeerConnection.h b/src/source/PeerConnection/PeerConnection.h index 9fc3638bce..6388218be8 100644 --- a/src/source/PeerConnection/PeerConnection.h +++ b/src/source/PeerConnection/PeerConnection.h @@ -85,7 +85,7 @@ typedef struct { PSctpSession pSctpSession; - SessionDescription remoteSessionDescription; + PSessionDescription pRemoteSessionDescription; PDoubleList pTransceivers; PDoubleList pFakeTransceivers; PDoubleList pAnswerTransceivers; diff --git a/src/source/PeerConnection/SessionDescription.c b/src/source/PeerConnection/SessionDescription.c index 3ec55159f6..94a0181388 100644 --- a/src/source/PeerConnection/SessionDescription.c +++ b/src/source/PeerConnection/SessionDescription.c @@ -413,9 +413,10 @@ STATUS populateSingleMediaSection(PKvsPeerConnection pKvsPeerConnection, PKvsRtp CHK_STATUS(hashTableGet(pUnknownCodecPayloadTypesTable, unknownCodecHashTableKey, &payloadType)); } else { CHK_STATUS(hashTableGet(pKvsPeerConnection->pCodecTable, pRtcMediaStreamTrack->codec, &payloadType)); - currentFmtp = fmtpForPayloadType(payloadType, &(pKvsPeerConnection->remoteSessionDescription)); + if (!pKvsPeerConnection->isOffer) { + currentFmtp = fmtpForPayloadType(payloadType, pRemoteSessionDescription); + } } - if (pRtcMediaStreamTrack->kind == MEDIA_STREAM_TRACK_KIND_VIDEO) { if (pRtcMediaStreamTrack->codec == RTC_CODEC_H264_PROFILE_42E01F_LEVEL_ASYMMETRY_ALLOWED_PACKETIZATION_MODE) { retStatus = hashTableGet(pKvsPeerConnection->pRtxTable, RTC_RTX_CODEC_H264_PROFILE_42E01F_LEVEL_ASYMMETRY_ALLOWED_PACKETIZATION_MODE, @@ -555,11 +556,14 @@ STATUS populateSingleMediaSection(PKvsPeerConnection pKvsPeerConnection, PKvsRtp attributeCount++; STRCPY(pSdpMediaDescription->sdpAttributes[attributeCount].attributeName, "mid"); - // check all session attribute lines to see if a line with mid is present. If it is present, copy its content and break - for (i = 0; i < pRemoteSessionDescription->mediaDescriptions[mediaSectionId].mediaAttributesCount; i++) { - if (STRCMP(pRemoteSessionDescription->mediaDescriptions[mediaSectionId].sdpAttributes[i].attributeName, MID_KEY) == 0) { - STRCPY(remoteSdpAttributeValue, pRemoteSessionDescription->mediaDescriptions[mediaSectionId].sdpAttributes[i].attributeValue); - break; + + if (!pKvsPeerConnection->isOffer) { + // check all session attribute lines to see if a line with mid is present. If it is present, copy its content and break + for (i = 0; i < pRemoteSessionDescription->mediaDescriptions[mediaSectionId].mediaAttributesCount; i++) { + if (STRCMP(pRemoteSessionDescription->mediaDescriptions[mediaSectionId].sdpAttributes[i].attributeName, MID_KEY) == 0) { + STRCPY(remoteSdpAttributeValue, pRemoteSessionDescription->mediaDescriptions[mediaSectionId].sdpAttributes[i].attributeValue); + break; + } } } @@ -620,7 +624,6 @@ STATUS populateSingleMediaSection(PKvsPeerConnection pKvsPeerConnection, PKvsRtp STRCPY(pSdpMediaDescription->sdpAttributes[attributeCount].attributeName, "rtcp-mux"); attributeCount++; - if (mediaSectionId != 0) { STRCPY(pSdpMediaDescription->sdpAttributes[attributeCount].attributeName, "rtcp-rsize"); attributeCount++; @@ -889,10 +892,8 @@ STATUS populateSessionDescriptionMedia(PKvsPeerConnection pKvsPeerConnection, PS UINT32 unknownHashTableBucketCount = 0; CHK_STATUS(dtlsSessionGetLocalCertificateFingerprint(pKvsPeerConnection->pDtlsSession, certificateFingerprint, CERTIFICATE_FINGERPRINT_LENGTH)); - if (pKvsPeerConnection->isOffer) { pDtlsRole = DTLS_ROLE_ACTPASS; - CHK_STATUS(doubleListGetHeadNode(pKvsPeerConnection->pTransceivers, &pCurNode)); while (pCurNode != NULL) { CHK_STATUS(doubleListGetNodeData(pCurNode, &data)); @@ -900,7 +901,6 @@ STATUS populateSessionDescriptionMedia(PKvsPeerConnection pKvsPeerConnection, PS pKvsRtpTransceiver = (PKvsRtpTransceiver) data; if (pKvsRtpTransceiver != NULL) { CHK(pLocalSessionDescription->mediaCount < MAX_SDP_SESSION_MEDIA_COUNT, STATUS_SESSION_DESCRIPTION_MAX_MEDIA_COUNT); - // If generating answer, need to check if Local Description is present in remote -- if not, we don't need to create a local // description for it or else our Answer will have an extra m-line, for offer the local is the offer itself, don't care about remote CHK_STATUS(populateSingleMediaSection( @@ -922,7 +922,6 @@ STATUS populateSessionDescriptionMedia(PKvsPeerConnection pKvsPeerConnection, PS // if an m-line does not have a corresponding transceiver created by the user, we create a fake transceiver CHK_STATUS(findTransceiversByRemoteDescription(pKvsPeerConnection, pRemoteSessionDescription, pUnknownCodecPayloadTypesTable, pUnknownCodecRtpmapTable)); - // pAnswerTransceivers contains transceivers created by the user as well as fake transceivers CHK_STATUS(doubleListGetHeadNode(pKvsPeerConnection->pAnswerTransceivers, &pCurNode)); while (pCurNode != NULL) { @@ -989,12 +988,14 @@ STATUS populateSessionDescription(PKvsPeerConnection pKvsPeerConnection, PSessio UINT32 i, sizeRemaining; INT32 charsCopied; - CHK(pKvsPeerConnection != NULL && pLocalSessionDescription != NULL && pRemoteSessionDescription != NULL, STATUS_NULL_ARG); + CHK(pKvsPeerConnection != NULL && pLocalSessionDescription != NULL, STATUS_NULL_ARG); + if (!pKvsPeerConnection->isOffer) { + CHK(pRemoteSessionDescription != NULL, STATUS_NULL_ARG); + } CHK_STATUS(populateSessionDescriptionMedia(pKvsPeerConnection, pRemoteSessionDescription, pLocalSessionDescription)); MEMSET(bundleValue, 0, MAX_SDP_ATTRIBUTE_VALUE_LENGTH); MEMSET(wmsValue, 0, MAX_SDP_ATTRIBUTE_VALUE_LENGTH); MEMSET(remoteSdpAttributeValue, 0, MAX_SDP_ATTRIBUTE_VALUE_LENGTH); - STRCPY(pLocalSessionDescription->sdpOrigin.userName, "-"); pLocalSessionDescription->sdpOrigin.sessionId = RAND(); pLocalSessionDescription->sdpOrigin.sessionVersion = 2; @@ -1011,7 +1012,6 @@ STATUS populateSessionDescription(PKvsPeerConnection pKvsPeerConnection, PSessio STRCPY(pLocalSessionDescription->sdpAttributes[0].attributeName, "group"); STRCPY(pLocalSessionDescription->sdpAttributes[0].attributeValue, BUNDLE_KEY); pLocalSessionDescription->sessionAttributesCount++; - if (pKvsPeerConnection->canTrickleIce.value) { STRCPY(pLocalSessionDescription->sdpAttributes[pLocalSessionDescription->sessionAttributesCount].attributeName, "ice-options"); STRCPY(pLocalSessionDescription->sdpAttributes[pLocalSessionDescription->sessionAttributesCount].attributeValue, "trickle"); @@ -1019,10 +1019,12 @@ STATUS populateSessionDescription(PKvsPeerConnection pKvsPeerConnection, PSessio } // check all session attribute lines to see if a line with BUNDLE is present. If it is present, copy its content and break - for (i = 0; i < pRemoteSessionDescription->sessionAttributesCount; i++) { - if (STRSTR(pRemoteSessionDescription->sdpAttributes[i].attributeValue, BUNDLE_KEY) != NULL) { - STRCPY(remoteSdpAttributeValue, pRemoteSessionDescription->sdpAttributes[i].attributeValue + ARRAY_SIZE(BUNDLE_KEY) - 1); - break; + if (!pKvsPeerConnection->isOffer) { + for (i = 0; i < pRemoteSessionDescription->sessionAttributesCount; i++) { + if (STRSTR(pRemoteSessionDescription->sdpAttributes[i].attributeValue, BUNDLE_KEY) != NULL) { + STRCPY(remoteSdpAttributeValue, pRemoteSessionDescription->sdpAttributes[i].attributeValue + ARRAY_SIZE(BUNDLE_KEY) - 1); + break; + } } }