After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 344667 - [siddec] sounds very broken
[siddec] sounds very broken
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-ugly
0.10.2
Other Linux
: Normal normal
: 0.10.4
Assigned To: Wim Taymans
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2006-06-12 14:22 UTC by Michael Schwendt
Modified: 2006-06-13 15:46 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Michael Schwendt 2006-06-12 14:22:44 UTC
Details at: https://bugzilla.livna.org/show_bug.cgi?id=912
Comment 1 Tim-Philipp Müller 2006-06-12 14:59:22 UTC
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?

Comment 2 Michael Schwendt 2006-06-12 16:01:28 UTC
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?"

Comment 3 Wim Taymans 2006-06-13 15:46:35 UTC
        * 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.