Oracle P2 batch 2 (refactor items). Drops both remaining design-shape
clippy warnings to zero without behavior change.
- avhw.rs build_filter_graph: drop unused _enc_width/_enc_height params
(Oracle caught them during P2 review — passed by EncState::new but
never read inside the function; the filter graph uses width/height
only). Signature: 8 args -> 6 args (under clippy's 7 threshold).
- state.rs InFlightSurface::CopyQueued: Box the drm_map field.
AVDRMFrameDescriptor is ~592 bytes (4 objects + 4 layers); the enum
size was being dominated by this variant, ballooning every
InFlightSurface value to 592 bytes even for the None/AllocQueued
variants. Box<AVDRMFrameDescriptor> shrinks the enum to ~32 bytes
regardless of variant. The drm_map field is currently destructured
under _drm_map (unused), so the boxing has no consumer-side impact.
- state_portal.rs webrtc_thread_loop: 10 args -> 4 args via two new
structs:
* WebRtcThreadConfig { fps, enc_width, enc_height, max_bitrate }
— immutable for the thread's lifetime; a tier change spawns a
new thread rather than mutating.
* WebRtcThreadChannels { webrtc_rx, sent_gap_tx, bitrate_tx,
resolution_tx } — channel endpoints owned exclusively by the
sender thread after spawn.
wrtc (WebRtcState) and paused (Arc<AtomicBool>) stay as separate
args because they have different ownership semantics (moved-in
state vs shared atomic). Documented as doc comments on the new
types so the next reader understands the bundle rationale.
All 79 unit tests + 3 integration tests pass. clippy: 0 errors.
Per-file warning counts: state_portal.rs down from 3 to 0; state.rs
down from 8 to 4 (remaining are unrelated dead-code on OutputInfo /
starting_timestamp).
Oracle step 4 (option A) — give the scale_*, transfer_*, encode_* stats
fields real producers instead of misleading zeros. The fields existed in
FrameTimings and PipelineStats already; producers just weren't passing
non-zero values.
- avhw.rs: new EncodeStages { scale_us, transfer_us, encode_us } struct.
EncState::encode_frame (HW VAAPI path) now times the filter graph
separately from avcodec_send_frame, returning EncodeStages. transfer_us
is honestly 0 because the HW path never reads back to CPU.
SwEncState::encode_frame (SW fallback path) returns EncodeStages too;
there import_and_scale bundles GPU scale + GPU→CPU readback into one
call, so scale_us includes transfer for SW. Documented inline.
- state.rs: StreamingEncoder::encode_frame return type bumps from
Result<()> to Result<EncodeStages>; wlr-screencopy path now feeds
real per-stage timings into FrameTimings instead of just total_us.
- state_portal.rs: HW portal path (enc.encode_frame) now extracts
stages.scale_us / stages.transfer_us / stages.encode_us into
FrameTimings. Removed the now-unused t_encode_start binding.
Deferred (documented):
- state_portal.rs SW portal path (line 525) calls import_and_scale +
enc_thread separately and bypasses SwEncState::encode_frame. To wire
scale/transfer timing there too, either route through SwEncState or
thread timing out of import_and_scale. Out of scope for this commit.
- SW path lumps transfer into scale_us. Splitting requires extending
import_and_scale's return type — left as a follow-up if operational
need arises (current default is HW VAAPI).
Oracle audit 2026-06-28 step 4 (option A: integrate, not delete).
All 79 unit tests + 3 integration tests pass. clippy: 0 errors.
Oracle-driven P1 fix plan. Resolves the StatsSnapshot 'computed but never
consumed' debt that was silently zeroing two real diagnostic fields and
leaving a dozen more unreported.
Bug fix (Oracle step 2):
- state_portal.rs: set_pipewire_dropped(0, 0) and set_queue_depths(0, 0)
were hardcoded, silently discarding real PipeWire diagnostics. Now wires
to self.cap.dropped_count() (with pw_dropped_prev delta tracking) and
self.cap.capture_queue_depth(). The encoded side stays 0 because the
encoder thread exposes no queue-depth API.
Display expansion (Oracle step 1):
- stats.rs: StatsSnapshot::Display now reports 12 previously-silent fields
paired with their existing p95/max counterparts — capture/encoded/sent
frame counts, elapsed_secs, *_avg_ms gap timing, frame_age_avg_ms,
per-stage import/sws/encode/total avg_ms, output_frame_bytes_p95. Each
line of the format string maps to one operational question (cadence,
drops, queue pressure, latency, bandwidth); layout note added.
Dead residue purge (Oracle steps 5 + 6):
- stats.rs: removed record_over_budget method + over_budget_count field
(no caller; total_p95_ms answers the useful question without an
arbitrary budget threshold).
- state.rs: removed InFlightSurface::Allocd variant (never constructed)
and CaptureSource::alloc_frame trait method (prototype leftover; the
sole impl in cap_wlr_screencopy.rs returned None unconditionally).
- cap_wlr_screencopy.rs: removed the alloc_frame stub; updated the
unit-type Frame doc to reference the asynchronicity rationale without
the deleted method.
- cap_portal.rs: removed redundant 'let dropped = dropped;' shadowing
flagged by clippy::redundant_locals (line 849).
Deferred (Oracle step 4 — needs product decision):
- scale_avg/scale_p95/transfer_avg/transfer_p95/send_wait_p95 fields
still appear in Display but producers in the live encode path don't
record them, so they often show misleading zeros. Either add real
EncState timing for scale/transfer stages, or remove the fields from
Display until then.
All 79 unit tests + 3 integration tests still pass. clippy: 0 errors.
Warning count: multiple_fields_never_read on StatsSnapshot,
method_never_used on record_over_budget/dropped_count/capture_queue_depth/
alloc_frame, variant_never_constructed on Allocd, redundant_locals on
dropped — all gone.
Audit-driven follow-up after the SAFETY-debt commit (Oracle steps 6-7).
End state: cargo clippy --release --all-targets still 0 errors; private_interfaces
and type_complexity warnings cleared.
Design cleanups (Oracle step 6):
- cap_portal.rs: introduce PortalFormatInfo struct to replace the
Rc<Cell<Option<(u32,u32,u32,u64)>>> cross-callback hand-off. Self-
documenting struct fields replace positional tuple access at the
format-change and process callbacks.
- avhw.rs: import_dma_buf_to_vaapi signature collapses from 8 args
(fd/width/height/drm_format/modifier/stride/offset) to
(*mut AVBufferRef, &PwDmaBufFrame). Callers in avhw.rs,
state_portal.rs, and vaapi_import_bench.rs now pass the frame by
reference instead of unpacking 7 fields just to repack them. Drops
the unused width parameter and the too_many_arguments(8/7) warning.
- state.rs: visibility hygiene. EncConstructionStage and WlrHeadInfo
downgrade pub -> pub(crate); State.stage field downgrades to
pub(crate). These are internal state-machine types not exposed
across the crate boundary; making them pub(crate) clears all
private_interfaces warnings without leaking more types.
Dead-code purge (Oracle step 7):
- transform.rs: remove unused Rect struct, transform_basis,
screen_to_frame, fit_inside_bounds helpers and their 18 dedicated
tests. Transform enum and transpose_if_transform_transposed remain
(both are actively used by state.rs and avhw.rs). File shrinks
from 409 -> 109 lines.
Repository housekeeping (Oracle step 7):
- .gitignore: add review.json (stray review-tool output that
regenerates per run).
- README.md: refresh CLI table to match src/args.rs (now lists
--backend, --no-persist, --port-as-WebRTC-signaling, --max-bitrate,
--stats). Add capture-backend explainer + 4 new usage examples.
Note in README points readers at src/args.rs as the authoritative
source. Remove stale 'WebTransport, unused in MVP' description.
avhw.rs: AsRawFd import annotated with a rustc-quirk explanation — the
import triggers a false 'unused_imports' warning but E0599 if removed.
Left as-is with explanatory comment rather than chasing the lint.
All 79 remaining unit tests + 3 integration tests still pass. Cargo
build --release clean.
Audit-driven cleanup pass. End state:
- cargo clippy --release --all-targets: 0 errors (was 4)
- undocumented_unsafe_blocks warnings: 0 (was 67)
- Cargo.toml: undocumented_unsafe_blocks escalated warn -> deny
Clippy correctness errors fixed:
- src/bin/{sw_encode_bench,vaapi_import_bench}.rs: receive_first_frame
rewritten per Oracle plan with total 10s deadline + 200ms wait slice +
while-let drain of all control events. The previous loop body always
exited on first iteration (never_loop); the new version actually retries
and matches production's repeated-poll semantics in state_portal.rs.
- src/avhw.rs: hash_sampled_y_plane tests now use a row_range(row, stride,
width) helper instead of inline stride * N. Preserves the row-index
intent across all sibling tests without tripping erasing_op (row==0) or
identity_op (row==1).
Machine-applicable clippy autofixes applied via 'cargo clippy --fix':
- unnecessary_cast, manual_is_multiple_of, needless_borrows_for_generic_args
- manual_abs_diff, derivable_impls, new_without_default
- unnecessary_map_or, unneeded_struct_pattern, redundant_locals
webrtc_gop_formula test rewritten to wrap the (fps * 2).max(20) formula in
a runtime lambda. The previous clippy --fix pass had constant-folded the
5fps case into assert_eq!(20, 20), silently stripping the floor-case
coverage. The lambda blocks the fold while keeping the formula exercisable.
67 SAFETY comments added across 7 files (cap_portal.rs 26, sw_encode_bench
21, state_portal.rs 7, vaapi_import_bench.rs 6, avhw.rs 5, state.rs 1,
main.rs 1). Two sites carry load-bearing invariant documentation:
- cap_portal.rs:806 process callback documents the PipeWire raw_buf
ownership contract across all 10 exit paths (audited: every path
correctly requeues; fd ownership via dup() is independent and also
exactly-once closed).
- avhw.rs:341 unsafe impl Send for EncState documents the single-thread
exclusivity assumption referenced by AGENTS.md.
All 97 unit tests + 3 integration tests still pass; cargo build --release
finishes clean. Lint escalation to deny freezes the SAFETY baseline: any
future patch adding an unsafe block without a // SAFETY: comment will fail
clippy at compile time.
Cleanup pass after the latency investigation concluded:
- state_portal.rs: remove TODO(#19) comment block (17 lines). The Option A
upgrade path was tentatively documented during the #19 fix, but the user
decided to accept current latency behavior. The TODO is now noise.
- state.rs:605: remove 'let _fps = self.args.fps as i64;' dead binding left
over from #25 time_base change. The variable became unused when PTS formula
switched from fps-multiplier to literal 90_000, and was renamed to _fps to
silence the warning. Removing it entirely is cleaner.
No behavior change. Build clean (0 new warnings). All 96+96+3 tests pass.
Root cause:
After #24 fixed PTS propagation, browser jitter buffer still accumulated
to 10+ seconds during active mouse movement. User reported: stop moving
mouse, client continues showing motion for ~10 seconds.
The encoder time_base was 1/fps (33ms granularity at 30fps). When KWin
delivers frames at 60fps (16.7ms apart), compute_capture_pts integer
math mapped multiple captures to the same tick. The monotonicity guard
then bumped them to sequential ticks (0, 1, 2, 3, ...).
Result: 60 captures in 1 real second produced 60 sequential RTP
timestamps spanning 60 * 33ms = 1.98 seconds of RTP time. Browser
played at RTP rate (half real speed), buffer accumulated.
Math verification from test8 log:
- 2067 frames * 3000 RTP jumps = 68s of active RTP time
- 61 frames * 54000 RTP jumps = 37s of static RTP time
- Total RTP time 105s vs real time 98.6s (7% inflation in short session;
long active sessions amplify to 50%+ inflation matching user-reported
10-second trailing).
Fix (per Oracle round review):
Change WebRTC encoder time_base from 1/fps to 1/90000 (90kHz). This
matches the RTP video clock directly, providing 11us PTS granularity.
Captures 16.7ms apart now produce distinct ticks (~1500 each), no
quantization, RTP timestamps accurately reflect real time.
Oracle-required revisions incorporated:
1. **set_frame_rate alongside time_base** — libx264 infers fps from
time_base when not explicit. With 1/90000 time_base and no explicit
framerate, x264 would assume ~90000fps and VBV rate control would
break. Setting framerate=fps/1 preserves real frame semantics while
using 90kHz PTS precision.
2. **rtp_timestamp_from_pts_ticks returns u64 not u32** — MediaTime::new
takes u64. Returning u32 would truncate at 13.25 hours and create
backwards MediaTime. str0m handles RTP u32 wrap internally; we feed
it full u64.
3. **wlr-screencopy path also updated** — state.rs:605 used fps-based
PTS formula. Changed to 90kHz ticks so wlr path matches Portal path
unit. Without this, wlr-screencopy users would have wrong PTS after
the time_base change.
4. **MP4 path (create_software_h264_muxer) UNCHANGED** — verified at
avhw.rs:1693-1789, keeps 1/fps time_base, no set_frame_rate added.
File output doesn't need real-time PTS.
Implementation:
- src/avhw.rs: WEBRTC_RTP_CLOCK_HZ=90_000 const; create_software_h264_encoder
uses 1/90000 time_base + explicit framerate
- src/state_portal.rs: compute_capture_pts uses WEBRTC_RTP_CLOCK_HZ for
tick conversion (was fps multiplier)
- src/state.rs: wlr PTS formula uses 90_000 (was fps multiplier)
- src/webrtc.rs: rtp_timestamp_from_pts_ticks simplified to identity
function (pts_ticks.max(0) as u64), drop fps parameter; write_h264_frame
signature drops fps (was only used for rtp conversion); 5 unit tests
updated to assert 90kHz identity (0->0, 1500->1500, 90000->90000)
Verification expectations:
- Active period jitterBufferDelay: 1000+ ms -> < 100 ms
- 'Stop mouse, client continues 10s' symptom: should disappear
- Static period behavior: unchanged (was already correct)
- MP4 file output: unchanged
- VBV-constrained IDR sizes: unchanged (framerate explicit preserves
rate control semantics)
- All prior fixes (#19, #23, #15, #18, #24) preserved
Out of scope (Oracle noted, not blocking):
- build_swenc_filter_graph still uses 1/fps time_base at avhw.rs:1603/1620
(semantic mismatch but no functional impact since scale_vaapi passes
PTS integers through)
- Runtime VBV update on bitrate change (separate pre-existing issue)
Tests:
- cargo build --release: 0 new warnings (23 baseline preserved)
- cargo test: 96 lib + 3 integration, 0 failed
- 5 rtp_timestamp_* tests updated for 90kHz identity
- SAFETY comments preserved verbatim
- 4 files changed, +48/-35 lines
Root cause (3-stage bug found via Oracle round 1+2 review):
Browser WebRTC clients accumulated 2-3 seconds jitter buffer under
damage-driven variable frame rate (KWin Portal/PipeWire). User moved
mouse, saw action 2-3 seconds later on client.
Three coordinated bugs formed a chain that defeated any single-point fix:
1. state_portal.rs:455 used sequential frame counter as PTS instead of
real capture time. (Portal path only — wlr-screencopy already correct.)
2. avhw.rs Channel output sent only Vec<u8>, DISCARDING AVPacket PTS.
Even with correct encoder PTS, timing metadata was thrown away.
3. webrtc.rs:695 computed RTP timestamp as 'frame_number * 90000 / fps'
from a counter, ignoring any real PTS. Browser saw uniform 33ms RTP
spacing regardless of actual 1.6-57fps variable delivery, growing
jitter buffer to compensate for perceived 'network jitter'.
Fix (7-step ordered implementation per Oracle round 2):
1. EncodedH264Frame struct in avhw.rs carries data + pts_ticks
2. FrameOutput::Channel type changed from Sender<Vec<u8>> to
Sender<EncodedH264Frame>; both new_webrtc signatures updated
3. Channel drain in avhw.rs extracts pkt.pts(), normalizes via
'p - start_ts' (mirrors existing Muxer branch logic). Drops
packets with missing PTS instead of silently emitting zero.
4. write_h264_frame signature: frame_number:u64 -> pts_ticks:i64
(both WebRtcState and WebRtcInner layers). Extracted pure function
rtp_timestamp_from_pts_ticks(pts_ticks, fps) with 5 unit tests
covering zero, one-frame, one-second, negative clamp, fps=0.
5. state_portal.rs receiver loop consumes EncodedH264Frame, passes
.data and .pts_ticks to write_h264_frame.
6. state.rs (wlr-screencopy) receiver loop updated for compile
compatibility — its existing real-time PTS computation at state.rs:606
was already correct, now properly propagates through new channel type.
7. state_portal.rs:467 PTS computation GATED on output mode:
- WebRTC branch: compute_capture_pts() uses PipeWire's ns timestamp
(or Instant fallback), normalizes to first-frame-origin, converts
to encoder time_base units with i128 intermediate math, enforces
monotonicity via safe checked_add pattern.
- MP4 branch: KEEPS self.frames_encoded as i64 (sequential counter).
File output does not need real-time PTS; changing it would alter
file playback speed during static periods.
Oracle round 2 critical revisions incorporated:
- Single-point PTS normalization (only in avhw Channel drain), NOT at
source. Avoids double-subtraction with existing Muxer logic.
- MP4 path explicitly preserved — real PTS only applied to WebRTC branch.
- Safe Rust monotonicity guard (no unsafe pointer tricks).
- i64::try_from(ticks_i128) instead of broken i128::try_from(...).unwrap_or(i64::MAX).
- pkt.pts() missing -> log + drop, not silent unwrap_or(0).
- Updates span 4 files (avhw.rs, webrtc.rs, state_portal.rs, state.rs)
because channel type change ripples through both Portal and wlr paths.
Verification expectations:
- Browser jitter buffer should stabilize at 100-500ms (typical) instead
of growing to 2-3 seconds under damage-driven delivery.
- chrome://webrtc-internals: jitterBufferDelay / jitterBufferEmittedCount
ratio should drop significantly.
- Server-side metrics (output_bps, frame rate, IDR size) unchanged.
- MP4 file output (--output mode) behavior unchanged.
Out of scope (deferred):
- frame_age metric fix (Oracle: separate commit to isolate behavioral
change from observability change)
- VFR encoder redesign (1/fps time_base sufficient for this fix)
- MP4 VFR recording (semantic change, separate decision)
- Existing client jitter buffers may not auto-shrink; reconnect may be
required for users with already-accumulated latency
Tests:
- cargo build --release: clean, 0 new warnings (19 pre-existing)
- cargo test: 96 lib + 96 bin + 3 integration, 0 failed
- 5 new RTP unit tests covering edge cases
- SAFETY comments preserved verbatim
- 4 files changed, +157/-27 lines
WebRTC client bandwidth estimate now drives both encoder bitrate and
resolution tier selection, replacing the previous static-target encoder.
- webrtc.rs: enable str0m BWE (seeded at 5 Mbps), surface
EgressBitrateEstimate + KeyframeRequest events, expose
get_bwe_estimate() / set_need_keyframe()
- state_portal.rs: wire bitrate/resolution channels between the WebRTC
thread and the encode thread; tier ladder [1440p, 1080p, 720p] with
downscale at 60% budget and upscale hysteresis (120% sustained 10s)
- avhw.rs: SwEncImport::poll_resolution_commands() rebuilds the import
filter graph on UpdateResolution; SwEncEncode::recreate_encoder()
rebuilds sws/enc_video/yuv_frame atomically; hash_sampled_y_plane()
skips duplicate frames; VBV x264opts cap IDR bursts; H.264 level 4.0
(muxer) / 4.2 (WebRTC)
- state.rs: sync wlr-screencopy GOP to fps*2 max 20 for parity
- fix: drain bitrate_rx + resolution_rx BEFORE the stride check in
encode_cpu_frame() so the new (smaller-stride) frame produced after
a resolution change does not hit the stale (larger) enc_width and
crash the encode thread
- WebRTC GOP widened to fps*2 max 20 (was fps/2 max 10)
- Replace 'let _ = tx.send()' with proper error handling: log warning,
set webrtc_disconnected flag, and break drain loop on SendError
- Add Arc<AtomicBool> webrtc_paused shared between State/StatePortal
and SwEncState, synced from wrtc.is_connected() in poll_webrtc()
- Skip encoding in encode_filtered_frame() when paused or disconnected
- Drain and discard stale channel frames on disconnect
- Resume encoding automatically on WebRTC reconnection
- Add src/webrtc.rs: HTTP signaling server + str0m Sans-IO WebRTC transport
with H.264 Annex-B → RTP packetization and key-frame request handling
- avhw: introduce FrameOutput enum (Muxer | Channel) so SwEncState can
output to either MP4 muxer or crossbeam channel for WebRTC
- cap_portal: support portal session restore tokens (PersistMode::ExplicitlyRevoked)
to skip re-authorization dialog; add --no-persist flag to force fresh dialog
- args: make --output optional when --port is used for WebRTC mode
- state_portal: integrate WebRTC pipeline (encoder channel → RTP forwarding)
with shorter GOP for WebRTC (fps/2, min 10)
- main: redirect tracing to stderr; validate --output or --port required
- Add dependencies: str0m 0.20, serde_json 1, dirs 6
BUG-2 (HIGH): SHM Buffer event caused permanent hang
In the ZwlrScreencopyFrameV1 dispatcher, receiving a SHM Buffer event
left in_flight_surface stuck at AllocQueued forever, preventing
queue_alloc_frame() from requesting new frames.
Fix: treat Buffer as a metadata offer (v3 protocol), wait for
BufferDone to decide failure, and add AllocQueued state guard to
LinuxDmabuf handler.
BUG-3 (MEDIUM): Portal backend picked wrong GPU on multi-GPU systems
state_portal.rs hardcoded /dev/dri/renderD128 then renderD129, which
selects the wrong GPU when PipeWire uses a different device.
Fix: extract find_drm_render_nodes() as shared utility; defer DRM
device selection to first PipeWire frame; test each candidate with
av_hwframe_transfer_data to find the GPU that can actually import
the DMA-BUF frame.
BUG-4 (LOW): VAAPI device context created twice unnecessarily
try_finalize_output() created an AvHwDevCtx stored in EverythingButFmt,
but negotiate_format() discarded it (_hw_device_ctx) and EncState::new
created a new one.
Fix: thread the existing hw_device_ctx through negotiate_format() and
create_encoder() to EncState::new() which reuses it when provided.
Add a second capture backend for compositors without wlr-screencopy
(KWin, GNOME, etc.) using the xdg-desktop-portal ScreenCast interface
and PipeWire DMA-BUF streaming.
New files:
- src/backend_detect.rs: auto-detect wlr-screencopy vs portal backend
- src/cap_portal.rs: Portal session setup + PipeWire DMA-BUF thread
- src/state_portal.rs: StatePortal encoder pipeline (DMA-BUF → VAAPI)
Changes:
- Cargo.toml: add ashpd 0.13, tokio 1, pipewire 0.9, libspa 0.9,
crossbeam-channel 0.5
- src/args.rs: add --backend CLI flag
- src/avhw.rs: extract create_encoder() from inline State code
- src/main.rs: route to portal or wlr-screencopy based on backend
- src/state.rs: fix params.destroy() on dup failure, cleanup
in_flight_surface on copy fail, use create_encoder()
- tests/integration_test.rs: add --backend flag tests
- registry_queue_init consumes registry events during its internal
roundtrip without forwarding them to Dispatch<WlRegistry>. Added
bind_initial_globals() to manually iterate GlobalList and bind all
initial globals (wl_output, xdg_output_manager, dmabuf, screencopy,
wlr_output_manager) at State::new time.
- Fix av_freep segfault in build_filter_graph: av_buffersrc_parameters_alloc
returns a plain pointer, use av_free instead of av_freep (which expects
pointer-to-pointer).
- Fix filter graph format negotiation: remove software format filter that
broke scale_vaapi hardware pipeline. Chain is now src -> scale -> sink.
- Downgrade repeat_pps error to warning (not available in FFmpeg 6.x).
dailz