No_std support for symphonia-core

[atoktoto]

Following the encouragement from #88 and seeing that #91 got stuck I decided to take a shot myself.

My approach:

• Get the crate to build with #![no_std] and extern crate alloc;
• Make minimal changes in the code base, introduce no extra dependencies
• Use core::Error even though it's still behind #![feature(error_in_core)], switch to Nightly until stabilized ( Tracking Issue for Error in core rust-lang/rust#103765 )
• Start with core and flac and get it running on actual hardware to see if a simple conversion is viable.

[atoktoto]

Quick update:

• Got the wav/PCM decoding working on my hardware
• FLAC compiles but I run out of heap space on initialization, waiting for a board with some more ram :)

Is there a reason why MediaSource trait requires Send + Sync ? It seems to build without those.

[atoktoto]

I will push the code later but for now another update, I got the FLAC playing but:

• I'm using 256 kB of heap on top of 128 kB normal RAM and running at 400 Mhz (stm32h747cm7)
• I had to disable CRC checks in flac Fragment
• I removed all i64 usage in flac decoder (replaced with i32)
• I replaced chained iterators with a for loop in lpc_predict

Some more thought is needed to make it work generically which makes this more of a proof of concept. To make it marge-able I will need some guidance on how these can be handled.

[pdeljanov]

Hi @atoktoto,

Very interesting work here and looking forward to seeing how far you can get!

I have not done a very thorough read-through of the changes, but I do have some concerns I'd like to bring up before you get in too far. I know this is early days and just proof-of-concept work!

First, I think there needs to be a very firm separation between changes required for no_std, and changes required make Symphonia suitable for microcontrollers. These are two different problems, and the latter is going to be much more difficult than the former. It is highly unlikely I will merge a single PR that addresses both at the same time. Hopefully it will become clear why below.

---

In terms of changes to symphonia-core:

• I am not comfortable with the copying of std::io enums and traits. We have a stricter license than that of the standard library, so this would not be permitted.

• Changing the byte/bit readers from returning io::Result to Result will likely create a performance hit since the former is smaller than the latter, and these functions are used in extremely hot and inlined code paths. Would need to benchmark this with something like MP3 decoding (uses the bit reader heavily).

---

In terms of the changes you're making to symphonia-bundle-flac:

• You cannot disable the CRC checks in Fragment. They are used to properly packetize the native FLAC container, and while it may work for some files (particularly error-free ones), that is not a guarantee if the checks are removed.

• i64 is required to not cause overflows. Intermediate values while decoding may exceed 32 bits. Perhaps with effort it is possible to use a pure 32-bit decode pipeline, but it would need significant validation.

• I'm not sure why removing iterators would be necessary? That is idiomatic Rust and in theory should optimize away.

---

As you can see, changing the decoders to suit small processors can quickly become a major task. For this reason, I think those changes should live in another PR, or a separate set of "micro" versions of the decoders should be created instead. Particularly if you want to use fixed-point instead of floating-point (as the lossy decoders do).

I don't think I am willing to compromise on the functionality, correctness, performance, or maintainability of the current set of decoders to better support microcontrollers. However, making such compromises in a separate complementary set of decoders whether, in-tree or out-of-tree, is okay.

---

Finally, to answer your question:

Is there a reason why MediaSource trait requires Send + Sync ? It seems to build without those.

This was a change request made long ago to simplify the effort required to use Symphonia in certain use-cases. It's not a requirement of the library itself, and I will likely be reverting it at the next sem-ver breaking release.

---

I hope this helps guide you!

[atoktoto]

Thank you for the response and it all makes a lot of sense for me. What I'm thinking now is:

• I will keep this branch focused on no_std support with minimal changes and keep the codecs changes separate (probably in another branch based on this one).
• I will try to address the io::Result concerns, probably start with running some benchmarks (on x64) and reading up on the differences :D
• I will try to get rid of std::io copied stuff, it felt like a hack from the get-go and most of those are not even used.

I'm not sure why removing iterators would be necessary? That is idiomatic Rust and in theory should optimize away.
I'm also not sure but I got there by measuring.

I changed

let predicted = coeffs
            .iter()
            .zip(&buf[i - N..i])
            .map(|(&c, &s)| c * c)
            .sum::<i32>();

into

let buf_s = &buf[i - N..i];
let mut predicted = 0;
for j in 0..N {
     predicted += buf_s[j] * coeffs[j]
}

I didn't check for correctness (or overflows), just wanted to finally hear some sound :D and it will stay on a different branch. I will measure the actual impact and share the data.

[atoktoto]

Errors

I reworked the Error structure. There is no copied code from std anymore and the whole thing looks much cleaner.
All errors are wrapped in SymphoniaError which has an IoError(Box<dyn core::error::Error>) variant. I also added variants for IoInterruptedError and EndOfFile (for #246). This way the cases can be handled when needed but the abstraction is less leaky. We are also not loosing the inner error reference which can be downcasted if needed. On the usage side (in codecs) I imported it as Error to reduce the amount of changes.

Performance

I run the ffbench vs symphonia-play a couple of times on the MLKDream.mp3 and there seems to be a no measurable impact on performance from those changes comparing to master. I did not manage to run the full suite as it seems to depend on ffmpeg a not the ffbench and I wasn't sure if that was intentional.

Next up

1. Roll-back the addition of optional fft and id3v2 features as this is not strictly necessary for no_std
2. Roll-back the move of apic_picture_type_to_visual_key function
3. Make all codecs no_std compatible to make sure std is not accidentally included by enabling any of those
4. Make sure I did not break any error-handling scenarios
5. Document the no_std additions
6. Run rustfmt

Open to any feedback :)

[pdeljanov]

Sounds like you have a pretty solid plan!

It looks like the Error part could be the most controversial. To be frank, I have little experience with no_std and what the best practices are, so it will probably be a while before I can have any reasonable feedback. Hopefully some other experts can also chime in. :)

For now, I think you could just continue with your plan. As long as you try to keep the changes as uninvasive as possible it should be workable.

[atoktoto]

I decided to not migrate all codecs in this PR. While the changes are straight-forward there is a lot of them and they would make the review of this PR harder. I left no_std versions of WAV and FLAC as an example what no_std compatibility entails but removed all changes that made them micro-controller-ready.