GNOME Bugzilla – Bug 576800
gstreamer SPC plugin requires non-portable x86 code
Last modified: 2009-08-07 21:50:09 UTC
gst-spc, the SNES SPC playback plugin uses OpenSPC, which is coded in raw, nearly unpenetrable x86 assembly, limiting it to x86 and x86_64. Luckily the emulation state of the art has advanced since then an a suitable replacement library written in nearly unpenetrable C++ is available (and useful on even PPC in theory for instance). This is Shay Smith's snes_spc (http://slack.net/~ant/libs/audio.html#snes_spc), as used by bsnes and now zSNES and available under the LGPL. I've prepared a patch utilizing this library (including it inline, v0.9.0, instead of making it a dependency, as done with the similar NSF plugin), which I will attach. I suppose the naming convention would move spc from /ext to /gst but I'll let you guys handle your directory layout. ;) There's probably a porting bug or two (notably, I've not been able to test seeking with gst-launch and I couldn't get eina to work, and I don't really feel like waiting for anything larger to compile to test... :-/). Hopefully nothing major, if you have any question feel free to drop me a line. Thanks.
Created attachment 131402 [details] GZipped patch porting gst-spc from libopenspc to snes_spc
I don't like the idea to copy this library into our source tree... wouldn't it be better to simply depend on it as done for openspc in the past? (FWIW, I also don't like the NSF library being including in our sources and the modplug library... but the latter has changed lately fortunately)
Sebastian: To be honest it is probably better to depend on the library instead of including it as it could be tracked/updated as a dependency instead of having to keep it updated in gstreamer, I was just trying to keep with the existing practice in gst-plugins-bad. (However, snes_spc itself is not designed to be installed as a shared library per se, so the packagers would have to take a little more care to build it as there's hardly any build system at all.) Either way the rest of the changes should be applicable then, except that configure.ac would need changed and the various Makefile.am patches would be excluded.
Right, could you contact the author and ask him to provide some kind of build system and a way to build it as library? Also openspc seems to be packaged by a lot of distributions right now while snes_spc doesn't seem to be packaged by any distribution yet... so we should probably make it a conditional dependency: use snes_spc if available, otherwise use openspc.
Sorry about the delayed reponse, I marked it as action item and then promptly forgot to do anything about it anyways. I've emailed the library author to see if he wants to touch it up a bit to make it a real library. If not I'll probably make a simple fork so we have something to depend on.
http://code.google.com/p/game-music-emu/ I got in touch with the author, and he gave me commit access to add a buildsystem to game-music-emu (libgme.so on disk). I'll still need to update my patch but there is an actual build-able release we can depend on now, 0.5.5.
Great, I'd be happy to commit it then :)
Created attachment 139717 [details] [review] Use libgme for SPC playback. This patch uses libgme (the shared library instead by Game_Music_Emu) for playback of SPC files. Note that the libgme 0.5.5 release I mentioned exported too many headers, only the gme/gme.h file is supposed to be used by client code, which will be rectified in the next release. I did forget that someone mentioned that it would be preferable to support both the old library and libgme so I guess I'll go back and add ifdef's or such. But this is what the patch will look like aside from that unless anyone sees other issues with it. I've tried to maintain the correct style but I don't have GNU indent and I wasn't going to install it just to satisfy git so please don't assume your precommit hooks caught my mistakes. :( One final point: libgme supports more systems than just SNES. I know that this plugin collection already supported other emulated music types so I'm not sure how you all would want to work that. Right now I've artificially constrained the plugin to only play real SPC files in case a different file type gets passed to it somehow but I'm not sure how to support multiple types in one plugin, or if you all would want to go that route anyways. Patch itself compiles with no warnings on gcc 4.4.1, and is tested using GST_PLUGIN_PATH to work on Phonon applications and a command-line gstreamer test application I have. How do I know GST_PLUGIN_PATH was right? Remove the gme_start_track() line and find out for yourself... ;)
I guess instead of #ifdef'ing to work with libgme and openspc the better solution would be to only work with libgme and at the same time add support for the other file formats that libgme understands. Some comments: * spc->seekpoint = dest / 1000000; // better divide by GST_MSECOND ;) * On errors in the _play function you should also stop the task and send EOS downstream additional to GST_ELEMENT_ERROR Other than that it looks good... change this and I'll commit it :) And afterwards it'd be great if you could provide an incremental patch to add support for other file formats than SPC :)
Created attachment 139744 [details] [review] Revised libgme support patch This patch includes Sebastian's recommendations as to fixing GST_MSECOND and properly stopping the task and sending EOS on error. Should work fine on libgme 0.5.5 or later. Talking with the libgme author, libgme 0.5.6 (or whatever the next release ends up being called) will switch out the SPC core to the one actually in use on bSNES and zSNES and will add pkg-config support so that it's more standard to detect and use.
Ok, it's committed now :) Are you going to add support for the other formats that are supported by libgme later? If you're going to do that please just add it to the current spc plugin, I'll care for renaming the plugin to gme then (otherwise the diffs are too hard to read) :) commit de03453f6d3e284de1247ada864e45f010d126e4 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Tue Aug 4 10:18:46 2009 +0200 spc: Make the SPC plugin work with the latest libgme release gme_enable_accuracy() was added in SVN trunk and is not yet in any release. commit 4394b1a61c081163ec457d6b7924943c80aa29f4 Author: Michael Pyne <mpyne@kde.org> Date: Tue Aug 4 10:06:54 2009 +0200 spc: Use the portable libgme instead of x86-only OpenSPC library This will later allow us to play other gaming console files that are supported by libgme. Fixes bug #576800.
I'll probably get around to it at some point, but it's not going to be #1 on my priority list either. :-/ However should I come back to it I'll definitely work by altering gst-spc and allowing you guys to tweak it from there. The work itself isn't hard though, as I mentioned I had to artificially constrain the plugin, if you remove the check for gme_type_spc and add the mimetypes and file extensions for the other formats then you should already be done. :)
I did this now, the gme plugin in gst-plugins-bad supports it all now and typefind from gst-plugins-base detects all formats ;)
Sebastian, indeed from git log it looks like you've made quite a few fixes, thanks! Looking forward to seeing this in a released version of gst-plugins-bad. In the meantime I will poke libgme's author to see if he will make the next release (for gme_enable_accuracy, sorry about that), or if I should.