search

ExPixel / miniaudio-rs

Rust bindings for miniaudio C library.
https://crates.io/crates/miniaudio
MIT License
50 stars 20 forks source link

Added missing pcm_frames functions to ma_decoder #22

Closed Nazariglez closed 4 years ago

Nazariglez commented 4 years ago

Close #18

ExPixel commented 4 years ago

I was just about to mention the problem with requiring exclusive access to the decoder for this functions but it looks like you noticed and changed the &'s to &mut's. This means we would need real synchronization for decoders though. Right now you can just clone an immutable reference to the decoder and get an owned value pointing to the same RawDecoder. I was thinking it would be good to have two versions Decoder and SyncDecoder which would both be smart pointers (so the methods would be on RawDecoder instead) with SyncDecoder having some locking mechanisms that plays nicely in the audio callback thread.

Nazariglez commented 4 years ago

Sure, just a question, with a Sync version you mean that we're going to have two versions of decoder? One of them will be safe to be used on a multithreaded environment and the another is not? And, the sync version will be the same but just using .lock or .read/.write to interact with the RawDecoder right?

I like the idea if you think that is the right way, I can try to do my best.

ExPixel commented 4 years ago

That's what I mean yeah. The only problem is that locks like Mutex and RwLock use a syscall when they are dropped/unlocked when there is contention which might cause some timing issues in the audio thread We might need a spinlock (using std::sync::atomic::spin_loop_hint in a loop while checking an atomic or something). I won't have time to implement it this week so if this is something you would like to implement let me know.

ExPixel commented 4 years ago

Actually might not even have to implement the spin lock. I just remembered there is a library for it that I've used before: https://crates.io/crates/spin which has a spinning RwLock .Would be good to make it optional and put the sync decoder behind a cfg(feature) though.

Nazariglez commented 4 years ago

I can do it, but not sure when right now, I don't have so much time available for the next weeks but I suppose that this is not a blocker for anything for now so it doesn't matter that it takes some time.

Btw the spin crate seems to be deprecated. I can try with the std sync or parking_lot and if there some issues we can change back to spin and maybe help with the crate.

ExPixel commented 4 years ago

Implemented by d52412ea05bb1074e8438b0843ee6e1e23865aba