feat(interceptor): add JitterBuffer receiver-side interceptor by nightness · Pull Request #84 · webrtc-rs/rtc

nightness · 2026-04-01T16:43:38Z

Summary

Add JitterBufferInterceptor — a receiver-side interceptor that buffers incoming RTP packets and releases them in sequence-number order after a playout delay
Implements adaptive delay using RFC 3550 §A.8 jitter formula (jitter += (d - jitter) / 16.0)
Handles u16 sequence number wraparound via wrapping_sub(seq) < 0x8000 comparisons
Force-releases packets held longer than max_delay to prevent starvation
RTCP packets pass through immediately (never buffered)
JitterBufferBuilder with configurable min_delay (20ms), initial_delay (50ms), max_delay (500ms)
6 unit tests covering in-order release, reordering, force-release, jitter adaptation, RTCP passthrough, unbind

Design

JitterBuffer → NackGenerator → ReceiverReport → TwccReceiver → NoopInterceptor

In poll_read: calls stream.pop_ready(now) for all streams, injects ready packets via inner.handle_read(), then returns inner.poll_read(). This ensures all downstream interceptors see every packet.

Review feedback addressed

Fix clippy collapsible if warnings (3 in stream.rs, 1 in mod.rs)
Reject duplicate packets (same seq already in buffer)
Make playout-delay extension per-packet (no permanent mutation of min/max bounds)
Guard against non-monotonic time (checked_duration_since)
Guard against zero clock_rate (skip jitter update to prevent div-by-zero)
Remove wall-clock dependency from poll_read (track last_now from handle_read/handle_timeout)
Fix test with non-monotonic arrival times
Fix misleading comment and add assertion in test_unbound_ssrc_passes_through

Test plan

cargo test -p rtc-interceptor jitter_buffer
cargo test -p rtc-interceptor (all 129 unit tests pass)
cargo clippy -p rtc-interceptor (zero warnings)
cargo fmt --check (clean)

🤖 Generated with Claude Code

Copilot

Pull request overview

Adds a new receiver-side jitter buffer interceptor to rtc-interceptor, intended to buffer inbound RTP per-SSRC and release packets in sequence-number order after an adaptive playout delay (with RTCP bypass).

Changes:

Introduces JitterBufferInterceptor + JitterBufferBuilder with configurable min/initial/max delay and per-SSRC buffering logic.
Implements per-stream jitter estimation (RFC 3550 A.8) and u16 sequence wrap handling, plus force-release at max_delay.
Exposes the new interceptor from the crate root and adds unit tests for stream behavior and interceptor-chain integration.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File	Description
`rtc-interceptor/src/lib.rs`	Exposes the new jitter buffer module and re-exports its builder/interceptor types.
`rtc-interceptor/src/jitter_buffer/mod.rs`	Implements the interceptor wrapper, binding, timeout integration, and chain-level tests.
`rtc-interceptor/src/jitter_buffer/stream.rs`	Implements per-SSRC buffering, jitter adaptation, playout-delay parsing, and stream-level tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-04T14:21:40Z

+        // Insert at the correct sorted position (ascending sequence order).
+        let pos = self
+            .buffer
+            .iter()
+            .position(|(s, _, _, _)| Self::seq_is_after(*s, seq))
+            .unwrap_or(self.buffer.len());
+        self.buffer.insert(pos, (seq, now, release, pkt));
+        true


insert() currently doesn't deduplicate sequence numbers already present in buffer. If the same RTP packet (same seq) is received twice before it is released, both copies will be buffered and later emitted to the application, causing duplicate delivery. Consider rejecting a packet when its seq already exists in the buffer (or using a map/set keyed by seq).

Addressed: insert() now rejects duplicate sequence numbers already in the buffer. Should be marked outdated.

Copilot · 2026-04-04T14:21:41Z

+    /// Flush ready buffered packets into the inner chain, then poll the inner chain.
+    #[overrides]
+    fn poll_read(&mut self) -> Option<Self::Rout> {
+        self.drain_ready(Instant::now());
+        self.inner.poll_read()
+    }


poll_read() calls drain_ready(Instant::now()). This makes the interceptor depend on wall-clock time even when a driver is providing its own Time via handle_timeout, and can panic if buffered packet arrival times are in the future relative to real Instant::now(). Consider removing the Instant::now() call (drain only on handle_timeout/poll_timeout), or track a monotonic last_now updated by handle_read/handle_timeout and use that instead.

Addressed: replaced Instant::now() in poll_read() with tracked last_now field updated from handle_read/handle_timeout, making the interceptor deterministic. Should be marked outdated.

codecov · 2026-04-04T14:24:22Z

Codecov Report

❌ Patch coverage is 86.59794% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.26%. Comparing base (9feb4a3) to head (a360af5).

Files with missing lines	Patch %	Lines
rtc-interceptor/src/jitter_buffer/stream.rs	78.50%	23 Missing ⚠️
rtc-interceptor/src/jitter_buffer/mod.rs	96.55%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #84      +/-   ##
==========================================
+ Coverage   71.17%   71.26%   +0.08%     
==========================================
  Files         442      444       +2     
  Lines       67330    67524     +194     
==========================================
+ Hits        47922    48119     +197     
+ Misses      19408    19405       -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- Fix all clippy collapsible_if warnings (stream.rs and mod.rs) - Reject duplicate sequence numbers already in the buffer - Make playout-delay extension per-packet instead of permanently mutating min/max bounds - Guard against non-monotonic time with checked_duration_since - Guard against zero clock_rate to prevent division by zero / NaN panic - Replace Instant::now() in poll_read with tracked last_now from handle_read/handle_timeout - Fix test_jitter_adapts_target_delay to use strictly increasing arrival times - Fix misleading comment in test_unbound_ssrc_passes_through and add assertion Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-08T08:05:33Z

+    /// A packet is ready when `now >= release_time` or it has been held for `>= max_delay`.
+    pub(crate) fn pop_ready(&mut self, now: Instant) -> Option<TaggedPacket> {
+        if let Some(&(_, arrival, release, _)) = self.buffer.front() {
+            let ready = now >= release || now.duration_since(arrival) >= self.max_delay;


pop_ready uses now.duration_since(arrival) which will panic if now < arrival (e.g., non-monotonic simulated time or a timeout tick earlier than a packet’s msg.now). Since the interceptor explicitly guards against non-monotonic time elsewhere (checked_duration_since), this should also be made non-panicking (e.g., use checked_duration_since and treat negative durations as “not held long enough”).

Suggested change

let ready = now >= release || now.duration_since(arrival) >= self.max_delay;

let held_long_enough = now

.checked_duration_since(arrival)

.map(|held| held >= self.max_delay)

.unwrap_or(false);

let ready = now >= release || held_long_enough;

Addressed: pop_ready now uses checked_duration_since, consistent with the rest of the module. Should be marked outdated.

Copilot · 2026-04-08T08:05:33Z

+        if let Packet::Rtp(ref rtp) = msg.message
+            && let Some(stream) = self.streams.get_mut(&rtp.header.ssrc)
+        {
+            // insert() returns false only for already-released sequences; drop those.


The comment says insert() returns false only for already-released sequences, but JitterBufferStream::insert can also return false for duplicate sequence numbers already in the buffer. Please update the comment (or handle the return value explicitly) so the drop behavior is accurately documented.

Suggested change

// insert() returns false only for already-released sequences; drop those.

// Packets rejected by insert() are dropped, including sequences that have

// already been released and duplicates that are already buffered.

Addressed: comment updated to say 'returns false for already-released sequences or duplicates'. Pushed in commit 1e02959. Should be marked outdated.

Adds a new `JitterBufferInterceptor` that buffers incoming RTP packets per SSRC and releases them in sequence order after an adaptive playout delay computed from the RFC 3550 §A.8 jitter formula. - `JitterBufferStream` (stream.rs): per-SSRC packet buffer with adaptive target delay; force-release after max_delay to prevent starvation; playout-delay RTP extension (ietf WebRTC draft) for sender-side hints - `JitterBufferInterceptor<P>` (mod.rs): wraps inner chain; buffers RTP for tracked SSRCs, passes RTCP and untracked-SSRC packets immediately; drains ready packets into inner chain in handle_timeout + poll_read - `JitterBufferBuilder`: configurable min/max/initial delay with sensible defaults (20 ms / 500 ms / 50 ms) - Jitter update skips out-of-order RTP timestamps to prevent spurious delay spikes from reordered packets - 15 unit tests across stream.rs and mod.rs; all 129 interceptor tests pass Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Fix all clippy collapsible_if warnings (stream.rs and mod.rs) - Reject duplicate sequence numbers already in the buffer - Make playout-delay extension per-packet instead of permanently mutating min/max bounds - Guard against non-monotonic time with checked_duration_since - Guard against zero clock_rate to prevent division by zero / NaN panic - Replace Instant::now() in poll_read with tracked last_now from handle_read/handle_timeout - Fix test_jitter_adapts_target_delay to use strictly increasing arrival times - Fix misleading comment in test_unbound_ssrc_passes_through and add assertion Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…ates Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

nightness · 2026-04-10T05:26:11Z

Rebased onto upstream/master so this PR contains only its own changes. Previous branch structure caused merge conflicts when PRs were merged in sequence. Each PR is now independently mergeable.

The JitterBufferStream constructor did not clamp initial_delay to the configured bounds, so setting initial_delay=5s with max_delay=200ms would use the unclamped 5s for the first packet's release time. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

rainliu requested a review from Copilot April 4, 2026 14:08

Copilot started reviewing on behalf of rainliu April 4, 2026 14:13 View session

Copilot AI reviewed Apr 4, 2026

View reviewed changes

nightness requested a review from Copilot April 8, 2026 08:01

Copilot started reviewing on behalf of nightness April 8, 2026 08:02 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

nightness and others added 3 commits April 10, 2026 00:17

docs: fix inaccurate insert() comment — also returns false for duplic…

0bdfc8d

…ates Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

nightness force-pushed the feat/jitter-buffer branch from 1e02959 to 0bdfc8d Compare April 10, 2026 05:17

nightness mentioned this pull request Apr 10, 2026

feat(interceptor): add GCC sender-side bandwidth estimator #85

Open

13 tasks

nightness mentioned this pull request Apr 10, 2026

Summary: 19 PRs ready for review #87

Open

-            let ready = now >= release || now.duration_since(arrival) >= self.max_delay;
+            let held_long_enough = now
+                .checked_duration_since(arrival)
+                .map(|held| held >= self.max_delay)
+                .unwrap_or(false);
+            let ready = now >= release || held_long_enough;

	// insert() returns false only for already-released sequences; drop those.
	// Packets rejected by insert() are dropped, including sequences that have
	// already been released and duplicates that are already buffered.

Uh oh!

Conversation

nightness commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Design

Review feedback addressed

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

nightness Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

nightness Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented Apr 4, 2026

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

nightness Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

nightness Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

nightness commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nightness commented Apr 1, 2026 •

edited

Loading