Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support the SPC music format #7

Open
est31 opened this issue Jan 11, 2017 · 15 comments
Open

Support the SPC music format #7

est31 opened this issue Jan 11, 2017 · 15 comments

Comments

@est31
Copy link
Member

est31 commented Jan 11, 2017

Recently, there has been a RCE-able c implementation. With all the rust based SNES emulators around, I'm sure there is one that also has support for the SPC music format? It would be great to provide a safe alternative to the dangerous C implementations our users.

@est31
Copy link
Member Author

est31 commented Jan 12, 2017

Yup, this should do it: https://github.com/emu-rs/snes-apu

However, seems it only compiles for OSX, guess due to missing audio output bindings for the other platforms. There is even an issue about this: emu-rs/emu#6

@est31
Copy link
Member Author

est31 commented Jan 12, 2017

That issue looks like a place where we could help. First we should research whether there are any resamplers now, and if not maybe we should write our own. Maybe some audio-utils crate that contains commonly needed things?

@mitchmindtree @ruuda @yupferris what are your thoughts?

@mitchmindtree
Copy link
Member

Sure! I've not ran into any use cases for SPC personally but I'd be happy to see it supported.

@est31 It looks like that issue is only related to the audio IO, not the codec itself. emu seems to use the spc crate for spc decoding, which I'm guessing is already portable as it is in pure rust?

Re: resampling and the audio-utils idea - I thought it might be best to open up a new issue for this specifically, see #11.

@est31
Copy link
Member Author

est31 commented Jan 12, 2017

It looks like that issue is only related to the audio IO, not the codec itself.

The spc crate only decodes the dsp program from the file, but actually get the samples you need to run an emulator. Also, I've looked, seems there is no feature to turn off playback support so that there is no requirement on mac os. Probably something that upstream should pursue either way as the decoders shouldn't pull in os playback api libraries I think :)

@est31
Copy link
Member Author

est31 commented Jan 12, 2017

About use cases... I don't have one either, but its one more format we support :)

@yupferris
Copy link

hi there! As the author of snes-apu, I'd love to see this integration happen. As for it only compiling for OS X, this is actually not true. The crate itself should compile fine on all platforms; the example only compiles for OS X, as it relies directly on our coreaudio-rs package currently for actual playback. The actual lib code containing the core emulation should be fully cross-platform though (if it's not that's a bigger problem and we need to fix that).

As for emu-rs/emu#6, I was kindof waiting around for cpal to incorporate coreaudio-rs again, then was going to add a driver for that there. I also want to redesign the emu audio driver api to be push-based rather than pull-based, and it'd be best to do that before incorporating additional platforms most likely. However, in the context of integrating with this audio library, that stuff shouldn't be necessary for now (however I'd be thrilled if you wanted to take a look at that).

Overall though I'm happy to help in any way possible if you're willing to incorporate this package :)

@yupferris
Copy link

Additionally for emu/snes-apu etc if relicensing under MIT/apache2 makes things simpler, I'm happy to accept that as well.

@yupferris
Copy link

Just did a quick clone/build; snes-apu indeed works fine on Windows (minus the example ofc, which actually needs playback).

@est31
Copy link
Member Author

est31 commented Jan 12, 2017

About licensing: BSD like licenses are okay. The only ones I'd avoid for this crate are strong copyleft ones like LGPL or GPL.

And for compiling it, I guess you are right. Missed that emu was listed as dev-dependency...

@yupferris
Copy link

@est31 snes-apu no longer has a dev dependency on emu, just cpal. The example should work just fine cross-platform now.

@est31
Copy link
Member Author

est31 commented Dec 11, 2017

@yupferris thank you!

@est31
Copy link
Member Author

est31 commented Dec 11, 2017

I'll look at doing an emu port within the next few weeks :).

@yupferris
Copy link

yupferris commented Dec 11, 2017

what do you mean by emu port? The API's in the emu libs are not very well fleshed out atm; I'd prefer if those were avoided for the time being. My hope is that after finishing up some more stuff for https://github.com/emu-rs/rustual-boy I can generalize some patterns from there, see how well they apply to snes-apu, and then look into turning those into the emu libs proper (as well as cleaning up the code in snes-apu like removing unsafe, providing a more sensible API, etc.). But as it is now I think they shouldn't be used for a more stable/reliable lib like audrey.

@yupferris
Copy link

Keep in mind I also haven't done a proper crate version bump+publish yet, as you mentioned you were waiting on a couple things before that. While the current snes-apu API isn't that great, it should definitely do what you need for this use case. If it works for you, I'd recommend implementing support on top of master in these repos, then I can publish new versions when support at this level is ready in case there's any additional changes we need to make. But I don't suspect there will be anything major.

@est31
Copy link
Member Author

est31 commented Dec 11, 2017

@yupferris one thing which would be nice to get fixed before this gets included in audrey would be to get rid of unsafe in the snes-apu crate.

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

No branches or pull requests

3 participants