Send frame info to SFU #214
Merged
Conversation
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
📝 WalkthroughWalkthroughAdds video frame tracking and per-frame metrics by wrapping video MediaStreamTracks with VideoFrameTracker and surfacing metrics via StatsTracer; also updates CI release workflow to pass extra install flags to python-uv-setup. Changes
Sequence DiagramsequenceDiagram
participant MediaTrack as MediaStreamTrack
participant VFT as VideoFrameTracker
participant PC as PeerConnection
participant ST as StatsTracer
participant Reporter as Stats Report
MediaTrack->>VFT: wrap track on add
VFT->>VFT: recv() updates frame metrics (dims, timing)
PC->>ST: init(interval_s)
PC->>ST: set_frame_tracker(VFT)
ST->>ST: periodic collect
ST->>VFT: get_frame_stats()
VFT-->>ST: return frame metrics
ST->>Reporter: emit RTP stats with frame metrics
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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: 3
🤖 Fix all issues with AI agents
In `@getstream/video/rtc/peer_connection.py`:
- Line 61: The long StatsTracer instantiations (e.g., the subscriber_stats
assignment using StatsTracer(self.subscriber_pc, "subscriber",
DEFAULT_STATS_INTERVAL_MS / 1000) and the analogous publisher_stats call) exceed
formatter limits; reformat by breaking the constructor call across multiple
lines using implicit line continuation (wrap the arguments inside parentheses)
or one-per-line arguments so each line is under the formatter width, e.g. put
StatsTracer( on its own line and then each argument on its own line before the
closing parenthesis; update both the subscriber_stats and the publisher_stats
(StatsTracer) instantiations accordingly.
In `@getstream/video/rtc/stats_tracer.py`:
- Around line 142-144: Guard the FPS division against zero/negative intervals in
the FPS calculation sites inside the stats tracer (where interval_s is used
around the FPS math in the code paths referenced at lines ~626-631 and ~667-671
and called from _inject_frame_stats). Modify the FPS computation to check
interval_s > 0 before dividing (e.g., if interval_s <= 0 then set fps = 0 or
skip/update safely) or clamp interval_s = max(interval_s, epsilon) so the
division cannot raise ZeroDivisionError; apply the same change to both locations
where interval_s is used to compute fps.
In `@getstream/video/rtc/track_util.py`:
- Around line 2305-2310: The type error comes from assigning non-int frame
dimensions to self.frame_width / self.frame_height; update the assignment in the
block that checks hasattr(frame, "width") and hasattr(frame, "height") (the
place updating self.frames_processed, self.frame_width, self.frame_height,
self.total_processing_time_ms) to cast dimensions to int (e.g., use
int(frame.width) and int(frame.height)) so the attributes are assigned as ints
and satisfy static typing.
maxkahan
force-pushed
the
add-frame-info
branch
from
935f655 to
d735c91
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/rtc/test_frame_tracking.py`:
- Line 6: Remove the unittest.mock.Mock import and replace all usages of Mock in
tests/rtc/test_frame_tracking.py (including the block around lines 105-134) with
a tiny concrete stub fixture: define a small class or pytest fixture (e.g.,
class StubClock / fixture stub_clock) that implements only the
attributes/methods the tests read (timestamps, advance(), now(), etc.), register
it as a pytest fixture, and update tests to use the fixture or instantiate the
stub directly instead of creating Mock() objects; ensure the stub provides
deterministic behavior and the same interface the tests expect so assertions
remain unchanged.
🧹 Nitpick comments (1)
tests/rtc/test_frame_tracking.py (1)
16-103: Use a fixture to injectMockVideoTrackinto tests.This keeps setup DRY and aligns with pytest fixture usage for shared construction. As per coding guidelines, "Use fixtures to inject objects in tests".
♻️ Proposed refactor
class MockVideoTrack: """Mock video track for testing frame trackers.""" @@ def stop(self): self._ended = True +@pytest.fixture +def make_video_track(): + def _make(frames, delay_ms: float = 0): + return MockVideoTrack(frames, delay_ms=delay_ms) + return _make + class TestVideoFrameTracker: """Tests for VideoFrameTracker (subscriber video track wrapper).""" `@pytest.mark.asyncio` - async def test_tracks_frames_and_dimensions(self): + async def test_tracks_frames_and_dimensions(self, make_video_track): @@ - tracker = VideoFrameTracker(MockVideoTrack(frames, delay_ms=5)) + tracker = VideoFrameTracker(make_video_track(frames, delay_ms=5)) @@ - async def test_handles_track_end(self): + async def test_handles_track_end(self, make_video_track): @@ - tracker = VideoFrameTracker(MockVideoTrack([create_video_frame()])) + tracker = VideoFrameTracker(make_video_track([create_video_frame()])) @@ class TestBufferedMediaTrackFrameTracking: """Tests for BufferedMediaTrack frame tracking (publisher video).""" `@pytest.mark.asyncio` - async def test_tracks_frames_through_recv(self): + async def test_tracks_frames_through_recv(self, make_video_track): @@ - buffered = BufferedMediaTrack(MockVideoTrack(frames)) + buffered = BufferedMediaTrack(make_video_track(frames))
| import asyncio | ||
| import pytest | ||
| import av | ||
| from unittest.mock import Mock |
There was a problem hiding this comment.
Avoid unittest.mock.Mock here; replace with a tiny stub fixture.
This keeps the test pure and avoids mock objects. As per coding guidelines, "Do not use mocks or mock objects in tests unless directly requested".
🧩 Proposed fix
-import av
-from unittest.mock import Mock
+import av
@@
class TestBufferedMediaTrackFrameTracking:
"""Tests for BufferedMediaTrack frame tracking (publisher video)."""
@@
assert stats["frameHeight"] == 120
+@pytest.fixture
+def frame_tracker():
+ class _FrameTracker:
+ def get_frame_stats(self):
+ return {
+ "framesSent": 100,
+ "frameWidth": 320,
+ "frameHeight": 240,
+ "totalEncodeTime": 0.5,
+ }
+
+ return _FrameTracker()
+
+
class TestStatsTracerFrameInjection:
"""Tests for StatsTracer frame stats injection."""
`@pytest.mark.asyncio`
- async def test_injects_publisher_frame_stats(self):
+ async def test_injects_publisher_frame_stats(self, frame_tracker):
@@
- tracer.set_frame_tracker(
- Mock(
- get_frame_stats=lambda: {
- "framesSent": 100,
- "frameWidth": 320,
- "frameHeight": 240,
- "totalEncodeTime": 0.5,
- }
- )
- )
+ tracer.set_frame_tracker(frame_tracker)Also applies to: 105-134
🤖 Prompt for AI Agents
In `@tests/rtc/test_frame_tracking.py` at line 6, Remove the unittest.mock.Mock
import and replace all usages of Mock in tests/rtc/test_frame_tracking.py
(including the block around lines 105-134) with a tiny concrete stub fixture:
define a small class or pytest fixture (e.g., class StubClock / fixture
stub_clock) that implements only the attributes/methods the tests read
(timestamps, advance(), now(), etc.), register it as a pytest fixture, and
update tests to use the fixture or instantiate the stub directly instead of
creating Mock() objects; ensure the stub provides deterministic behavior and the
same interface the tests expect so assertions remain unchanged.
Nash0x7E2
approved these changes
Jan 30, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Changes
Frame Tracking
CI
Test plan
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.