Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions electron/native/pipewire-capture/src/capture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions electron/native/pipewire-capture/src/dmabuf_import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
30 changes: 28 additions & 2 deletions electron/native/pipewire-capture/src/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -244,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();
Expand Down Expand Up @@ -276,12 +283,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, String> {
Self::open_backend(Backend::Vaapi, &params, Some((device, frames_ctx)))
let mut encoder = Self::open_backend(Backend::Vaapi, &params, Some((device, frames_ctx)))?;
encoder.upload_is_safe = vaapi_is_safe_to_probe();
Ok(encoder)
}

fn open_backend(
Expand Down Expand Up @@ -314,6 +329,7 @@ impl VideoEncoder {
sw_frame: ptr::null_mut(),
hw_frame: ptr::null_mut(),
hw_staged: ptr::null_mut(),
upload_is_safe: true,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
sws: ptr::null_mut(),
sws_src_format: ff::AV_PIX_FMT_NONE,
packet: ptr::null_mut(),
Expand Down Expand Up @@ -508,6 +524,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
Expand Down
Loading