Two stacked refactors on the same branch (16 commits total).
Part 1 — June 2026 (this PR's original scope)
Split monolithic src/avhw.rs into focused src/avhw/* modules while preserving the crate::avhw::* facade and public re-exports (d53e881).
Address cargo-audit findings: bump anyhow to 1.0.103 (823dd53); narrow .cargo/audit.toml exception for two quick-xml advisories pulled in via wayland-scanner (to be removed once upstream supports quick-xml ≥0.41).
CI fixes: libavfilter dev package, cargo git registry index, libclang lookup.
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 079611a)
- cap_portal::fourcc() const fn — superseded by drm_fourcc::DrmFourcc
- PwThreadCtx.fps — destructured as `fps: _`, never consumed
Lifetime/ownership invariants (kept with #[allow(dead_code)] + reason):
- CapPortal.rt — ashpd caches a zbus::Connection in a process-global
OnceCell; runtime must outlive CapPortal or the connection hangs
- EncState.hw_device_ctx — root AVHWDeviceContext; consumers hold their
own ref_clone() but the root ref must stay alive for ownership
False-positive warning (annotated):
- state_portal use AsRawFd — required at FFI boundary but rustc mis-attributes
the call to OwnedFd's inherent method; E0599 if removed
Cosmetic:
- drop 5 redundant `as *const i32` casts on linesize.as_ptr() in
avhw/encode.rs and bin/vaapi_import_bench.rs
Drops PartialOutputInfo.physical_size and .logical_position. After the
warning cleanup in 6332472 these fields were probe-time-only gates with
no downstream consumer (encoder reads geometry from the dmabuf frame
itself, not from wl_output description).
Readiness simplification (try_finalize_output):
- xdg-output path (Sway/Hyprland): done_count >= 2 + name + transform
- wlr-output-management path (niri): done_count >= 1 + wlr_manager_done + transform
Safe because Wayland protocol guarantees Geometry/Mode/Position events
fire before Done, so done_count >= N implies the prior events arrived.
done_count is the real signal; the per-field .is_none() checks were
redundant belt-and-suspenders.
Cascading cleanup of writers that only fed the deleted fields:
- WlOutput::Geometry handler — drop physical_size write, keep transform
- XdgOutputEvent::LogicalPosition arm — deleted
- WlrHeadEvent::Position arm — deleted (was the only reader of
wlr_head_proxy_to_name; both maps now write-only markers)
- WlrHeadInfo.position field — deleted; struct becomes empty marker
Behavior change risk: probe may finalize slightly earlier in cases where
a compositor fires Done before physical_size/logical_position events
(protocol violation, but possible). Verified with cargo test --release
(79 unit + 3 integration, 1 ignored). Hardware/Wayland-session test
deferred to user.
Net: -48 lines.
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 d53e881)
- Stale 'avoid moving wrappers across threads' guidance — Send is sound,
the audit just confirmed why
No code behavior change. Verified: cargo build --release,
cargo clippy --release --all-targets (0 warnings),
cargo test --release (79 unit + 3 integration, 1 ignored).
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.
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
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
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.
dailz
changed title from Refactor avhw encoder module and address cargo audit findings to refactor: decompose oversized modules into directory form (avhw + state + cap_portal + state_portal + webrtc + bench bins)2026-07-14 11:44:38 +08:00
dailz
merged commit 5902df63c2 into master2026-07-14 11:44:56 +08:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
Two stacked refactors on the same branch (16 commits total).
Part 1 — June 2026 (this PR's original scope)
src/avhw.rsinto focusedsrc/avhw/*modules while preserving thecrate::avhw::*facade and public re-exports (d53e881).823dd53); narrow.cargo/audit.tomlexception for two quick-xml advisories pulled in via wayland-scanner (to be removed once upstream supports quick-xml ≥0.41).Part 2 — July 2026 (stacked on Part 1)
File-level decomposition of 4 oversized core modules + bench binary dedup:
src/state.rs(1594 LOC) →src/state/mod.rs(999) +src/state/dispatch/(6 files holding 13 WaylandDispatchimpls).src/cap_portal.rs(1313 LOC) →src/cap_portal.rs(176) + 6 submodules (types / setup / token_fs / pipewire_thread / fourcc / logging).src/state_portal.rs(1241 LOC) →src/state_portal.rs(829) +{bitrate,threads}.rs.src/webrtc.rs(913 LOC) →src/webrtc.rs(741) +html_page.rs.src/bin/common/mod.rs(~120 LOC verbatim duplication removed).avhw::av_err_to_stringfrompub(crate)topubso bench binaries can drop their copy.Intentionally NOT done (per Oracle audit on Part 2):
pipewire_thread3-way fn split — pipewire-rs uses a typestate builder pattern; the listener chain cannot cross a function boundary.try_finalize_output3-way fn split — would forcepub(crate)on 4 invariant-bearing private State fields.Verification (green on the final state)
cargo build --release --binscargo test: 79 lib + 3 integration = 82 pass, 1 ignored (unchanged from baseline)cargo clippy --all-targets -- -D warningscleancargo fmt --checkclean--help// SAFETY:comments preserved with theirunsafeblocksRollback
pre-refactor-baseline(75ad4bb) covers Part 2 only.git revert -m 1 <merge_commit>.Post-merge follow-ups (non-blocking)
cargo docwarnings (cosmetic, existed before either refactor).pub(crate)on ~35 internal fields for cross-submodule access — could be narrowed via accessor methods if desired.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_benchStep 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 binariesUpdate 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.Refactor avhw encoder module and address cargo audit findingsto refactor: decompose oversized modules into directory form (avhw + state + cap_portal + state_portal + webrtc + bench bins)