RustAudio / ogg

Ogg container decoder and encoder written in pure Rust
Other
113 stars 21 forks source link

`no_std` support #33

Open AlexTMjugador opened 2 years ago

AlexTMjugador commented 2 years ago

As mentioned in https://github.com/RustAudio/ogg/pull/32#issuecomment-1107841292, it'd be nice to make this crate work with #![no_std], which would allow its usage by embedded developers or other freestanding environments. Moreover, as this requires carefully choosing any dependencies, those that use the crate in std environments will benefit from reducing their number as well, slightly speeding up build times.

An obvious first start would be getting rid of the byteorder dependency, as the core library of the MSRV, which is also available in no #![no_std], already contains methods to convert between bytes and integers.

Getting rid of the Read and Write traits is a bit more involved. However, some days ago I've stumbled upon the acid_io crate, which provides suitable, drop-in replacements for these traits. Using conditional compilation and feature flags to choose between acid_io's traits and the std ones is easy enough to be worth a try, in my opinion.

est31 commented 2 years ago

Interesting crate! I'm just a bit worried that it has no users on crates.io apparently. I wonder if there are crates that do the same with more users.

AlexTMjugador commented 2 years ago

There's also not-io, which was published by the same person behind the popular, well-maintained png, gif and deflate crates, but it also does not have any publicly known dependents on crates.io either. core_io is yet another alternative, and this time there are some other crates using it. Still, acid_io looks like the most "drop-in replacement" of them all to me.

The rust-lang/rust repository has an interesting issue about why the Read and Write traits are not available in no_std environment and mentions some of these crates: https://github.com/rust-lang/rust/issues/48331.

est31 commented 2 years ago

core_io is the only one with actual users, but it seems to require nightly, so isn't really an option. What about rolling our own traits? They shouldn't be too complicated.

AlexTMjugador commented 2 years ago

That could work relatively elegantly, too. After all, the core parsing logic is mostly concerned with memory buffers than calling I/O.

When targeting std it would be possible to create a blanket implementation for the custom traits to delegate the operations to the real Read, Write and Seek traits. On the other hand, when targeting no_std, those traits can be manually on core types, such as slices.

On a side note, I'm thinking it's a bit sad that there are no good off-the-shelf solutions for the seemingly basic problem of byte source and sink abstractions for non-std environments, but I guess if we do it right a worth-publishing subcrate may spawn, and compete with the other solutions.

est31 commented 2 years ago

The bytes crate looks like it has what we need, except that the operations panic if there is an issue instead of returning a Result, which isn't optimal.

AlexTMjugador commented 2 years ago

The types provided by the bytes crate are meant to work with in-memory buffers. Memory allocation failures are usually treated as hard, panic-worthy errors through the ecosystem, so it's not worse than what we have now (we already allocate Vecs, which can panic). So, for in-memory buffers, I think that bytes is a good solution for no_std.

The problem would then be handling I/O from other sources, which I think it's important on embedded devices with constrained memory that can't afford to just read everything to RAM.

I think we could extend the idea of the custom trait blanket implementations to be able to read from any type that implements bytes::Buf, and then let users implement that fallible trait themselves to handle reading from other sources.

est31 commented 2 years ago

The issue is if the end of the buffer is reached, which can occur with corrupted data. bytes in this instance panics, which is not helpful. Instead, something that returns an error would be helpful, but of course it can't be std::io::Error...