From dba9a75ca597cd3ae92b80a937aea10ae25ed5df Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 6 Nov 2026 00:29:05 -0800 Subject: [PATCH] feat(vpp)!: resize into recycled surfaces, one pool per output size `Processor::resize` scales a surface into NV12 and returns it exported, writing into a surface pool kept per output size. Once the caller drops a frame, its surface is the next one written at that size; a frame still held is never written over, and the next resize allocates beside it. At most one free surface is kept per size. The pool moves out of the decoder into its own module so both share it. `Processor::process` becomes private: `resize` covers what it was public for. Co-Authored-By: Claude Opus 5.5 --- README.md | 3 +- src/decode.rs | 174 +++++++++----------------------------------------- src/lib.rs | 1 + src/pool.rs | 137 +++++++++++++++++++++++++++++++++++++++ src/vpp.rs | 172 +++++++++++++++++++++++++++++++++++++++---------- 5 files changed, 308 insertions(+), 179 deletions(-) create mode 100644 src/pool.rs diff --git a/README.md b/README.md index 3699868..93b11e1 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,8 @@ pulled in as a git dependency. - **DMA-BUF import and video post-processing.** `dmabuf` wraps a DRM PRIME buffer as a surface without copying it; `vpp` blits one surface into another through `VAEntrypointVideoProc`, scaling, converting, and re-tiling on the - way. + way. `Processor::resize` scales into NV12 surfaces it recycles per size, so a + stream of resizes costs no allocation per frame. - **H.264 decode** over VA-API: one Annex-B access unit in, tightly-packed NV12 out, with in-stream parameter sets, a conformant DPB (reference marking, reordering, frame_num gaps), and mid-stream resolution changes. Progressive diff --git a/src/decode.rs b/src/decode.rs index 91c5874..fe25506 100644 --- a/src/decode.rs +++ b/src/decode.rs @@ -32,11 +32,10 @@ //! stream this crate's encoder, WebRTC, or a browser's `VideoEncoder` produces. //! Left and top cropping are rejected; right and bottom cropping are supported. -use std::cell::Cell; use std::io::Cursor; use std::path::PathBuf; use std::rc::Rc; -use std::sync::{Arc, Mutex, Weak}; +use std::sync::Arc; use anyhow::{anyhow, bail, Context as _}; @@ -46,6 +45,7 @@ use crate::codec::h264::parser::{ SliceType, Sps, }; use crate::codec::h264::picture::{Field, IsIdr, PictureData, Reference}; +use crate::pool::{PooledSurface, SurfacePool}; use crate::{ bindings, Buffer, BufferType, Config as VaConfig, Context, Display, DrmPrimeSurfaceDescriptor, H264PicFields, H264SeqFields, IQMatrix, IQMatrixBufferH264, Image, Picture, PictureH264, PictureParameter, @@ -146,9 +146,23 @@ impl ExportedFrame { /// Fails when the surface is not NV12, cannot be synced, or the driver will /// not export it. pub fn from_surface(surface: Surface<()>, timestamp: u64) -> anyhow::Result { - surface.sync().map_err(|e| anyhow!("surface sync: {e:?}"))?; - let (width, height) = surface.size(); + let size = surface.size(); + Self::export(Arc::new(PooledSurface::unpooled(surface)), timestamp, size) + } + + /// Exports `surface`, whose picture is the top-left `width` by `height` of it, copying nothing. + /// + /// The frame holds a reference to the surface, so a pool does not hand it + /// out again while the caller holds the descriptor, and + /// [`download`](Self::download) can still map it. + pub(crate) fn export( + surface: Arc, + timestamp: u64, + (width, height): (u32, u32), + ) -> anyhow::Result { + surface.get().sync().map_err(|e| anyhow!("surface sync: {e:?}"))?; let descriptor = surface + .get() .export_prime() .map_err(|e| anyhow!("export the surface: {e:?}"))?; if descriptor.fourcc != VA_FOURCC_NV12 { @@ -159,7 +173,7 @@ impl ExportedFrame { width, height, descriptor, - surface: Arc::new(PooledSurface::unpooled(surface)), + surface, }) } @@ -1161,153 +1175,25 @@ impl Sequence { info, width, height, - pool: SurfacePool { - display: Arc::clone(display), - width: coded_width, - height: coded_height, - free: Arc::new(FreeList { - surfaces: Mutex::new(Vec::new()), - capacity, - }), - allocated: Cell::new(0), - }, + pool: SurfacePool::new( + display, + (coded_width, coded_height), + UsageHint::USAGE_HINT_DECODER, + capacity, + ), context, _config: config, }) } } -/// A recycling pool of decode target surfaces. -/// -/// Surfaces are allocated on demand and come back when the last reference to -/// one drops: the DPB's, and an [`ExportedFrame`]'s if the picture was handed -/// out. So a picture a consumer holds is never decoded over, and one it has let -/// go of is decoded into again rather than replaced by a fresh allocation. -/// -/// The pool keeps at most [`FreeList::capacity`] surfaces free and destroys -/// any returned past that, so a consumer that held many pictures and then let -/// them all go does not leave that many parked here for the rest of the -/// sequence. -struct SurfacePool { - display: Arc, - width: u32, - height: u32, - /// Shared with every surface on loan, which puts itself back from whichever - /// thread drops it last. - free: Arc, - /// Surfaces allocated over the pool's life. - allocated: Cell, -} - -/// The surfaces a [`SurfacePool`] has free to decode into. -struct FreeList { - surfaces: Mutex>>, - /// The most surfaces kept; one returned past this is destroyed. - capacity: usize, -} - -impl SurfacePool { - fn alloc(&self) -> anyhow::Result { - let recycled = self.free.surfaces.lock().expect("poisoned").pop(); - let surface = match recycled { - Some(surface) => surface, - None => { - let surface = self - .display - .create_surfaces::<()>( - VA_RT_FORMAT_YUV420, - Some(VA_FOURCC_NV12), - self.width, - self.height, - Some(UsageHint::USAGE_HINT_DECODER), - vec![()], - ) - .map_err(|e| anyhow!("create decode surface: {e:?}"))? - .pop() - .context("VA-API returned an empty surface list")?; - self.allocated.set(self.allocated.get() + 1); - log::debug!( - "allocated decode surface {} at {}x{}", - self.allocated.get(), - self.width, - self.height - ); - surface - } - }; - - Ok(PooledSurface { - surface: Some(surface), - free: Arc::downgrade(&self.free), - }) - } -} - -/// A surface on loan from a [`SurfacePool`], returned to it on drop. -/// -/// Shared behind an [`Arc`] by the DPB and by an [`ExportedFrame`], so it goes -/// back only once neither refers to it. The pool is held weakly: a surface that -/// outlives its sequence is destroyed rather than kept for a size nobody -/// decodes at any more. -struct PooledSurface { - /// Always `Some` until dropped; the `Option` is only there so [`Drop`] can - /// move the surface back into the pool. - surface: Option>, - /// Where to return the surface. Empty for a surface that came from - /// somewhere other than a pool, which is destroyed on drop. - free: Weak, -} - -impl PooledSurface { - /// Wraps a surface that belongs to no pool. - fn unpooled(surface: Surface<()>) -> Self { - Self { - surface: Some(surface), - free: Weak::new(), - } - } - - fn get(&self) -> &Surface<()> { - self.surface.as_ref().expect("surface is taken only on drop") - } - - fn id(&self) -> bindings::VASurfaceID { - self.get().id() - } -} - -impl Drop for PooledSurface { - fn drop(&mut self) { - let (Some(surface), Some(free)) = (self.surface.take(), self.free.upgrade()) else { - return; - }; - let mut surfaces = free.surfaces.lock().expect("poisoned"); - if surfaces.len() < free.capacity { - surfaces.push(surface); - } - } -} - /// Exports a decoded surface as a DRM PRIME descriptor, copying nothing. -/// -/// The frame holds a reference to the surface, so the pool does not hand it out -/// again while the caller holds the descriptor, and [`ExportedFrame::download`] -/// can still map it. fn export(handle: &Handle) -> anyhow::Result { - let surface = handle.surface.get(); - surface.sync().map_err(|e| anyhow!("surface sync: {e:?}"))?; - - let descriptor = surface - .export_prime() - .map_err(|e| anyhow!("export the decode surface: {e:?}"))?; - - Ok(ExportedFrame { - timestamp: handle.timestamp, - width: handle.width, - height: handle.height, - descriptor, - surface: Arc::clone(&handle.surface), - }) + ExportedFrame::export( + Arc::clone(&handle.surface), + handle.timestamp, + (handle.width, handle.height), + ) } /// Reads a decoded surface back to client memory as tightly packed NV12. diff --git a/src/lib.rs b/src/lib.rs index 5d33ab2..1dd7449 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -21,6 +21,7 @@ mod display; mod generic_value; mod image; mod picture; +mod pool; mod surface; mod usage_hint; diff --git a/src/pool.rs b/src/pool.rs new file mode 100644 index 0000000..7960bc4 --- /dev/null +++ b/src/pool.rs @@ -0,0 +1,137 @@ +//! A recycling pool of NV12 surfaces of one size, shared by the decoder's +//! targets and the video processor's outputs. + +use std::cell::Cell; +use std::sync::{Arc, Mutex, Weak}; + +use anyhow::{anyhow, Context as _}; + +use crate::{bindings, Display, Surface, UsageHint, VA_FOURCC_NV12, VA_RT_FORMAT_YUV420}; + +/// A recycling pool of NV12 surfaces. +/// +/// Surfaces are allocated on demand and come back when the last reference to +/// one drops, such as an [`ExportedFrame`](crate::decode::ExportedFrame)'s. So +/// a picture a consumer holds is never written over, and one it has let go of +/// is written into again rather than replaced by a fresh allocation. +/// +/// The pool keeps at most [`FreeList::capacity`] surfaces free and destroys +/// any returned past that, so a consumer that held many pictures and then let +/// them all go does not leave that many parked here. +pub(crate) struct SurfacePool { + display: Arc, + width: u32, + height: u32, + hint: UsageHint, + /// Shared with every surface on loan, which puts itself back from whichever + /// thread drops it last. + pub(crate) free: Arc, + /// Surfaces allocated over the pool's life. + pub(crate) allocated: Cell, +} + +/// The surfaces a [`SurfacePool`] has free to write into. +pub(crate) struct FreeList { + pub(crate) surfaces: Mutex>>, + /// The most surfaces kept; one returned past this is destroyed. + pub(crate) capacity: usize, +} + +impl SurfacePool { + /// Returns an empty pool of `width` by `height` surfaces allocated with `hint`, keeping at most `capacity` free. + pub(crate) fn new(display: &Arc, (width, height): (u32, u32), hint: UsageHint, capacity: usize) -> Self { + Self { + display: Arc::clone(display), + width, + height, + hint, + free: Arc::new(FreeList { + surfaces: Mutex::new(Vec::new()), + capacity, + }), + allocated: Cell::new(0), + } + } + + /// Returns a free surface, or allocates one when none is. + pub(crate) fn alloc(&self) -> anyhow::Result { + let recycled = self.free.surfaces.lock().expect("poisoned").pop(); + let surface = match recycled { + Some(surface) => surface, + None => { + let surface = self + .display + .create_surfaces::<()>( + VA_RT_FORMAT_YUV420, + Some(VA_FOURCC_NV12), + self.width, + self.height, + Some(self.hint), + vec![()], + ) + .map_err(|e| anyhow!("create a {}x{} surface: {e:?}", self.width, self.height))? + .pop() + .context("VA-API returned an empty surface list")?; + self.allocated.set(self.allocated.get() + 1); + log::debug!( + "allocated surface {} at {}x{}", + self.allocated.get(), + self.width, + self.height + ); + surface + } + }; + + Ok(PooledSurface { + surface: Some(surface), + free: Arc::downgrade(&self.free), + }) + } +} + +/// A surface on loan from a [`SurfacePool`], returned to it on drop. +/// +/// Shared behind an [`Arc`] by whatever still reads it, such as the decoder's +/// DPB and an [`ExportedFrame`](crate::decode::ExportedFrame), so it goes back +/// only once none refers to it. The pool is held weakly: a surface that +/// outlives its pool is destroyed rather than kept for a size nobody asks for +/// any more. +pub(crate) struct PooledSurface { + /// Always `Some` until dropped; the `Option` is only there so [`Drop`] can + /// move the surface back into the pool. + surface: Option>, + /// Where to return the surface. Empty for a surface that came from + /// somewhere other than a pool, which is destroyed on drop. + free: Weak, +} + +impl PooledSurface { + /// Wraps a surface that belongs to no pool. + pub(crate) fn unpooled(surface: Surface<()>) -> Self { + Self { + surface: Some(surface), + free: Weak::new(), + } + } + + pub(crate) fn get(&self) -> &Surface<()> { + self.surface.as_ref().expect("surface is taken only on drop") + } + + pub(crate) fn id(&self) -> bindings::VASurfaceID { + self.get().id() + } +} + +impl Drop for PooledSurface { + fn drop(&mut self) { + let (Some(surface), Some(free)) = (self.surface.take(), self.free.upgrade()) else { + return; + }; + let mut surfaces = free.surfaces.lock().expect("poisoned"); + if surfaces.len() < free.capacity { + surfaces.push(surface); + } + } +} diff --git a/src/vpp.rs b/src/vpp.rs index 186571f..f97a7a4 100644 --- a/src/vpp.rs +++ b/src/vpp.rs @@ -4,7 +4,8 @@ //! processor stretches to fit the destination and converts to the destination's //! format on the way. That covers three jobs without the pixels leaving the GPU: //! -//! - Scaling: blit into a smaller or larger surface. +//! - Scaling: blit into a smaller or larger surface. [`Processor::resize`] +//! blits into NV12 surfaces it recycles per size and exports the result. //! - Color conversion: blit a packed RGB or YUYV surface into an NV12 one, the //! format an encoder takes. [`Processor::blit`] takes the YUV space to write. //! - Re-tiling: blit into a surface of the same size and format. The destination @@ -35,7 +36,9 @@ use std::path::Path; use std::rc::Rc; use std::sync::Arc; +use crate::decode::ExportedFrame; use crate::dmabuf::{rt_format, DmaBuf}; +use crate::pool::SurfacePool; use crate::{ bindings, BufferType, Color, Config as VaConfig, Context, Display, Picture, ProcColorProperties, ProcPipelineParameterBuffer, Surface, SurfaceMemoryDescriptor, UsageHint, VAConfigAttrib, VAConfigAttribType, @@ -59,12 +62,14 @@ pub fn probe(display: &Display) -> anyhow::Result<()> { /// A video post-processor on one device. /// -/// Holds the display, the VPP configuration, and one processing context per -/// destination size, which are what cost something to build. A caller -/// converting every frame of a stream, or of a few renditions, pays for each -/// context once. +/// Holds the display, the VPP configuration, and per destination size a +/// processing context and the surfaces [`resize`](Self::resize) writes into, +/// which are what cost something to build. A caller converting every frame of +/// a stream, or of a few renditions, pays for each once. pub struct Processor { display: Arc, + /// The NV12 surfaces [`resize`](Self::resize) writes into, recycled per size. + outputs: RefCell>, // Drop order: the contexts before the config they were created from. contexts: RefCell>>, config: VaConfig, @@ -129,6 +134,7 @@ impl Processor { Ok(Self { display, + outputs: RefCell::new(HashMap::new()), contexts: RefCell::new(HashMap::new()), config, }) @@ -191,6 +197,41 @@ impl Processor { result } + /// Scales `input` to `width` by `height` and converts it to NV12, returning the result exported. + /// + /// The output comes from a pool per size, so a stream of frames costs no + /// allocation per frame: once the returned frame drops, its surface is the + /// next one written at that size. A frame still held is never written over; + /// the next resize allocates another surface instead. At most one free + /// surface is kept per size, and any returned past that is destroyed. See + /// [`blit`](Self::blit) for the colors. + /// + /// The frame's timestamp is zero, for the caller to set. + /// + /// # Errors + /// + /// Fails when the destination cannot be allocated or exported, and as + /// [`blit`](Self::blit) does. + pub fn resize( + &self, + input: &Surface, + size: (u32, u32), + input_color: Option, + output_color: Option, + ) -> anyhow::Result { + let output = self + .outputs + .borrow_mut() + .entry(size) + .or_insert_with(|| { + let hint = UsageHint::USAGE_HINT_VPP_WRITE | UsageHint::USAGE_HINT_EXPORT; + SurfacePool::new(&self.display, size, hint, 1) + }) + .alloc()?; + self.blit(input, output.get(), input_color, output_color)?; + ExportedFrame::export(Arc::new(output), 0, size) + } + /// Blits `input` into a new driver-allocated surface of `fourcc`, `width` and `height`. /// /// The destination is hinted as post-processing output that will be @@ -201,7 +242,7 @@ impl Processor { /// /// Fails for a format with no render target mapping, when the destination /// cannot be allocated, and as [`blit`](Self::blit) does. - pub fn process( + fn process( &self, input: &Surface, fourcc: u32, @@ -396,21 +437,8 @@ mod tests { "the blit preserves the plane count" ); } - /// A surface scaled by the processor exports and reads back at its new - /// size, with the picture scaled rather than cropped. - #[test] - fn a_scaled_surface_reads_back_at_its_new_size() { - let Some(display) = Display::open() else { - eprintln!("skipping: no VA-API display"); - return; - }; - let Ok(processor) = Processor::with_display(display) else { - eprintln!("skipping: device has no video post-processing entrypoint"); - return; - }; - - // Left half dark, right half bright, so a crop and a scale differ. - let (width, height) = (640u32, 480u32); + /// An NV12 surface whose left half has luma `left` and right half `right`, so a crop and a scale differ. + fn split_surface(processor: &Processor, (width, height): (u32, u32), [left, right]: [u8; 2]) -> Surface<()> { let source = processor .display() .create_surfaces( @@ -439,7 +467,7 @@ mod tests { for y in 0..height as usize { let row = va_image.offsets[0] as usize + y * va_image.pitches[0] as usize; for x in 0..width as usize { - data[row + x] = if x < width as usize / 2 { 40 } else { 200 }; + data[row + x] = if x < width as usize / 2 { left } else { right }; } } for y in 0..height as usize / 2 { @@ -447,20 +475,96 @@ mod tests { data[row..row + width as usize].fill(128); } } + source + } + + /// Reads `frame` back and returns the luma at the middle of its left and right halves. + fn halves(frame: &ExportedFrame) -> [u8; 2] { + let data = frame.download().expect("read the frame back").data; + let (w, h) = (frame.width as usize, frame.height as usize); + assert_eq!(data.len(), w * h * 3 / 2); + [data[h / 2 * w + w / 8], data[h / 2 * w + w * 7 / 8]] + } + + /// Whether every value is within 4 of the expected one. + fn near(actual: [u8; 2], expected: [u8; 2]) -> bool { + actual.iter().zip(expected).all(|(a, e)| a.abs_diff(e) < 4) + } - let scaled = processor - .process(&source, VA_FOURCC_NV12, (width / 2, height / 2), None, None) + /// The inode of an exported frame's descriptor, which names the allocation + /// behind it: every DMA-BUF gets its own on the anonymous dma-buf filesystem, + /// while each export of one surface is a new descriptor. + fn inode(frame: &ExportedFrame) -> u64 { + use std::os::unix::fs::MetadataExt as _; + + let fd = frame.descriptor.objects[0] + .fd + .try_clone() + .expect("duplicate the descriptor"); + std::fs::File::from(fd).metadata().expect("stat the descriptor").ino() + } + + /// A surface scaled by the processor exports and reads back at its new + /// size, with the picture scaled rather than cropped. + #[test] + fn a_scaled_surface_reads_back_at_its_new_size() { + let Some(display) = Display::open() else { + eprintln!("skipping: no VA-API display"); + return; + }; + let Ok(processor) = Processor::with_display(display) else { + eprintln!("skipping: device has no video post-processing entrypoint"); + return; + }; + + let source = split_surface(&processor, (640, 480), [40, 200]); + let exported = processor + .resize(&source, (320, 240), None, None) .expect("scale the surface"); - let exported = crate::decode::ExportedFrame::from_surface(scaled, 7).expect("export the scaled surface"); - assert_eq!((exported.width, exported.height), (width / 2, height / 2)); - assert_eq!(exported.timestamp, 7); - let frame = exported.download().expect("read the scaled surface back"); - let (w, h) = ((width / 2) as usize, (height / 2) as usize); - assert_eq!(frame.data.len(), w * h * 3 / 2); - let (left, right) = (frame.data[h / 2 * w + w / 8], frame.data[h / 2 * w + w * 7 / 8]); - assert!( - left.abs_diff(40) < 4 && right.abs_diff(200) < 4, - "read back {left} and {right}" + assert_eq!((exported.width, exported.height), (320, 240)); + assert_eq!(exported.descriptor.fourcc, VA_FOURCC_NV12); + let read = halves(&exported); + assert!(near(read, [40, 200]), "read back {read:?}"); + } + + /// A resize writes into the surface of an output that was let go, never + /// into one still held, and keeps at most one free surface per size. + #[test] + fn a_released_output_is_written_into_again() { + let Some(display) = Display::open() else { + eprintln!("skipping: no VA-API display"); + return; + }; + let Ok(processor) = Processor::with_display(display) else { + eprintln!("skipping: device has no video post-processing entrypoint"); + return; + }; + let dark_left = split_surface(&processor, (640, 480), [40, 200]); + let dark_right = split_surface(&processor, (640, 480), [200, 40]); + let size = (320, 240); + let resize = |source| processor.resize(source, size, None, None).expect("resize"); + + let first = resize(&dark_left); + let released = inode(&first); + drop(first); + let held = resize(&dark_left); + assert_eq!(inode(&held), released, "the released output was not written into again"); + + let other = resize(&dark_right); + assert_ne!(inode(&other), inode(&held), "an output still held was written over"); + let (read, other_read) = (halves(&held), halves(&other)); + assert!(near(read, [40, 200]), "the held output now reads {read:?}"); + assert!(near(other_read, [200, 40]), "the second output reads {other_read:?}"); + + drop(held); + drop(other); + let outputs = processor.outputs.borrow(); + let pool = &outputs[&size]; + assert_eq!(pool.allocated.get(), 2, "two outputs held at once need two surfaces"); + assert_eq!( + pool.free.surfaces.lock().unwrap().len(), + 1, + "one free surface is kept per size, and the other destroyed" ); } }