Skip to content

Conversation

@tbarbugli
Copy link
Member

@tbarbugli tbarbugli commented Nov 11, 2025

Summary by CodeRabbit

  • Breaking Changes

    • Updated AudioStreamTrack constructor: replaced max_queue_size parameter with audio_buffer_size_ms (default 30000ms) for improved buffer management control
  • Tests

    • Added comprehensive test suite for AudioStreamTrack covering initialization, format/sample rate conversions, buffer overflow scenarios, and frame timing validation

@coderabbitai
Copy link

coderabbitai bot commented Nov 11, 2025

Warning

Rate limit exceeded

@tbarbugli has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 54 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between a7483bd and bd5ca9d.

📒 Files selected for processing (1)
  • tests/test_audio_stream_track.py (1 hunks)

Walkthrough

The AudioStreamTrack class transitions from an async queue-based architecture to an internal byte buffer for managing PCM data. The refactor introduces buffer overflow handling with a configurable maximum duration, timestamp-based frame pacing, and a new constructor parameter audio_buffer_size_ms replacing max_queue_size. Comprehensive test coverage validates initialization, data flow, format/rate/channel conversions, buffer behavior, and frame timing.

Changes

Cohort / File(s) Summary
Core Audio Track Implementation
getstream/video/rtc/audio_track.py
Refactored AudioStreamTrack to use an internal byte buffer with locking instead of async queue. Replaced max_queue_size parameter with audio_buffer_size_ms (default 30000 ms). Writes normalize PCM to target format, convert to bytes, and append to buffer with overflow handling (drops oldest data). Frame production uses timestamp-based pacing with optional drift warnings. Simplified _normalize_pcm to always resample to target sample_rate and channels.
Test Suite
tests/test_audio_stream_track.py
New comprehensive test file covering initialization defaults, write/receive flow, format conversion (s16 to f32), sample rate resampling (16kHz to 48kHz), channel conversion (mono↔stereo), buffer overflow behavior, silence emission, flush operations, frame timing (20ms intervals), continuous streaming scenarios, and error handling after track stop.

Sequence Diagram

sequenceDiagram
    participant Client
    participant AudioStreamTrack
    participant Buffer
    participant Frame

    Note over AudioStreamTrack,Buffer: New Buffering Architecture
    
    Client->>AudioStreamTrack: write(pcm_data)
    activate AudioStreamTrack
    AudioStreamTrack->>AudioStreamTrack: _normalize_pcm()
    Note over AudioStreamTrack: Convert format/rate/channels
    AudioStreamTrack->>Buffer: append bytes (with lock)
    alt Buffer overflow
        Buffer->>Buffer: drop oldest data
        Buffer->>AudioStreamTrack: log overflow
    end
    deactivate AudioStreamTrack

    Client->>AudioStreamTrack: recv()
    activate AudioStreamTrack
    AudioStreamTrack->>Buffer: read from buffer (with lock)
    alt Data available
        Buffer-->>AudioStreamTrack: byte chunk
        AudioStreamTrack->>Frame: fill AudioFrame planes
    else Buffer empty
        AudioStreamTrack->>Frame: emit silence (20ms)
    end
    AudioStreamTrack-->>Client: AudioFrame
    Note over AudioStreamTrack: Timestamp-based pacing
    deactivate AudioStreamTrack
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Key areas requiring attention:
    • Buffer management logic in write() and recv() methods, including overflow handling and the locking mechanism
    • Timestamp-based frame pacing implementation and drift warning logic
    • PCM normalization and byte buffer construction for various format/rate/channel combinations
    • Test comprehensiveness for edge cases (overflow, empty buffer, timing precision)
    • Interaction between buffer clearing in flush() and concurrent read/write operations

Possibly related PRs

  • PR #173: Makes overlapping direct changes to getstream/video/rtc/audio_track.py, introducing PcmData-based audio pipeline with write signature changes, resampling/normalization logic, and internal buffering mechanisms.

Poem

🐰 A buffer blooms where queues once flowed,
No async roads, just bytes bestowed,
With timestamps ticking, frames aligned,
Twenty mills of silence, ever kind,
Tests guard each frame—no buffer feared!

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'New audio track' is vague and generic, failing to convey the specific nature of the changes made to the AudioStreamTrack implementation. Use a more descriptive title that highlights the main change, such as 'Replace AudioStreamTrack queue with byte buffer' or 'Refactor AudioStreamTrack to use byte buffer for PCM data'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 62b1538 and a7483bd.

📒 Files selected for processing (2)
  • getstream/video/rtc/audio_track.py (6 hunks)
  • tests/test_audio_stream_track.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
getstream/video/rtc/audio_track.py (1)
getstream/video/rtc/track_util.py (5)
  • PcmData (87-1353)
  • to_bytes (566-601)
  • duration_ms (267-269)
  • resample (542-564)
  • resample (1392-1448)
tests/test_audio_stream_track.py (2)
getstream/video/rtc/audio_track.py (4)
  • AudioStreamTrack (15-264)
  • write (83-148)
  • recv (161-243)
  • flush (150-159)
getstream/video/rtc/track_util.py (5)
  • PcmData (87-1353)
  • AudioFormat (29-79)
  • kind (1675-1676)
  • readyState (1683-1684)
  • append (709-832)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Tests (3.12)
  • GitHub Check: Tests (3.13)
  • GitHub Check: Tests (3.10)
  • GitHub Check: Tests (3.11)

@tbarbugli tbarbugli merged commit 12c8694 into main Nov 11, 2025
5 checks passed
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.

2 participants