Commit Graph
12 Commits
Author SHA1 Message Date
dailz 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: 75ad4bb (covers Part 2 only).
2026-07-14 11:44:56 +08:00
dailz 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.
2026-07-14 10:32:43 +08:00
dailz 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
2026-07-13 19:33:07 +08:00
dailz 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
2026-07-13 19:09:39 +08:00
dailz 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
2026-07-13 16:35:44 +08:00
dailz 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
2026-07-13 16:20:56 +08:00
dailz 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
2026-07-13 16:04:30 +08:00
dailz 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.
2026-07-13 15:47:42 +08:00
dailz 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.
2026-07-13 15:40:54 +08:00
dailz 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 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).
2026-07-09 15:20:16 +08:00
dailz 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 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.
2026-07-09 14:48:10 +08:00
dailz 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 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
2026-07-09 14:36:10 +08:00