refactor(client-core): the first fence comes off — video_vulkan.rs is at zero
`video_vulkan.rs` was fenced with the other thirteen GPU/FFI backends, but it never belonged with them: its four sites are not ash calls, they are RAW POINTER DEREFERENCES inside the two FFmpeg `lock_queue`/`unlock_queue` callback trampolines — exactly the case where the lint pays, and where a proof carries an argument instead of restating a signature. Both bodies had none at all. They keep `unsafe extern "C"`: FFmpeg invokes them with a `*mut AVHWDeviceContext`, which is a real contract, now written down. What was missing is why the dereference chain is sound, and it is worth stating because it is not obvious — the trampoline casts between pf_ffvk's `AVHWDeviceContext` and ffmpeg-sys's (the same C struct declared twice), reads `user_opaque`, and dereferences it as a `QueueLock`. That pointer borrows `VkCtxStorage::_queue_lock`, an `Arc<QueueLock>` whose field doc already says it exists to outlive every call the context can make. The proof now connects those two facts, so a future edit to the storage's lifetime has something to contradict. With that, the file needs no exemption and the workspace `deny` covers it: 14 fenced files -> 13. This is what removing a fence is supposed to look like — the sites that are worth narrowing get narrowed, and the exemption disappears rather than being renewed. Verified on .21: fmt + `clippy --workspace --all-targets -- -D warnings` + the feature-gated `-p pf-encode` step, all rc=0 with no allow in the file.
This commit is contained in:
@@ -1,10 +1,4 @@
|
|||||||
//! FFmpeg Vulkan Video decode over the presenter's own VkDevice (zero-copy VkImage).
|
//! FFmpeg Vulkan Video decode over the presenter's own VkDevice (zero-copy VkImage).
|
||||||
// UNSAFE-LINT EXEMPTION (rationale + exit criteria: `unsafe_op_in_unsafe_fn` in the workspace
|
|
||||||
// Cargo.toml). This body is raw libav + ash calls on the presenter's VkDevice almost line for line;
|
|
||||||
// narrowing it would add one `unsafe {}` plus one SAFETY comment per call that could only restate
|
|
||||||
// the signature. Clearing this file means DELETING the markers that carry no caller contract, not
|
|
||||||
// wrapping the calls — until then the lint is off HERE and enforced everywhere else.
|
|
||||||
#![allow(unsafe_op_in_unsafe_fn)]
|
|
||||||
#![allow(clippy::unnecessary_cast)]
|
#![allow(clippy::unnecessary_cast)]
|
||||||
|
|
||||||
use crate::video::{
|
use crate::video::{
|
||||||
@@ -67,25 +61,45 @@ struct VkCtxStorage {
|
|||||||
/// [`VkCtxStorage`], which outlives the context). Replaces FFmpeg's internal default,
|
/// [`VkCtxStorage`], which outlives the context). Replaces FFmpeg's internal default,
|
||||||
/// which only serializes FFmpeg against itself — the presenter submits to the same
|
/// which only serializes FFmpeg against itself — the presenter submits to the same
|
||||||
/// graphics queue from another thread and holds this same lock around its calls.
|
/// graphics queue from another thread and holds this same lock around its calls.
|
||||||
|
///
|
||||||
|
/// # Safety
|
||||||
|
/// FFmpeg calls this with the `AVHWDeviceContext` it owns, whose `user_opaque` we set to a
|
||||||
|
/// `*const QueueLock` before handing the context over.
|
||||||
unsafe extern "C" fn ffvk_lock_queue(
|
unsafe extern "C" fn ffvk_lock_queue(
|
||||||
ctx: *mut pf_ffvk::AVHWDeviceContext,
|
ctx: *mut pf_ffvk::AVHWDeviceContext,
|
||||||
_queue_family: u32,
|
_queue_family: u32,
|
||||||
_index: u32,
|
_index: u32,
|
||||||
) {
|
) {
|
||||||
let dev = ctx as *mut ffmpeg::ffi::AVHWDeviceContext;
|
// SAFETY: `ctx` is the live context FFmpeg passes to its own callback, and the two
|
||||||
let lock = (*dev).user_opaque as *const QueueLock;
|
// `AVHWDeviceContext` declarations (pf_ffvk's and ffmpeg-sys's) describe the same C struct, so
|
||||||
(*lock).lock();
|
// the cast reads the same `user_opaque` field. That field holds the pointer we stored, which
|
||||||
|
// borrows `VkCtxStorage::_queue_lock` — an `Arc<QueueLock>` the storage keeps alive for as long
|
||||||
|
// as the hw device context can fire this trampoline (see its field doc), so the lock outlives
|
||||||
|
// every call FFmpeg can make.
|
||||||
|
unsafe {
|
||||||
|
let dev = ctx as *mut ffmpeg::ffi::AVHWDeviceContext;
|
||||||
|
let lock = (*dev).user_opaque as *const QueueLock;
|
||||||
|
(*lock).lock();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The matching `unlock_queue` trampoline — see [`ffvk_lock_queue`].
|
/// The matching `unlock_queue` trampoline — see [`ffvk_lock_queue`].
|
||||||
|
///
|
||||||
|
/// # Safety
|
||||||
|
/// As [`ffvk_lock_queue`]; additionally, FFmpeg only calls this after a matching `lock_queue`, so
|
||||||
|
/// the lock it releases is one this pair took.
|
||||||
unsafe extern "C" fn ffvk_unlock_queue(
|
unsafe extern "C" fn ffvk_unlock_queue(
|
||||||
ctx: *mut pf_ffvk::AVHWDeviceContext,
|
ctx: *mut pf_ffvk::AVHWDeviceContext,
|
||||||
_queue_family: u32,
|
_queue_family: u32,
|
||||||
_index: u32,
|
_index: u32,
|
||||||
) {
|
) {
|
||||||
let dev = ctx as *mut ffmpeg::ffi::AVHWDeviceContext;
|
// SAFETY: as `ffvk_lock_queue` — same live context from FFmpeg, same `user_opaque` pointer into
|
||||||
let lock = (*dev).user_opaque as *const QueueLock;
|
// the `Arc<QueueLock>` that `VkCtxStorage` keeps alive for the context's whole lifetime.
|
||||||
(*lock).unlock();
|
unsafe {
|
||||||
|
let dev = ctx as *mut ffmpeg::ffi::AVHWDeviceContext;
|
||||||
|
let lock = (*dev).user_opaque as *const QueueLock;
|
||||||
|
(*lock).unlock();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl VulkanDecoder {
|
impl VulkanDecoder {
|
||||||
|
|||||||
Reference in New Issue
Block a user