fix(llc): fix for multicall audio issues, per-call pc factory#1230
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughPer-call native PeerConnectionFactory support, call-scoped factory caching, audio suspend/resume, speech-activity-driven speaking detection, consolidated thermal monitoring, RTC wiring for native factories, dependency overrides, test updates, and platform plugin/listing adjustments. ChangesPer-Call Native Factory Architecture & Audio Suspension
Speech Activity Detection & Thermal Monitoring
Build Configuration, Dependencies & Testing
🎯 4 (Complex) | ⏱️ ~75 minutes Suggested reviewers:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/stream_video/lib/src/webrtc/rtc_manager.dart (2)
59-75:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't fall back to the global Apple audio policy here.
RtcManagernow carries the per-call factory, butsetAudioOutputDevice()still reapplies_streamVideo.options.audioConfigurationPolicy. On Apple platforms that drops the call-level override as soon as output routing changes or a resumed audio track reapplies the current sink.Suggested fix
Future<Result<None>> setAudioOutputDevice({ required RtcMediaDevice device, }) async { + final policy = + stateManager.callState.preferences.audioConfigurationPolicy ?? + _streamVideo.options.audioConfigurationPolicy; + // Get all remote audio tracks. final audioTracks = tracks.values.whereType<RtcRemoteTrack>().where( (it) => it.trackType == SfuTrackType.audio, ); @@ if (CurrentPlatform.isIos && device.id.equalsIgnoreCase( AudioSettingsRequestDefaultDeviceEnum.speaker.value, )) { await setAppleAudioConfiguration( speakerOn: true, - policy: _streamVideo.options.audioConfigurationPolicy, + policy: policy, ); } else { await setAppleAudioConfiguration( - policy: _streamVideo.options.audioConfigurationPolicy, + policy: policy, ); }Also applies to: 1332-1373
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stream_video/lib/src/webrtc/rtc_manager.dart` around lines 59 - 75, The RtcManager's setAudioOutputDevice currently reapplies the global _streamVideo.options.audioConfigurationPolicy which causes per-call overrides to be lost on Apple platforms; update setAudioOutputDevice (and the analogous logic around lines 1332-1373) to use the call-specific factory's policy instead of the global one by replacing uses of _streamVideo.options.audioConfigurationPolicy with the per-call StreamPeerConnectionFactory's audioConfigurationPolicy (pcFactory.audioConfigurationPolicy) so the call-level audioConfigurationPolicy carried by RtcManager is respected when changing output routing or reapplying sinks.
107-145:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKey the generic SDP cache by factory identity.
getGenericSdp()now depends on the supplied native factory, but_cachedGenericSdpis still keyed only by direction. After the first caller fills the cache, later calls with a different per-call factory or audio policy never regenerate SDP, so negotiation silently reuses the wrong factory’s capabilities.Suggested fix
- static final Map<rtc.TransceiverDirection, String> _cachedGenericSdp = {}; + static final Map<(rtc.TransceiverDirection, String?), String> + _cachedGenericSdp = {}; @@ static Future<String> getGenericSdp( rtc.TransceiverDirection direction, { required rtc.NativePeerConnectionFactory? pcFactory, }) async { + final cacheKey = (direction, pcFactory?.factoryId); + // Check cache first - if (_cachedGenericSdp.containsKey(direction)) { - return _cachedGenericSdp[direction]!; + if (_cachedGenericSdp.containsKey(cacheKey)) { + return _cachedGenericSdp[cacheKey]!; } @@ - _cachedGenericSdp[direction] = sdp; + _cachedGenericSdp[cacheKey] = sdp; return sdp; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stream_video/lib/src/webrtc/rtc_manager.dart` around lines 107 - 145, The cache _cachedGenericSdp is currently keyed only by rtc.TransceiverDirection so getGenericSdp(...) can return an SDP created with a different pcFactory; change the cache key to include the factory identity (e.g., combine direction with a unique id from rtc.NativePeerConnectionFactory or its object hash) and use that composite key when reading/writing _cachedGenericSdp; ensure getGenericSdp still falls back to creating a tempPC when pcFactory is null and that disposal and null‑sdp handling (taggedLogger, return '') remain unchanged.packages/stream_video/lib/src/call/stats/sfu_stats_reporter.dart (1)
99-108:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard timer rescheduling after
stop().If
stop()runs while one of the initial one-shot timers is already executing, that callback still calls_scheduleNextReport()after_stoppedis set. This can recreate_timerand leave a periodic wake-up running forever even thoughsendSfuStats()now early-returns.Suggested fix
void _scheduleNextReport(Duration regularInterval) { + if (_stopped) return; if (_reportCount < _initialReportingDelays.length) { _timer = Timer(_initialReportingDelays[_reportCount], () { + if (_stopped) return; _reportCount++; _invokeSendStats(); + if (_stopped) return; _scheduleNextReport(regularInterval); }); } else { - _timer = Timer.periodic(regularInterval, (_) => _invokeSendStats()); + _timer = Timer.periodic(regularInterval, (_) { + if (_stopped) return; + _invokeSendStats(); + }); } }Also applies to: 317-323
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stream_video/lib/src/call/stats/sfu_stats_reporter.dart` around lines 99 - 108, The _scheduleNextReport logic can reschedule timers even after stop() sets _stopped; modify _scheduleNextReport to check the _stopped flag and bail out before creating any Timer (both one-shot and periodic) and also check _stopped inside the one-shot callback before calling _scheduleNextReport or creating the next timer; ensure _timer is only set when not _stopped and that the one-shot callback increments _reportCount and calls _invokeSendStats only if !_stopped. This change touches _scheduleNextReport, the one-shot Timer callback that references _reportCount and _initialReportingDelays, the periodic Timer creation, and relies on the existing stop()/_stopped behavior to prevent resurrecting timers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/stream_video/CHANGELOG.md`:
- Line 14: Update the changelog to correct the misspelled API name: change
`NoiceCancellationSettingsMode.autoOn` to the correct
`NoiseCancellationSettingsMode.autoOn` in the sentence describing the audio
processor teardown so users can find the API by its accurate identifier.
In
`@packages/stream_video/lib/src/audio_processing/audio_recognition_webrtc.dart`:
- Around line 22-52: The start() subscription only reacts to future events so it
can miss an already-active started state; after subscribing to
RtcMediaDeviceNotifier.instance.speechActivityStream, query the notifier for the
current speech activity (e.g. a current/last event getter on
RtcMediaDeviceNotifier.instance) and if that value is SpeechActivityStarted(),
set _isSpeaking = true, cancel any _trailingSilenceTimer, and invoke
onSoundStateChanged(const SoundState(isSpeaking: true, audioLevel: 1)) so
initial speaking state is seeded; keep the existing handling for
SpeechActivityStarted() and SpeechActivityEnded() intact.
In `@packages/stream_video/lib/src/call/call.dart`:
- Around line 919-947: The guard flag _leaveCallTriggered is being reset in
end()'s finally block which reopens the gate even when _disconnect() returned
false (another teardown is in flight); instead, remove the finally reset from
both end() and leave() and make _disconnect() (or the internal _clear() path it
invokes) responsible for clearing _leaveCallTriggered after it has finished the
full teardown; update the implementations of _disconnect(), _clear(), and any
code paths that currently clear _leaveCallTriggered so that only the code that
actually performs session.leave()/endCall() and cleanup clears the flag, leaving
end(), leave(), and callers to trust _disconnect() to manage the guard.
- Around line 302-335: In suspendAudio, set the CallState flag isAudioSuspended
on _stateManager immediately after checking factory (before the first await) so
new incoming audio tracks see the suspended state; i.e. call _stateManager.state
= _stateManager.callState.copyWith(isAudioSuspended: true) before awaiting
factory.suspendAudio() and before scanning tracks into _suspendedTrackStates;
additionally, if you want to preserve original behavior on errors, wrap
factory.suspendAudio() in try/catch and reset isAudioSuspended to false on
failure.
In `@packages/stream_video/lib/src/call/session/call_session.dart`:
- Around line 865-881: The remote-track handling still unconditionally calls
start in _onTrackPublished (and the alternate resolved-remote branch around the
other publish handling), so add the same suspension guard used in
_onLocalTrackPublished: if the track is an audio track and
stateManager.callState.isAudioSuspended is true, do not call track.start() and
instead invoke onSuspendedAudioTrackRecorded(track.trackId); otherwise proceed
to await track.start() and (for audio) call _applyCurrentAudioOutputDevice().
Ensure you apply this change in both _onTrackPublished and the other
resolved-remote publish handler referenced in the review.
In `@packages/stream_video/lib/src/call/stats/sfu_stats_reporter.dart`:
- Around line 36-43: The code currently only sets _thermalStatus when the
platform emits a transition; seed the current status immediately by reading
_ThermalMonitor.lastStatus and handling it (assign to _thermalStatus and call
_deviceTracer.trace('device.thermalState', status.name) if non-null) before
creating the stream subscription in SfuStatsReporter; then create the
subscription as you already do (_thermalStatusSubscription =
_ThermalMonitor.stream.listen(...)). Apply the same one-time seeding change to
the other similar block (the later occurrence around the 334-355 region) so both
paths mirror the snapshot logic used in StatsReporter.
In `@packages/stream_video/lib/src/core/client_state.dart`:
- Around line 129-140: When setActiveCall receives a call that's already present
in activeCalls, avoid treating it as a new activation: detect existing entries
by callCid, remove that existing entry from activeCalls (to prevent duplicates
and preserve ordering used by removeActiveCall), resume the target call's audio
(call.resumeAudio()) with a try/catch around it (best-effort like suspendAudio),
and then append the call to activeCalls.value; do this short-circuit before the
loop that suspends other calls so you don't leave an already-active call muted
or duplicated.
In
`@packages/stream_video/lib/src/webrtc/rtc_media_device/rtc_media_device_notifier.dart`:
- Around line 32-42: The SpeechActivityEvent types are unscoped singletons and
must carry a source identifier so concurrent detectors/calls can distinguish
events; update SpeechActivityEvent (in rtc_media_device_notifier.dart) to
include an immutable identifier field (e.g., callId or audioModuleId) and make
SpeechActivityStarted and SpeechActivityEnded require and propagate that id,
then update any code subscribing/producing these events (e.g.,
AudioRecognitionWebRTC.start()) to include the appropriate id when emitting so
detectors can filter by their own id. Ensure the identifier is final/const where
appropriate and that equals/hashCode or value comparisons consider the id so
listeners can reliably match events to their session.
In `@packages/stream_video/pubspec.yaml`:
- Around line 44-48: The pubspec currently contains a long-lived
dependency_overrides entry for stream_webrtc_flutter pinned to git ref
e8678886871fb08aa6646aef870a2560d49199ea; remove or gate this override in
packages/stream_video/pubspec.yaml (and other publishable packages) before
release so the declared semver dependency (^2.2.6) is honored. Replace the git
override with the published semver in the package pubspecs used for publishing,
or move the git override into a non-published/dev-only location (e.g., a
local/dev-only override in a root-level pubspec or CI step) and ensure
dependency_overrides: stream_webrtc_flutter is not present in any package
intended for publishing.
---
Outside diff comments:
In `@packages/stream_video/lib/src/call/stats/sfu_stats_reporter.dart`:
- Around line 99-108: The _scheduleNextReport logic can reschedule timers even
after stop() sets _stopped; modify _scheduleNextReport to check the _stopped
flag and bail out before creating any Timer (both one-shot and periodic) and
also check _stopped inside the one-shot callback before calling
_scheduleNextReport or creating the next timer; ensure _timer is only set when
not _stopped and that the one-shot callback increments _reportCount and calls
_invokeSendStats only if !_stopped. This change touches _scheduleNextReport, the
one-shot Timer callback that references _reportCount and
_initialReportingDelays, the periodic Timer creation, and relies on the existing
stop()/_stopped behavior to prevent resurrecting timers.
In `@packages/stream_video/lib/src/webrtc/rtc_manager.dart`:
- Around line 59-75: The RtcManager's setAudioOutputDevice currently reapplies
the global _streamVideo.options.audioConfigurationPolicy which causes per-call
overrides to be lost on Apple platforms; update setAudioOutputDevice (and the
analogous logic around lines 1332-1373) to use the call-specific factory's
policy instead of the global one by replacing uses of
_streamVideo.options.audioConfigurationPolicy with the per-call
StreamPeerConnectionFactory's audioConfigurationPolicy
(pcFactory.audioConfigurationPolicy) so the call-level audioConfigurationPolicy
carried by RtcManager is respected when changing output routing or reapplying
sinks.
- Around line 107-145: The cache _cachedGenericSdp is currently keyed only by
rtc.TransceiverDirection so getGenericSdp(...) can return an SDP created with a
different pcFactory; change the cache key to include the factory identity (e.g.,
combine direction with a unique id from rtc.NativePeerConnectionFactory or its
object hash) and use that composite key when reading/writing _cachedGenericSdp;
ensure getGenericSdp still falls back to creating a tempPC when pcFactory is
null and that disposal and null‑sdp handling (taggedLogger, return '') remain
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a8662d73-da54-4b7a-882b-751a7fa8c39f
⛔ Files ignored due to path filters (2)
packages/stream_video_flutter/test/src/call_screen/call_content/goldens/ci/stream_call_content_extend_body_false.pngis excluded by!**/*.pngpackages/stream_video_flutter/test/src/call_screen/call_content/goldens/ci/stream_call_content_extend_body_true.pngis excluded by!**/*.png
📒 Files selected for processing (33)
dogfooding/ios/Flutter/AppFrameworkInfo.plistdogfooding/lib/screens/call_screen.dartdogfooding/linux/flutter/generated_plugin_registrant.ccdogfooding/linux/flutter/generated_plugins.cmakedogfooding/windows/flutter/generated_plugins.cmakepackages/stream_video/CHANGELOG.mdpackages/stream_video/lib/src/audio_processing/audio_recognition_webrtc.dartpackages/stream_video/lib/src/call/call.dartpackages/stream_video/lib/src/call/session/call_session.dartpackages/stream_video/lib/src/call/session/call_session_factory.dartpackages/stream_video/lib/src/call/stats/sfu_stats_reporter.dartpackages/stream_video/lib/src/call_state.dartpackages/stream_video/lib/src/core/client_state.dartpackages/stream_video/lib/src/models/call_preferences.dartpackages/stream_video/lib/src/stream_video.dartpackages/stream_video/lib/src/webrtc/media/media_constraints.dartpackages/stream_video/lib/src/webrtc/peer_connection_factory.dartpackages/stream_video/lib/src/webrtc/rtc_manager.dartpackages/stream_video/lib/src/webrtc/rtc_manager_factory.dartpackages/stream_video/lib/src/webrtc/rtc_media_device/rtc_media_device_notifier.dartpackages/stream_video/lib/src/webrtc/rtc_track/rtc_local_track.dartpackages/stream_video/pubspec.yamlpackages/stream_video/test/src/call/call_allow_multiple_active_calls_test.dartpackages/stream_video/test/src/call/call_apply_settings_test.dartpackages/stream_video/test/src/call/fixtures/call_test_helpers.dartpackages/stream_video/test/src/core/client_state_test.dartpackages/stream_video_filters/pubspec.yamlpackages/stream_video_flutter/example/linux/flutter/generated_plugins.cmakepackages/stream_video_flutter/example/windows/flutter/generated_plugins.cmakepackages/stream_video_flutter/lib/src/call_screen/lobby_video.dartpackages/stream_video_flutter/pubspec.yamlpackages/stream_video_noise_cancellation/pubspec.yamlpackages/stream_video_push_notification/pubspec.yaml
💤 Files with no reviewable changes (6)
- packages/stream_video/test/src/call/call_apply_settings_test.dart
- dogfooding/linux/flutter/generated_plugin_registrant.cc
- dogfooding/ios/Flutter/AppFrameworkInfo.plist
- packages/stream_video/test/src/call/call_allow_multiple_active_calls_test.dart
- packages/stream_video/test/src/core/client_state_test.dart
- packages/stream_video/lib/src/stream_video.dart
|
|
||
| - Fixed sibling-call audio capture being silently broken when another concurrently-active call ended (e.g. a 1:1 ringing call ending alongside a running livestream, or a previous ringing call ending before a new one was accepted). | ||
| - Fixed a sibling call's audio breaking when a ringing 1:1 call ended via `dropIfAloneInRingingFlow` (the remote party hung up first). `Call.end()` and `Call.leave()` now share a single `_disconnect` cleanup path. | ||
| - Made the audio processor teardown in `Call._clear` multi-call aware. The audio processor is owned by `StreamVideo`, not by an individual `Call`, so disabling it on one call's teardown silently dropped noise cancellation on any other still-active call. `_clear` now only stops the global processor when no other active call is configured to use `NoiceCancellationSettingsMode.autoOn`. |
There was a problem hiding this comment.
Fix incorrect API name spelling in changelog.
Line 14 uses NoiceCancellationSettingsMode.autoOn, which appears to be a typo and can mislead users when searching the API.
📝 Suggested fix
- - Made the audio processor teardown in `Call._clear` multi-call aware. The audio processor is owned by `StreamVideo`, not by an individual `Call`, so disabling it on one call's teardown silently dropped noise cancellation on any other still-active call. `_clear` now only stops the global processor when no other active call is configured to use `NoiceCancellationSettingsMode.autoOn`.
+ - Made the audio processor teardown in `Call._clear` multi-call aware. The audio processor is owned by `StreamVideo`, not by an individual `Call`, so disabling it on one call's teardown silently dropped noise cancellation on any other still-active call. `_clear` now only stops the global processor when no other active call is configured to use `NoiseCancellationSettingsMode.autoOn`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Made the audio processor teardown in `Call._clear` multi-call aware. The audio processor is owned by `StreamVideo`, not by an individual `Call`, so disabling it on one call's teardown silently dropped noise cancellation on any other still-active call. `_clear` now only stops the global processor when no other active call is configured to use `NoiceCancellationSettingsMode.autoOn`. | |
| - Made the audio processor teardown in `Call._clear` multi-call aware. The audio processor is owned by `StreamVideo`, not by an individual `Call`, so disabling it on one call's teardown silently dropped noise cancellation on any other still-active call. `_clear` now only stops the global processor when no other active call is configured to use `NoiseCancellationSettingsMode.autoOn`. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/stream_video/CHANGELOG.md` at line 14, Update the changelog to
correct the misspelled API name: change `NoiceCancellationSettingsMode.autoOn`
to the correct `NoiseCancellationSettingsMode.autoOn` in the sentence describing
the audio processor teardown so users can find the API by its accurate
identifier.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/stream_video/lib/src/call/call.dart`:
- Around line 309-320: The code clears _suspendedTrackStates unconditionally
before calling factory.suspendAudio(), which makes suspendAudio()
non-idempotent; change suspendAudio() so it only captures the current tracks'
enabled state into _suspendedTrackStates if that snapshot is empty (or if the
call isn't already in a suspended state), and avoid overwriting an existing
snapshot on repeated suspendAudio() calls; set
_stateManager.state.isAudioSuspended only when you perform the initial snapshot,
call await factory.suspendAudio(), and on error only clear the snapshot/state if
this invocation created it (i.e., revert only if you altered
_suspendedTrackStates/_stateManager in this call); reference suspendAudio(),
_suspendedTrackStates, _stateManager.state, and resumeAudio() when making these
changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 36ece14a-a8c0-4a1c-8e77-a656e85865f2
📒 Files selected for processing (1)
packages/stream_video/lib/src/call/call.dart
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stream_video/lib/src/call/call.dart (1)
302-365:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSerialize
suspendAudio()andresumeAudio().These two methods still mutate
_suspendedTrackStates,isAudioSuspended, and the native factory without a shared lock. IfresumeAudio()runs whilesuspendAudio()is insideawait factory.suspendAudio(),resumeAudio()can clear the snapshot and flip the state back to false, then the first call disables tracks afterward. That leaves audio suspended while the call state says it is resumed.🔒 Proposed fix
@@ late final _callReconnectLock = Lock(); late final _callClosedCaptionsLock = Lock(); late final _multitaskingCameraLock = Lock(); + late final _audioSuspendLock = Lock(); @@ - Future<void> suspendAudio() async { - final factory = _pcFactory; - if (factory == null) { - _logger.w(() => '[suspendAudio] no factory yet'); - return; - } - - if (_stateManager.callState.isAudioSuspended) { - _logger.d(() => '[suspendAudio] already suspended'); - return; - } - - _suspendedTrackStates.clear(); - _stateManager.state = _stateManager.callState.copyWith( - isAudioSuspended: true, - ); - - try { - await factory.suspendAudio(); - } catch (e, stk) { - _logger.e(() => '[suspendAudio] native suspend failed: $e\n$stk'); - _suspendedTrackStates.clear(); - _stateManager.state = _stateManager.callState.copyWith( - isAudioSuspended: false, - ); - return; - } - - final tracks = _session?.rtcManager?.tracks; - if (tracks != null) { - for (final entry in tracks.entries) { - final track = entry.value; - if (track.isAudioTrack) { - final wasEnabled = track.mediaTrack.enabled; - _suspendedTrackStates[entry.key] = wasEnabled - ? SuspendedTrackState.wasEnabled - : SuspendedTrackState.wasDisabled; - - track.disable(); - - _logger.d( - () => - '[suspendAudio] disabled track ${entry.key} ' - '(was enabled: $wasEnabled)', - ); - } - } - } - } + Future<void> suspendAudio() { + return _audioSuspendLock.synchronized(() async { + final factory = _pcFactory; + if (factory == null) { + _logger.w(() => '[suspendAudio] no factory yet'); + return; + } + + if (_stateManager.callState.isAudioSuspended) { + _logger.d(() => '[suspendAudio] already suspended'); + return; + } + + _suspendedTrackStates.clear(); + _stateManager.state = _stateManager.callState.copyWith( + isAudioSuspended: true, + ); + + try { + await factory.suspendAudio(); + } catch (e, stk) { + _logger.e(() => '[suspendAudio] native suspend failed: $e\n$stk'); + _suspendedTrackStates.clear(); + _stateManager.state = _stateManager.callState.copyWith( + isAudioSuspended: false, + ); + return; + } + + final tracks = _session?.rtcManager?.tracks; + if (tracks != null) { + for (final entry in tracks.entries) { + final track = entry.value; + if (track.isAudioTrack) { + final wasEnabled = track.mediaTrack.enabled; + _suspendedTrackStates[entry.key] = wasEnabled + ? SuspendedTrackState.wasEnabled + : SuspendedTrackState.wasDisabled; + track.disable(); + } + } + } + }); + } @@ - Future<void> resumeAudio() async { - final factory = _pcFactory; - if (factory == null) { - _logger.w(() => '[resumeAudio] no factory yet'); - return; - } - await factory.resumeAudio(); - - await _session?.resumeSuspendedAudioTracks(_suspendedTrackStates); - _suspendedTrackStates.clear(); - - _stateManager.state = _stateManager.callState.copyWith( - isAudioSuspended: false, - ); - } + Future<void> resumeAudio() { + return _audioSuspendLock.synchronized(() async { + final factory = _pcFactory; + if (factory == null) { + _logger.w(() => '[resumeAudio] no factory yet'); + return; + } + + await factory.resumeAudio(); + await _session?.resumeSuspendedAudioTracks(_suspendedTrackStates); + _suspendedTrackStates.clear(); + _stateManager.state = _stateManager.callState.copyWith( + isAudioSuspended: false, + ); + }); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stream_video/lib/src/call/call.dart` around lines 302 - 365, suspendAudio() and resumeAudio() race because they mutate _suspendedTrackStates and _stateManager and call the native factory without synchronization; wrap both methods' critical sections in a shared mutex/lock so only one of suspendAudio or resumeAudio runs at a time (acquire the lock at the start, perform checks, call factory.suspendAudio()/factory.resumeAudio(), update _suspendedTrackStates, call _session?.resumeSuspendedAudioTracks, and update _stateManager, then release the lock) to ensure the snapshot and state flips cannot be interleaved.
🧹 Nitpick comments (1)
melos.yaml (1)
25-25: Confirmstream_webrtc_flutterpinning is already in package manifests
- The removed/commented
stream_webrtc_flutter: ^2.2.6inmelos.yamldoesn’t leave the workspace unpinned:stream_video,stream_video_flutter,stream_video_filters,stream_video_noise_cancellation, andstream_video_push_notificationall declarestream_webrtc_flutter: ^2.2.6and include adependency_overridesgit pin tohttps://github.com/GetStream/webrtc-flutter.gitwithref: e8678886871fb08aa6646aef870a2560d49199ea.packages/stream_video_flutter/example/pubspec.yamllacks the gitdependency_overridesforstream_webrtc_flutter(it only has^2.2.6), so add the same override there if the example must use the exact git-pinned source.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@melos.yaml` at line 25, The example package pubspec for stream_video_flutter currently only lists stream_webrtc_flutter: ^2.2.6 and lacks the git dependency_overrides used elsewhere; update the stream_video_flutter example pubspec.yaml to add a dependency_overrides entry for stream_webrtc_flutter pointing to the same git repo and commit ref (https://github.com/GetStream/webrtc-flutter.git with ref e8678886871fb08aa6646aef870a2560d49199ea) so the example uses the exact git-pinned source consistent with stream_video, stream_video_filters, stream_video_noise_cancellation, and stream_video_push_notification.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/stream_video/lib/src/call/call.dart`:
- Around line 302-365: suspendAudio() and resumeAudio() race because they mutate
_suspendedTrackStates and _stateManager and call the native factory without
synchronization; wrap both methods' critical sections in a shared mutex/lock so
only one of suspendAudio or resumeAudio runs at a time (acquire the lock at the
start, perform checks, call factory.suspendAudio()/factory.resumeAudio(), update
_suspendedTrackStates, call _session?.resumeSuspendedAudioTracks, and update
_stateManager, then release the lock) to ensure the snapshot and state flips
cannot be interleaved.
---
Nitpick comments:
In `@melos.yaml`:
- Line 25: The example package pubspec for stream_video_flutter currently only
lists stream_webrtc_flutter: ^2.2.6 and lacks the git dependency_overrides used
elsewhere; update the stream_video_flutter example pubspec.yaml to add a
dependency_overrides entry for stream_webrtc_flutter pointing to the same git
repo and commit ref (https://github.com/GetStream/webrtc-flutter.git with ref
e8678886871fb08aa6646aef870a2560d49199ea) so the example uses the exact
git-pinned source consistent with stream_video, stream_video_filters,
stream_video_noise_cancellation, and stream_video_push_notification.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: de9f8d20-5990-4044-ae24-eb218847afd4
📒 Files selected for processing (2)
melos.yamlpackages/stream_video/lib/src/call/call.dart
…-video-flutter into fix/multicall-audio-fix # Conflicts: # dogfooding/android/gradle.properties
180b03f to
8d5f069
Compare
…-video-flutter into fix/multicall-audio-fix
Summary by CodeRabbit
New Features
Improvements
Chores