CF SDK: Negotiate callee media based on the Remote SDP #1125
Conversation
🦋 Changeset detectedLatest commit: 02d1cf2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
packages/js/src/fabric/WSClient.ts
Outdated
| const call = this.wsClient.makeCallFabricObject({ | ||
| audio: params.audio ?? true, | ||
| video: params.video ?? video, | ||
| negotiateAudio: params.negotiateAudio ?? true, |
There was a problem hiding this comment.
The remote SDP needs to be considered for audio, too.
There was a problem hiding this comment.
Makes sense to me, thanks! 👍 I am gonna update this.
packages/js/src/fabric/WSClient.ts
Outdated
| audio: params.audio ?? true, | ||
| video: params.video ?? video, | ||
| negotiateAudio: params.negotiateAudio ?? true, | ||
| negotiateVideo: params.negotiateVideo ?? negotiateVideo, |
There was a problem hiding this comment.
The developer shouldn't be allowed to override the SDP terms.
| } | ||
|
|
||
| const call = this.wsClient.makeCallFabricObject({ | ||
| audio: params.audio ?? audio, |
There was a problem hiding this comment.
ATM, we can't let the params take precedence or we will get the onnegotiationneeded loop.
There was a problem hiding this comment.
Ummm... this is a self user's outgoing audio/video stream. This does not affect the SDP. It's just the local stream.
The negotiate params are below. Am I missing something?
There was a problem hiding this comment.
Yes, exactly, but if the remote SDP doesn't accept video or audio we don't want to:
a) waste resources opening the unsupported media in for the localStream
b) add an unsupported media track to the RTPPeerConnection (to avoid the onn loop)
The final SDP negotiation depends on whether the localStream has the media tracks or whether we decide to negotiate the media even without the tracks.
But in cases where the remote SDP doesn't support a type of media, we don't want to force a renegotiation.
There was a problem hiding this comment.
That's what I am trying to understand; the negotiate params used in the SDP for the media negotiation are the below two params, or not?
Also, if we do not accept the audio and video params from the developer, would that be a good user experience? Considering that the callee may want to answer the audio-only call with his video open.
Maybe we should reconsider and try to reproduce the loopback scenario to understand which parameters affect the SDP.
There was a problem hiding this comment.
SDP for the media negotiation are the below two params, or not?
It's both the local media (influenced by video and audio params) and negotiateVideo, negotiatedAudio.
I agree it isn't the best user experience, but since the SDK and the backend don't support the required renegotiation ATM. We have no option but to ignore the parameter in that case.
You can reproduce the issue by creating an audio-only call to a subscriber and having the App to set video:true when answering.
There was a problem hiding this comment.
At the moment, I am unable to reproduce any of the above. It seems no matter what I pass while dialing a call, the server always sends the remote SDP to the callee including both video and audio. As far as a local video is concerned, the SDK opens the camera based on the video param.
Let's put it on hold, we might want to discuss this with the RTC.
Description
The CF SDK handles the Caller user media based on the destination address channel. This PR will now handle the Callee user media based on the incoming/remote SDP.
Additional changes;
getMediaConstraintsfunction to remove the dependency and keep it a generic method.Type of change
Code snippets
No changes are required from the client side.