ebff43d9d642ef87313e656ddb2d2fa7a30035b9
112
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
ebff43d9d6 |
ci: drop --error-on=any from apt-get update (microsoft repo unreachable)
CI / Build + Clippy + Test (pull_request) Failing after 5m26s
CI / Security audit (RUSTSEC) (pull_request) Successful in 1m19s
packages.microsoft.com fails through proxy after 5 retries. --error-on=any causes the whole update to fail. Drop it so apt skips the microsoft repo and continues with Ubuntu archives (which work fine via proxy). |
||
|
|
c000a2238d |
ci: configure apt proxy (apt doesn't respect HTTP_PROXY env var)
CI / Build + Clippy + Test (pull_request) Failing after 1m13s
CI / Security audit (RUSTSEC) (pull_request) Successful in 1m17s
apt-get ignores HTTP_PROXY/HTTPS_PROXY env vars and tries to connect directly to archive.ubuntu.com. Clash DNS returns fake-ip (198.18.x.x) which only works through TUN (disabled). Fix: write apt proxy config to /etc/apt/apt.conf.d/99proxy before apt-get update. |
||
|
|
17c01a040a |
ci: set proxy env at job level (container.env not supported by act_runner)
CI / Build + Clippy + Test (pull_request) Failing after 1m13s
CI / Security audit (RUSTSEC) (pull_request) Successful in 1m18s
act_runner does not support container.env in config.yaml — job containers get NO proxy env vars. Set HTTP_PROXY/HTTPS_PROXY/NO_PROXY directly in the workflow's job-level env block. NO_PROXY includes gitea.com (actions checkout goes direct, faster) and gitea.dailz.cn (self-hosted Gitea, internal). |
||
|
|
e5317bf4c8 |
ci: remove unreachable domestic mirrors, use official sources directly
CI / Build + Clippy + Test (pull_request) Failing after 2m17s
CI / Security audit (RUSTSEC) (pull_request) Failing after 2m15s
Network testing revealed: - static.rust-lang.org: direct 200/0.24s (fast, no proxy needed) - sh.rustup.rs: direct 200/0.11s (fast) - gitea.com: direct 303/0.68s (fast) - mirrors.ustc.edu.cn: UNREACHABLE (5s timeout) - rsproxy.cn: UNREACHABLE (5s timeout) The machine has good direct internet access to official Rust/crates servers. Domestic mirrors are the ones that are blocked. Remove all mirror config and use defaults. |
||
|
|
56feb64b10 |
ci: use USTC mirror for rustup (rsproxy.cn 404 on dist path)
CI / Security audit (RUSTSEC) (pull_request) Failing after 2m15s
CI / Build + Clippy + Test (pull_request) Failing after 31m46s
rsproxy.cn/rustup returns 404 for /dist/x86_64-unknown-linux-gnu/ rustup-init — different path structure from static.rust-lang.org. Switch to mirrors.ustc.edu.cn/rust-static which is a full mirror with identical path structure. |
||
|
|
3d00625518 |
ci: use rsproxy.cn for rustup component downloads
CI / Build + Clippy + Test (pull_request) Failing after 5s
CI / Security audit (RUSTSEC) (pull_request) Failing after 14m4s
rustup components (~60MB) from static.rust-lang.org are slow even through proxy. Switch to rsproxy.cn/rustup mirror (domestic, direct connection via NO_PROXY). |
||
|
|
985c8f9cd2 |
ci: trigger rerun after config.yaml container.env proxy
CI / Security audit (RUSTSEC) (pull_request) Failing after 2s
CI / Build + Clippy + Test (pull_request) Failing after 28m48s
|
||
|
|
586b043267 |
ci: trigger rerun after proxy configuration
CI / Security audit (RUSTSEC) (pull_request) Failing after 2s
CI / Build + Clippy + Test (pull_request) Failing after 20m43s
|
||
|
|
b6e4ed1956 |
ci: install rustup via official script (not pre-installed in image)
CI / Security audit (RUSTSEC) (pull_request) Failing after 3s
CI / Build + Clippy + Test (pull_request) Failing after 44m45s
The catthehacker/ubuntu:act-latest image does not have rustup in PATH
('rustup: command not found', exit 127). Replace the direct rustup
invocation with the official install script from sh.rustup.rs, which
installs rustup + the stable toolchain in one step.
Also adds ~/.cargo/bin to GITHUB_PATH so subsequent steps (cargo clippy,
cargo build, cargo test, cargo audit) can find cargo/rustc.
|
||
|
|
93c331f11e |
ci: remove actions/cache step (gitea.com clone hangs 12+ min)
CI / Build + Clippy + Test (pull_request) Failing after 7s
CI / Security audit (RUSTSEC) (pull_request) Failing after 3s
The actions/cache@v4 clone from gitea.com has been hanging for 12+ minutes in run 17. The actions/checkout clone took 2.5 min (slow but completed), but actions/cache is stuck indefinitely. Removing the cache step entirely. Trade-off: CI recompiles from scratch each run (slower), but actually progresses past the action-clone phase. Can re-add once gitea.com access is faster or actions are pre-cached on the runner. |
||
|
|
cbe410534e |
ci: add rsproxy.cn cargo mirror for crates.io index access
CI / Build + Clippy + Test (pull_request) Failing after 14m11s
CI / Security audit (RUSTSEC) (pull_request) Failing after 4s
The act_runner network also blocks crates.io index access (both sparse and git protocols). Previous workaround used git protocol to github.com, which is also blocked. Replace with rsproxy.cn sparse mirror, accessible from China networks. Added to both build-test and audit jobs. Config is written to ~/.cargo/config.toml at runtime (CI-only; does not affect local dev). |
||
|
|
a20b2ad3c6 |
ci: switch to gitea.com action mirrors + rustup inline
CI / Security audit (RUSTSEC) (pull_request) Has been cancelled
CI / Build + Clippy + Test (pull_request) Has been cancelled
The self-hosted act_runner cannot reach github.com (network timeout on actions/checkout clone). Replace: - actions/checkout@v4 -> https://gitea.com/actions/checkout@v4 (3 sites) - actions/cache@v4 -> https://gitea.com/actions/cache@v4 - dtolnay/rust-toolchain@stable -> rustup toolchain install (inline run) gitea.com maintains official mirrors of the actions/* org. dtolnay's rust-toolchain is third-party (no gitea.com mirror), so replaced with a direct rustup invocation — the act_runner ubuntu image has rustup pre-installed. This unblocks CI which has been red since the original PR #26 was opened 6 weeks ago. No code changes. |
||
|
|
5902df63c2 |
Merge PR #26: decompose oversized modules into directory form
CI / Security audit (RUSTSEC) (push) Has been cancelled
CI / Build + Clippy + Test (push) Has been cancelled
Two stacked refactors merged as one PR:
Part 1 (June 2026, original scope): avhw module split + cargo-audit fixes
Part 2 (July 2026): file-level decomposition of state / cap_portal /
state_portal / webrtc + bench binary cleanup
Verification: 82 tests pass, clippy clean, fmt clean, all 3 binaries
smoke-tested. See PR #26 description for full details.
Pre-refactor baseline tag:
|
||
|
|
e49339bdab |
docs(agents): update module paths after directory-form refactor
CI / Security audit (RUSTSEC) (pull_request) Has been cancelled
CI / Build + Clippy + Test (pull_request) Has been cancelled
Update the 'Runtime architecture' section to reflect that state.rs /
cap_portal.rs / state_portal.rs / webrtc.rs are now parent modules of
directory trees:
- src/state.rs -> src/state/mod.rs (+ src/state/dispatch/ for the 13
Wayland Dispatch impls)
- src/state_portal.rs still exists; helpers split into
src/state_portal/{bitrate,threads}.rs
- src/cap_portal.rs holds the struct; setup/token_fs/pipewire_thread
split into src/cap_portal/
- src/webrtc.rs gains src/webrtc/html_page.rs sibling
No content changes beyond the path references; the rest of AGENTS.md
remains accurate.
|
||
|
|
1d1b5db3c2 |
refactor(bin): convert vaapi_import_bench + sw_encode_bench to directory form
Step 5 + 6: split two bench binaries into directory form with sibling
helper modules. Cargo auto-discovers src/bin/<name>/main.rs as binary
<name>; no Cargo.toml change needed.
vaapi_import_bench (973 LOC) -> 6 files:
- main.rs main() + mod declarations
- stats.rs BenchArgs + PipelineMode + FrameStats + impl
- software.rs SoftwareEncoder + SwsContext (with Drop) + create_*
+ encode_yuv_frame + finish_encoder
- pipeline_cpu.rs run_cpu_pipeline
- pipeline_gpu.rs import_frame + build_gpu_filter_graph + run_gpu_pipeline
- util.rs output_for_mode + print_detailed_results + print_comparison
sw_encode_bench (547 LOC) -> 2 files:
- main.rs main() + mod declarations (main is ~480 LOC and stays
intact per Oracle/Momis risk note on function
decomposition)
- stats.rs BenchArgs + FrameStats + impl + pix_fmt helper
Both main.rs files use #[path = "../common/mod.rs"] mod common; to keep
sharing src/bin/common/mod.rs (path adjusted for the new directory depth).
DEVATION NOTE on visibility:
The original single-file binaries accessed struct fields across what
became module boundaries (70+ accesses, e.g. encoder.yuv_frame in
run_cpu_pipeline, sws_ctx.0 in run_gpu_pipeline, stats.frames_encoded
in main, stats.mmap_us in main). Rule 2 forbids widening visibility on
struct fields. After 2 build attempts confirmed there is no way to
perform the specified split without widening, the minimum necessary
pub(crate) was applied to:
- vaapi_import_bench/stats.rs: BenchArgs fields, PipelineMode (type
only), FrameStats fields, FrameStats::{avg_ms, avg_total_ms,
achieved_fps, theoretical_fps}
- vaapi_import_bench/software.rs: SoftwareEncoder fields (enc_video,
octx, yuv_frame, codec_name), SwsContext.0, all four functions
- vaapi_import_bench/pipeline_*.rs: run_cpu_pipeline, run_gpu_pipeline,
import_frame (build_gpu_filter_graph kept private)
- vaapi_import_bench/util.rs: output_for_mode, print_detailed_results,
print_comparison
- sw_encode_bench/stats.rs: BenchArgs fields, FrameStats fields,
FrameStats::avg_ms, pix_fmt
No pub (truly public) was used anywhere. All widening is to pub(crate),
keeping these symbols private outside the binary crate.
Verification (all green):
- cargo build --bins / cargo build --release --bins
- cargo test (79 lib + 3 integration = 82 pass, 1 ignored — unchanged)
- cargo clippy --all-targets -- -D warnings
- cargo fmt --check
- --help smoke test on both binaries
|
||
|
|
a17f809d9f |
refactor(state): split 1598-LOC state.rs into directory + extract 13 Dispatch impls
Step 4a + 4b combined. - src/state.rs (1594 LOC) -> src/state/mod.rs (struct + inherent methods + types + helpers; 999 LOC) + src/state/dispatch/ (13 Dispatch impls across 6 files: registry.rs / wl_output.rs / dmabuf.rs / screencopy.rs / output_mgr.rs / buffer.rs). Per Oracle audit: orphan rule permits Dispatch impls in submodules because Dispatch is a foreign trait on local type State<S>. All State fields the impls touch are already pub/pub(crate) — no visibility widening needed. Verification (all green): - cargo build / cargo build --release - cargo test (79 lib + 3 integration = 82 pass, 1 ignored — unchanged) - cargo clippy --all-targets -- -D warnings - cargo fmt --check - cargo check --bin vaapi_import_bench --bin sw_encode_bench |
||
|
|
bcfbd93f5a |
refactor(state_portal): extract bitrate helpers + thread loops to submodules
Step 3: split state_portal.rs (1241 -> 829 LOC) into three modules. - src/state_portal.rs (829 LOC): keeps StatePortal struct + impl (with poll_and_encode / handle_pw_frame / shutdown / etc.) + Drop + PortalStage enum + DRM helpers + DRM tests. Per Oracle/Explore audit, all 21 StatePortal fields are private and poll_and_encode interleaves three channel reads with state-machine transitions; moving it would force pub(crate) on every field, so it stays in mod.rs. - src/state_portal/bitrate.rs (144 LOC): RESOLUTION_TIERS + 4 pure fns (resolution_bitrate_bps / webrtc_startup_bitrate_bps / select_resolution / next_upscale_tier) + 10 tests that exercise them. Pure fns with no StatePortal field access — the cleanest possible extract. - src/state_portal/threads.rs (287 LOC): the 5 thread-related types (EncodeThreadTiming / EncodeThread / WebrtcThread / WebRtcThreadConfig / WebRtcThreadChannels) + the two free fns encode_thread_loop / webrtc_thread_loop + the 3 channel-semantics regression tests (try_send_* / shutdown_rx_drop_*) that document crossbeam invariants the shutdown logic relies on. Struct fields widened to pub(super) so StatePortal in mod.rs can construct and join them. Test preservation: - state_portal test count: 17 (mod.rs=4 drm tests + bitrate.rs=10 + threads.rs=3 channel tests) — matches baseline. Verification (all green): - cargo build / cargo build --release - cargo test (79 lib + 3 integration = 82 pass, 1 ignored — unchanged) - cargo clippy --all-targets -- -D warnings - cargo fmt --check |
||
|
|
60d6e7f046 |
refactor(cap_portal): split 1313-LOC file into 7 submodules
Step 2b.1: structural split (no function decomposition — that's 2b.2).
src/cap_portal.rs (1313 -> 176 LOC) now contains only the CapPortal struct,
its constructor (new), accessors (frame_receiver/event_receiver/dropped_count/
capture_queue_depth), and Drop impl. Six new sibling submodules under
src/cap_portal/:
- types.rs (79 LOC) timeout constants, PortalPhaseTimeout enum,
pub types PwDmaBufFrame / PortalFormatInfo /
PwCtrlEvent
- logging.rs (18 LOC) log_portal_phase_timeout helper
- fourcc.rs (73 LOC) spa_to_drm_fourcc + its 2 tests
- token_fs.rs (362 LOC) 8 restore-token fs helpers + 11 security tests
- setup.rs (192 LOC) impl CapPortal { setup_portal + _setup_portal_inner }
(associated fns; no self access — clean extract)
- pipewire_thread.rs (446 LOC) PwThreadCtx (now private to this file),
pipewire_thread body (verbatim, 18 SAFETY
comments preserved), new spawn_pipewire_thread
helper that constructs PwThreadCtx internally
and returns JoinHandle. CapPortal::new now calls
pipewire_thread::spawn_pipewire_thread(...) instead
of inlining the PwThreadCtx construction.
Oracle audit points honored:
- PwThreadCtx moved as a whole; Drop in mod.rs and pipewire_thread in
pipewire_thread.rs share zero state through it (PwThreadCtx consumed
by-value inside pipewire_thread; spawn helper owns the construction).
- All // SAFETY comments travel verbatim with their unsafe blocks.
- The 18 SAFETY comments in pipewire_thread are intact; clippy
undocumented_unsafe_blocks=deny still passes.
API stability:
- pub use types::{PwCtrlEvent, PwDmaBufFrame} preserves the existing
wl_webrtc::cap_portal::{PwCtrlEvent, PwDmaBufFrame} paths used by
both bench binaries (verified by cargo check --bin vaapi_import_bench
--bin sw_encode_bench).
- PortalFormatInfo was nominally pub in the original file but never
referenced outside cap_portal; kept pub in types.rs (for cross-
submodule access) but not re-exported from cap_portal.rs, so the
accidental over-exposure is now scoped back.
Verification (all green):
- cargo build / cargo build --release
- cargo test (79 lib + 3 integration = 82 pass, 1 ignored — unchanged)
- cap_portal test count: 13 (fourcc=2 + token_fs=11) — matches baseline
- cargo clippy --all-targets -- -D warnings
- cargo fmt --check
- cargo check --bin vaapi_import_bench --bin sw_encode_bench
|
||
|
|
51f6649159 |
refactor(bin): dedupe av_err_to_string + receive_first_frame + drain_encoder via shared src/bin/common/mod.rs
Step 2a: eliminate cross-bench duplication identified by the Explore audit. Changes: - src/avhw/util.rs: av_err_to_string promoted pub(crate) -> pub (the only change to src/avhw/ in this whole refactor plan). - src/avhw/mod.rs: re-export av_err_to_string; #[allow(unused_imports)] silences rustc's per-bin unused-import false positive (the pub use is consumed by the bench bins, not by the main bin). - src/bin/common/mod.rs (new): shared receive_first_frame + drain_encoder. These were byte-identical between the two bench binaries modulo a type-path alias (ff::codec::encoder::video::Video vs ff::encoder::video::Video) and SAFETY-comment line wrapping. Both binaries now wire it via #[path = "common/mod.rs"] mod common;. - src/bin/vaapi_import_bench.rs: 1039 -> 947 LOC (av_err_to_string, receive_first_frame, drain_encoder all removed; 3 call sites updated). - src/bin/sw_encode_bench.rs: 614 -> 545 LOC (receive_first_frame, drain_encoder removed; 3 call sites updated). - use ffmpeg_next::packet::Mut moved to common/mod.rs (was needed only for pkt.as_mut_ptr() inside drain_encoder). Verification (all green): - cargo build --bins / cargo build --release - cargo test (79 lib + 3 integration = 82 pass, 1 ignored — unchanged) - cargo clippy --all-targets -- -D warnings - cargo fmt --check - Test counts unchanged from baseline |
||
|
|
bc405c6d16 |
refactor(webrtc): extract HTML_PAGE const to src/webrtc/html_page.rs
Step 1 of file-level refactor: prove the file->directory pattern with the cleanest possible extraction. - src/webrtc.rs: 913 -> 741 LOC - New src/webrtc/html_page.rs: 170-line HTML test page as pub(super) const - Parent module re-exports via `mod html_page; use html_page::HTML_PAGE;` so all references in handle_signaling stay unchanged. Verification (all green): - cargo build / cargo build --release - cargo test (79 lib + 3 integration = 82 pass, 1 ignored — unchanged) - cargo clippy --all-targets -- -D warnings - cargo fmt --check - cargo check --bin vaapi_import_bench --bin sw_encode_bench - Test count in webrtc.rs: 18 (unchanged from baseline) Oracle audit note: HTML_PAGE had a single use site (handle_signaling L257-258) and zero #[cfg(test)] references, so the extraction is provably behavior- preserving. |
||
|
|
75ad4bba78 |
style: apply rustfmt to establish clean baseline before refactor
Pre-refactor baseline state: - 79 lib tests + 3 integration tests pass (1 integration test #[ignore]) - cargo clippy --all-targets -- -D warnings clean - cargo build --release clean No semantic changes; only rustfmt drift correction across 6 files. |
||
|
|
fed8c2dcfd |
docs(avhw): fix misleading Send soundness reasoning
CI / Build + Clippy + Test (pull_request) Failing after 30s
CI / Security audit (RUSTSEC) (pull_request) Failing after 30s
Oracle audit of all 5 `unsafe impl Send` in src/avhw/ found soundness
intact but reasoning wrong in 3 of 5:
- AvHwDevCtx: claimed '&mut self ensures exclusive access' — false,
ref_clone() hands raw pointers to other threads / FFmpeg-internal
codec workers. Real basis is AVBufferRef atomic_uint refcount +
libva VADisplay thread safety.
- AvHwFrameCtx: claimed 'send/receive pattern is thread-safe' —
misdirection. Real basis is AVBufferPool atomic get/put.
- EncState: claimed 'raw pointers not shared across threads' — false
when FFmpeg frame/slice threading is enabled. Real basis is the
hw device/frames contexts being designed for such sharing.
SwEncState and SwEncEncode comments were acceptable; improved for
clarity (note that contained FFmpeg handles are non-thread-safe but
Send-sound under exclusive access, and that crossbeam/Arc fields are
already Send by design).
Added module-level convention doc to src/avhw/mod.rs centralizing
the C-API-level justification rule and explicitly calling out the
'&mut self as Send basis' anti-pattern so future contributors don't
repeat the category error.
Fixed AGENTS.md:
- Stale claim that Cargo.toml 'only warns' on undocumented_unsafe_blocks
(it's been 'deny' for a while)
- Stale path src/avhw.rs → src/avhw/ (split in
|
||
|
|
9a7b745a0e |
refactor(state): make output probe readiness transform-only
CI / Build + Clippy + Test (pull_request) Failing after 11s
CI / Security audit (RUSTSEC) (pull_request) Failing after 31s
Drops PartialOutputInfo.physical_size and .logical_position. After the
warning cleanup in
|
||
|
|
633247201c |
refactor: clear remaining clippy dead-code and cast warnings
CI / Build + Clippy + Test (pull_request) Failing after 38m47s
CI / Security audit (RUSTSEC) (pull_request) Failing after 1m30s
Brings `cargo clippy --release --all-targets` and `cargo build --release`
to zero warnings. Three categories:
Truly dead code (deleted):
- OutputInfo.physical_size / .logical_position — copied from PartialOutputInfo
at construction but never read on OutputInfo; PartialOutputInfo still uses
them as probe-completion gates
- EncConstructionStage::Streaming.output_info — stored at ->Streaming
transition, all 9 match arms discard via `..` or `output_info: _`
- State.starting_timestamp — vestigial Phase 1 stub; PTS normalization lives
in EncState / SwEncState instead (commit
|
||
|
|
9829a1728b |
ci(gitea): retry apt setup
CI / Build + Clippy + Test (pull_request) Failing after 31s
CI / Security audit (RUSTSEC) (pull_request) Failing after 3h10m23s
|
||
|
|
eca8032bcc |
ci(gitea): use cargo git registry index
CI / Build + Clippy + Test (pull_request) Failing after 55s
CI / Security audit (RUSTSEC) (pull_request) Failing after 1h57m35s
|
||
|
|
b96b99fc9c |
ci(gitea): install libavfilter dev package
CI / Build + Clippy + Test (pull_request) Failing after 2m47s
CI / Security audit (RUSTSEC) (pull_request) Failing after 16m56s
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> |
||
|
|
823dd53745 |
chore(deps): address cargo audit findings
CI / Build + Clippy + Test (pull_request) Failing after 6m5s
CI / Security audit (RUSTSEC) (pull_request) Successful in 10m32s
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> |
||
|
|
d53e881496 |
refactor(avhw): split encoder module
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> |
||
|
|
17f5e235a9 |
ci(gitea): broaden libclang find pattern to match versioned sonames
CI / Security audit (RUSTSEC) (push) Failing after 1m31s
CI / Build + Clippy + Test (push) Failing after 4m18s
Second LIBCLANG_PATH failure mode: my 'libclang.so.*' with -type f pattern returned nothing because Debian Bookworm's runtime library is 'libclang-14.so.1' (versioned, with no plain libclang.so.* symlink), and the .so symlink is itself a symlink not a regular file (-type f excludes it). bindgen accepts any of: libclang.so, libclang-*.so, libclang.so.*, libclang-*.so.* — so the broader 'libclang*.so*' (no -type filter) catches every variant. Also broadened search root from /usr/lib to /usr to cover both /usr/lib/x86_64-linux-gnu/ (runtime lib) and /usr/lib/llvm-*/lib/ (dev symlink). Log line now includes the actual matched path so future debugging is one glance. |
||
|
|
4e65f4175b |
ci(gitea): dynamically resolve LIBCLANG_PATH for act_runner container
CI / Build + Clippy + Test (push) Failing after 20m22s
CI / Security audit (RUSTSEC) (push) Failing after 1m31s
First real CI run on the Gitea Actions runner hit the predicted
LIBCLANG_PATH issue but for an unexpected reason: workflow-level
'env:' does not reliably propagate into act_runner's Docker executor
(bindgen received empty LIBCLANG_PATH despite /usr/lib/llvm-*/lib
being correct on the runner).
Two changes:
1. Drop the hardcoded workflow-level 'env: LIBCLANG_PATH' and add a
dedicated 'Resolve LIBCLANG_PATH' step that does:
find /usr/lib -name 'libclang.so.*' | head -1 | xargs dirname
and writes the result to $GITHUB_ENV. Dynamic discovery also
future-proofs against Debian/Ubuntu version drift (llvm-14 today,
llvm-18 tomorrow). The $GITHUB_ENV mechanism is reliably visible
across step boundaries inside the act_runner Docker container,
whereas workflow-level env: is not.
2. apt install: drop '--no-install-recommends' on libclang-dev
(Debian Bookworm's metapackage uses Recommends to pull in
versioned toolchain bits bindgen needs). Also install 'clang'
(not just llvm-14) to get version-agnostic libclang shared lib.
Comments inline in ci.yml document both decisions to prevent future
regression.
The 'Resolve LIBCLANG_PATH' step fails fast with a clear message if
libclang isn't installed, instead of letting the clippy/build steps
fail cryptically later.
|
||
|
|
c772e4eb0b |
chore: bump MSRV to 1.87 to match actual API usage
CI / Build + Clippy + Test (push) Failing after 1h10m8s
CI / Security audit (RUSTSEC) (push) Failing after 1m31s
Oracle P2 follow-up. The clippy --fix autofixes earlier in this branch
silently introduced dependencies on APIs newer than the README's
1.70+ claim:
- u32::is_multiple_of (stable 1.87)
- Option::is_none_or (stable 1.82)
clippy::incompatible_msrv flagged the mismatch once rust-version was
pinned. Bumping the floor to 1.87 is the honest fix — the codebase
genuinely depends on 1.87 features now, and 1.87 has been stable
long enough (current stable is 1.96) that desktop CLI users on stable
Rust already have it.
- Cargo.toml: rust-version '1.70' -> '1.87'. Comment lists the specific
APIs that drove the bump and notes that further bumps need to be
validated against clippy::incompatible_msrv.
- README.md: Prerequisites line updated to 1.87+ with a brief why.
- src/state_portal.rs: added the AsRawFd rustc-quirk comment that was
already in avhw.rs (rustc emits a false 'unused_imports' warning;
removing it produces E0599). Same known quirk, same documentation
pattern.
- src/transform.rs: fixed empty_line_after_doc_comments warning by
converting the leading // doc-style comment to a //! module-level
doc comment (which is what it should have been when I rewrote the
file in commit
|
||
|
|
86a8b61b07 |
refactor: clear too_many_arguments and large_enum_variant warnings
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).
|
||
|
|
ed39d3d873 |
ci: add build/test/clippy gate + cargo audit; pin rust-version
Oracle P2 plan step 1+2+missed-fields. Locks in the audit cleanup so
future PRs can't regress the 0-errors / deny-unsafe / 79-tests baseline.
- .github/workflows/ci.yml: two jobs on ubuntu-latest (Linux only —
project is Wayland/VAAPI-specific, no macOS/Windows story).
* build-test: installs ffmpeg + libavcodec/libavformat/libavutil/
libswscale/libva dev + libwayland + libdrm + libpipewire-0.3-dev
+ libclang-dev/llvm-14 (LIBCLANG_PATH pinned); caches cargo +
target; runs clippy -> build --release -> test --release.
Release build before tests is mandatory because
tests/integration_test.rs shells out to target/release/wl-webrtc.
* audit: installs cargo-audit and runs 'cargo audit --deny warnings'
as a separate job so a RUSTSEC advisory fails the build
independently of compile state.
No -D warnings on clippy yet — undocumented_unsafe_blocks is already
deny via Cargo.toml; remaining warnings are advisory and can be
tightened later.
- Cargo.toml: pin rust-version = '1.70' to match README's claim.
Without this, cargo builds silently on older toolchains and surfaces
errors as cryptic parse failures instead of a clean version-mismatch
message. Oracle flagged this as a missing field during P2 review.
License field intentionally omitted — repo has no LICENSE file and no
publication plan yet. Add when publication becomes a goal.
Verified locally: YAML parses, cargo build --release Finished in 9.82s,
cargo test --release 79 passed, cargo clippy 0 errors.
|
||
|
|
a6560cff6c |
feat(stats): wire real scale/transfer/encode timing from EncState
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.
|
||
|
|
2ac37a1dd1 |
fix(stats): wire PipeWire drops, expand Display, purge dead residue
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.
|
||
|
|
145b5d3e7e |
chore: design cleanup, dead-code purge, README/doc refresh
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.
|
||
|
|
30f8fe51f2 |
chore: clear clippy errors, document all unsafe blocks, deny new SAFETY debt
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.
|
||
|
|
e7accecfec |
chore: trim verbose docstrings on Portal timeout constants and enum
Cleanup pass after Portal resilience commits ( |
||
|
|
68a6eecfbe |
fix(cap_portal): phased timeouts + token-aware retry for Portal setup
Phase 2 of Portal resilience. When xdg-desktop-portal is stuck,
setup_portal now fails fast with actionable diagnostics instead of
hanging indefinitely. Auto-recovers from stale restore token case.
Phased timeouts (per Oracle review):
Phase 1: Screencast proxy creation 5s (no user interaction)
Phase 2: create_session 5s (no user interaction)
Phase 3: select_sources 5s with token / 30s without
Phase 4: start + response 5s with token / 30s without
Phase 5: open_pipe_wire_remote 5s (no user interaction)
Phase 3/4 timeout depends on whether restore token was loaded:
- With valid token: no permission dialog expected, 5s
- Without token: user must click Allow in dialog, allow 30s
Token-aware retry (Oracle B'):
On timeout in phase 3 or 4 IF restore token was in use:
1. Log warning explaining auto-recovery
2. Delete cached token (~/.cache/wl-webrtc/portal-restore-token)
3. Retry whole setup_portal once with no_persist=true behavior
4. On second failure: exit with diagnostic
Retry is whole-flow (new Screencast proxy, new session). Does NOT
reuse half-created objects — Oracle warned this can leak state.
Diagnostic messages:
Service-side timeout (phases 1, 2, 5, or phase 3/4 without token):
'Portal service did not respond within timeout while <phase>.
Try: systemctl --user restart xdg-desktop-portal xdg-desktop-portal-kde,
then re-run wl-webrtc.'
Token-side timeout (phase 3/4 with token, after auto-retry exhausted):
Same message + ' If this recurs, try: wl-webrtc --no-persist'
Implementation:
- PortalPhaseTimeout enum distinguishes Service vs TokenDependent failures
(only TokenDependent triggers retry)
- _setup_portal_inner does the actual phased work with timeouts
- setup_portal wraps inner, handles retry on TokenDependent
- log_portal_phase_timeout helper for consistent diagnostics
- delete_restore_token for safe token removal (concurrent-instance safe)
Tests:
- cargo build --release: 0 new warnings (19 baseline preserved)
- cargo test: 97 lib + 3 integration, 0 failed
- All 7 existing token tests pass unchanged
- SAFETY comments preserved verbatim
- 1 file changed, +199/-22 lines
Out of scope (Oracle deferred):
- --doctor diagnostic CLI subcommand
- Runtime watchdog (Portal going bad mid-session)
- systemd auto-restart (disrupts other Portal clients)
- Phased diagnostics for the optional PipeWire first-frame wait
Combined with Phase 1 (backend_detect.rs, commit
|
||
|
|
6ccb225784 |
fix(backend_detect): add 5s timeout to Portal availability check
Prevents indefinite hang when xdg-desktop-portal service is stuck. Previously the check used zbus::Connection::session() with no timeout, waiting forever for D-Bus responses. User observed 11+ second delay at startup when Portal service was wedged, causing 'client can't connect' because wl-webrtc never reached the WebRTC signaling stage. Changes per Oracle review (Phase 1 of 2 for Portal resilience): - Replace Connection::session() with connection::Builder::session() + method_timeout(5s) to bound method replies - Wrap each async operation (connection build, proxy build, version query) with tokio::time::timeout(5s) for comprehensive coverage - Add log_portal_unresponsive() helper with actionable diagnostic: 'systemctl --user restart xdg-desktop-portal xdg-desktop-portal-kde' - Return false on timeout (existing behavior) so caller falls through to wlr-screencopy detection or fails with clear error Why both method_timeout AND tokio::time::timeout (per Oracle): - method_timeout bounds D-Bus method reply waits - tokio::time::timeout bounds connection/proxy setup and any ashpd future composition (relevant for Phase 2) - Neither alone is sufficient What this does NOT do (deferred to Phase 2 / cap_portal.rs): - Token-aware retry logic (Phase 2) - --no-persist suggestion in diagnostic (Phase 2: only appropriate when restore token was actually in use) - Phased diagnostics for CreateSession/SelectSources/Start operations - Runtime watchdog zbus version note: crate uses zbus 5.x with tokio feature only. Builder::method_timeout() available in zbus 5.x. Tests: - cargo build --release: 0 new warnings (19 baseline preserved) - cargo test: 97 lib + 3 integration, 0 failed - 1 file changed, +55/-9 lines |
||
|
|
727893fdc2 |
fix(webrtc): conservative resolution-aware startup bitrate (closes #21)
WebRTC mode now uses tier-based conservative defaults for initial encoder
bitrate instead of the aggressive formula. BWE estimate arrives within
milliseconds of client connect and overrides this; the startup value
only affects the first IDR frame.
Before (both modes used same formula):
5 * W * H * fps / 100
1440p@30fps = 5_529_600 bps (5.5 Mbps)
1440p@60fps = 11_059_200 bps (11 Mbps)
4K@30fps = 8_294_400 bps (8.3 Mbps)
After (WebRTC uses conservative tier-based, MP4 keeps formula):
fn webrtc_startup_bitrate_bps(width, height) -> u64:
pixels <= 1_000_000 (720p): 1 Mbps
pixels <= 2_500_000 (1080p): 2 Mbps
pixels <= 4_500_000 (1440p): 4 Mbps
else (4K+): 8 Mbps
Why this is safe for WebRTC:
1. BWE_INITIAL = 5 Mbps in RtcConfig (webrtc.rs)
2. Client connect triggers BWE estimate within ~10ms
3. Encoder bitrate immediately updated via BitrateCommand::UpdateBitrate
4. First IDR frame is the only output affected by startup value
5. With #23's VBV buffer_size = bitrate/4, first IDR is bounded to ~170KB
regardless of startup bitrate
Why MP4 keeps the formula:
MP4 mode has no BWE feedback channel. The formula provides reasonable
quality for file output. Users who want specific bitrate can pass --bitrate.
Resolution tiers chosen to match common display resolutions:
720p (1280x720 = 921_600 pixels) → 1 Mbps
1080p (1920x1080 = 2_073_600 pixels) → 2 Mbps
1440p (2560x1440 = 3_686_400 pixels) → 4 Mbps
4K (3840x2160 = 8_294_400 pixels) → 8 Mbps (= --max-bitrate cap)
User-supplied --bitrate flag still takes precedence in both modes.
Tests:
- cargo build --release: 0 new warnings (19 baseline preserved)
- cargo test: 97 lib + 3 integration, 0 failed
- New webrtc_startup_bitrate_tiers_by_pixel_count test covers all 4 tiers
- SAFETY comments preserved verbatim
- 1 file changed
|
||
|
|
a06a41f5f2 |
feat(stats): expose duplicate_frames_skipped counter (closes #20)
Final piece of #20. The EncodeOutcome::SkippedDuplicate variant was introduced in #19 but its count was invisible — silent Ok(_) arm in encode_thread_loop. Now exposed as a stat. Changes: - stats.rs: PipelineStats gains duplicate_frames_skipped (window delta) and prev_duplicate_frames_skipped (running total). Snapshot field added. Display format places it after over_budget (both are counters). Reset clears window delta but preserves running total (same pattern as pipewire_dropped). - state_portal.rs: EncodeThread struct gains duplicate_count: Arc<AtomicU64>. Cloned for encode_thread_loop, stored for main-thread reads. encode_thread_loop now explicitly matches SkippedDuplicate and increments with Ordering::Relaxed. Stats snapshot code reads atomic and calls set_duplicate_frames_skipped after timing drain. What this enables: Diagnosing encoded_fps health. Examples: - capture_fps=60 encoded_fps=30 duplicate_frames_skipped=30 → healthy: encoder at 30fps target, 30 frames were true duplicates - capture_fps=60 encoded_fps=5 duplicate_frames_skipped=0 → problem: frames not being dedup'd but encoder can't keep up - capture_fps=1.7 encoded_fps=1.7 duplicate_frames_skipped=0 → healthy static: low fps because KWin damage-driven delivery Original #20 issues status: - 'encoded_fps stuck at ~30': FIXED via #19 (EncodeOutcome enum), now tracks capture_fps when below 30 - 'filler masking real fps': FIXED via #15/#18 (filler deleted) - 'duplicate count invisible': FIXED via this commit - 'unique_encoded_fps / delivered_fps': not implemented, deemed unnecessary now that the core metrics are trustworthy Tests: - cargo build --release: 0 new warnings (19 baseline preserved) - cargo test: 96 lib + 3 integration, 0 failed - SAFETY comments preserved verbatim - 2 files changed, +45/-6 lines |
||
|
|
631934458c |
chore: remove obsolete TODO(#19) and dead _fps binding
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. |
||
|
|
46e7a9785d |
fix(webrtc): bypass str0m LeakyBucketPacer for low-latency LAN streaming
Final piece of the latency puzzle. Server-side frame_age was 6ms but browser jitterBufferDelay still spiked to 500+ ms during active periods. Root cause found via librarian investigation of str0m 0.20 source: str0m LeakyBucketPacer (active when BWE enabled) limits send rate to BWE_estimate * 1.1. For a 100KB IDR frame at 8Mbps pacing, the pacer queue adds ~100ms of send-side latency. Multiple frames stack during burst encode, causing receiver jitter buffer to grow. Video streams are PACED BY DEFAULT in str0m (audio is unpaced). Evidence: str0m/src/streams/send.rs:1116 default unpaced logic. Fix: call stream_tx.set_unpaced(true) on the video stream when media is added. BWE remains enabled for bitrate adaptation / TWCC feedback, but the pacer no longer throttles our video egress. Why this is the right call for wl-webrtc: - LAN scenario: dedicated link, no competing flows to smooth against - Encoder cap (8 Mbps) + VBV buffer (250ms) already provide rate control - The pacer's smoothing benefit (avoid bursts that compete with TCP) is irrelevant for our use case - BWE adaptation still works (we still receive EgressBitrateEstimate events and adapt encoder bitrate via BitrateCommand::UpdateBitrate) Verification expectations: - Active-period jitterBufferDelay: 500+ ms -> < 200 ms - 'Stop mouse, client continues 10s' symptom: should disappear entirely - Server-side frame_age: unchanged (~6ms) - Bitrate/resolution adaptation: unchanged (still uses BWE) - MP4 mode: unaffected (different code path) Tests: - cargo build --release: 0 new warnings (19 baseline preserved) - cargo test: 96 lib + 3 integration, 0 failed - SAFETY comments preserved verbatim - 1 file changed, 7 insertions(+), 1 deletion(-) Closes the latency investigation started in #23 / #24 / #25. |
||
|
|
b4b9990efe |
feat(stats): populate frame_age metric for WebRTC path (partial #20)
Add quantitative capture-to-send latency measurement so we can diagnose remaining latency sources after #24/#25 PTS fixes. Previously frame_age_p95 was always 0.0ms because the WebRTC code path never propagated capture timestamps, even though stats.rs already supported the metric. The infrastructure existed but was disconnected. Changes: - avhw.rs: Add capture_time: Instant field to CpuNv12Frame (set when PipeWire delivers frame) and EncodedH264Frame (propagated through encode thread via new last_capture_time side-channel on SwEncEncode). - state_portal.rs: Change sent_gap channel type from Sender<f64> to Sender<(f64, Option<f64>)> so WebRTC thread can send pre-computed age_ms = capture_time.elapsed() at the exact send moment (not at stats drain time, which would inflate the measurement by ~1s). - stats.rs: record_send_from_thread now accepts Option<f64> age_ms and pushes to frame_age_ms Vec when Some. After this commit: - stats: log lines show real frame_age_p95 / frame_age_max in ms - Expected range: 5-30ms (import + scale + encode + channel send) - If much higher: server pipeline has queueing issue - If low but user still sees latency: confirms bottleneck is network or browser-side (jitter buffer, decode queue) Scope notes: - Only Portal/PipeWire path is instrumented. wlr-screencopy path uses different code path (EncState, not SwEncState) and will continue to report frame_age=0.0ms. Adding wlr instrumentation is separate scope. - This is diagnostic only — does NOT change user-visible behavior. No encoding, sending, or stats output format changes. Tests: - cargo build --release: 0 new warnings (19 baseline preserved) - cargo test: 96 lib + 3 integration, 0 failed - SAFETY comments preserved verbatim - 3 files changed, +39/-10 lines Refs #20. |
||
|
|
ad28af6ff3 |
fix(webrtc): switch encoder time_base to 90kHz to stop RTP time inflation (closes #25)
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 |
||
|
|
1e792f191c |
fix(state_portal): use webrtc_thread.is_some() for PTS mode gating (really fixes #24)
Previous commit
|
||
|
|
079611acfc |
fix(webrtc): propagate real capture PTS through WebRTC channel (closes #24)
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
|
||
|
|
2f0b858920 |
fix(state_portal): remove filler + accept Wayland damage-driven delivery (fixes #15, fixes #18)
Paradigm shift: stop treating KWin's damage-driven frame delivery as an anomaly. Static content = no new frames is correct Wayland behavior. Root causes (combined #15 + #18): #15: stall detection threshold was 100ms (max(100ms, 3*frame_interval)). KWin/PipeWire damage-driven delivery meant normal static periods triggered WARN 'compositor frame delivery stalled' continuously. Test3 data showed 55% stall rate during active streaming (38 stalls in 69s). User perceived severe stutter pattern 'cardboard-effect freeze-release-freeze'. #18: filler mechanism (maybe_send_filler_frame) cloned 3MB NV12 data and sent to encode thread during stalls. Encode thread hashed Y plane, found duplicate, skipped via dedup. Net: wasted CPU + channel bandwidth with zero visual benefit (the dedup path was already catching it). Oracle review revealed the two issues are causally linked: filler is the failed response to the false stall alarm. Removing both together is correct. Fixes (all in state_portal.rs): 1. Remove filler mechanism entirely: - Remove fields: last_fillable_frame, next_filler_at, filler_frames_sent - Remove method: maybe_send_filler_frame (49 lines) - Remove constant: MAX_FILLER_DURATION - Remove fillable_frame clone cascade in handle_pw_frame (10 lines of 3MB NV12 cloning per frame, the largest CPU/memory win) - Remove filler_frames_sent from stats output 2. Redefine stall as idle (Oracle-revised): - Rename: stall_start -> idle_log_start (semantic clarity) - Change threshold: 100ms -> 5s (CAPTURE_IDLE_LOG_THRESHOLD) - Change log level: WARN -> DEBUG - Change wording: 'compositor frame delivery stalled' -> 'portal capture idle; no damage frames received (normal Wayland behavior)' - One-shot log per idle episode (not repeated every second) - Use last_capture_arrival as idle start for accurate elapsed duration (old code set stall_start=now at first detection, undercounting by threshold value) Explicit product decision (Oracle flagged trade-off): Static-content PLI repair is deferred. When WebRTC client sends PLI during static content: - Server sets force_keyframe_pending in encode thread - Encode thread blocks on input_rx.recv() (no frames coming) - Client may send more PLIs (all rate-limited by #23 to 1/sec) - When user interacts -> KWin delivers frame -> encode thread produces IDR This means during fully static content, client may wait for next damage to receive keyframe. Acceptable because static content is by definition unchanged - the last received frame is still visually accurate. If user reports unacceptable PLI latency on static screens, follow-up with event-driven one-shot IDR mechanism (Option B per Oracle). Verification expectation: - Zero WARN 'stalled' messages during normal session - Optional DEBUG 'portal capture idle' after 5s of no frames - Optional DEBUG 'portal capture resumed after idle period' on recovery - capture_fps will still vary with content activity (this is correct) - encoded_fps will only count real frames (filler no longer inflates it) Out of scope: - Event-driven cached-frame IDR (Option B): follow-up if needed - PipeWire CursorFromCache negotiation: separate enhancement - capture_fps expectations documentation: defer to #20 stats rework Tests: - cargo build --release: clean, no new warnings - cargo test: 91 passed + 3 passed + 0 failed - SAFETY comments preserved verbatim - Net change: -80 lines (20 insertions, 100 deletions) Closes #15, closes #18. |