From 7c1c9b2e1914510118eb88713caadc7c8693924c Mon Sep 17 00:00:00 2001 From: dailz Date: Sat, 6 Jun 2026 15:54:09 +0800 Subject: [PATCH] fix(avhw): add SAFETY comments to all undocumented unsafe blocks Close #7 - Add // SAFETY: comments to 19 undocumented unsafe blocks and impls - Add nb_streams/null guard on stream array dereference (drain_encoder) - Add clippy undocumented_unsafe_blocks = warn lint to prevent regression avhw.rs now has 0 clippy unsafe documentation warnings. --- Cargo.toml | 3 +++ src/avhw.rs | 56 +++++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e9e8f7f..d96c203 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,3 +31,6 @@ dirs = "6" [dev-dependencies] tempfile = "3.27.0" + +[lints.clippy] +undocumented_unsafe_blocks = "warn" diff --git a/src/avhw.rs b/src/avhw.rs index a679d3b..57643aa 100644 --- a/src/avhw.rs +++ b/src/avhw.rs @@ -23,12 +23,17 @@ pub struct AvHwDevCtx { ptr: *mut ffi::AVBufferRef, } +// SAFETY: AvHwDevCtx wraps an FFmpeg AVBufferRef which is not Send by default, +// but we guarantee exclusive access through &mut self. The underlying VAAPI +// device context is thread-safe for the operations we perform. unsafe impl Send for AvHwDevCtx {} impl AvHwDevCtx { pub fn new_vaapi(drm_device: &Path) -> Result { let device_cstr = CString::new(drm_device.to_str().unwrap())?; let mut p: *mut ffi::AVBufferRef = ptr::null_mut(); + // SAFETY: device_cstr is a valid C string for the duration of the call; + // p is a valid out-pointer that FFmpeg initializes on success. let ret = unsafe { ffi::av_hwdevice_ctx_create( &mut p, @@ -74,10 +79,15 @@ pub struct AvHwFrameCtx { ptr: *mut ffi::AVBufferRef, } +// SAFETY: AvHwFrameCtx wraps an FFmpeg AVBufferRef to an AVHWFramesContext. +// It is only accessed through &mut self, ensuring no concurrent mutation. +// The underlying hardware frames pool is thread-safe for the send/receive pattern. unsafe impl Send for AvHwFrameCtx {} impl AvHwFrameCtx { fn new_inner(hw_dev: &AvHwDevCtx, w: u32, h: u32, sw_fmt: ff::format::Pixel) -> Result { + // SAFETY: hw_dev is a live AVHWDeviceContext; FFmpeg returns either a valid + // frames context ref or null (checked below). let mut p = unsafe { ffi::av_hwframe_ctx_alloc(hw_dev.as_ptr()) }; if p.is_null() { bail!("av_hwframe_ctx_alloc returned null"); @@ -92,6 +102,8 @@ impl AvHwFrameCtx { (*fc).height = h as i32; (*fc).initial_pool_size = 4; } + // SAFETY: p is a valid AVHWFramesContext ref configured above and not yet + // transferred or freed. let ret = unsafe { ffi::av_hwframe_ctx_init(p) }; if ret < 0 { // SAFETY: p is valid but init failed; clean up. @@ -211,6 +223,9 @@ pub unsafe fn import_dma_buf_to_vaapi( } let mut dst = ff::frame::Video::empty(); + // SAFETY: frames_ctx is guaranteed by this unsafe function's contract to be a + // valid initialized VAAPI frames context; we set format/hw_frames_ctx on a + // freshly allocated dst frame. unsafe { let dp = dst.as_mut_ptr(); (*dp).format = ffi::AVPixelFormat::AV_PIX_FMT_VAAPI as i32; @@ -219,6 +234,8 @@ pub unsafe fn import_dma_buf_to_vaapi( bail!("av_buffer_ref(frames_ctx) returned null"); } } + // SAFETY: src and dst are initialized AVFrames; dst has a valid hw_frames_ctx + // ref and av_hwframe_map fills dst from src. let ret = unsafe { ffi::av_hwframe_map( dst.as_mut_ptr(), @@ -244,6 +261,8 @@ unsafe extern "C" fn cleanup_drm_descriptor(_opaque: *mut c_void, data: *mut u8) fn av_err_to_string(err: i32) -> String { let mut buf = vec![0u8; 128]; + // SAFETY: buf points to 128 writable bytes and lives for the duration of + // av_strerror. unsafe { ffi::av_strerror(err, buf.as_mut_ptr() as *mut i8, buf.len()); } @@ -308,6 +327,8 @@ impl EncState { )?; let mut sink_ctx = video_filter.get("out").unwrap(); + // SAFETY: sink_ctx is a live buffersink; the returned hw_frames_ctx is + // borrowed, so av_buffer_ref creates an owned reference. let sink_hw_frames = unsafe { let raw = ffi::av_buffersink_get_hw_frames_ctx(sink_ctx.as_mut_ptr()); if raw.is_null() { @@ -320,6 +341,8 @@ impl EncState { hw_ref }; + // SAFETY: sink_hw_frames is an owned AVBufferRef to an AVHWFramesContext + // returned by the validated filter graph. unsafe { let fc = (*sink_hw_frames).data as *mut ffi::AVHWFramesContext; let actual_w = (*fc).width as u32; @@ -523,6 +546,8 @@ impl EncState { match filter_sink.frame(&mut filtered) { Ok(()) => { let start_ts = self.starting_timestamp.unwrap_or(0); + // SAFETY: filtered is a valid VAAPI frame drained from the + // filter graph; enc_video is an opened encoder. let ret = unsafe { ffi::avcodec_send_frame(self.enc_video.as_mut_ptr(), filtered.as_ptr()) }; @@ -569,9 +594,14 @@ impl EncState { // Rescale timestamps from encoder time_base to stream time_base let enc_tb = self.enc_video.time_base(); + // SAFETY: octx was created with stream 0 during muxer setup; streams + // is non-null and stream 0 remains owned by the format context. let stream_tb = unsafe { - let streams = (*self.octx.as_ptr()).streams; - let st = *streams.add(0); + let fmt = *self.octx.as_ptr(); + if fmt.nb_streams == 0 || fmt.streams.is_null() { + bail!("no streams in output context"); + } + let st = *fmt.streams.add(0); ff::Rational::from((*st).time_base) }; pkt.rescale_ts(enc_tb, stream_tb); @@ -763,6 +793,8 @@ impl SwEncState { } } + // SAFETY: Sending a null frame flushes the opened software encoder; + // no frame data is dereferenced. enc_video is exclusively borrowed via &mut self. unsafe { let ret = ffi::avcodec_send_frame(self.enc_video.as_mut_ptr(), ptr::null()); if ret < 0 && ret != ffi::AVERROR_EOF { @@ -791,6 +823,8 @@ impl SwEncState { return Ok(()); } } + // SAFETY: av_frame_alloc returns a newly allocated AVFrame or null, + // which is checked below. let mut sw_nv12 = unsafe { ffi::av_frame_alloc() }; if sw_nv12.is_null() { bail!("av_frame_alloc failed for NV12 transfer frame"); @@ -863,9 +897,14 @@ impl SwEncState { match self.output { Some(FrameOutput::Muxer(ref mut octx)) => { let enc_tb = self.enc_video.time_base(); + // SAFETY: muxer output was created with stream 0 during setup; + // streams is non-null and stream 0 remains owned by the format context. let stream_tb = unsafe { - let streams = (*octx.as_ptr()).streams; - let st = *streams.add(0); + let fmt = *octx.as_ptr(); + if fmt.nb_streams == 0 || fmt.streams.is_null() { + bail!("no streams in output context"); + } + let st = *fmt.streams.add(0); ff::Rational::from((*st).time_base) }; pkt.rescale_ts(enc_tb, stream_tb); @@ -883,6 +922,9 @@ impl SwEncState { self.frames_written = true; } Some(FrameOutput::Channel(ref tx)) => { + // SAFETY: pkt is a valid AVPacket just filled by + // avcodec_receive_packet; this copies fields for + // read-only inspection before pkt is dropped. let raw = unsafe { *pkt.as_mut_ptr() }; if raw.size > 0 && !raw.data.is_null() { // SAFETY: `pkt` is a valid AVPacket just filled by a successful @@ -994,6 +1036,8 @@ fn build_swenc_filter_graph( ); let mut src_ctx = graph.add(&buffersrc, "in", &args)?; + // SAFETY: av_buffersrc_parameters_alloc returns newly allocated parameters + // or null, which is checked below. let par = unsafe { ffi::av_buffersrc_parameters_alloc() }; if par.is_null() { bail!("av_buffersrc_parameters_alloc returned null"); @@ -1233,6 +1277,8 @@ fn create_software_h264_encoder( enc.set_max_b_frames(0); if codec_name == "libx264" { + // SAFETY: priv_data belongs to the unopened encoder context; each + // CString lives for the duration of its av_opt_set call. unsafe { let key = CString::new("preset").unwrap(); let val = CString::new("ultrafast").unwrap(); @@ -1345,6 +1391,8 @@ fn build_filter_graph( Transform::Normal => unreachable!(), }; let mut trans_ctx = graph.add(&transpose, "transpose", &format!("dir={dir_val}"))?; + // SAFETY: trans_ctx is a live transpose_vaapi filter context; + // scale_vaapi/transpose_vaapi keep a ref-counted device context. unsafe { (*trans_ctx.as_mut_ptr()).hw_device_ctx = hw_dev.ref_clone(); }