GNOME Bugzilla – Bug 344667
[siddec] sounds very broken
Last modified: 2006-06-13 15:46:35 UTC
Details at: https://bugzilla.livna.org/show_bug.cgi?id=912
In response to https://bugzilla.livna.org/show_bug.cgi?id=912#c2 : > * There are several mistakes in the code. The official IANA MIME type > for SID files is 'audio/prs.sid' not 'audio/x-sid'. > libgsttypefindfunctions.so for gst-typefind also knows about that > wrong MIME type for SID files. As both need a correction, this > cannot be done in this package. This doesn't really need fixing (nor can it be fixed in a stable series for API/ABI compatibility reasons), since audio/x-sid is not a MIME type in this context, it's a 'GStreamer media type', which may or may not correspond to the official MIME type :) > * It doesn't become clear who the author of the plugin is. Compare > header with inline author information. The inline author information tends to be more correct, the header is probably a boilerplate copyright header from the plugin skeleton generator in this case. In any case, patches or bugs should generally go into bugzilla and not be mailed to the author (since the people actively working on things change a lot, or e-mail addresses are out of date etc.). > * Several engine paramters are customised in a weird way. Memory > mode MPU_BANK_SWITCHING should be the safe default. Enabling > 'forceSongSpeed' mode is clearly wrong. Stereo is well, eh, an > inappropriate default for music which consists of mostly three > voices. Default filter parameters should not be touched unless > the developer knows what he's doing. It's not necessary. The > code should read the default configuration and modify only > where necessary. Could you provide us with a sample file or point us towards a sample file that demonstrates the problem by any chance? Regarding the patch: if we get the default configuration from the lib, we should probably also use the retrieved default values as the default values in the GObject properties registration in class_init, no?
Once more in words, since you don't need to accept the patch unmodified, of course. I don't care about the actual implementation, just about facts: - memory mode MPU_PLAYSID_ENVIRONMENT is a bad/wrong configuration setting - use MPU_BANK_SWITCHING instead or don't touch the default mode at all - enabling 'forceSongSpeed' mode is clearly wrong - enabling stereo mode for 3-voice SID music is bad/wrong - hence by default, libsidplay does proper mixing in mono mode if you let it - chip mode MOS8580 with libsidplay 1.x (as opposed to 2.x) is questionable - overriding the default start song is another issue that makes one ask "why?"
* ext/sidplay/gstsiddec.cc: Fix copyright, email addresses and descriptions. Use saner defaults for arguments. Fixes #344667. constify some stuff. Fix memleaks. Add tags. Fix negotiation to do mono/44100 by default. Post error messages. Use _scale_int where possible.