Repository navigation
[flutter_webrtc] Fix data channel map race, key derivation option, and setParameters encodings - #1208
Open
seungsoo47 wants to merge 3 commits into
Open
seungsoo47 wants to merge 3 commits into
seungsoo47 wants to merge 3 commits into
Conversation
Data channel observers are added to data_channel_observers_ under the base lock, and one of those inserts runs on the WebRTC signaling thread (OnDataChannel). DataChannelClose and DataChannelForId read and erase the map on the platform thread without the lock, which is a data race on std::map. Take the lock around those accesses too. Upstream flutter_webrtc has the same code in common/cpp and fixed it the same way in flutter-webrtc/flutter-webrtc#2194 (8ecd2e526fd1cf6d3ad0fd4fe53696e0c0f210ec, 1.6.2+hotfix.4).
…ovider FrameCryptorFactoryCreateKeyProvider did not read keyDerivationAlgorithm from KeyProviderOptions, so libwebrtc always used PBKDF2 even when HKDF was requested. Parse the option into key_derivation_algorithm. The helper in flutter_data_packet_cryptor.cc named KeyDerivationAlgorithmFromInt actually mapped the cipher algorithm, so it is replaced by AlgorithmFromInt from flutter_frame_cryptor.cc. The mapping is unchanged. Ported from flutter-webrtc/flutter-webrtc#2049 (bcf08738e576f988312e1d7c46b9eff1489daf70).
updateRtpParameters modified the objects returned by RTCRtpParameters::encodings(), but libwebrtc returns copies, so every change to encodings (maxBitrate, maxFramerate, active, ...) was dropped and setParameters still returned true. Write the encodings back with set_encodings(). Once the encodings are written back, two placeholder values that rtpParametersToMap reports for unset fields come back from Dart and are rejected by libwebrtc: an empty scalabilityMode, and ssrc 0 before negotiation. setParameters then returned false for the usual read-modify-write call, so skip these two values. Ported from flutter-webrtc/flutter-webrtc#2211 (db81cea2730ae65b53fd3ffa97f45ad71200344a).
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or 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
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.
Port three fixes from the upstream common C++ implementation:
KeyDerivationAlgorithmoption flutter-webrtc/flutter-webrtc#2049Integration tests are added for the last two fixes. All 13 tests passed 3 times each on RPi4, TV 10.0, and the TV 9.0 / 10.0 emulators.