Add a=ice-options:trickle to session level of SDP as viewer (#1813)

* Add a=ice-options:trickle to session level of SDP as viewer * negative test case added * clang format * add chk_log_err to confirm what is null in the stack
awslabs · Dec 5, 2023 · 157fb64 · 157fb64
1 parent 751bc91
commit 157fb64
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 2 deletions.
diff --git a/samples/kvsWebRTCClientViewer.c b/samples/kvsWebRTCClientViewer.c
@@ -78,6 +78,7 @@ INT32 main(INT32 argc, CHAR* argv[])
 
     MEMSET(&offerSessionDescriptionInit, 0x00, SIZEOF(RtcSessionDescriptionInit));
 
+    offerSessionDescriptionInit.useTrickleIce = pSampleStreamingSession->remoteCanTrickleIce;
     CHK_STATUS(setLocalDescription(pSampleStreamingSession->pPeerConnection, &offerSessionDescriptionInit));
     DLOGI("[KVS Viewer] Completed setting local description");
 

diff --git a/src/include/com/amazonaws/kinesis/video/webrtcclient/Include.h b/src/include/com/amazonaws/kinesis/video/webrtcclient/Include.h
@@ -1206,6 +1206,7 @@ typedef struct {
  */
 typedef struct {
     SDP_TYPE type;                                      //!< Indicates an offer/answer SDP type
+    BOOL useTrickleIce;                                 //!< Indicates if an offer should set trickle ice
     CHAR sdp[MAX_SESSION_DESCRIPTION_INIT_SDP_LEN + 1]; //!< SDP Data containing media capabilities, transport addresses
                                                         //!< and related metadata in a transport agnostic manner
                                                         //!<

diff --git a/src/source/Ice/IceUtils.c b/src/source/Ice/IceUtils.c
@@ -131,6 +131,8 @@ STATUS iceUtilsGenerateTransactionId(PBYTE pBuffer, UINT32 bufferLen)
 
 CleanUp:
 
+    CHK_LOG_ERR(retStatus);
+
     return retStatus;
 }
 

diff --git a/src/source/PeerConnection/PeerConnection.c b/src/source/PeerConnection/PeerConnection.c
@@ -1170,6 +1170,9 @@ STATUS createOffer(PRtcPeerConnection pPeerConnection, PRtcSessionDescriptionIni
     CHK(NULL != (pSessionDescription = (PSessionDescription) MEMCALLOC(1, SIZEOF(SessionDescription))), STATUS_NOT_ENOUGH_MEMORY);
     pSessionDescriptionInit->type = SDP_TYPE_OFFER;
     pKvsPeerConnection->isOffer = TRUE;
+    if (pSessionDescriptionInit->useTrickleIce) {
+        NULLABLE_SET_VALUE(pKvsPeerConnection->canTrickleIce, TRUE);
+    }
 
 #ifdef ENABLE_DATA_CHANNEL
     pKvsPeerConnection->sctpIsEnabled = TRUE;

diff --git a/src/source/PeerConnection/SessionDescription.c b/src/source/PeerConnection/SessionDescription.c
@@ -877,6 +877,13 @@ 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");
+        pLocalSessionDescription->sessionAttributesCount++;
+    }
 
     // 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++) {
@@ -908,7 +915,6 @@ STATUS populateSessionDescription(PKvsPeerConnection pKvsPeerConnection, PSessio
         STRCPY(pLocalSessionDescription->mediaDescriptions[i].sdpConnectionInformation.addressType, "IP4");
         STRCPY(pLocalSessionDescription->mediaDescriptions[i].sdpConnectionInformation.connectionAddress, "127.0.0.1");
     }
-    pLocalSessionDescription->sessionAttributesCount++;
 
     STRCPY(pLocalSessionDescription->sdpAttributes[pLocalSessionDescription->sessionAttributesCount].attributeName, "msid-semantic");
     STRCPY(pLocalSessionDescription->sdpAttributes[pLocalSessionDescription->sessionAttributesCount].attributeValue, " WMS myKvsVideoStream");

diff --git a/src/source/Stun/Stun.c b/src/source/Stun/Stun.c
@@ -1215,6 +1215,8 @@ STATUS getStunAttribute(PStunPacket pStunPacket, STUN_ATTRIBUTE_TYPE attributeTy
 
 CleanUp:
 
+    CHK_LOG_ERR(retStatus);
+
     if (ppStunAttribute != NULL) {
         *ppStunAttribute = pTargetAttribute;
     }

diff --git a/tst/SdpApiTest.cpp b/tst/SdpApiTest.cpp
@@ -1133,6 +1133,66 @@ a=ice-options:trickle
     });
 }
 
+TEST_F(SdpApiTest, noMediaTrickleIce) {
+    PRtcPeerConnection offerPc = NULL;
+    PRtcPeerConnection answerPc = NULL;
+    RtcConfiguration configurationOffer;
+    RtcConfiguration configurationAnswer;
+    RtcSessionDescriptionInit sessionDescriptionInitViewer;
+    RtcSessionDescriptionInit sessionDescriptionInitMaster;
+
+    MEMSET(&configurationOffer, 0x00, SIZEOF(RtcConfiguration));
+    MEMSET(&configurationAnswer, 0x00, SIZEOF(RtcConfiguration));
+
+    // Create peer connection
+    EXPECT_EQ(createPeerConnection(&configurationOffer, &offerPc), STATUS_SUCCESS);
+    EXPECT_EQ(createPeerConnection(&configurationAnswer, &answerPc), STATUS_SUCCESS);
+
+    sessionDescriptionInitViewer.useTrickleIce = TRUE;
+
+    EXPECT_EQ(STATUS_SUCCESS, createOffer(offerPc, &sessionDescriptionInitViewer));
+    STRCPY(sessionDescriptionInitMaster.sdp, sessionDescriptionInitViewer.sdp);
+    sessionDescriptionInitMaster.type = SDP_TYPE_OFFER;
+    EXPECT_EQ(setRemoteDescription(answerPc, &sessionDescriptionInitMaster), STATUS_SUCCESS);
+    EXPECT_EQ(TRUE, canTrickleIceCandidates(answerPc).value);
+
+    closePeerConnection(offerPc);
+    freePeerConnection(&offerPc);
+
+    closePeerConnection(answerPc);
+    freePeerConnection(&answerPc);
+}
+
+TEST_F(SdpApiTest, noMediaTrickleIceNegativeCase) {
+    PRtcPeerConnection offerPc = NULL;
+    PRtcPeerConnection answerPc = NULL;
+    RtcConfiguration configurationOffer;
+    RtcConfiguration configurationAnswer;
+    RtcSessionDescriptionInit sessionDescriptionInitViewer;
+    RtcSessionDescriptionInit sessionDescriptionInitMaster;
+
+    MEMSET(&configurationOffer, 0x00, SIZEOF(RtcConfiguration));
+    MEMSET(&configurationAnswer, 0x00, SIZEOF(RtcConfiguration));
+
+    // Create peer connection
+    EXPECT_EQ(createPeerConnection(&configurationOffer, &offerPc), STATUS_SUCCESS);
+    EXPECT_EQ(createPeerConnection(&configurationAnswer, &answerPc), STATUS_SUCCESS);
+
+    sessionDescriptionInitViewer.useTrickleIce = FALSE;
+
+    EXPECT_EQ(STATUS_SUCCESS, createOffer(offerPc, &sessionDescriptionInitViewer));
+    STRCPY(sessionDescriptionInitMaster.sdp, sessionDescriptionInitViewer.sdp);
+    sessionDescriptionInitMaster.type = SDP_TYPE_OFFER;
+    EXPECT_EQ(setRemoteDescription(answerPc, &sessionDescriptionInitMaster), STATUS_SUCCESS);
+    EXPECT_EQ(FALSE, canTrickleIceCandidates(answerPc).value);
+
+    closePeerConnection(offerPc);
+    freePeerConnection(&offerPc);
+
+    closePeerConnection(answerPc);
+    freePeerConnection(&answerPc);
+}
+
 TEST_F(SdpApiTest, answerMlinesOrderSameAsOfferMLinesOrder)
 {
     auto offer = std::string(R"(v=0
@@ -2340,4 +2400,4 @@ INSTANTIATE_TEST_SUITE_P(SdpApiTest_SdpMatch_Safari, SdpApiTest_SdpMatch, ::test
 } // namespace video
 } // namespace kinesis
 } // namespace amazonaws
-} // namespace com
+} // namespace com
-Original file line number
+Diff line change
@@ Expand Up @@
     CleanUp:
+        CHK_LOG_ERR(retStatus);
         return retStatus;
     }
@@ Expand Down @@