[BUG] addTransceiver does not handle PRtcMediaStreamTrack nullability

[max-khm]

Description
The Doxygen documentation of addTransceiver states this about PRtcMediaStreamTrack parameter:

[in] PRtcMediaStreamTrack Stream track information for the codec appropriate codec, or NULL for RECVONLY

Therefore, for adding RECVONLY transceiver I wrote:

CHK_STATUS(addTransceiver(connection, NULL /* receive only */,
                              &(RtcRtpTransceiverInit){.direction = RTC_RTP_TRANSCEIVER_DIRECTION_RECVONLY},
                              &audio_transceiver));

However, the code above fails during runtime because of trying to member-access that NULL parameter, causing segmentation fault here:

amazon-kinesis-video-streams-webrtc-sdk-c/src/source/PeerConnection/PeerConnection.c

Line 1207 in 4127014

switch (pRtcMediaStreamTrack->codec) {

SDK version number
Current version on master branch, HEAD at 4127014

Expected behavior
Receive-only transceiver being added correctly, as it's stated in documentation.

Desktop:

• OS: Ubuntu 20.04.3, GCC/9.3.0 Linux/5.11.0-40-generic x86_64

[niyatim23]

Thank you for bringing this up. Does setting PRtcMediaStreamTrack to a valid stream track work?

[max-khm]

Thank you for bringing this up. Does setting PRtcMediaStreamTrack to a valid stream track work?

Yes, it works fine if the parameter is set, for example like this:

CHK_STATUS(addTransceiver(
        connection, &(RtcMediaStreamTrack){.codec = RTC_CODEC_OPUS, .kind = MEDIA_STREAM_TRACK_KIND_AUDIO},
        &(RtcRtpTransceiverInit){.direction = RTC_RTP_TRANSCEIVER_DIRECTION_RECVONLY}, &self->audio_transceiver));

Additional observations:
The above snippet produces SDP answer with a=recvonly given that a=sendonly track was offered.
If a=sendrecv track is offered, the answer will be a=sendrecv as well, even is transceiver direction is set to RECVONLY

[hassanctech]

Are you using the C SDK for both the Viewer and the Master? If not can you please clarify which SDKs you're using for viewer and master. Since you're wanting to use recvonly I'm assuming that's the viewer, so how about the master side is it using this same SDK?

In the C SDK if the offer is recvonly the answer will be sendonly and if the offer is sendrecv than the response will be sendrecv. This handles the direction setting logic for both the offer (viewer) and answer (master) side: https://github.com/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/blob/master/src/source/PeerConnection/SessionDescription.c#L516-L548

Is anything unexpected there?

[hassanctech]

In case you're using the JS SDK the master then the direction being set to sendrecv in the answer produced by the SDK is expected (on certain browsers) when the offer (C SDK) direction is set to recvonly. The reason for this is this code in our JS SDK:

https://github.com/awslabs/amazon-kinesis-video-streams-webrtc-sdk-js/blob/e6d3be4751d7afadd5e43780aee76c3fa9d096ab/examples/master.js#L181-L182

However certain browsers like Safari (last time I checked) completely ignore this, however last I checked in FF and Chrome it will respect that parameter so the answer direction WILL be sendrecv even if the offer direction was recvonly. Hope that makes sense.

[max-khm]

Are you using the C SDK for both the Viewer and the Master? If not can you please clarify which SDKs you're using for viewer and master. Since you're wanting to use recvonly I'm assuming that's the viewer, so how about the master side is it using this same SDK?

Yes, I'm using C SDK for both sides.

Is anything unexpected there?

No, there isn't, that makes sense. I left those observation in my previous comment just to make sure this behavior is intentional.
Thank you for shedding more light on this topic!

[disa6302]

Closing since the PR is merged.