Skip to content

fix(rtc): filter media events from the room FFI subscription - #821

Open
swayamg20 wants to merge 1 commit into
livekit:mainfrom
swayamg20:fix/812-filter-room-ffi-events
Open

swayamg20 wants to merge 1 commit into
livekit:mainfrom
swayamg20:fix/812-filter-room-ffi-events

Conversation

@swayamg20

Copy link
Copy Markdown
Contributor

Summary

Fixes #812.

Filter audio_stream_event and video_stream_event from the long-lived subscription created by Room.connect(). Audio/video streams already have their own subscriptions, so forwarding these events to every room only adds event-loop callbacks and room-queue work.

Keep all other events eligible, including the publish_track and unpublish_track callbacks that LocalParticipant receives through the room queue. The temporary connect subscription and AudioSource.capture_frame() are unchanged.

Thanks to @wizd for the report and the correction identifying the publication-callback requirement. The separate publication-state robustness issue is intentionally out of scope. This change also leaves the connection-cancellation work in #813 untouched.

Regression coverage

The new tests exercise the real room connection, FFI queue, room listener, and participant publication methods with a fake native request boundary:

  • Audio/video events schedule no room callbacks while still reaching an independent media subscriber.
  • Room events, RPC invocations, publication callbacks, and audio capture callbacks still arrive.
  • Public publish/unpublish calls complete, and disconnect cleans up subscriptions.

Reverting only the filter makes both media regressions fail: 100 media events schedule 200 callbacks instead of the expected 100. Restoring it passes all eight new cases. This is deterministic dispatch-count evidence, not a production throughput benchmark.

Validation

  • Focused regression and existing FFI queue tests: 15 passed, with warnings treated as errors.
  • Full tests/ and livekit-rtc/tests/ suite: 121 passed, 72 skipped on Python 3.13.12/macOS arm64. Live-server credentials were unset.
  • Repository-wide Ruff lint and formatting: passed.
  • Mypy across protocol/API/RTC packages and the new test: passed, 100 source files.
  • RTC wheel build and git diff --check: passed.

Local fixture note: git-lfs is unavailable in this checkout. The full-suite run used the exact upstream WAV files in a temporary directory, verified against the tracked LFS SHA-256 hashes; a pytest collection hook redirected only the three audio test modules' fixture directory. No assertions or tracked fixtures were changed. The existing JWT test-key and AudioStream test-double destructor warnings remain (34 warnings). Skipped/live-server tests have not been validated.

Exclude audio and video stream events before scheduling room callbacks, while preserving publication and other request callbacks. Add regression coverage for dispatch filtering, publish/unpublish completion, and subscription cleanup.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@cloudwebrtc cloudwebrtc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Room.connect FFI subscription is unfiltered; audio_stream_event fans out O(rooms × events) and starves the asyncio loop

2 participants