fix: improve processor handling - #859
Conversation
This allows external users to e.g. apply rounded edges. Without this patch, VideoTrackRenderer will expand to its available space with no way for parent widgets to know how large the video actually is
otherwise unblurred video is visible for a few frames
|
I'll try to get the tests to pass later today, but they don't seem to be related to my changes |
|
I checked that the tests run through locally, all the other breaking stuff should be unrelated to my PR. Does anyone know what's up with dart formatter? On some of my repos it keeps finding stuff to format without any other changes 🤷 |
📝 WalkthroughWalkthroughChanges enhance track processing infrastructure by introducing asynchronous processor lifecycle management, nullable track restoration in setProcessedTrack, and expanding camera capture options with focus and exposure mode parameters. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (6)
✏️ Tip: You can disable this entire section by setting Comment |
|
@cloudwebrtc @hiroshihorie do you have feedback on this PR? |
|
That seems really cool, especially the background blur stuff etc. Hope this gets some feedback and might be merged. |
|
@holzgeist can you fix conflicts, pls? As I understend, you variant it one of the best solution |
|
@cloudwebrtc who could check it? Could you please? |
|
It looks like #1038 (merged and released) and #1039 (draft) contain almost identical copies of this PR. Only difference I could spot in a quick glance is the way how the track is updated in the mediaStream. Also #1014 (merged and released) was added after this PR although it uses a different approach. Is there a reason for not iterating on this PR but create new ones? On the bright side, it looks like processor support is improving for LiveKit's Flutter SDK 🎥 |
|
@cloudwebrtc @hiroshihorie any updates on this? |
…ivekit#1106) ## Summary - Align Flutter default local-video degradation behavior with the Swift SDK. - Default unset `VideoPublishOptions.degradationPreference` to `maintainResolution` for camera and screen-share publishing. - Keep explicit degradation preferences overrideable by apps. ## Context Related to livekit#1097, which explores preserving video quality through a live-streaming option. This PR takes the smaller SDK-default approach instead: use maintain-resolution by default, matching Swift, without adding a separate app-facing toggle for this behavior. ## Testing - `dart analyze` - `flutter test test/core/room_e2e_test.dart`
## Summary Migrates the Android plugin to AGP 9's built-in Kotlin while keeping older toolchains building (same pattern as flutter-webrtc/flutter-webrtc#2075): - Apply the **Kotlin Gradle Plugin only when built-in Kotlin is inactive** — AGP < 9, or AGP 9 with `android.builtInKotlin=false` (the configuration Flutter currently ships by default while the ecosystem migrates). When AGP 9's built-in Kotlin is active it registers the `kotlin` extension itself and rejects KGP, so applying it is skipped. - Set the JVM target through the `kotlin { compilerOptions {} }` DSL **when the extension supports it** (KGP 1.9+ / AGP 9 built-in Kotlin), falling back to the legacy `kotlinOptions` DSL for apps still on KGP 1.8.x. - Bump the standalone buildscript fallback KGP to 2.1.0 so it is self-consistent. - Add a changeset entry for the generated release notes. ## Context AGP 9 uses built-in Kotlin support and rejects Android plugins that still apply KGP directly. This follows the Flutter compatibility migration path instead of raising the minimum supported toolchain. ## Verification - Example app builds (`flutter build apk --debug`) on the current stable toolchain (AGP 8.x + modern KGP path). - The AGP 9 built-in path mirrors the reviewed and merged flutter-webrtc implementation.
## Summary - Add `deployment` field to `RoomAgentDispatch` for targeting specific agent deployments - Add `agentDeployment` to `TokenRequestOptions` to pass deployment through token requests - Update generated JSON serialization code The `deployment` field allows targeting a specific agent deployment (e.g., "staging"). Leave empty to target the production deployment. Related PRs: - node-sdks: livekit/node-sdks#675 - python-sdks: livekit/python-sdks#722 - rust-sdks: livekit/rust-sdks#1176 - client-sdk-swift: livekit/client-sdk-swift#1043 - client-sdk-js: livekit/client-sdk-js#1971 ## Usage ```dart final options = TokenRequestOptions( roomName: 'my-room', agentName: 'my-agent', agentDeployment: 'staging', // Optional: target specific deployment ); ``` Or directly via `RoomAgentDispatch`: ```dart final dispatch = RoomAgentDispatch( agentName: 'my-agent', metadata: 'my-metadata', deployment: 'staging', ); ``` ## Test plan ### Unit Tests ```bash flutter test flutter test test/token/token_source_test.dart -v ``` ### Manual Verification **1. Verify JSON serialization includes deployment:** ```dart final dispatch = RoomAgentDispatch( agentName: 'my-agent', deployment: 'staging', ); final json = dispatch.toJson(); print(json); // Should include 'deployment': 'staging' ``` **2. Verify TokenRequestOptions converts to request correctly:** ```dart final options = TokenRequestOptions( roomName: 'test-room', agentName: 'my-agent', agentDeployment: 'staging', ); final request = options.toRequest(); print(request.roomConfiguration?.agents?.first?.deployment); // Should print 'staging' ``` **3. Verify JSON round-trip:** ```dart final original = RoomAgentDispatch( agentName: 'my-agent', deployment: 'staging', ); final json = original.toJson(); final restored = RoomAgentDispatch.fromJson(json); assert(restored.deployment == 'staging'); ``` ### End-to-End Verification 1. Use TokenSource to get credentials with agentDeployment set 2. Connect to room - agent with matching deployment should join 3. Verify only staging agent receives the dispatch 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| // force re-setting of the mediaStreamTrack on the sender | ||
| //await this.setMediaStreamTrack(this._mediaStreamTrack, true); | ||
|
|
||
| await setProcessedTrack(null); |
There was a problem hiding this comment.
🟡 Stopping a camera with an active effect leaves the track pointing at a dead video source
When a local track is stopped, the newly added restore step (setProcessedTrack(null) at lib/src/track/local/local.dart:382) silently does nothing because the saved original camera source was already discarded moments earlier, so the track keeps referring to the destroyed processed source.
Impact: After stopping a track that had a processor, the track still exposes the dead processed source instead of the real camera source, so anything that inspects or re-uses the track afterwards sees an unusable source.
Why the reset branch never runs during stop()
LocalTrack.stop() (lib/src/track/local/local.dart:217-241) first calls super.stop(), and Track.stop() (lib/src/track/track.dart:139-142) stops _originalTrack and sets it to null while _mediaStreamTrack still holds the processed track. stopProcessor() then calls setProcessedTrack(null), whose reset branch is guarded by else if (_originalTrack != null) (lib/src/track/track.dart:236) — already null — so _mediaStreamTrack is never restored. Only the event at lib/src/track/track.dart:243-246 is emitted.
A fix would be to restore the original track before/inside Track.stop() (e.g. call the processor teardown before clearing _originalTrack), or make stop() reset _mediaStreamTrack = _originalTrack when it clears it.
Prompt for agents
In lib/src/track/track.dart, Track.stop() stops and nulls _originalTrack while _mediaStreamTrack still holds the processor output. The new reset logic added in LocalTrack.stopProcessor() (await setProcessedTrack(null)) is therefore a no-op during stop, leaving _mediaStreamTrack pointing at the destroyed processed track. Consider ordering the processor teardown/restore before Track.stop() clears _originalTrack, or having Track.stop() restore _mediaStreamTrack = _originalTrack when it stops and clears the original.
Was this helpful? React with 👍 or 👎 to provide feedback.
| events.emit(TrackStreamUpdatedEvent( | ||
| track: this, | ||
| stream: _mediaStream, | ||
| )); |
There was a problem hiding this comment.
🟡 Video widgets are told to re-attach to a media stream that was just destroyed
Stopping a local track that uses a processor announces a stream update (setProcessedTrack(null) at lib/src/track/local/local.dart:382) after the underlying media stream has already been released, so the video widget tries to re-attach to a destroyed stream.
Impact: Video renderers may log platform errors or throw unhandled async errors when a camera with an effect is stopped.
Ordering of dispose and the new event
LocalTrack.stop() disposes the media stream (lib/src/track/local/local.dart:227) and only afterwards calls stopProcessor(), which now ends with setProcessedTrack(null); that method unconditionally emits TrackStreamUpdatedEvent(stream: _mediaStream) (lib/src/track/track.dart:243-246) with the disposed stream. VideoTrackRenderer reacts by assigning _renderer?.srcObject = event.stream (lib/src/widgets/video_track_renderer.dart:219-222).
Emitting only when something actually changed (i.e. inside the two branches of setProcessedTrack), or calling stopProcessor() before disposing the stream, avoids this.
Prompt for agents
setProcessedTrack in lib/src/track/track.dart always emits TrackStreamUpdatedEvent, even when neither branch changed anything (e.g. reset requested while _originalTrack is already null). Combined with LocalTrack.stop() disposing mediaStream before calling stopProcessor(), listeners such as VideoTrackRenderer receive an event carrying an already-disposed stream and re-assign it as srcObject. Consider emitting the event only when the track/stream actually changed, and/or calling stopProcessor() before disposing the media stream in LocalTrack.stop().
Was this helpful? React with 👍 or 👎 to provide feedback.
| void setProcessedTrack(rtc.MediaStreamTrack track) { | ||
| _originalTrack = _mediaStreamTrack; | ||
| _mediaStreamTrack = track; | ||
| Future<void> setProcessedTrack(rtc.MediaStreamTrack? track) async { |
There was a problem hiding this comment.
🟡 Release notes entry is missing for a user-visible change
The pull request changes published SDK behaviour and the signature of a track method but adds no changeset file under .changes/, which the repository requires for every PR affecting the published package.
Impact: The release automation check fails and the fix will not show up in the changelog.
Repository rule
AGENTS.md: "Every PR that affects the published package needs a changeset file in .changes/ (format: patch|minor|major type=\"fixed|added|changed|...\" \"description\"); CI checks for it and runs dart-apitool against main to require a major changeset for breaking public-API changes." The .changes/ directory is currently empty, while Track.setProcessedTrack changed from void setProcessedTrack(rtc.MediaStreamTrack track) to Future<void> setProcessedTrack(rtc.MediaStreamTrack? track) — an API signature change.
Was this helpful? React with 👍 or 👎 to provide feedback.
6 participants
This PR improves handling of processors:
copyWithwhen copying camera capture optionsTrackStreamUpdatedEventon processor changes to allow widgets to update accordinglyI'm currently working on a federated plugin to add support for virtual backgrounds and blurring on web, iOS and Android. This PR contains required changes/bug fixes I found during development
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.