test(encode): the VAAPI dmabuf path verified on AMD silicon, and its owner-only fields say so
Closes the last unverified leg of the AvBuffer work. `DmabufInner` was the heaviest
ownership change in the crate — four owned objects (DRM device, derived VAAPI device,
DRM-PRIME frames ctx, filter graph) whose eight failure branches each repeated the
same four-line unwind, once inside a macro, plus a ninth copy in `Drop` — and it had
no coverage at all. `vaapi_cpu_encode_smoke` does not reach it: that drives the
swscale/CPU-upload path, which uses `VaapiHw` and never builds a graph.
`dmabuf_inner_alloc_drop_cycles` loops construct/drop, which is the whole contract
now that every handle releases itself. **It passes on a Radeon 780M**, as do the two
pre-existing VAAPI tests, so `VaapiHw` and `DmabufInner` are both hardware-verified
rather than compile-only.
Also fixes three more owner-only fields the AMD box surfaced: `graph`,
`vaapi_device` and `drm_device` are `never read` since the hand-written `Drop` that
used to read them is gone. Same call as the decoders and the QSV pair — annotate
rather than delete (removing `graph` would free it while `src`/`sink` still point
into it) or underscore-rename (which hides what they hold). `drm_frames` is NOT
annotated: `submit` genuinely reads it per frame.
I had missed these twice: my Linux log greps searched "never constructed|never used",
which does not match "never read". The check now includes all three spellings.
Two environment notes worth keeping, both diagnosed by testing OUTSIDE our code
first (ffmpeg's own CLI reproduced each):
* Inside a distrobox on an immutable host, VAAPI needs
`LIBVA_DRIVERS_PATH=/run/host/usr/lib64/dri`. The container's mesa (25.3.6) is
older than the host's (26.0.4) and every encoder open fails with a bare ENOSYS —
"Function not implemented", naming nothing. The test doc says so.
* `cargo check --workspace` in that container fails on `glib-sys` (no GTK dev
headers). Unrelated to this branch; .21 checks the full workspace clean.
Verified: AMD .116 (Radeon 780M, Mesa 26.0.4) full pf-encode suite 33 passed / 0
failed plus all 3 ignored VAAPI hardware tests green. Linux .21 workspace check exit
0 / zero errors, pf-encode 33/0, pf-client-core 34/0, and zero dead-code warnings
across all three spellings.
This commit is contained in:
@@ -744,12 +744,23 @@ struct DmabufInner {
|
||||
// so freed all four ahead of ffmpeg-next dropping the encoder. Every one of these holds its own
|
||||
// reference, so refcounting makes any order sound; the point is that a field reorder must not
|
||||
// silently change what ships. Do not move `enc`, and do not reorder the four below it.
|
||||
/// The filter graph. Owner-only: `src`/`sink` below are borrowed filter contexts the graph owns,
|
||||
/// and everything per-frame goes through those, so nothing reads this field again — it exists so
|
||||
/// the graph outlives them and is freed exactly once. (`dead_code` answered here rather than by
|
||||
/// removing the field, which would free the graph while `src`/`sink` still point into it.)
|
||||
#[allow(dead_code)]
|
||||
graph: AvFilterGraph,
|
||||
/// DRM-PRIME frames context for the imported dmabufs (buffersrc input).
|
||||
/// DRM-PRIME frames context for the imported dmabufs (buffersrc input). Read per frame by
|
||||
/// `submit`, which tags each imported `AVFrame` with a new ref of it.
|
||||
drm_frames: AvBuffer,
|
||||
/// VAAPI device driving `hwmap`/`scale_vaapi`/the encoder.
|
||||
/// VAAPI device driving `hwmap`/`scale_vaapi`/the encoder. Owner-only: the two filters and the
|
||||
/// encoder each took their own ref at open, so nothing reads it again — it keeps the device
|
||||
/// alive behind them.
|
||||
#[allow(dead_code)]
|
||||
vaapi_device: AvBuffer,
|
||||
/// DRM device the source dmabuf frames reference (the buffersrc's `hw_frames_ctx` device).
|
||||
/// Owner-only for the same reason: `drm_frames` holds its own ref on it.
|
||||
#[allow(dead_code)]
|
||||
drm_device: AvBuffer,
|
||||
src: *mut ffi::AVFilterContext,
|
||||
sink: *mut ffi::AVFilterContext,
|
||||
@@ -1348,6 +1359,42 @@ impl Encoder for VaapiEncoder {
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// Construct/drop `DmabufInner` repeatedly on real VAAPI silicon.
|
||||
///
|
||||
/// This is the heaviest ownership change in the crate and the one with no other coverage.
|
||||
/// `open` builds four owned objects — a DRM device, a VAAPI device derived from it, a DRM-PRIME
|
||||
/// frames context, and a filter graph — and each of its eight failure branches used to unwind
|
||||
/// them by hand, the same four-line block copied eight times (once inside a macro) plus a ninth
|
||||
/// copy in `Drop`. All of that is now `AvBuffer`/`AvFilterGraph` field drops in declaration
|
||||
/// order, so *construct and drop* is the entire contract. Looping is what separates the
|
||||
/// outcomes: a double-free trips glibc, a missed one leaks a VAAPI device, a DRM device and a
|
||||
/// whole filter graph per iteration.
|
||||
///
|
||||
/// `vaapi_cpu_encode_smoke` does NOT cover this — it drives the swscale/CPU-upload path, which
|
||||
/// uses `VaapiHw` and never builds a graph.
|
||||
///
|
||||
/// `#[ignore]`d (needs a real VAAPI device):
|
||||
/// `cargo test -p pf-encode dmabuf_inner_alloc_drop_cycles -- --ignored --nocapture`
|
||||
/// ⚠️ Inside a distrobox on an immutable host, also set
|
||||
/// `LIBVA_DRIVERS_PATH=/run/host/usr/lib64/dri` — the container's own mesa is typically older
|
||||
/// than the host kernel's amdgpu and fails every encoder open with a bare ENOSYS.
|
||||
#[test]
|
||||
#[ignore = "needs a real VAAPI device (run on an AMD/Intel host, not the build box)"]
|
||||
fn dmabuf_inner_alloc_drop_cycles() {
|
||||
for i in 0..8 {
|
||||
let inner = DmabufInner::open(Codec::H264, PixelFormat::Bgrx, 640, 480, 30, 8_000_000)
|
||||
.unwrap_or_else(|e| panic!("DmabufInner::open failed on iteration {i}: {e:#}"));
|
||||
assert!(!inner.graph.as_ptr().is_null(), "graph went null");
|
||||
assert!(!inner.drm_frames.as_ptr().is_null(), "drm_frames went null");
|
||||
assert!(
|
||||
!inner.vaapi_device.as_ptr().is_null(),
|
||||
"vaapi_device went null"
|
||||
);
|
||||
assert!(!inner.drm_device.as_ptr().is_null(), "drm_device went null");
|
||||
}
|
||||
eprintln!("8 DmabufInner alloc/drop cycles completed without abort");
|
||||
}
|
||||
|
||||
/// The operator pin tries exactly one mode; a cached resolution ([`LP_MODE`]) tries only the
|
||||
/// mode that worked; anything else runs the full ladder, full-feature FIRST — AMD's first-try
|
||||
/// open must stay byte-for-byte unchanged.
|
||||
|
||||
Reference in New Issue
Block a user