feat: configurable video frame buffering with blackhole drain#231
feat: configurable video frame buffering with blackhole drain#231
Conversation
Pass video_buffered=False through ConnectionManager → PeerConnectionManager → SubscriberPeerConnection to relay.subscribe(buffered=False) for video tracks. Default True preserves current behavior. Without buffering, only the latest video frame is kept in memory instead of an unbounded queue. This prevents OOM on voice-only agents that subscribe to video but never consume frames (~400 MiB/10sec leak).
When video_buffered=False, attach a MediaBlackhole to the video proxy track to continuously consume and discard frames. This prevents unbounded queue growth in RTCRtpReceiver._queue (aiortc issue #554). Without draining, unconsumed video frames accumulate at ~400 MiB/10sec because aiortc's receiver queue has no size limit.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 6 minutes and 36 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughA new boolean parameter Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@getstream/video/rtc/pc.py`:
- Around line 188-192: Current code overwrites single _video_blackhole and
_video_drain_task when multiple video tracks arrive, leaking the previous
MediaBlackhole/task and preventing deterministic cleanup; change the
implementation to store blackholes and drain tasks in maps keyed by track.id
(e.g., self._video_blackholes: Dict[track_id, MediaBlackhole] and
self._video_drain_tasks: Dict[track_id, asyncio.Task]) when creating them in the
track handler (where MediaBlackhole() and asyncio.create_task are called) and
update handle_track_ended to look up by track.id, cancel and await/cancel the
task, and remove both entries to ensure proper cleanup.
- Around line 184-194: The current code reuses the same RelayStreamTrack
subscription (proxy returned by relay.subscribe(tracked_track)) for both
downstream consumers and the MediaBlackhole, which makes them compete for
frames; change the blackhole logic to create a separate subscription for
draining by calling relay.subscribe(tracked_track) again (e.g., drain_proxy) and
add that separate track to MediaBlackhole, keep the original proxy for
emit("track_added", ...), and assign the blackhole and start task to
self._video_blackhole and self._video_drain_task as before so only the
drain_proxy is consumed by MediaBlackhole and downstream consumers get the
original proxy.
🪄 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: 480a4aa8-5710-4ea2-ba60-0d612588d906
📒 Files selected for processing (3)
getstream/video/rtc/connection_manager.pygetstream/video/rtc/pc.pygetstream/video/rtc/peer_connection.py
…racks - Create a dedicated relay subscription for blackhole drain so it doesn't compete with downstream consumers for frames - Use dicts keyed by track ID instead of single references to support multiple concurrent video tracks
| self._video_drain_tasks[track.id] = asyncio.create_task( | ||
| blackhole.start() |
There was a problem hiding this comment.
Let's use add_done_callback to clean up the tasks and stop blackholes in case the track swithches on/off
3 participants
Why
aiortc's
RTCRtpReceiveruses an unboundedasyncio.Queuefor decoded video frames (aiortc#554). When nobody callsrecv()on the video track, frames accumulate at ~400 MiB per 10 seconds. Voice-only agents that receive but don't consume video hit OOM within minutes.Changes
drain_video_framesparameter (defaultFalse) propagated throughConnectionManager→PeerConnectionManager→SubscriberPeerConnectiondrain_video_frames=True, attach aMediaBlackholeto video proxy tracks to continuously consume and discard frames, preventing unbounded queue growthHow it works
Without drain (
drain_video_frames=False, default):With drain (
drain_video_frames=True):Benchmark (local k8s, 30sec video call)
Usage
Companion PR
Summary by CodeRabbit