Make AdmProxy worker-thread-affine#1212
Open
hiroshihorie wants to merge 7 commits into
Open
Conversation
Platform ADM implementations (Windows CoreAudio2, iOS, the upcoming Apple AudioEngine device) bind a sequence checker to their construction thread and expect every control call on it. Previously AdmProxy called the platform ADM directly on whatever thread the caller was on, guarded only by a mutex, which violates that contract and races with WebRTC's own worker-thread calls. AdmProxy now owns all of its state on the worker thread. Every public method marshals once at the boundary via RunOnWorker (inline when the caller is already the worker, which covers all WebRTC-internal calls) and mode transitions execute as plain sequential worker code. This removes the mutex entirely along with the races it couldn't prevent: stale mode snapshots interleaving transitions, and the synthetic ADM being driven from caller threads. The proxy must be constructed on the worker thread (the factory already does this) so the eagerly created platform ADM binds to it. Destruction may happen on any thread and hops to the worker for teardown.
On Android the platform ADM is created lazily on first acquire, after WebRTC has already registered its audio transport with the proxy and after the app may have selected devices. Pass the stored transport and selection along to the newly created ADM. Previously the transport was never registered, so platform recording delivered no audio to the pipeline.
Rust fetches the controller per call, so a controller copy can briefly be the last reference to the AdmProxy on a Rust thread during factory teardown. The proxy marshals onto the runtime's worker thread, so the runtime must outlive every reference to the proxy that Rust can reach. Follows the existing pattern used by AudioTrack and VideoTrack.
The doc described a lazy create/terminate-at-ref-0 lifecycle and an in-proxy stub playout task that no longer exist. Document the actual design: eager platform ADM creation kept until proxy destruction, SyntheticAudioDevice as a separate sub ADM, and the worker-thread-affine threading model.
The selected device GUIDs were captured on every SetPlayoutDevice and SetRecordingDevice call but never read anywhere. They were meant for restoring selection across ADM re-creation, which no longer happens, and the one re-creation path left (Android lazy creation) re-applies by index. Keep only the indices and document the index 0 caveat.
Contributor
ChangesetThe following package versions will be affected by this PR:
|
Demonstrates the PlatformAudio API and doubles as an offline regression check for the worker-thread-affine AdmProxy: lifecycle and full runtime teardown/reacquire cycles, device enumeration/selection/hot-swap, recording on real hardware, audio processing configuration, and heavy concurrent access from many threads. Runs without a LiveKit server, needs audio hardware and mic permission so it lives as an example rather than a CI test.
hiroshihorie
force-pushed
the
hiroshi/adm-proxy-worker-thread
branch
from
d6c7b0a to
58e946b
Compare
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
1 participant
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.
What
Rewrites
AdmProxy(webrtc-sys) so that every touch of the platform ADM, the synthetic ADM, and all proxy state happens on the WebRTC worker thread. Every public method marshals once at the boundary (RunOnWorker, aBlockingCallthat runs inline when the caller is already the worker), all mutable state is worker-owned and annotatedRTC_GUARDED_BY(worker_thread_), and both mutexes are removed. Mode transitions are now plain sequential worker code.Why
Platform ADM implementations (Windows CoreAudio2, iOS, and the upcoming Apple AudioEngine device) bind a sequence checker to their construction thread and
RTC_DCHECK_RUN_ONevery control method. Previously the proxy called the platform ADM directly on whatever thread the Rust FFI caller was on, guarded only by a mutex — violating that contract and racing with WebRTC's own worker-thread calls. This is a prerequisite for switching the Apple platform ADM to the AudioEngine device (follow-up branchhiroshi/audioengine-platformaudio, depends on webrtc-sdk/webrtc#260).Making the proxy thread-affine removes by construction the races a mutex couldn't prevent: stale mode snapshots interleaving transitions, the synthetic ADM being driven from caller threads, and
Terminate()racing mode switches.This mirrors upstream WebRTC's own pattern:
RTCPeerConnectionFactoryconstructs the ADM inside_workerThread->BlockingCall(...), andWebRtcVoiceEngine/AudioStateguard every ADM call with worker-thread sequence checkers.Also fixed along the way
AudioTransportat factory init, before the Android ADM exists;EnsurePlatformAdmCreatednow passes it (and any pre-acquire device selection) to the fresh ADM. Previously platform recording delivered no audio to the pipeline.AudioDeviceControllernow holds ashared_ptr<RtcRuntime>(same pattern as AudioTrack/VideoTrack) so the worker thread outlives every proxy reference reachable from Rust.playout_initialized_/recording_initialized_flags and never-read device GUID tracking.docs/ADM_PROXY_DESIGN.mdupdated to match the actual lifecycle (eager idle ADM, release does not terminate) and to document the threading model.Threading contract
BlockingCall); the eagerly created platform ADM binds its sequence checker there.AudioTransportdata) are NOT marshaled — they flow directly through the sub-ADMs' own I/O threads, matching the platform ADMs' design (separate io/render thread checkers).Behavior parity
Apart from the marshaling and the fixes listed above, behavior is unchanged: all no-platform-ADM defaults, state-update ordering, and gate semantics match the previous implementation method-for-method.
Testing
cargo check -p webrtc-sysandcargo build -p livekitpass.BlockingCallboundary (factory teardown ordering, dtor-on-any-thread, SyntheticAudioDevice task queue). Threading contract verified against the ADM implementations in webrtc-sdk (Windows/iOS/AudioEngine sequence checkers, voice engine and AudioState worker guards).cargo run -p livekit --example platform_audio— runs offline, no LiveKit server. Passing on macOS (arm64):playing_ = trueswitch branch) and DCHECK-level thread validation (release prebuilt compilesRTC_DCHECKout; needs a debug webrtc viaLK_CUSTOM_WEBRTC).