Copy dav1d as the Rust API
#1362
Conversation
Found when implementing a Rust API in #1362.
There was a problem hiding this comment.
Having a public Rust API matching dav1d-rs seems like it'd be very nice, as that Rusty API has already been designed for us evidently. However, this is way too much unsafe code that is entirely undocumented/unjustified, and I don't really see why we can't wrap the safe but private Rust API that we have right now (the rav1d_* versions of all of the dav1d_* functions, which are all safe, but otherwise have very similar APIs besides converting the data types).
| bitflags = "2.4.0" | ||
| cfg-if = "1.0.0" | ||
| libc = "0.2" | ||
| parking_lot = "0.12.2" | ||
| paste = "1.0.14" | ||
| raw-cpuid = "11.0.1" | ||
| static_assertions = "1" |
There was a problem hiding this comment.
Can we keep this like the other dependencies with a full major.minor.patch? We just used what cargo add gave us.
| impl From<Rav1dError> for Error { | ||
| fn from(err: Rav1dError) -> Self { | ||
| match err { | ||
| Rav1dError::EAGAIN => Error::Again, | ||
| Rav1dError::ENOMEM => Error::NotEnoughMemory, | ||
| Rav1dError::EINVAL => Error::InvalidArgument, | ||
| Rav1dError::ENOPROTOOPT => Error::UnsupportedBitstream, | ||
| _ => Error::UnknownError(err), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Do we just want to use this as Rav1dError in the first place?
| unsafe { | ||
| let mut dav1d_settings = mem::MaybeUninit::uninit(); | ||
|
|
||
| dav1d_default_settings(NonNull::new(dav1d_settings.as_mut_ptr()).unwrap()); | ||
|
|
||
| Self { | ||
| dav1d_settings: dav1d_settings.assume_init(), | ||
| } | ||
| } |
There was a problem hiding this comment.
This one is pretty trivial to do safely using the Rust API.
| bitflags::bitflags! { | ||
| #[derive(Copy, Clone, Debug, PartialEq, Eq, Default)] | ||
| pub struct InloopFilterType: u32 { | ||
| const DEBLOCK = DAV1D_INLOOPFILTER_DEBLOCK; | ||
| const CDEF = DAV1D_INLOOPFILTER_CDEF; | ||
| const RESTORATION = DAV1D_INLOOPFILTER_RESTORATION; | ||
| } | ||
| } |
There was a problem hiding this comment.
We'd prefer just making Rav1dInloopFilterType this instead of doing a conversion here at the edge of the API.
| impl TryFrom<u32> for DecodeFrameType { | ||
| type Error = TryFromEnumError; | ||
|
|
||
| fn try_from(value: u32) -> Result<Self, Self::Error> { | ||
| match value { | ||
| DAV1D_DECODEFRAMETYPE_ALL => Ok(DecodeFrameType::All), | ||
| DAV1D_DECODEFRAMETYPE_REFERENCE => Ok(DecodeFrameType::Reference), | ||
| DAV1D_DECODEFRAMETYPE_INTRA => Ok(DecodeFrameType::Intra), | ||
| DAV1D_DECODEFRAMETYPE_KEY => Ok(DecodeFrameType::Key), | ||
| _ => Err(TryFromEnumError(())), | ||
| } | ||
| } | ||
| } |
| #[derive(Debug, Copy, Clone, PartialEq, Eq)] | ||
| pub struct TryFromEnumError(()); | ||
|
|
||
| impl std::fmt::Display for TryFromEnumError { |
There was a problem hiding this comment.
We've preferred fully importing these kinds of items (e.x. use std::fmt::Display).
| cookie: Option<SendSyncNonNull<std::ffi::c_void>>, | ||
| ) { | ||
| let cookie = cookie.unwrap().as_ptr().as_ptr(); | ||
| let buf = unsafe { Box::from_raw(cookie as *mut T) }; |
There was a problem hiding this comment.
We haven't enabled it in CI yet, but we have #![deny(clippy::undocumented_unsafe_blocks)] on. All unsafe blocks should be documented with // SAFETY comments.
| /// Pixel layout of a frame. | ||
| #[derive(Debug, Eq, PartialEq, Copy, Clone)] | ||
| pub enum PixelLayout { | ||
| /// Monochrome. | ||
| I400, | ||
| /// 4:2:0 planar. | ||
| I420, | ||
| /// 4:2:2 planar. | ||
| I422, | ||
| /// 4:4:4 planar. | ||
| I444, | ||
| } | ||
|
|
||
| /// Frame component. | ||
| #[derive(Eq, PartialEq, Copy, Clone, Debug)] | ||
| pub enum PlanarImageComponent { | ||
| /// Y component. | ||
| Y, | ||
| /// U component. | ||
| U, | ||
| /// V component. | ||
| V, | ||
| } |
There was a problem hiding this comment.
These are much better names than we came up with (we don't know much about AV1 or video codecs themselves). We should try to just use these internally more.
This basically copy-pastes https://github.com/rust-av/dav1d-rs/blob/master/src/lib.rs (actually via https://github.com/rerun-io/rav1d-rs/blob/master/src/lib.rs) into rav1d to give rav1d a proper Rust interface.
It is a bit silly to wrap the C FFI, but it works.
The upshot is that this makes
rav1da drop-in replacement for https://crates.io/crates/dav1d