Skip to content

PoC: switch to rav1d for AVIF decoding#2849

Draft
Shnatsel wants to merge 3 commits intoimage-rs:mainfrom
Shnatsel:ravid
Draft

PoC: switch to rav1d for AVIF decoding#2849
Shnatsel wants to merge 3 commits intoimage-rs:mainfrom
Shnatsel:ravid

Conversation

@Shnatsel
Copy link
Member

@Shnatsel Shnatsel commented Mar 14, 2026

Depends on memorysafety/rav1d#1439

We don't have any in-tree AVIF tests, so I tested manually with Shnatsel/wondermagick#92 and it seems to work great!

@fintelia
Copy link
Contributor

We'll probably want to get a sense of the crate with fuzzing/Miri before switching over.

From what I understand zlib-rs was created by the same organization. And my experience with zlib-rs was that the marketing got a bit ahead of the implementation: When I saw it declared as ready to try for production use, I ran it through some fuzzing and discovered that it instantly crashed because a few decode code-paths were still marked todo!.

@Shnatsel
Copy link
Member Author

FWIW I don't see the two projects sharing any key developers. But I don't see a fuzzing harness in-tree, which is a potential red flag.

@Shnatsel
Copy link
Member Author

Shnatsel commented Mar 14, 2026

Oh, https://github.com/imazen/rav1d-safe also exists where people ported rav1d's handwritten assembly to Rust's safe SIMD intrinsics plus safe_unaligned_simd. And it already comes with a safe Rust API. I believe that could be a better fit for image-rs, provided it works as advertised; we're not operating under video codec level of performance constraints.

However, the license is AGPL-3.0, which is a very aggressive copyleft variant. But the README includes the following:

##Upstream Contribution

This fork exists because maintaining a separate safe SIMD implementation is the fastest path to getting safe Rust AV1 decoding into production. If the rav1d maintainers are interested in upstreaming any of this work under the original BSD-2-Clause license, we'd be happy to contribute. Open an issue or reach out.

Should I talk to the memorysafety.org folks (with you cc'd) about getting the safe Rust codepaths upstreamed?

Or perhaps we could reach out to rav1d-safe ourselves and discuss inclusion into image directly?

@197g
Copy link
Member

197g commented Mar 14, 2026

In any case it brought archmage to my attention which proxies safe-unaligned-simd for the memory access 🎉

@awxkee
Copy link
Contributor

awxkee commented Mar 14, 2026

Whole rav1d-safe seems to me have been made by AI. Don't get me wrong I'm fine with AI, but there is like ~100K lines fairly complex AI-generated code that likely no one at least partially understands. If there are logical or safety errors, which is almost inevitable, they will be quite difficult to track and fix, especially since no one really knows what the code is actually supposed to do.

@Shnatsel
Copy link
Member Author

Shnatsel commented Mar 14, 2026

#[forbid(unsafe_code)] insulates us from safety errors. Since the entire thing is safe Rust, the consequences of AI mistakes are far less drastic. I'd take 100k of safe Rust over 160k unsafe handwritten assembly.

especially since no one really knows what the code is actually supposed to do.

That's not true: there are scalar Rust functions implementing the same behavior. So we always have known-good reference code and can automatically generate tests that compare the output of any optimized implementation to the scalar reference. This is precisely the property that could make an AI-driven conversion manageable; without this ground truth it would be a lost cause.

Still, the fact that such a faithful conversion is potentially feasible doesn't mean that rav1d-safe did it with an appropriate level of rigor. Their claims still need to be verified.

@awxkee
Copy link
Contributor

awxkee commented Mar 14, 2026

That's not true: there are scalar Rust functions implementing the same behavior. So we always have known-good reference code and can automatically generate tests that compare the output of any optimized implementation to the scalar reference. This is precisely the property that could make an AI-driven conversion manageable; without this ground truth it would be a lost cause.

I meant that if something goes wrong in the decoding path without an obvious panic, and the chatbot can't guess where the mistake is, one would have to debug the entire decoding path from scratch, because no one would be able to point out even where to start looking.

@197g
Copy link
Member

197g commented Mar 14, 2026

I have to do that in other repositories, too, both that I wrote and did not write myself. The argument is not exactly a quantitative methodology for getting rid of faults in comitted code. Using an LLM for translation of code is as close to a qualified use case for it as it gets; and intrinsic instructions rarely have panic side effects so really besides weird behavior I'm not too concerned.

@Shnatsel
Copy link
Member Author

Shnatsel commented Mar 14, 2026

The upstream rav1d (not -safe) seems to be decently complete. I've written a fuzzing harness and the fuzzer didn't find any todo!()s. It did find a panic on invalid indexing due to arithmetic overflow, but that's precisely why I want a Rust implementation. No AddressSanitizer errors so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants