ReactNative: transceiver.mid is not available until setRemoteDescription is called #264
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I think sendingRtpParameters should also be updated. |
and what exactly needs to be updated do not tell me? |
|
|
you correctly noted. But will it work or should the following actions be rewritten too? it might say @ibc |
ibc
reviewed
Jun 23, 2023
|
Related issue reported in react-native-webrtc project: react-native-webrtc/react-native-webrtc#1404 |
|
Here something I personally hate:
BTW I've reported the issue in react-native-webrtc project: react-native-webrtc/react-native-webrtc#1404 |
ibc
added a commit
that referenced
this pull request
Jun 23, 2023
…tc 111 on iOS (unset transceiver.mid) - Fixes issue #263 - Replaces PR #264 - Issue in react-native-webrtc: react-native-webrtc/react-native-webrtc#1404 ### Details - `ReactNativeWithUnifiedPlan`: Read `transceiver.mid` after calling `pc.setRemoteDescription()` since it's set then (and not before, bug in react-native-webrtc). - Problem of this approach is that we are generating the remote SDP without MID (since we don't know it yet). Not sure about the implications. I'm pretty sure that the new media section in the SDP **does** contain proper `a=mid` (it's just that `transceiver.mid` returns `null`), and in this PR we are calling `setRemoteDescription()` with a remote media section that doesn't contain `a=mid`.
|
Feedback given in this PR makes sense plus this PR fails at lint task. I'm doing another one here: #271 |
ibc
added a commit
that referenced
this pull request
Jun 23, 2023
…tc 111 on iOS (unset transceiver.mid) (#271) - Fixes issue #263 - Replaces PR #264 - Issue in react-native-webrtc: react-native-webrtc/react-native-webrtc#1404 ### Details - `ReactNativeWithUnifiedPlan`: Read `transceiver.mid` after calling `pc.setRemoteDescription()` since it's set then (and not before, bug in react-native-webrtc). - Problem of this approach is that we are generating the remote SDP without MID (since we don't know it yet). Not sure about the implications. I'm pretty sure that the new media section in the SDP **does** contain proper `a=mid` (it's just that `transceiver.mid` returns `null`), and in this PR we are calling `setRemoteDescription()` with a remote media section that doesn't contain `a=mid`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR attempts to fix #263.
The issue is
transciever.midis not available after callingpc.setLocalDescription. Since mediasoup-client usesmidas the key to_mapMidTransceivermap, without a valid mid, future call toproducer.close()finds wrong transceiver to turn off.After enabling react-native-webrtc logs,I noticed that
transeiver.midis properly updated inpc.setRemoteDescription, therefore I added code to update themidafter the call.