From 5e85d96fb2ccd23ba6beda13e3fa2496c3b68eb5 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Mon, 14 Sep 2026 11:47:49 +0200 Subject: [PATCH 1/2] fix(capture-linux): re-apply the vaMapBuffer2 guard to the dmabuf encoder's CPU upload The dmabuf path opens its encoder through open_importing, which skips the ladder's vaapi_is_safe_to_probe guard. The import itself only maps, so it never reaches vaMapBuffer2. But since ea22c149 a CPU frame staged on that encoder (after a renegotiation to a modifier-less format) is uploaded into the importer's pool with av_hwframe_transfer_data, the call that aborts the process on a libva without the symbol (#576). open_importing now records the guard and VideoEncoder::stage refuses a CPU frame when it failed, before hw_staged is freed, so the recording ends with an error and a playable file instead of SIGABRT. Comments at the three bypass points state the invariant, as #534 asks. Fixes #534 --- .../native/pipewire-capture/src/capture.rs | 8 ++++++ .../pipewire-capture/src/dmabuf_import.rs | 7 ++++++ .../native/pipewire-capture/src/encoder.rs | 25 ++++++++++++++++++- 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/electron/native/pipewire-capture/src/capture.rs b/electron/native/pipewire-capture/src/capture.rs index ab6ef115a..8eb8b7b01 100644 --- a/electron/native/pipewire-capture/src/capture.rs +++ b/electron/native/pipewire-capture/src/capture.rs @@ -297,6 +297,14 @@ impl Capture { // for a monitor, or the window's crop rectangle for a window. The // encoder is FORCED to VAAPI — the only backend that can consume the // mapped surface; a non-VAAPI machine never negotiates dmabuf. + // + // This branch never goes through `VideoEncoder::open`, so it skips the + // ladder's `vaMapBuffer2` guard, as `dmabuf_import::available` does. + // That is safe only while nothing here uploads a CPU frame: the import + // maps and never transfers, and `open_importing` re-applies the guard + // to the one upload left, in `VideoEncoder::stage`. On a libva without + // the symbol this branch is also the ONLY route to hardware H.264, so + // anything that disables it there downgrades to software (issue #534). let importer = crate::dmabuf_import::DmabufImporter::new( desc.width, desc.height, diff --git a/electron/native/pipewire-capture/src/dmabuf_import.rs b/electron/native/pipewire-capture/src/dmabuf_import.rs index c3dc23553..4dc60412f 100644 --- a/electron/native/pipewire-capture/src/dmabuf_import.rs +++ b/electron/native/pipewire-capture/src/dmabuf_import.rs @@ -10,6 +10,13 @@ //! The importer owns the VAAPI device and the filtergraph. The encoder is opened //! against [`Self::output_frames_ctx`] so the NV12 surface this produces is one //! `avcodec_send_frame` accepts directly. See docs/dmabuf-vaapi-plan.md. +//! +//! NEVER READ OR WRITE A SURFACE FROM SYSTEM MEMORY HERE. Nothing on this path, +//! `available()` included, consults the `vaMapBuffer2` guard in encoder.rs +//! (`vaapi_is_safe_to_probe`). It gets away with that only because it maps and +//! never transfers. A CPU fallback added here, such as `av_hwframe_transfer_data` +//! or a map into a software frame, would reach that symbol unguarded and abort +//! the helper on a libva that lacks it (issues #534 and #576). use crate::ffmpeg as ff; use std::os::fd::AsRawFd; diff --git a/electron/native/pipewire-capture/src/encoder.rs b/electron/native/pipewire-capture/src/encoder.rs index 8e197dddd..8ac41049e 100644 --- a/electron/native/pipewire-capture/src/encoder.rs +++ b/electron/native/pipewire-capture/src/encoder.rs @@ -210,6 +210,10 @@ pub struct VideoEncoder { /// and held across the clock-driven re-encodes until the next frame replaces /// it. Null on the shm/software path. hw_staged: *mut ff::AVFrame, + /// False only for an encoder from [`Self::open_importing`] on a libva + /// without `vaMapBuffer2`, where [`Self::stage`] refuses CPU frames because + /// their upload would abort the process (issue #534). + upload_is_safe: bool, sws: *mut ff::SwsContext, sws_src_format: ff::AVPixelFormat, packet: *mut ff::AVPacket, @@ -276,12 +280,20 @@ impl VideoEncoder { /// /// SAFETY: `device` and `frames_ctx` must be a live VAAPI device and an NV12 /// VAAPI frames context on it; the encoder takes its own references. + /// + /// BYPASSES THE LADDER'S `vaMapBuffer2` GUARD. The importer only maps, so + /// nothing it does reaches that symbol. A CPU frame staged later (after a + /// renegotiation to a modifier-less format) would be uploaded into this pool + /// through `av_hwframe_transfer_data`, which does. So the guard is taken here + /// and enforced in [`Self::stage`], the only way to reach that upload. pub unsafe fn open_importing( params: VideoParams, device: *mut ff::AVBufferRef, frames_ctx: *mut ff::AVBufferRef, ) -> Result { - Self::open_backend(Backend::Vaapi, ¶ms, Some((device, frames_ctx))) + let mut encoder = Self::open_backend(Backend::Vaapi, ¶ms, Some((device, frames_ctx)))?; + encoder.upload_is_safe = vaapi_is_safe_to_probe(); + Ok(encoder) } fn open_backend( @@ -314,6 +326,7 @@ impl VideoEncoder { sw_frame: ptr::null_mut(), hw_frame: ptr::null_mut(), hw_staged: ptr::null_mut(), + upload_is_safe: true, sws: ptr::null_mut(), sws_src_format: ff::AV_PIX_FMT_NONE, packet: ptr::null_mut(), @@ -508,6 +521,16 @@ impl VideoEncoder { stride: usize, src_format: ff::AVPixelFormat, ) -> Result<(), String> { + // Before anything is touched, `hw_staged` above all: `Capture::finish` + // re-sends that last imported surface as its tail write, so refusing here + // ends the recording with a playable file instead of a core dump. + if !self.upload_is_safe { + return Err(concat!( + "a CPU frame reached the dmabuf encoder, and libva.so.2 does not export ", + "vaMapBuffer2: uploading it would abort the process" + ) + .to_owned()); + } // The LAST row needs only its own pixels, not a further stride's worth of // padding. Demanding `stride * height` rejected exactly the frames a // window crop produces: `pixels` there starts partway into the buffer, so From ad4af0337e4aa56f5c0d7621e8fec5e66ee3768d Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Mon, 14 Sep 2026 18:09:15 +0200 Subject: [PATCH 2/2] fix(capture-linux): apply the vaMapBuffer2 guard to a forced VAAPI encoder too --- electron/native/pipewire-capture/src/encoder.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/electron/native/pipewire-capture/src/encoder.rs b/electron/native/pipewire-capture/src/encoder.rs index 8ac41049e..a2e041bfe 100644 --- a/electron/native/pipewire-capture/src/encoder.rs +++ b/electron/native/pipewire-capture/src/encoder.rs @@ -248,7 +248,10 @@ impl VideoEncoder { let mut failures = Vec::new(); for backend in candidates { - if backend == Backend::Vaapi && forced.is_none() && !vaapi_is_safe_to_probe() { + // Forced too: every CPU frame on this path is uploaded, so a forced VAAPI + // encoder on such a libva would abort on its first frame instead of + // failing here with a readable reason. + if backend == Backend::Vaapi && !vaapi_is_safe_to_probe() { let reason = "libva.so.2 does not export vaMapBuffer2, so this ffmpeg build \ would abort inside VA-API rather than fail cleanly" .to_owned();