From a6951c7e69b4daa62f0046a3ebda2f0530c630e1 Mon Sep 17 00:00:00 2001 From: Sludge <96552222+SludgePhD@users.noreply.github.com> Date: Fri, 8 Mar 2024 19:52:15 +0100 Subject: [PATCH] Work around Mesa bug, improve some APIs a bit --- Cargo.toml | 3 ++ examples/infodump.rs | 12 +++++-- examples/jpeg-decode.rs | 20 ++++++++--- src/display.rs | 2 +- src/dlopen.rs | 35 +++++++++++++++++-- src/jpeg.rs | 33 +++++++++--------- src/pixelformat.rs | 9 +++-- src/surface.rs | 52 +++++++++++++++++++---------- src/vpp.rs | 74 +++++++++++++++++++++++++---------------- 9 files changed, 167 insertions(+), 73 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 68ab93d..dfa6d59 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,3 +21,6 @@ softbuffer = "0.3.0" jpeg-decoder = "0.3.0" anyhow = "1.0.68" expect-test = "1.4.0" + +[profile.dev.package."*"] +opt-level = 3 diff --git a/examples/infodump.rs b/examples/infodump.rs index 9769258..17e1ca3 100644 --- a/examples/infodump.rs +++ b/examples/infodump.rs @@ -144,8 +144,16 @@ fn main() -> Result<(), Box> { "- Output Color Standards: {:?}", caps.output_color_standards() ); - println!("- Input Pixel Formats: {:?}", caps.input_pixel_formats()); - println!("- Output Pixel Formats: {:?}", caps.output_pixel_formats()); + print!("- Input Pixel Formats: "); + match caps.input_pixel_formats() { + Some(fmts) => println!("{fmts:?}"), + None => println!(""), + } + print!("- Output Pixel Formats: "); + match caps.output_pixel_formats() { + Some(fmts) => println!("{fmts:?}"), + None => println!(""), + } } Ok(()) diff --git a/examples/jpeg-decode.rs b/examples/jpeg-decode.rs index cb7ce7d..a1cef21 100644 --- a/examples/jpeg-decode.rs +++ b/examples/jpeg-decode.rs @@ -3,10 +3,11 @@ use std::{num::NonZeroU32, rc::Rc, time::Instant}; use anyhow::bail; use fev::{ display::Display, + image::{Image, ImageFormat}, jpeg::{JpegDecodeSession, JpegInfo}, surface::ExportSurfaceFlags, + PixelFormat, }; -use softbuffer::{Context, Surface}; use winit::{ dpi::PhysicalSize, event::{ElementState, Event, KeyboardInput, MouseButton, VirtualKeyCode, WindowEvent}, @@ -49,8 +50,8 @@ fn main() -> anyhow::Result<()> { .build(&ev)?; let win = Rc::new(win); - let graphics_context = unsafe { Context::new(&win).unwrap() }; - let mut surface = unsafe { Surface::new(&graphics_context, &win).unwrap() }; + let graphics_context = unsafe { softbuffer::Context::new(&win).unwrap() }; + let mut surface = unsafe { softbuffer::Surface::new(&graphics_context, &win).unwrap() }; let PhysicalSize { width, height } = win.inner_size(); surface .resize( @@ -68,8 +69,19 @@ fn main() -> anyhow::Result<()> { .export_prime(ExportSurfaceFlags::SEPARATE_LAYERS | ExportSurfaceFlags::READ)?; log::debug!("PRIME export: {prime:#?}"); + let mut image = Image::new( + &display, + ImageFormat::new(PixelFormat::RGBA), + jpeg_info.width().into(), + jpeg_info.height().into(), + )?; + + log::debug!(""); + let start = Instant::now(); let surf = context.decode_and_convert(&jpeg)?; - let mapping = surf.map_sync()?; + log::debug!(" took {:?}", start.elapsed()); + surf.copy_to_image(&mut image)?; + let mapping = image.map()?; log::debug!("{} byte output", mapping.len()); diff --git a/src/display.rs b/src/display.rs index 82322bb..b634760 100644 --- a/src/display.rs +++ b/src/display.rs @@ -328,7 +328,7 @@ impl Display { Ok(Entrypoints { vec: entrypoints }) } - /// Queries the supported [`ImageFormat`][crate::image::ImageFormat]s. + /// Queries the supported [`ImageFormat`]s. pub fn query_image_formats(&self) -> Result { unsafe { let max = self.d.libva.vaMaxNumImageFormats(self.d.raw) as usize; diff --git a/src/dlopen.rs b/src/dlopen.rs index 95ed942..3a2a3c5 100644 --- a/src/dlopen.rs +++ b/src/dlopen.rs @@ -5,8 +5,7 @@ #![allow(bad_style)] use std::{ - ffi::c_void, - os::raw::{c_char, c_float, c_int, c_uchar, c_uint}, + os::raw::{c_char, c_float, c_int, c_short, c_uchar, c_uint, c_ulong, c_ushort, c_void}, sync::OnceLock, }; @@ -133,7 +132,19 @@ dylib! { fn vaDestroyImage(dpy: VADisplay, image: VAImageID) -> VAStatus; fn vaSetImagePalette(dpy: VADisplay, image: VAImageID, palette: *mut c_uchar) -> VAStatus; fn vaGetImage(dpy: VADisplay, surface: VASurfaceID, x: c_int, y: c_int, width: c_uint, height: c_uint, image: VAImageID) -> VAStatus; - fn vaPutImage(dpy: VADisplay, surface: VASurfaceID, image: VAImageID, src_x: c_int, src_y: c_int, src_width: c_uint, src_height: c_uint, dest_x: c_int, dest_y: c_int, dest_width: c_uint, dest_height: c_uint) -> VAStatus; + fn vaPutImage( + dpy: VADisplay, + surface: VASurfaceID, + image: VAImageID, + src_x: c_int, + src_y: c_int, + src_width: c_uint, + src_height: c_uint, + dest_x: c_int, + dest_y: c_int, + dest_width: c_uint, + dest_height: c_uint + ) -> VAStatus; fn vaDeriveImage(dpy: VADisplay, surface: VASurfaceID, image: *mut VAImage) -> VAStatus; fn vaMaxNumSubpictureFormats(dpy: VADisplay) -> c_int; fn vaQuerySubpictureFormats(dpy: VADisplay, format_list: *mut ImageFormat, flags: *mut c_uint, num_formats: *mut c_uint) -> VAStatus; @@ -172,6 +183,22 @@ dylib! { pub struct libva_x11; fn vaGetDisplay(dpy: *mut Display) -> VADisplay; + fn vaPutSurface( + dpy: VADisplay, + surface: VASurfaceID, + draw: Drawable, + srcx: c_short, + srcy: c_short, + srcw: c_ushort, + srch: c_ushort, + destx: c_short, + desty: c_short, + destw: c_ushort, + desth: c_ushort, + cliprects: *mut Rectangle, + number_cliprects: c_uint, + flags: c_uint + ) -> VAStatus; } dylib! { @@ -193,3 +220,5 @@ pub struct wl_buffer; /// Opaque type representing the Xlib X11 `Display` type. pub struct Display; + +pub type Drawable = c_ulong; diff --git a/src/jpeg.rs b/src/jpeg.rs index 5eb533a..748ce9c 100644 --- a/src/jpeg.rs +++ b/src/jpeg.rs @@ -15,8 +15,8 @@ use crate::{ context::Context, display::Display, error::Error, - raw::{VA_PADDING_LOW, VA_PADDING_MEDIUM}, - surface::SurfaceWithImage, + raw::{Rectangle, VA_PADDING_LOW, VA_PADDING_MEDIUM}, + surface::{RTFormat, Surface}, vpp::{ColorProperties, ColorStandardType, ProcPipelineParameterBuffer, SourceRange}, Entrypoint, PixelFormat, Profile, Result, Rotation, SliceParameterBufferBase, }; @@ -63,7 +63,8 @@ pub struct PictureParameterBuffer { num_components: u8, color_space: ColorSpace, rotation: Rotation, - va_reserved: [u32; VA_PADDING_MEDIUM - 1], + crop_rectangle: Rectangle, + va_reserved: [u32; VA_PADDING_MEDIUM - 3], } #[derive(Clone, Copy)] @@ -420,8 +421,8 @@ pub struct JpegDecodeSession { width: u32, height: u32, - jpeg_surface: SurfaceWithImage, - vpp_surface: SurfaceWithImage, + jpeg_surface: Surface, + vpp_surface: Surface, jpeg_context: Context, vpp_context: Context, @@ -446,10 +447,8 @@ impl JpegDecodeSession { let config = Config::new(&display, Profile::None, Entrypoint::VideoProc)?; let vpp_context = Context::new(&config, width, height)?; - let jpeg_surface = SurfaceWithImage::new(&display, width, height, PixelFormat::NV12)?; - let vpp_surface = SurfaceWithImage::new(&display, width, height, PixelFormat::RGBA)?; - - log::debug!("image format = {:?}", vpp_surface.image()); + let jpeg_surface = Surface::new(&display, width, height, RTFormat::YUV420)?; + let vpp_surface = Surface::with_pixel_format(&display, width, height, PixelFormat::RGBA)?; Ok(Self { width, @@ -462,11 +461,11 @@ impl JpegDecodeSession { } #[inline] - pub fn surface(&mut self) -> &mut SurfaceWithImage { + pub fn surface(&mut self) -> &mut Surface { &mut self.jpeg_surface } - /// Decodes a baseline JPEG, returning a [`SurfaceWithImage`] containing the decoded image. + /// Decodes a baseline JPEG, returning a [`Surface`] containing the decoded image. /// /// The decoded image is in the JPEG's native color space and uses an unspecified pixel format. /// @@ -474,7 +473,7 @@ impl JpegDecodeSession { /// /// This method returns an error when the JPEG is malformed or VA-API returns an error during /// decoding. - pub fn decode(&mut self, jpeg: &[u8]) -> Result<&mut SurfaceWithImage> { + pub fn decode(&mut self, jpeg: &[u8]) -> Result<&mut Surface> { // TODO make this more flexible and move to `error` module macro_rules! bail { ($($args:tt)*) => { @@ -589,8 +588,12 @@ impl JpegDecodeSession { } } - let Some(ppbuf) = ppbuf else { bail!("file is missing SOI segment") }; - let Some((slice_params, slice_data)) = slice else { bail!("file is missing SOS header") }; + let Some(ppbuf) = ppbuf else { + bail!("file is missing SOI segment") + }; + let Some((slice_params, slice_data)) = slice else { + bail!("file is missing SOS header") + }; let mut buf_dht = Buffer::new_param(&self.jpeg_context, BufferType::HuffmanTable, dhtbuf)?; let mut buf_iq = Buffer::new_param(&self.jpeg_context, BufferType::IQMatrix, iqbuf)?; @@ -612,7 +615,7 @@ impl JpegDecodeSession { Ok(&mut self.jpeg_surface) } - pub fn decode_and_convert(&mut self, jpeg: &[u8]) -> Result<&mut SurfaceWithImage> { + pub fn decode_and_convert(&mut self, jpeg: &[u8]) -> Result<&mut Surface> { self.decode(jpeg)?; let mut pppbuf = ProcPipelineParameterBuffer::new(&self.jpeg_surface); diff --git a/src/pixelformat.rs b/src/pixelformat.rs index 768ff62..5aaed2e 100644 --- a/src/pixelformat.rs +++ b/src/pixelformat.rs @@ -23,9 +23,14 @@ impl PixelFormat { /// Interleaved YUV 4:2:2, stored in memory as `yyyyyyyy uuuuuuuu YYYYYYYY vvvvvvvv`. /// - /// `uuuuuuuu` and `vvvvvvvv` are shared by 2 neighboring pixels. + /// `uuuuuuuu` and `vvvvvvvv` are shared by 2 horizontally neighboring pixels. + /// + /// Also known as [`YUYV`](Self::YUYV). pub const YUY2: Self = f(b"YUY2"); + /// Identical to [`YUY2`](Self::YUY2). + pub const YUYV: Self = f(b"YUYV"); + /// Interleaved YUV 4:2:2, stored in memory as `uuuuuuuu yyyyyyyy vvvvvvvv YYYYYYYY`. /// /// `uuuuuuuu` and `vvvvvvvv` are shared by 2 neighboring pixels. @@ -72,7 +77,7 @@ impl PixelFormat { pub fn to_rtformat(self) -> Option { Some(match self { Self::NV12 | Self::NV21 => RTFormat::YUV420, - Self::YUY2 | Self::UYVY => RTFormat::YUV422, + Self::YUY2 | Self::YUYV | Self::UYVY => RTFormat::YUV422, Self::RGBA | Self::RGBX | Self::ARGB | Self::BGRA | Self::BGRX => RTFormat::RGB32, _ => return None, }) diff --git a/src/surface.rs b/src/surface.rs index 4d06463..79226a1 100644 --- a/src/surface.rs +++ b/src/surface.rs @@ -326,9 +326,33 @@ pub struct Surface { impl Surface { pub fn new(display: &Display, width: u32, height: u32, format: RTFormat) -> Result { + log::trace!("creating {width}x{height} surface with {format:?}"); Self::with_attribs(display, width, height, format, &mut []) } + pub fn with_pixel_format( + display: &Display, + width: u32, + height: u32, + format: PixelFormat, + ) -> Result { + let rtformat = format.to_rtformat().ok_or_else(|| { + Error::from(format!( + "no RTFormat to go with the requested pixel format {:?}", + format + )) + })?; + + log::trace!("creating {width}x{height} surface with format {format:?} and {rtformat:?}"); + Self::with_attribs( + &display, + width, + height, + rtformat, + &mut [SurfaceAttribEnum::PixelFormat(format).into()], + ) + } + pub fn with_attribs( display: &Display, width: u32, @@ -458,7 +482,9 @@ impl Surface { /// Creates an [`Image`] that allows direct access to the surface's image data. /// /// Only supported by some drivers, and only for some surface formats. Will return - /// [`VAError::ERROR_OPERATION_FAILED`] if it's not supported. + /// [`VAError::ERROR_OPERATION_FAILED`] if it's not supported. In that case, the caller should + /// fall back to creating an [`Image`] manually and using [`Surface::copy_to_image`]. The + /// [`SurfaceWithImage`] type encapsulates that pattern and should be used for this if possible. pub fn derive_image(&mut self) -> Result { unsafe { let mut image = MaybeUninit::uninit(); @@ -531,28 +557,14 @@ impl SurfaceWithImage { /// The [`RTFormat`] of the [`Surface`] will be determined automatically based on the specified /// [`PixelFormat`]. pub fn new(display: &Display, width: u32, height: u32, format: PixelFormat) -> Result { - let rtformat = format.to_rtformat().ok_or_else(|| { - Error::from(format!( - "pixel format {:?} is unknown or unimplemented", - format - )) - })?; - - let mut surface = Surface::with_attribs( - &display, - width, - height, - rtformat, - &mut [SurfaceAttribEnum::PixelFormat(format).into()], - )?; + let mut surface = Surface::with_pixel_format(display, width, height, format)?; // Try to use `vaDeriveImage` first, fall back if that fails. match surface.derive_image() { Ok(image) => { log::trace!( "using vaDeriveImage for fast surface access \ - (surface format = {:?}, image format = {:?})", - rtformat, + (image format = {:?})", format, ); @@ -563,7 +575,10 @@ impl SurfaceWithImage { }) } Err(e) if e.as_libva() == Some(VAError::ERROR_OPERATION_FAILED) => { - log::trace!("vaDeriveImage not supported, using vaGetImage (surface format = {:?}, image format = {:?})", rtformat, format); + log::trace!( + "vaDeriveImage not supported, using vaGetImage (simage format = {:?})", + format + ); let image = Image::new(display, ImageFormat::new(format), width, height)?; Ok(Self { @@ -586,6 +601,7 @@ impl SurfaceWithImage { &self.image } + /// Synchronizes the [`Surface`] and [`Image`] contents and maps the [`Image`] into memory. pub fn map_sync(&mut self) -> Result> { if self.derived { self.surface.sync()?; diff --git a/src/vpp.rs b/src/vpp.rs index 727ad29..14e7fa9 100644 --- a/src/vpp.rs +++ b/src/vpp.rs @@ -58,10 +58,13 @@ impl Context { &self, filters: &mut Filters, ) -> Result { - let mut input_color_standards = vec![ColorStandardType(0); 32]; - let mut output_color_standards = vec![ColorStandardType(0); 32]; - let mut input_pixel_formats = vec![PixelFormat::from_u32_le(0); 32]; - let mut output_pixel_formats = vec![PixelFormat::from_u32_le(0); 32]; + const BUFLEN: usize = 32; + const EMPTY_COLOR_STANDARDS: &[ColorStandardType] = &[ColorStandardType(0); BUFLEN]; + const EMPTY_PIXEL_FORMATS: &[PixelFormat] = &[PixelFormat::from_u32_le(0); BUFLEN]; + let mut input_color_standards = EMPTY_COLOR_STANDARDS.to_vec(); + let mut output_color_standards = EMPTY_COLOR_STANDARDS.to_vec(); + let mut input_pixel_formats = EMPTY_PIXEL_FORMATS.to_vec(); + let mut output_pixel_formats = EMPTY_PIXEL_FORMATS.to_vec(); unsafe { let mut caps: RawProcPipelineCaps = mem::zeroed(); @@ -87,33 +90,44 @@ impl Context { // Intel's and Mesa's implementation doesn't use the user-provided buffers, but changes // the pointer to point to static data, despite the `va_vpp.h` docs implying otherwise. // Support both by copying to our buffer. - let num_in_color_stds = caps.num_input_color_standards as usize; - let num_out_color_stds = caps.num_output_color_standards as usize; if caps.input_color_standards != input_color_standards.as_mut_ptr() { - input_color_standards[..num_in_color_stds].copy_from_slice(slice::from_raw_parts( - caps.input_color_standards, - num_in_color_stds, - )); + let num = caps.num_input_color_standards as usize; + input_color_standards[..num] + .copy_from_slice(slice::from_raw_parts(caps.input_color_standards, num)); } if caps.output_color_standards != output_color_standards.as_mut_ptr() { - output_color_standards[..num_out_color_stds].copy_from_slice( - slice::from_raw_parts(caps.output_color_standards, num_out_color_stds), - ); + let num = caps.num_output_color_standards as usize; + output_color_standards[..num] + .copy_from_slice(slice::from_raw_parts(caps.output_color_standards, num)); } - - // Aaaand of course nobody actually supports the pixel format part of the interface. - // So when the number is unchanged, assume the data is too, and clear it. - if caps.num_input_pixel_formats == input_pixel_formats.len() as _ { - input_pixel_formats.clear(); + if caps.input_pixel_format != input_pixel_formats.as_mut_ptr() { + let num = caps.num_input_pixel_formats as usize; + input_pixel_formats[..num] + .copy_from_slice(slice::from_raw_parts(caps.input_pixel_format, num)); } - if caps.num_output_pixel_formats == output_pixel_formats.len() as _ { - output_pixel_formats.clear(); + if caps.output_pixel_format != output_pixel_formats.as_mut_ptr() { + let num = caps.num_output_pixel_formats as usize; + output_pixel_formats[..num] + .copy_from_slice(slice::from_raw_parts(caps.output_pixel_format, num)); } + // Aaaand of course nobody actually supports the pixel format part of the interface. + // So when the data is unchanged, set them to `None`. + let input_pixel_formats = if input_pixel_formats == EMPTY_PIXEL_FORMATS { + None + } else { + input_pixel_formats.truncate(caps.num_input_pixel_formats as _); + Some(input_pixel_formats) + }; + let output_pixel_formats = if output_pixel_formats == EMPTY_PIXEL_FORMATS { + None + } else { + output_pixel_formats.truncate(caps.num_output_pixel_formats as _); + Some(output_pixel_formats) + }; + input_color_standards.truncate(caps.num_input_color_standards as _); output_color_standards.truncate(caps.num_output_color_standards as _); - input_pixel_formats.truncate(caps.num_input_pixel_formats as _); - output_pixel_formats.truncate(caps.num_output_pixel_formats as _); Ok(ProcPipelineCaps { raw: caps, @@ -566,8 +580,8 @@ pub struct ProcPipelineCaps { raw: RawProcPipelineCaps, input_color_standards: Vec, output_color_standards: Vec, - input_pixel_formats: Vec, - output_pixel_formats: Vec, + input_pixel_formats: Option>, + output_pixel_formats: Option>, } impl ProcPipelineCaps { @@ -601,14 +615,18 @@ impl ProcPipelineCaps { &self.output_color_standards } + /// Returns the list of supported [`PixelFormat`]s for the source surface, or [`None`] if + /// unknown. #[inline] - pub fn input_pixel_formats(&self) -> &[PixelFormat] { - &self.input_pixel_formats + pub fn input_pixel_formats(&self) -> Option<&[PixelFormat]> { + self.input_pixel_formats.as_deref() } + /// Returns the list of supported [`PixelFormat`]s for the destination surface, or [`None`] if + /// unknown. #[inline] - pub fn output_pixel_formats(&self) -> &[PixelFormat] { - &self.output_pixel_formats + pub fn output_pixel_formats(&self) -> Option<&[PixelFormat]> { + self.output_pixel_formats.as_deref() } }