Conversation
kkysen
left a comment
There was a problem hiding this comment.
Thanks for the much safer implementation! I'm still taking a closer look at the unsafes, as the few remaining are still quite critical, but I wanted to give some initial feedback in general first.
Pulled out the docs additions from #1439 into its own PR.
kkysen
left a comment
There was a problem hiding this comment.
@leo030303, could you rebase this on main? There are a lot of other commits in here now, so it's harder to review.
Co-authored-by: Khyber Sen <kkysen@gmail.com>
Co-authored-by: Khyber Sen <kkysen@gmail.com>
Co-authored-by: Khyber Sen <kkysen@gmail.com>
Co-authored-by: Khyber Sen <kkysen@gmail.com>
Co-authored-by: Khyber Sen <kkysen@gmail.com>
Co-authored-by: Khyber Sen <kkysen@gmail.com>
Co-authored-by: Khyber Sen <kkysen@gmail.com>
Co-authored-by: Khyber Sen <kkysen@gmail.com>
Co-authored-by: Khyber Sen <kkysen@gmail.com>
|
Think that's most of the outstanding issues wrapped up! |
|
I can't wait for this to be finished! |
|
@kkysen if fixing the API for that to never be null isn't straightforward then it could probably be left for a follow up pr since I think the core of this pr should be good to go |
|
I've opened a proof-of-concept PR for It's not quite a drop-in replacement but the conversion was fairly straightforward. I've wired everything up to |
Co-authored-by: Khyber Sen <kkysen@gmail.com>
…=> `pixel::ChromaLocation` conversion
…mageComponent> for usize`
| impl TryFrom<usize> for PlanarImageComponent { | ||
| type Error = Rav1dError; | ||
| fn try_from(index: usize) -> Result<Self, Self::Error> { | ||
| match index { | ||
| 0 => Ok(PlanarImageComponent::Y), | ||
| 1 => Ok(PlanarImageComponent::U), | ||
| 2 => Ok(PlanarImageComponent::V), | ||
| _ => Err(Rav1dError::InvalidArgument), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Is this needed by image-rs or something?
I've copy pasted the PR from #1362 and updated it with some of the suggestions made on that pull request. The main changes are:
enums fromrav1dinstead of redefining new ones, and added in doc comments from the originaldav1d-rslibrary to a few items.unsafecode as I could and replaced it with the Rust methods fromrav1das much as possible.It currently works as a drop-in replacement for
dav1d-rs; adding inuse rav1d as dav1d;to my fork of image makes everything work fine.The only functional changes I made are I removed the
unsafe impls ofSendandSyncforInnerPicturesoPictureis no longerSyncorSend. I looked through the code and I don't believeDisjointMut<Rav1dPictureDataComponentInner>, which is a field of one of its children, is thread safe, though I'm open to correction there; I'm pretty unfamiliar withunsafeRust.I also don't have safety comments on the two
unsafeblocks inrust_api.rs; I'm unsure what these would look like, so open to suggestions there. These are mostly taken verbatim from the old pull request.