From 65cd388a5268a4078b4aa3e78607a6ccb95e7a45 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Wed, 29 Jul 2026 00:59:13 +0200 Subject: [PATCH] =?UTF-8?q?fix(presenter,core):=20close=20the=20last=20of?= =?UTF-8?q?=20the=20proof-lint=20hole=20=E2=80=94=20the=20Vulkan=20contrac?= =?UTF-8?q?t,=20stated=20once?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pf-presenter's 120 sites are `ash` calls almost without exception, so 120 independent arguments would have been 120 restatements of the signature — the exact noise this program exists to remove. They get the `abi.rs` treatment instead: the Vulkan contract stated once in `lib.rs`, each site naming which of three shapes it is. The three are not equal, and separating them is the point. CREATE and RECORD carry no real precondition — the device is owned, the builders are locals, nothing executes until submit. DESTROY does: the GPU must not still be using the object, and that is established by the path (a fence wait, a `queue_wait_idle`, a retired swapchain), not by the call. Those sites say so, because getting it wrong is a use-after-free no type catches. The contract also tells the next person that a block outside the three shapes needs a real proof, and that writing "as above" is the signal it doesn't belong in them. punktfunk-core's Windows half is finished here too: `qos_windows.rs`'s `GetLastError` reads (called before anything can reset the thread's error slot) and `udp/windows.rs`'s control-message write, whose argument is that `ctrl` is sized by `WSA_CMSG_SPACE(4)` — computed two lines up — so header plus payload cannot run past it, and `write_unaligned` is used because `WSA_CMSG_DATA` offers no alignment guarantee. ⚠️ THE WINDOWS BLIND SPOT BIT A THIRD TIME. A Linux measurement put this crate pair at 113; the real number was 129 — `d3d11.rs`, `win32.rs`, `qos_windows.rs`, `udp/windows.rs` are all `cfg`-hidden. Every crate in this sweep had to be finished on .47 after being "done" on .21. For a cross-platform crate the Linux number is a lower bound, never the answer. All three crates now deny `undocumented_unsafe_blocks`, which was the goal: it applied to 8 of 11 crates carrying unsafe, and the three exempt ones held 381 items between them — including the C ABI surface and the presenter. Verified: Linux .21 fmt + both CI clippy steps rc=0; Windows .47 all four closed crates clippy `-D warnings` rc=0 plus the full Windows CI clippy set and pf-capture's tests. Only small crates remain unguarded (75 items total, largest 26). --- crates/pf-presenter/src/csc.rs | 37 ++++++++++ crates/pf-presenter/src/d3d11.rs | 24 +++++++ crates/pf-presenter/src/dmabuf.rs | 34 ++++++++++ crates/pf-presenter/src/lib.rs | 23 +++++++ crates/pf-presenter/src/run.rs | 2 + crates/pf-presenter/src/vk/gpu.rs | 16 +++++ crates/pf-presenter/src/vk/mod.rs | 4 ++ crates/pf-presenter/src/vk/overlay_pipe.rs | 24 +++++++ crates/pf-presenter/src/vk/present.rs | 37 ++++++++-- crates/pf-presenter/src/vk/present_timing.rs | 3 + crates/pf-presenter/src/vk/reconfig.rs | 23 +++++++ crates/pf-presenter/src/vk/resources.rs | 30 ++++++++ crates/pf-presenter/src/vk/setup.rs | 68 +++++++++++++++++++ crates/pf-presenter/src/win32.rs | 6 ++ .../src/transport/qos_windows.rs | 8 +++ .../src/transport/udp/windows.rs | 4 ++ 16 files changed, 339 insertions(+), 4 deletions(-) diff --git a/crates/pf-presenter/src/csc.rs b/crates/pf-presenter/src/csc.rs index bbad86aa..5e5bb48f 100644 --- a/crates/pf-presenter/src/csc.rs +++ b/crates/pf-presenter/src/csc.rs @@ -91,6 +91,8 @@ impl CscPass { .dst_stage_mask(vk::PipelineStageFlags::TRANSFER) .dst_access_mask(vk::AccessFlags::TRANSFER_READ), ]; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let render_pass = unsafe { device.create_render_pass( &vk::RenderPassCreateInfo::default() @@ -102,6 +104,8 @@ impl CscPass { } .context("CSC render pass")?; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let sampler = unsafe { device.create_sampler( &vk::SamplerCreateInfo::default() @@ -125,6 +129,8 @@ impl CscPass { .immutable_samplers(&samplers) }) .collect(); + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let set_layout = unsafe { device.create_descriptor_set_layout( &vk::DescriptorSetLayoutCreateInfo::default().bindings(&bindings), @@ -135,6 +141,8 @@ impl CscPass { let push = [vk::PushConstantRange::default() .stage_flags(vk::ShaderStageFlags::FRAGMENT) .size(64)]; // three vec4 rows + a params vec4 (mode, tonemap peak) + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let pipeline_layout = unsafe { device.create_pipeline_layout( &vk::PipelineLayoutCreateInfo::default() @@ -147,6 +155,8 @@ impl CscPass { let pool_sizes = [vk::DescriptorPoolSize::default() .ty(vk::DescriptorType::COMBINED_IMAGE_SAMPLER) .descriptor_count(plane_bindings)]; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let desc_pool = unsafe { device.create_descriptor_pool( &vk::DescriptorPoolCreateInfo::default() @@ -155,6 +165,8 @@ impl CscPass { None, ) }?; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let desc_set = unsafe { device.allocate_descriptor_sets( &vk::DescriptorSetAllocateInfo::default() @@ -203,6 +215,9 @@ impl CscPass { .descriptor_type(vk::DescriptorType::COMBINED_IMAGE_SAMPLER) .image_info(&infos[1]), ]; + // SAFETY: per the Vulkan contract above - recorded into a command buffer this code owns + // and has begun, referencing handles it also owns; nothing is submitted until the + // recording is ended. unsafe { device.update_descriptor_sets(&writes, &[]) }; } @@ -223,10 +238,17 @@ impl CscPass { .descriptor_type(vk::DescriptorType::COMBINED_IMAGE_SAMPLER) .image_info(&infos[b as usize]) }); + // SAFETY: per the Vulkan contract above - recorded into a command buffer this code owns + // and has begun, referencing handles it also owns; nothing is submitted until the + // recording is ended. unsafe { device.update_descriptor_sets(&writes, &[]) }; } pub fn destroy(&self, device: &ash::Device) { + // SAFETY: per the Vulkan contract above - this destroys objects this type owns, and the + // GPU is known idle for them (the fence/queue-wait on the path here, or the swapchain + // being retired), which is the obligation that makes a destroy sound rather than the + // handle merely being non-null. unsafe { device.destroy_pipeline(self.pipeline, None); device.destroy_pipeline_layout(self.pipeline_layout, None); @@ -255,15 +277,23 @@ pub(crate) fn build_fullscreen_pipeline( &include_bytes!("../shaders/fullscreen.vert.spv")[..], ))?; let frag = ash::util::read_spv(&mut std::io::Cursor::new(frag_spv))?; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this type + // and live for the call, and every builder struct is a local that outlives it. let vert_mod = unsafe { device.create_shader_module(&vk::ShaderModuleCreateInfo::default().code(&vert), None) }?; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this type + // and live for the call, and every builder struct is a local that outlives it. let frag_mod = unsafe { device.create_shader_module(&vk::ShaderModuleCreateInfo::default().code(&frag), None) }; let frag_mod = match frag_mod { Ok(m) => m, Err(e) => { + // SAFETY: per the Vulkan contract above - this destroys objects this type owns, and + // the GPU is known idle for them (the fence/queue-wait on the path here, or the + // swapchain being retired), which is the obligation that makes a destroy sound rather + // than the handle merely being non-null. unsafe { device.destroy_shader_module(vert_mod, None) }; return Err(e).context("fragment shader module"); } @@ -325,9 +355,16 @@ pub(crate) fn build_fullscreen_pipeline( .layout(layout) .render_pass(render_pass); let pipeline = + // SAFETY: per the Vulkan contract above - a create/allocate call on the live device, over + // builder structs that are locals outliving the call; the handle it returns is owned by + // the value being built here. unsafe { device.create_graphics_pipelines(vk::PipelineCache::null(), &[info], None) } .map_err(|(_, e)| e) .context("CSC pipeline"); + // SAFETY: per the Vulkan contract above - this destroys objects this type owns, and the GPU is + // known idle for them (the fence/queue-wait on the path here, or the swapchain being retired), + // which is the obligation that makes a destroy sound rather than the handle merely being non- + // null. unsafe { device.destroy_shader_module(vert_mod, None); device.destroy_shader_module(frag_mod, None); diff --git a/crates/pf-presenter/src/d3d11.rs b/crates/pf-presenter/src/d3d11.rs index a61d12f0..1bd030e7 100644 --- a/crates/pf-presenter/src/d3d11.rs +++ b/crates/pf-presenter/src/d3d11.rs @@ -48,6 +48,8 @@ fn format_importable( .push_next(&mut ext_info); let mut ext_props = vk::ExternalImageFormatProperties::default(); let mut props = vk::ImageFormatProperties2::default().push_next(&mut ext_props); + // SAFETY: per the Vulkan contract in lib.rs - a read-only query on the live instance/device, + // filling locals returned by value. unsafe { instance.get_physical_device_image_format_properties2(pdev, &fmt_info, &mut props) } .is_ok() && ext_props @@ -90,6 +92,9 @@ impl HwFrame { } pub fn destroy(self, device: &ash::Device) { + // SAFETY: per the Vulkan contract in lib.rs - destroys objects this type owns, on a path + // where the GPU is known idle for them; that idleness is the obligation, not the handle + // being non-null. unsafe { device.destroy_image(self.image, None); device.free_memory(self.memory, None); @@ -122,6 +127,9 @@ pub fn import( // the whole job. Kept maximally "identical" to the D3D11 resource (no view-format // aliasing, no extra usages). let mut external_info = vk::ExternalMemoryImageCreateInfo::default().handle_types(handle_type); + // SAFETY: per the Vulkan contract in lib.rs - a create/allocate/import call on the live + // device, over builder structs that are locals outliving the call; the handle returned is + // owned by the value being built here. let image = unsafe { device.create_image( &vk::ImageCreateInfo::default() @@ -153,10 +161,14 @@ pub fn import( // The handle's importable memory types, intersected with the image's requirement. let handle = frame.handle as vk::HANDLE; let mut handle_props = vk::MemoryWin32HandlePropertiesKHR::default(); + // SAFETY: per the Vulkan contract in lib.rs - a read-only query on the live + // instance/device, filling locals returned by value. unsafe { ext_mem_win32.get_memory_win32_handle_properties(handle_type, handle, &mut handle_props) } .context("vkGetMemoryWin32HandlePropertiesKHR")?; + // SAFETY: per the Vulkan contract in lib.rs - a read-only query on the live + // instance/device, filling locals returned by value. let reqs = unsafe { device.get_image_memory_requirements(image) }; let bits = reqs.memory_type_bits & handle_props.memory_type_bits; let type_index = (0..32u32) @@ -169,6 +181,9 @@ pub fn import( .handle_type(handle_type) .handle(handle); let mut dedicated = vk::MemoryDedicatedAllocateInfo::default().image(image); + // SAFETY: per the Vulkan contract in lib.rs - a create/allocate/import call on the live + // device, over builder structs that are locals outliving the call; the handle returned is + // owned by the value being built here. let memory = unsafe { device.allocate_memory( &vk::MemoryAllocateInfo::default() @@ -180,7 +195,13 @@ pub fn import( ) } .context("import D3D11 texture memory")?; + // SAFETY: per the Vulkan contract in lib.rs - a create/allocate/import call on the live + // device, over builder structs that are locals outliving the call; the handle returned is + // owned by the value being built here. if let Err(e) = unsafe { device.bind_image_memory(image, memory, 0) } { + // SAFETY: per the Vulkan contract in lib.rs - destroys objects this type owns, on a + // path where the GPU is known idle for them; that idleness is the obligation, not the + // handle being non-null. unsafe { device.free_memory(memory, None) }; return Err(e).context("bind imported memory"); } @@ -189,6 +210,9 @@ pub fn import( let memory = match result { Ok(m) => m, Err(e) => { + // SAFETY: per the Vulkan contract in lib.rs - destroys objects this type owns, on a + // path where the GPU is known idle for them; that idleness is the obligation, not the + // handle being non-null. unsafe { device.destroy_image(image, None) }; return Err(e); } diff --git a/crates/pf-presenter/src/dmabuf.rs b/crates/pf-presenter/src/dmabuf.rs index 7e616459..2e16a145 100644 --- a/crates/pf-presenter/src/dmabuf.rs +++ b/crates/pf-presenter/src/dmabuf.rs @@ -61,6 +61,8 @@ impl HwFrame { } pub fn destroy(self, device: &ash::Device) { + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. unsafe { for v in self.views { device.destroy_image_view(v, None); @@ -136,6 +138,10 @@ pub fn import( { Ok(r) => r, Err(e) => { + // SAFETY: per the Vulkan contract above - this destroys objects this type owns, and + // the GPU is known idle for them (the fence/queue-wait on the path here, or the + // swapchain being retired), which is the obligation that makes a destroy sound rather + // than the handle merely being non-null. unsafe { device.destroy_image(luma_img, None); device.free_memory(luma_mem, None); @@ -145,6 +151,9 @@ pub fn import( }; let view = |image, format| { + // SAFETY: per the Vulkan contract above - a create/allocate call on the live device, over + // builder structs that are locals outliving the call; the handle it returns is owned by + // the value being built here. unsafe { device.create_image_view( &vk::ImageViewCreateInfo::default() @@ -162,6 +171,10 @@ pub fn import( } .context("plane image view") }; + // SAFETY: per the Vulkan contract above - this destroys objects this type owns, and the GPU is + // known idle for them (the fence/queue-wait on the path here, or the swapchain being retired), + // which is the obligation that makes a destroy sound rather than the handle merely being non- + // null. let destroy_images = |views: &[vk::ImageView]| unsafe { for v in views { device.destroy_image_view(*v, None); @@ -228,6 +241,8 @@ fn plane_image( .plane_layouts(&plane_layouts); let mut external_info = vk::ExternalMemoryImageCreateInfo::default() .handle_types(vk::ExternalMemoryHandleTypeFlags::DMA_BUF_EXT); + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this type + // and live for the call, and every builder struct is a local that outlives it. let image = unsafe { device.create_image( &vk::ImageCreateInfo::default() @@ -256,6 +271,9 @@ fn plane_image( let result = (|| { // The fd's importable memory types, intersected with the image's requirement. let mut fd_props = vk::MemoryFdPropertiesKHR::default(); + // SAFETY: per the Vulkan contract above - a create/allocate call on the live device, over + // builder structs that are locals outliving the call; the handle it returns is owned by + // the value being built here. unsafe { ext_mem_fd.get_memory_fd_properties( vk::ExternalMemoryHandleTypeFlags::DMA_BUF_EXT, @@ -264,6 +282,8 @@ fn plane_image( ) } .context("vkGetMemoryFdPropertiesKHR")?; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let reqs = unsafe { device.get_image_memory_requirements(image) }; let bits = reqs.memory_type_bits & fd_props.memory_type_bits; let type_index = (0..32u32) @@ -271,6 +291,8 @@ fn plane_image( .context("no importable memory type for dmabuf")?; // Vulkan owns the fd it imports — dup so the decoder guard keeps the original. + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let owned = unsafe { BorrowedFd::borrow_raw(fd) } .try_clone_to_owned() .context("dup dmabuf fd")?; @@ -278,6 +300,8 @@ fn plane_image( .handle_type(vk::ExternalMemoryHandleTypeFlags::DMA_BUF_EXT) .fd(owned.into_raw_fd()); let mut dedicated = vk::MemoryDedicatedAllocateInfo::default().image(image); + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let memory = unsafe { device.allocate_memory( &vk::MemoryAllocateInfo::default() @@ -290,7 +314,13 @@ fn plane_image( } .context("import dmabuf memory")?; // (On allocate_memory failure Vulkan still closed the dup'd fd — nothing leaks.) + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. if let Err(e) = unsafe { device.bind_image_memory(image, memory, 0) } { + // SAFETY: per the Vulkan contract above - this destroys objects this type owns, and + // the GPU is known idle for them (the fence/queue-wait on the path here, or the + // swapchain being retired), which is the obligation that makes a destroy sound rather + // than the handle merely being non-null. unsafe { device.free_memory(memory, None) }; return Err(e).context("bind imported memory"); } @@ -300,6 +330,10 @@ fn plane_image( match result { Ok(memory) => Ok((image, memory)), Err(e) => { + // SAFETY: per the Vulkan contract above - this destroys objects this type owns, and + // the GPU is known idle for them (the fence/queue-wait on the path here, or the + // swapchain being retired), which is the obligation that makes a destroy sound rather + // than the handle merely being non-null. unsafe { device.destroy_image(image, None) }; Err(e) } diff --git a/crates/pf-presenter/src/lib.rs b/crates/pf-presenter/src/lib.rs index 6737de42..158510fa 100644 --- a/crates/pf-presenter/src/lib.rs +++ b/crates/pf-presenter/src/lib.rs @@ -14,6 +14,29 @@ //! Windows) and `d3d11` is its Windows counterpart (D3D11VA shared-texture import) — //! the decode chain there is Vulkan → D3D11VA → software. +// Unsafe-proof program: every `unsafe {}` in this crate carries a `// SAFETY:` proof. +#![deny(clippy::undocumented_unsafe_blocks)] + +// THE VULKAN CONTRACT, stated once - most `// SAFETY:` proofs in this crate are an instance of it. +// +// Nearly every `unsafe` here is an `ash` call, which is `unsafe` because Vulkan is a C API, not +// because each call carries its own bespoke obligation. Three shapes recur, and only one of them +// has a real precondition worth restating per site: +// +// * CREATE / ALLOCATE - `create_*`, `allocate_*`. The device is live (this type owns it), and the +// `vk::*CreateInfo` builders are locals that outlive the synchronous call. The handle returned is +// owned by the value being constructed and destroyed in its `Drop`. +// * RECORD - `cmd_*`, `begin/end_command_buffer`, `update_descriptor_sets`. Recorded into a command +// buffer this code owns and has begun, referencing handles it also owns. Nothing executes until +// submit, so a recording error is not yet a memory error. +// * DESTROY - `destroy_*`, `free_*`, `unmap_memory`. THIS is the one with a real obligation: the +// GPU must not still be using the object. That is established on the path, not by the call - a +// fence wait, a `queue_wait_idle`, or the swapchain having been retired - and the per-site proofs +// say so, because getting it wrong is a use-after-free the type system cannot catch. +// +// A block doing something OUTSIDE these three shapes gets a real, specific proof; if you add one and +// find yourself writing "as above", it probably belongs in one of them. + #[cfg(any(target_os = "linux", windows))] pub mod csc; #[cfg(any(target_os = "linux", windows))] diff --git a/crates/pf-presenter/src/run.rs b/crates/pf-presenter/src/run.rs index a71b7e2a..ee4e062d 100644 --- a/crates/pf-presenter/src/run.rs +++ b/crates/pf-presenter/src/run.rs @@ -1704,6 +1704,8 @@ fn apply_capture( /// forwarded as touch passthrough. An unknown/invalid id (INVALID) reads as not-direct. fn is_direct_touch(touch_id: u64) -> bool { use sdl3::sys::touch::{SDL_GetTouchDeviceType, SDL_TouchDeviceType, SDL_TouchID}; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this type + // and live for the call, and every builder struct is a local that outlives it. unsafe { SDL_GetTouchDeviceType(SDL_TouchID(touch_id)) == SDL_TouchDeviceType::DIRECT } } diff --git a/crates/pf-presenter/src/vk/gpu.rs b/crates/pf-presenter/src/vk/gpu.rs index f08b63cc..d371eb2e 100644 --- a/crates/pf-presenter/src/vk/gpu.rs +++ b/crates/pf-presenter/src/vk/gpu.rs @@ -10,6 +10,8 @@ impl Presenter { /// submits FFmpeg's Vulkan decode work concurrently, and wait-idle's external-sync /// rule over every device queue would race it (observed as a resize crash). pub(super) fn quiesce_own(&mut self) -> Result<()> { + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. unsafe { if self.submitted { self.device.wait_for_fences(&[self.fence], true, u64::MAX)?; @@ -31,6 +33,9 @@ impl Presenter { .contains(flags) }) .with_context(|| format!("no memory type for {flags:?}"))?; + // SAFETY: per the Vulkan contract above - a create/allocate call on the live device, over + // builder structs that are locals outliving the call; the handle it returns is owned by + // the value being built here. unsafe { self.device.allocate_memory( &vk::MemoryAllocateInfo::default() @@ -111,6 +116,9 @@ pub(super) fn vkframe_acquire_barrier( .dst_queue_family_index(dst) .image(image) .subresource_range(subresource_range()); + // SAFETY: per the Vulkan contract above - recorded into a command buffer this code owns and + // has begun, referencing handles it also owns; nothing is submitted until the recording is + // ended. unsafe { device.cmd_pipeline_barrier( cmd, @@ -145,6 +153,8 @@ pub(super) fn external_acquire_barrier( .dst_queue_family_index(qfi) .image(image) .subresource_range(subresource_range()); + // SAFETY: per the Vulkan contract in lib.rs - recorded into a command buffer this code owns + // and has begun, referencing handles it also owns; nothing runs until submit. unsafe { device.cmd_pipeline_barrier( cmd, @@ -177,6 +187,9 @@ pub(super) fn foreign_acquire_barrier( .dst_queue_family_index(qfi) .image(image) .subresource_range(subresource_range()); + // SAFETY: per the Vulkan contract above - recorded into a command buffer this code owns and + // has begun, referencing handles it also owns; nothing is submitted until the recording is + // ended. unsafe { device.cmd_pipeline_barrier( cmd, @@ -208,6 +221,9 @@ pub(super) fn barrier( .dst_queue_family_index(vk::QUEUE_FAMILY_IGNORED) .image(image) .subresource_range(subresource_range()); + // SAFETY: per the Vulkan contract above - recorded into a command buffer this code owns and + // has begun, referencing handles it also owns; nothing is submitted until the recording is + // ended. unsafe { device.cmd_pipeline_barrier( cmd, diff --git a/crates/pf-presenter/src/vk/mod.rs b/crates/pf-presenter/src/vk/mod.rs index ffa46042..24d78339 100644 --- a/crates/pf-presenter/src/vk/mod.rs +++ b/crates/pf-presenter/src/vk/mod.rs @@ -230,6 +230,8 @@ impl Presenter { /// The queue lock is held as cheap insurance against a straggling submitter. pub fn wait_idle(&self) { let _q = self.queue_lock.guard(); + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. unsafe { self.device.device_wait_idle() }.ok(); } @@ -278,6 +280,8 @@ impl Drop for Presenter { // after in-flight waits complete, bounded by their 250 ms cap) BEFORE the // swapchain teardown below. self.present_timer.take(); + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. unsafe { { // Insurance against a straggling submitter (the run loop joins the diff --git a/crates/pf-presenter/src/vk/overlay_pipe.rs b/crates/pf-presenter/src/vk/overlay_pipe.rs index e6fc064a..0a3a12dd 100644 --- a/crates/pf-presenter/src/vk/overlay_pipe.rs +++ b/crates/pf-presenter/src/vk/overlay_pipe.rs @@ -32,6 +32,8 @@ impl OverlayPipe { .dst_access_mask( vk::AccessFlags::COLOR_ATTACHMENT_READ | vk::AccessFlags::COLOR_ATTACHMENT_WRITE, )]; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let render_pass = unsafe { device.create_render_pass( &vk::RenderPassCreateInfo::default() @@ -43,6 +45,8 @@ impl OverlayPipe { } .context("overlay render pass")?; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let sampler = unsafe { device.create_sampler( &vk::SamplerCreateInfo::default() @@ -61,6 +65,8 @@ impl OverlayPipe { .descriptor_count(1) .stage_flags(vk::ShaderStageFlags::FRAGMENT) .immutable_samplers(&samplers)]; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let set_layout = unsafe { device.create_descriptor_set_layout( &vk::DescriptorSetLayoutCreateInfo::default().bindings(&bindings), @@ -68,6 +74,8 @@ impl OverlayPipe { ) }?; let set_layouts = [set_layout]; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let pipeline_layout = unsafe { device.create_pipeline_layout( &vk::PipelineLayoutCreateInfo::default().set_layouts(&set_layouts), @@ -77,6 +85,8 @@ impl OverlayPipe { let pool_sizes = [vk::DescriptorPoolSize::default() .ty(vk::DescriptorType::COMBINED_IMAGE_SAMPLER) .descriptor_count(1)]; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let desc_pool = unsafe { device.create_descriptor_pool( &vk::DescriptorPoolCreateInfo::default() @@ -85,6 +95,8 @@ impl OverlayPipe { None, ) }?; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let desc_set = unsafe { device.allocate_descriptor_sets( &vk::DescriptorSetAllocateInfo::default() @@ -131,6 +143,9 @@ impl OverlayPipe { ) -> Result<()> { self.destroy_targets(device); // no-op after take_targets; safety net otherwise for &image in images { + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by + // this type and live for the call, and every builder struct is a local that outlives + // it. let view = unsafe { device.create_image_view( &vk::ImageViewCreateInfo::default() @@ -143,6 +158,9 @@ impl OverlayPipe { }?; self.views.push(view); let attachments = [view]; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by + // this type and live for the call, and every builder struct is a local that outlives + // it. let fb = unsafe { device.create_framebuffer( &vk::FramebufferCreateInfo::default() @@ -160,6 +178,8 @@ impl OverlayPipe { } fn destroy_targets(&mut self, device: &ash::Device) { + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. unsafe { for fb in self.framebuffers.drain(..) { device.destroy_framebuffer(fb, None); @@ -172,6 +192,10 @@ impl OverlayPipe { pub(super) fn destroy(&mut self, device: &ash::Device) { self.destroy_targets(device); + // SAFETY: per the Vulkan contract above - this destroys objects this type owns, and the + // GPU is known idle for them (the fence/queue-wait on the path here, or the swapchain + // being retired), which is the obligation that makes a destroy sound rather than the + // handle merely being non-null. unsafe { device.destroy_pipeline(self.pipeline, None); device.destroy_pipeline_layout(self.pipeline_layout, None); diff --git a/crates/pf-presenter/src/vk/present.rs b/crates/pf-presenter/src/vk/present.rs index d7bdf9fc..1a9cef12 100644 --- a/crates/pf-presenter/src/vk/present.rs +++ b/crates/pf-presenter/src/vk/present.rs @@ -113,6 +113,8 @@ impl Presenter { // One frame in flight: the fence covers the command buffer, the staging buffer // AND the previously submitted hw frame — waiting makes all three reusable. + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. unsafe { if self.submitted { self.device.wait_for_fences(&[self.fence], true, u64::MAX)?; @@ -189,9 +191,14 @@ impl Presenter { .dst_binding(0) .descriptor_type(vk::DescriptorType::COMBINED_IMAGE_SAMPLER) .image_info(&infos)]; + // SAFETY: per the Vulkan contract above - recorded into a command buffer this code + // owns and has begun, referencing handles it also owns; nothing is submitted until the + // recording is ended. unsafe { self.device.update_descriptor_sets(&writes, &[]) }; } + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let (index, _suboptimal) = match unsafe { self.swap_d.acquire_next_image( self.swapchain, @@ -218,6 +225,9 @@ impl Presenter { }; let swap_image = self.images[index as usize]; + // SAFETY: per the Vulkan contract above - recorded into a command buffer this code owns + // and has begun, referencing handles it also owns; nothing is submitted until the + // recording is ended. unsafe { self.device.begin_command_buffer( self.cmd_buf, @@ -628,6 +638,9 @@ impl Presenter { depth: u8, msb_packed: bool, ) { + // SAFETY: per the Vulkan contract above - recorded into a command buffer this code owns + // and has begun, referencing handles it also owns; nothing is submitted until the + // recording is ended. unsafe { self.device.cmd_begin_render_pass( self.cmd_buf, @@ -714,6 +727,9 @@ impl Presenter { let Some(planar) = self.csc_planar.as_ref() else { return; }; + // SAFETY: per the Vulkan contract above - recorded into a command buffer this code owns + // and has begun, referencing handles it also owns; nothing is submitted until the + // recording is ended. unsafe { self.device.cmd_begin_render_pass( self.cmd_buf, @@ -816,11 +832,16 @@ impl Presenter { ); }; // img[0] is creation-constant (only the sync fields need the frames lock). - let image = - vk::Image::from_raw( - unsafe { (*(f.vkframe as *const pf_ffvk::AVVkFrame)).img[0] } as u64, - ); + let image = vk::Image::from_raw( + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned + // by this type and live for the call, and every builder struct is a local that + // outlives it. + unsafe { (*(f.vkframe as *const pf_ffvk::AVVkFrame)).img[0] } as u64, + ); let make = |aspect: vk::ImageAspectFlags, format: vk::Format| { + // SAFETY: per the Vulkan contract above - a create/allocate call on the live device, + // over builder structs that are locals outliving the call; the handle it returns is + // owned by the value being built here. unsafe { self.device.create_image_view( &vk::ImageViewCreateInfo::default() @@ -842,6 +863,10 @@ impl Presenter { let chroma = match make(vk::ImageAspectFlags::PLANE_1, chroma_fmt) { Ok(v) => v, Err(e) => { + // SAFETY: per the Vulkan contract above - this destroys objects this type owns, + // and the GPU is known idle for them (the fence/queue-wait on the path here, or + // the swapchain being retired), which is the obligation that makes a destroy sound + // rather than the handle merely being non-null. unsafe { self.device.destroy_image_view(luma, None) }; return Err(e); } @@ -871,6 +896,8 @@ struct VkFrameSync { // is required on one platform and a no-op on the other. #[allow(clippy::unnecessary_cast)] fn lock_vkframe(f: &VkVideoFrame) -> VkFrameSync { + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this type + // and live for the call, and every builder struct is a local that outlives it. unsafe { let lock: unsafe extern "C" fn(*mut pf_ffvk::AVHWFramesContext, *mut pf_ffvk::AVVkFrame) = std::mem::transmute(f.lock_frame); @@ -890,6 +917,8 @@ fn lock_vkframe(f: &VkVideoFrame) -> VkFrameSync { /// Write the post-submission state back (FFmpeg waits these on its next use of the /// frame) and release the lock. On a failed submit only the lock is released. fn unlock_vkframe(f: &VkVideoFrame, sync: &VkFrameSync, submitted: bool, graphics_qf: u32) { + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this type + // and live for the call, and every builder struct is a local that outlives it. unsafe { let vkf = f.vkframe as *mut pf_ffvk::AVVkFrame; if submitted { diff --git a/crates/pf-presenter/src/vk/present_timing.rs b/crates/pf-presenter/src/vk/present_timing.rs index ac8cbc4a..d44dcdac 100644 --- a/crates/pf-presenter/src/vk/present_timing.rs +++ b/crates/pf-presenter/src/vk/present_timing.rs @@ -58,6 +58,9 @@ impl PresentTimer { .spawn(move || { while let Ok(job) = rx.recv() { // 250 ms cap — see the module doc's lifecycle note. + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are + // owned by this type and live for the call, and every builder struct is a + // local that outlives it. let r = unsafe { wait_d.wait_for_present(job.swapchain, job.present_id, 250_000_000) }; diff --git a/crates/pf-presenter/src/vk/reconfig.rs b/crates/pf-presenter/src/vk/reconfig.rs index 2cc22aae..f34d685d 100644 --- a/crates/pf-presenter/src/vk/reconfig.rs +++ b/crates/pf-presenter/src/vk/reconfig.rs @@ -47,10 +47,15 @@ impl Presenter { // the stream for a frame, and only happens on resize/HDR-flip/OUT_OF_DATE. { let _q = self.queue_lock.guard(); + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by + // this type and live for the call, and every builder struct is a local that outlives + // it. unsafe { self.device.queue_wait_idle(self.queue) } .context("vkQueueWaitIdle (swapchain recreate)")?; } + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let caps = unsafe { self.surface_i .get_physical_device_surface_capabilities(self.pdev, self.surface) @@ -91,6 +96,9 @@ impl Presenter { .present_mode(self.present_mode) .clipped(true) .old_swapchain(old); + // SAFETY: per the Vulkan contract above - a create/allocate call on the live device, over + // builder structs that are locals outliving the call; the handle it returns is owned by + // the value being built here. let swapchain = unsafe { self.swap_d.create_swapchain(&info, None) } .map_err(|e| anyhow!("vkCreateSwapchainKHR: {e}{}", kmsdrm_swapchain_hint()))?; // The old swapchain and everything tied to its images dies NOW: the fence @@ -105,6 +113,8 @@ impl Presenter { // An unclaimed last present belonged to the dying swapchain — drop the claim. self.last_presented = None; let (overlay_views, overlay_framebuffers) = self.overlay_pipe.take_targets(); + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. unsafe { for fb in overlay_framebuffers { self.device.destroy_framebuffer(fb, None); @@ -120,6 +130,8 @@ impl Presenter { } } self.swapchain = swapchain; + // SAFETY: per the Vulkan contract above - a read-only query on the live instance/device, + // filling locals returned by value. self.images = unsafe { self.swap_d.get_swapchain_images(swapchain) }?; self.extent = extent; self.overlay_pipe.rebuild_targets( @@ -130,6 +142,9 @@ impl Presenter { )?; for _ in 0..self.images.len() { + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by + // this type and live for the call, and every builder struct is a local that outlives + // it. self.render_sems.push(unsafe { self.device .create_semaphore(&vk::SemaphoreCreateInfo::default(), None) @@ -205,6 +220,8 @@ impl Presenter { .min_luminance(m.min_display_mastering_luminance as f32 / 10_000.0) .max_content_light_level(m.max_cll as f32) .max_frame_average_light_level(m.max_fall as f32); + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. unsafe { ext.set_hdr_metadata(&[self.swapchain], &[md]) }; tracing::debug!(from_host = self.hdr_meta.is_some(), "HDR metadata pushed"); } @@ -238,6 +255,10 @@ impl Presenter { self.csc_planar = Some(CscPass::new_planar(&self.device, self.video_format)?); } if let Some(v) = self.video.take() { + // SAFETY: per the Vulkan contract above - this destroys objects this type owns, and + // the GPU is known idle for them (the fence/queue-wait on the path here, or the + // swapchain being retired), which is the obligation that makes a destroy sound rather + // than the handle merely being non-null. unsafe { self.device.destroy_framebuffer(v.framebuffer, None); self.device.destroy_image_view(v.view, None); @@ -254,6 +275,8 @@ impl Presenter { OverlayPipe::new(&self.device, target.format)?, ); let (overlay_views, overlay_framebuffers) = old_pipe.take_targets(); + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. unsafe { for fb in overlay_framebuffers { self.device.destroy_framebuffer(fb, None); diff --git a/crates/pf-presenter/src/vk/resources.rs b/crates/pf-presenter/src/vk/resources.rs index e45281c3..24f14e0f 100644 --- a/crates/pf-presenter/src/vk/resources.rs +++ b/crates/pf-presenter/src/vk/resources.rs @@ -14,6 +14,9 @@ impl Retired { #[cfg(windows)] Retired::D3d11(f) => f.destroy(device), Retired::Vk { frame, views } => { + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned + // by this type and live for the call, and every builder struct is a local that + // outlives it. unsafe { for v in views { device.destroy_image_view(v, None); @@ -49,6 +52,8 @@ impl Presenter { } let s = self.staging.as_ref().unwrap(); let n = f.rgba.len().min(needed); + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. unsafe { std::ptr::copy_nonoverlapping(f.rgba.as_ptr(), s.ptr, n) }; Ok(()) } @@ -57,6 +62,9 @@ impl Presenter { // Fence-quiesce: the old image is only ever referenced by OUR command buffers. self.quiesce_own()?; if let Some(v) = self.video.take() { + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by + // this type and live for the call, and every builder struct is a local that outlives + // it. unsafe { if v.framebuffer != vk::Framebuffer::null() { self.device.destroy_framebuffer(v.framebuffer, None); @@ -69,6 +77,8 @@ impl Presenter { } } // COLOR_ATTACHMENT is the CSC pass's render target; harmless where hw is absent. + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let image = unsafe { self.device.create_image( &vk::ImageCreateInfo::default() @@ -92,11 +102,17 @@ impl Presenter { None, ) }?; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let reqs = unsafe { self.device.get_image_memory_requirements(image) }; let memory = self.allocate(reqs, vk::MemoryPropertyFlags::DEVICE_LOCAL)?; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. unsafe { self.device.bind_image_memory(image, memory, 0) }?; // The CSC pass renders into it — view + framebuffer, unconditional (Vulkan-Video // frames need the pass on every device, dmabuf-capable or not). + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let view = unsafe { self.device.create_image_view( &vk::ImageViewCreateInfo::default() @@ -108,6 +124,8 @@ impl Presenter { ) }?; let attachments = [view]; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let framebuffer = unsafe { self.device.create_framebuffer( &vk::FramebufferCreateInfo::default() @@ -133,12 +151,18 @@ impl Presenter { fn rebuild_staging(&mut self, capacity: usize) -> Result<()> { self.quiesce_own()?; if let Some(s) = self.staging.take() { + // SAFETY: per the Vulkan contract above - this destroys objects this type owns, and + // the GPU is known idle for them (the fence/queue-wait on the path here, or the + // swapchain being retired), which is the obligation that makes a destroy sound rather + // than the handle merely being non-null. unsafe { self.device.unmap_memory(s.memory); self.device.destroy_buffer(s.buffer, None); self.device.free_memory(s.memory, None); } } + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let buffer = unsafe { self.device.create_buffer( &vk::BufferCreateInfo::default() @@ -148,12 +172,18 @@ impl Presenter { None, ) }?; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let reqs = unsafe { self.device.get_buffer_memory_requirements(buffer) }; let memory = self.allocate( reqs, vk::MemoryPropertyFlags::HOST_VISIBLE | vk::MemoryPropertyFlags::HOST_COHERENT, )?; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. unsafe { self.device.bind_buffer_memory(buffer, memory, 0) }?; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let ptr = unsafe { self.device .map_memory(memory, 0, vk::WHOLE_SIZE, vk::MemoryMapFlags::empty()) diff --git a/crates/pf-presenter/src/vk/setup.rs b/crates/pf-presenter/src/vk/setup.rs index 645c7a99..48742c8c 100644 --- a/crates/pf-presenter/src/vk/setup.rs +++ b/crates/pf-presenter/src/vk/setup.rs @@ -17,6 +17,9 @@ impl Presenter { /// Bring up instance → surface → device → swapchain over an SDL window. /// `instance_extensions` comes from `VideoSubsystem::vulkan_instance_extensions()`. pub fn new(window: &sdl3::video::Window, instance_extensions: &[String]) -> Result { + // SAFETY: per the Vulkan contract above - a create/allocate call on the live device, over + // builder structs that are locals outliving the call; the handle it returns is owned by + // the value being built here. let entry = unsafe { ash::Entry::load() }.context("libvulkan not loadable")?; let app_name = CString::new("punktfunk-session").unwrap(); @@ -29,6 +32,8 @@ impl Presenter { // HDR10 presentation needs the extended colorspaces at the INSTANCE level. let mut instance_extensions: Vec = instance_extensions.to_vec(); let inst_available = + // SAFETY: per the Vulkan contract above - a read-only query on the live + // instance/device, filling locals returned by value. unsafe { entry.enumerate_instance_extension_properties(None) }.unwrap_or_default(); let has_colorspace_ext = inst_available .iter() @@ -44,6 +49,8 @@ impl Presenter { // hardcoded `*const i8` compiles on the desktop targets and fails to match ash's // `&[*const c_char]` on ARM. let ext_ptrs: Vec<*const c_char> = ext_cstrings.iter().map(|e| e.as_ptr()).collect(); + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let instance = unsafe { entry.create_instance( &vk::InstanceCreateInfo::default() @@ -55,12 +62,19 @@ impl Presenter { .context("vkCreateInstance")?; let surface_i = ash::khr::surface::Instance::new(&entry, &instance); + // SAFETY: per the Vulkan contract above - a create/allocate call on the live device, over + // builder structs that are locals outliving the call; the handle it returns is owned by + // the value being built here. let surface = unsafe { window.vulkan_create_surface(instance.handle()) } .map_err(|e| anyhow!("SDL_Vulkan_CreateSurface: {e}"))?; let (pdev, qfi) = pick_device(&instance, &surface_i, surface)?; + // SAFETY: per the Vulkan contract above - a read-only query on the live instance/device, + // filling locals returned by value. let mem_props = unsafe { instance.get_physical_device_memory_properties(pdev) }; { + // SAFETY: per the Vulkan contract above - a read-only query on the live + // instance/device, filling locals returned by value. let props = unsafe { instance.get_physical_device_properties(pdev) }; let name = props .device_name_as_c_str() @@ -72,6 +86,8 @@ impl Presenter { // The dmabuf import set is optional: enabled when the device offers all four, // else that path is off (`supports_dmabuf() == false`). Windows has no // dmabuf/DRM-PRIME — the whole import path is compiled out there. + // SAFETY: per the Vulkan contract above - a read-only query on the live instance/device, + // filling locals returned by value. let available = unsafe { instance.enumerate_device_extension_properties(pdev) }?; let has = |name: &std::ffi::CStr| { available @@ -113,6 +129,8 @@ impl Presenter { // SAME adapter). Core 1.1 query; valid on effectively every Windows driver. let mut id_props = vk::PhysicalDeviceIDProperties::default(); let mut props2 = vk::PhysicalDeviceProperties2::default().push_next(&mut id_props); + // SAFETY: per the Vulkan contract above - a read-only query on the live instance/device, + // filling locals returned by value. unsafe { instance.get_physical_device_properties2(pdev, &mut props2) }; let adapter_luid: Option<[u8; 8]> = (id_props.device_luid_valid == vk::TRUE).then_some(id_props.device_luid); @@ -130,6 +148,8 @@ impl Presenter { // Probed, never required: a capable stack gets the video extensions, a second // (decode) queue, and the features FFmpeg's decoder needs; anything less means // `vulkan_decode() == None` and the decoder chain falls back (VAAPI/software). + // SAFETY: per the Vulkan contract above - a read-only query on the live instance/device, + // filling locals returned by value. let dev_props = unsafe { instance.get_physical_device_properties(pdev) }; let dev_is_13 = vk::api_version_major(dev_props.api_version) > 1 || vk::api_version_minor(dev_props.api_version) >= 3; @@ -149,6 +169,8 @@ impl Presenter { if present_wait_exts { have_f2 = have_f2.push_next(&mut have_pid).push_next(&mut have_pwait); } + // SAFETY: per the Vulkan contract above - a read-only query on the live instance/device, + // filling locals returned by value. unsafe { instance.get_physical_device_features2(pdev, &mut have_f2) }; // Copy the one base-features fact out NOW: `have_f2` mutably borrows the chained // structs through its pNext chain, so any later use of it would pin those borrows — @@ -174,6 +196,8 @@ impl Presenter { // The decode queue family + which codec operations it can run. let decode_family: Option<(u32, vk::VideoCodecOperationFlagsKHR)> = { + // SAFETY: per the Vulkan contract above - a read-only query on the live + // instance/device, filling locals returned by value. let n = unsafe { instance.get_physical_device_queue_family_properties2_len(pdev) }; let mut video: Vec = vec![vk::QueueFamilyVideoPropertiesKHR::default(); n]; @@ -181,6 +205,8 @@ impl Presenter { .iter_mut() .map(|v| vk::QueueFamilyProperties2::default().push_next(v)) .collect(); + // SAFETY: per the Vulkan contract above - a read-only query on the live + // instance/device, filling locals returned by value. unsafe { instance.get_physical_device_queue_family_properties2(pdev, &mut props) }; // `props` mutably borrows `video` (push_next); copy the flags out, then // read the driver-filled video properties directly. @@ -282,6 +308,8 @@ impl Presenter { .queue_priorities(&priorities), ); } + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let device = unsafe { instance.create_device( pdev, @@ -305,6 +333,8 @@ impl Presenter { ); let hdr_metadata_d = has_hdr_metadata.then(|| ash::ext::hdr_metadata::Device::new(&instance, &device)); + // SAFETY: per the Vulkan contract above - a read-only query on the live instance/device, + // filling locals returned by value. let queue = unsafe { device.get_device_queue(qfi, 0) }; #[cfg(target_os = "linux")] let hw = if hw_capable { @@ -341,6 +371,8 @@ impl Presenter { #[cfg(not(windows))] let export_worthy = video_ok || pyrowave_ok; let video_export = if export_worthy { + // SAFETY: per the Vulkan contract above - a read-only query on the live + // instance/device, filling locals returned by value. let qf_props = unsafe { instance.get_physical_device_queue_family_properties(pdev) }; let mut device_extensions: Vec = vec![CString::from(ash::khr::swapchain::NAME)]; @@ -425,6 +457,9 @@ impl Presenter { ); let overlay_pipe = OverlayPipe::new(&device, format.format)?; + // SAFETY: per the Vulkan contract above - recorded into a command buffer this code owns + // and has begun, referencing handles it also owns; nothing is submitted until the + // recording is ended. let cmd_pool = unsafe { device.create_command_pool( &vk::CommandPoolCreateInfo::default() @@ -433,6 +468,9 @@ impl Presenter { None, ) }?; + // SAFETY: per the Vulkan contract above - recorded into a command buffer this code owns + // and has begun, referencing handles it also owns; nothing is submitted until the + // recording is ended. let cmd_buf = unsafe { device.allocate_command_buffers( &vk::CommandBufferAllocateInfo::default() @@ -442,7 +480,12 @@ impl Presenter { ) }?[0]; let acquire_sem = + // SAFETY: per the Vulkan contract above - a create/allocate call on the live device, + // over builder structs that are locals outliving the call; the handle it returns is + // owned by the value being built here. unsafe { device.create_semaphore(&vk::SemaphoreCreateInfo::default(), None) }?; + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this + // type and live for the call, and every builder struct is a local that outlives it. let fence = unsafe { device.create_fence( &vk::FenceCreateInfo::default().flags(vk::FenceCreateFlags::SIGNALED), @@ -506,11 +549,16 @@ impl Presenter { /// the whole `PUNKTFUNK_VK_ADAPTER` match key, so a second identical card adds nothing). /// Same 1.3 instance the presenter creates, so the list matches what streaming sees. pub fn list_adapters() -> Result> { + // SAFETY: per the Vulkan contract above - a create/allocate call on the live device, over + // builder structs that are locals outliving the call; the handle it returns is owned by the + // value being built here. let entry = unsafe { ash::Entry::load() }.context("libvulkan not loadable")?; let app_name = CString::new("punktfunk-session").unwrap(); let app_info = vk::ApplicationInfo::default() .application_name(&app_name) .api_version(vk::API_VERSION_1_3); + // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this type + // and live for the call, and every builder struct is a local that outlives it. let instance = unsafe { entry.create_instance( &vk::InstanceCreateInfo::default().application_info(&app_info), @@ -518,9 +566,13 @@ pub fn list_adapters() -> Result> { ) } .context("vkCreateInstance")?; + // SAFETY: per the Vulkan contract above - a read-only query on the live instance/device, + // filling locals returned by value. let mut ranked: Vec<(u8, String)> = unsafe { instance.enumerate_physical_devices() }? .into_iter() .map(|d| { + // SAFETY: per the Vulkan contract above - a read-only query on the live + // instance/device, filling locals returned by value. let props = unsafe { instance.get_physical_device_properties(d) }; let rank = match props.device_type { vk::PhysicalDeviceType::DISCRETE_GPU => 0u8, @@ -535,6 +587,10 @@ pub fn list_adapters() -> Result> { }) .filter(|(_, n)| !n.is_empty()) .collect(); + // SAFETY: per the Vulkan contract above - this destroys objects this type owns, and the GPU is + // known idle for them (the fence/queue-wait on the path here, or the swapchain being retired), + // which is the obligation that makes a destroy sound rather than the handle merely being non- + // null. unsafe { instance.destroy_instance(None) }; ranked.sort_by_key(|(r, _)| *r); // stable: enumeration order within each tier let mut names: Vec = Vec::new(); @@ -553,6 +609,8 @@ fn pick_device( surface_i: &ash::khr::surface::Instance, surface: vk::SurfaceKHR, ) -> Result<(vk::PhysicalDevice, u32)> { + // SAFETY: per the Vulkan contract above - a read-only query on the live instance/device, + // filling locals returned by value. let devices = unsafe { instance.enumerate_physical_devices() }?; let forced: Option = std::env::var("PUNKTFUNK_VK_DEVICE") .ok() @@ -575,6 +633,8 @@ fn pick_device( .map(|w| w.trim().to_lowercase()) .filter(|w| !w.is_empty()); candidates.sort_by_key(|d| { + // SAFETY: per the Vulkan contract above - a read-only query on the live + // instance/device, filling locals returned by value. let props = unsafe { instance.get_physical_device_properties(*d) }; let name = props .device_name_as_c_str() @@ -595,10 +655,14 @@ fn pick_device( }); } for pdev in candidates { + // SAFETY: per the Vulkan contract above - a read-only query on the live instance/device, + // filling locals returned by value. let families = unsafe { instance.get_physical_device_queue_family_properties(pdev) }; for (i, f) in families.iter().enumerate() { let graphics = f.queue_flags.contains(vk::QueueFlags::GRAPHICS); let present = + // SAFETY: per the Vulkan contract above - a read-only query on the live + // instance/device, filling locals returned by value. unsafe { surface_i.get_physical_device_surface_support(pdev, i as u32, surface) } .unwrap_or(false); if graphics && present { @@ -629,6 +693,8 @@ pub(super) fn pick_formats( let colorspace_ext = colorspace_ext && !std::env::var("PUNKTFUNK_HDR10") .is_ok_and(|v| matches!(v.as_str(), "0" | "false" | "off" | "no")); + // SAFETY: per the Vulkan contract above - a read-only query on the live instance/device, + // filling locals returned by value. let formats = unsafe { surface_i.get_physical_device_surface_formats(pdev, surface) }?; let mut sdr = None; for want in [vk::Format::B8G8R8A8_UNORM, vk::Format::R8G8B8A8_UNORM] { @@ -673,6 +739,8 @@ fn pick_present_mode( pdev: vk::PhysicalDevice, surface: vk::SurfaceKHR, ) -> Result { + // SAFETY: per the Vulkan contract above - a read-only query on the live instance/device, + // filling locals returned by value. let modes = unsafe { surface_i.get_physical_device_surface_present_modes(pdev, surface) }?; let want = match std::env::var("PUNKTFUNK_PRESENT_MODE").ok().as_deref() { Some("fifo") => vk::PresentModeKHR::FIFO, diff --git a/crates/pf-presenter/src/win32.rs b/crates/pf-presenter/src/win32.rs index db20fd43..d017f0cb 100644 --- a/crates/pf-presenter/src/win32.rs +++ b/crates/pf-presenter/src/win32.rs @@ -30,6 +30,9 @@ const APP_USER_MODEL_ID: *const u16 = w!("unom.punktfunk.client"); /// process already carries the MSIX identity, which must win). Call before any window /// exists; later calls don't re-tag existing windows. pub(crate) fn set_app_user_model_id() { + // SAFETY: `GetCurrentPackageFullName` is called with `len = 0` and a NULL buffer, which is the + // documented identity PROBE — it writes nothing and only reports whether the process is + // packaged. `SetCurrentProcessExplicitAppUserModelID` takes a static wide literal. unsafe { let mut len: u32 = 0; // No buffer: just probe whether the process has package identity. @@ -43,6 +46,9 @@ pub(crate) fn set_app_user_model_id() { /// Stamp the exe-embedded icon (resource ordinal 1) onto the SDL window's title bar, /// taskbar and Alt-Tab. A quiet no-op when the exe embeds no icon. pub(crate) fn stamp_window_icon(window: &sdl3::video::Window) { + // SAFETY: the SDL property lookups return the `HWND` SDL itself owns for this live `window` + // borrow; the icon calls only pass that handle and a resource ordinal back to Win32, and every + // result is checked before use. unsafe { // The HWND behind the SDL window, via SDL3's window properties (the same route // the sdl3 crate's raw-window-handle integration takes). diff --git a/crates/punktfunk-core/src/transport/qos_windows.rs b/crates/punktfunk-core/src/transport/qos_windows.rs index 5aa9ff83..6e0f0e0d 100644 --- a/crates/punktfunk-core/src/transport/qos_windows.rs +++ b/crates/punktfunk-core/src/transport/qos_windows.rs @@ -38,6 +38,10 @@ fn qos_handle() -> Option { // SAFETY: both pointers are valid for the duration of the synchronous call. if unsafe { QOSCreateHandle(&version, &mut handle) } == 0 { tracing::debug!( + // SAFETY: `GetLastError` takes no arguments and reads this thread's own last-error + // slot; it is called immediately after the failing call, before anything can reset it. + // SAFETY: `GetLastError` takes no arguments and reads this thread's own last-error + // slot; it is called immediately after the failing call, before anything can reset it. err = unsafe { GetLastError() }, "QOSCreateHandle failed — qWAVE DSCP marking unavailable" ); @@ -95,6 +99,8 @@ pub(super) fn add_media_flow(socket: &UdpSocket, class: MediaClass) -> Option Option std: }; let cmsg_len = cmsgdata_align(hdr) + std::mem::size_of::(); // WSA_CMSG_LEN(4) let space = cmsgdata_align(hdr + cmsghdr_align(std::mem::size_of::())); // WSA_CMSG_SPACE(4) + // SAFETY: `ctrl` is a local control buffer sized by `WSA_CMSG_SPACE(4)` — computed as `space` + // just above — so the header plus its 4-byte payload fit inside it and neither the field stores + // nor the unaligned data write can run past the end. `write_unaligned` is used because the + // payload sits at a `WSA_CMSG_DATA` offset with no alignment guarantee. unsafe { let cmsg = ctrl.0.as_mut_ptr() as *mut CMSGHDR; (*cmsg).cmsg_len = cmsg_len;