[camera-app] Support WebRTC Trickle ICE #41381
Closed
Conversation
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
There was a problem hiding this comment.
Pull Request Overview
This PR implements Trickle ICE support for the camera app's WebRTC functionality, allowing ICE candidates to be sent immediately as they're discovered rather than waiting to batch them all together.
Key changes:
- Added callback mechanism for immediate ICE candidate handling
- Implemented state tracking to distinguish between initial ICE candidate batches and subsequent trickle updates
- Enhanced the WebRTC provider manager to handle trickle ICE candidates by reusing existing send mechanisms
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| examples/camera-app/linux/src/webrtc-transport.cpp | Added trickle ICE logic with state tracking and immediate candidate sending |
| examples/camera-app/linux/src/clusters/webrtc-provider/webrtc-provider-manager.cpp | Updated callback registration and added trickle ICE candidate handler |
| examples/camera-app/linux/include/webrtc-transport.h | Added new callback type and member variables for trickle ICE support |
| examples/camera-app/linux/include/clusters/webrtc-provider/webrtc-provider-manager.h | Added OnTrickleICECandidate callback declaration |
There was a problem hiding this comment.
Code Review
This pull request adds support for Trickle ICE, allowing ICE candidates to be sent as they are discovered after the initial exchange. The implementation correctly hooks into the WebRTC stack to receive new candidates and triggers sending them.
I've identified a couple of areas for improvement. The most significant one is a potential inefficiency where all previously gathered ICE candidates are resent with each new trickle candidate. I've detailed this in a specific comment. Additionally, I've found some unused public methods that could be removed to clean up the API.
Overall, the changes are well-structured and address the goal of supporting Trickle ICE. Addressing the feedback will make the implementation more efficient and maintainable.
f5b5085 to
013d0e3
Compare
|
PR #41381: Size comparison from e2f86ee to c9270b4 Full report (34 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #41381 +/- ##
==========================================
- Coverage 51.04% 50.94% -0.10%
==========================================
Files 1378 1378
Lines 100663 100698 +35
Branches 13014 13058 +44
==========================================
- Hits 51383 51302 -81
- Misses 49280 49396 +116 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
c9270b4 to
15e379e
Compare
|
PR #41381: Size comparison from e2f86ee to 15e379e Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
marktrayer
approved these changes
Oct 10, 2025
|
PR #41381: Size comparison from 876fcf5 to 1099cea Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
1099cea to
b73c542
Compare
|
PR #41381: Size comparison from 4523f7f to b73c542 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
pidarped
approved these changes
Oct 11, 2025
chulspro
approved these changes
Oct 11, 2025
ab00289 to
a86ed79
Compare
|
Currently, the TC_WEBRTC_1X tests expect ICE commands to occur in a specific order. We need to update those test cases to remove this restriction before we can land the change. |
|
PR #41381: Size comparison from 966e378 to a86ed79 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Summary
Currently, the device gathers the ICECandidates from the underlying WebRTC stack and sends once after the initial Offer/Answer interchange via a ProvideICECandidates() command.
However, there could be more ICE candidates that can trickle up from the WebRTC stack after the initial send. These would get collected in a member list. But would also need to be sent at some point.
A simple approach could be to hook up the up-call from the WebRTC stack to send the ICECandidate right away after the initial list has been dispatched(post Offer/Answer flow). This might be the preferred approach for Livestream setup instead of waiting to accumulate a minimum set of ICE candidates.
Related issues
Fixes: #39232
Testing
Validate by CI
Readability checklist
The checklist below will help the reviewer finish PR review in time and keep the
code readable:
descriptive
“When in Rome…”
rule (coding style)
See: Pull Request Guidelines