-
Notifications
You must be signed in to change notification settings - Fork 14
Added PyAVResampler - a stateful resampler implementation
#206
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: g711
Are you sure you want to change the base?
Conversation
WalkthroughThe changes introduce PyAVResampler, a new stateful audio resampling class that wraps PyAV's AudioResampler with internal frame buffering and PTS clock management. PcmData is extended with an empty property and bidirectional PyAV frame conversion methods (to_av_frame with PTS assignment, from_av_frame for deserializing PyAV AudioFrames). Resampler.repr is updated to include the runtime class name. Comprehensive tests are added for the new resampler across multiple scenarios. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
getstream/video/rtc/track_util.py (2)
489-494: Inconsistent shape for empty frames.Empty frames create different array shapes for mono vs. multi-channel:
- Mono:
np.array([], dtype=dtype)→ shape(0,)- Multi-channel:
np.zeros((channels, 0), dtype=dtype)→ shape(channels, 0)For consistency, mono should also return a 2D array when
channels == 1:🔎 Proposed fix for consistent empty frame shapes
# Handle empty frames if frame.samples == 0: - if channels == 1: - samples_array = np.array([], dtype=dtype) - else: - samples_array = np.zeros((channels, 0), dtype=dtype) + samples_array = np.zeros((channels, 0), dtype=dtype) else:Then flatten to 1D for mono later in the normalization logic (line 522-524) which already handles this case.
1755-1759: Consider clearer condition for empty frame check.The condition
if frame is not None and not frame.samples:works but could be more explicit. Consider:🔎 More explicit empty frame check
def _pyav_resample(self, frame: av.AudioFrame | None) -> list[av.AudioFrame]: - if frame is not None and not frame.samples: + if frame is not None and frame.samples == 0: # pyav resampler fails if audioframe has no samples return [] return self._pyav_resampler.resample(frame)Using
== 0makes it clearer that we're checking for zero samples, not a falsy samples object.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
getstream/video/rtc/track_util.py(4 hunks)tests/rtc/test_pcm_data.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*test*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*test*.py: Use fixtures to inject objects in tests
Keep tests well organized and use test classes for similar tests
Tests that rely on file assets should use files fromtests/assets/folder; new files should be added there, existing ones reused if possible
Do not use mocks or mock things in tests unless explicitly asked to
Files:
tests/rtc/test_pcm_data.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Plugins that work with audio, video, or WebRTC functionality should depend on
getstream[webrtc]instead of justgetstreamto access WebRTC-related dependencies likeaiortc,numpy,torch,torchaudio,soundfile,scipy,deepgram-sdk, andelevenlabs
Files:
tests/rtc/test_pcm_data.pygetstream/video/rtc/track_util.py
**/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/test_*.py: Use fixtures to inject objects in tests; test client fixtures can use the Stream API client
Load credentials from .env file; the client fixture will load credentials automatically
Keep tests well organized and use test classes to group similar tests
Tests that rely on file assets should use files fromtests/assets/folder; add new files there and reuse existing ones when possible. Do not use files larger than 256 kilobytes
Do not use mocks or mock objects in tests unless directly requested
Files:
tests/rtc/test_pcm_data.py
🧬 Code graph analysis (1)
getstream/video/rtc/track_util.py (2)
getstream/video/rtc/recording.py (1)
AudioFrame(64-70)getstream/video/rtc/audio_track.py (1)
flush(150-159)
🔇 Additional comments (7)
getstream/video/rtc/track_util.py (5)
811-811: LGTM!Assigning PTS to the AudioFrame enables proper timestamp tracking when converting PcmData to PyAV frames, which is essential for the stateful resampler's time management.
1669-1672: LGTM!The
emptyproperty provides a clean, intuitive way to check if PcmData contains any samples. This is especially useful for the resampler's flush behavior.
1708-1738: LGTM!The initialization logic properly validates format strings and handles both string and enum format types. The PyAV format determination and internal state setup are correct.
1796-1802: LGTM!The PTS clock management is correct. The logic tracks the next sample timestamp in
self._ptsand calculates the result's starting PTS by subtracting the result length, ensuring continuous monotonic timestamps across multiple resample calls.
2032-2032: LGTM!Using
self.__class__.__name__instead of a hardcoded class name is a best practice that ensures subclasses (likePyAVResampler) display the correct class name in their string representation.tests/rtc/test_pcm_data.py (2)
1979-2326: Excellent test coverage for PyAVResampler!The test suite comprehensively covers:
- Upsampling and downsampling scenarios
- Frame size buffering behavior
- Channel conversions (mono ↔ stereo)
- Format conversions (s16 ↔ f32)
- Real-time streaming simulation with 20ms chunks
- PTS/DTS timestamp tracking and monotonic progression
- Edge cases (empty frames, flush behavior)
- Cross-chunk consistency
The tests are well-organized in a dedicated class and follow the existing test patterns in the file.
2042-2082: Good defensive testing for input validation.These tests verify that the stateful resampler correctly rejects inconsistent inputs (different sample rates or formats after initialization), which is critical for preventing subtle bugs in streaming scenarios. Using
pytest.raiseswith thematchparameter ensures the error messages are clear.
Added a stateful resampler based on pyav
It is intended to be once for the audio track and reused.
Key differences from the stateless implementation:
PyAVResamplerkeeps its own monotonic PTS clock, and it's meant to be used with a single audio stream only.It ignores the PTS/DTS from
PcmData.PTS always starts from 0 for the first output.
pyav.AudioResamplerconfigures itself based on the first input frame. Feeding data in a different formator sample rate will fail.
PyAVResampleris not thread-safe.The source PCMs must have the same sample rate, format, and number of channels.
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.