libsidplayfp / sidplayfp

Console SID/MUS player
GNU General Public License v2.0
34 stars 8 forks source link

Certain songs with certain filter curve values causes SIDPlayFp to crash with segfault #36

Closed TCH68k closed 4 weeks ago

TCH68k commented 1 month ago
/usr/bin/sidplayfp -vf -mn -fs -p16 -rr -f48000 -o2 -b0:0 -t0 --digiboost --fcurve=256 "/a/WORKDISK/MUSICS/C64Music/MUSICIANS/O/Ouwehand_Reyn/Last_Ninja_Remix.sid" -v15

This song also dies with 65536, 65537, 800, 200000 and 257, with the latter one interestingly not instantly, but delayedly causing the segfault.

I guess these values are "invalid" anyway, but while the manual states that the valid range is from 0.0 to 1.0 the player.cpp contains a wired in filter curve list (https://github.com/libsidplayfp/sidplayfp/blob/master/src/player.cpp#L184) which contains values above 1.0, and the sound really differs between for instance 1.0 and 1.5.

What is the real interval?

drfiemost commented 1 month ago

I guess these values are "invalid" anyway, but while the manual states that the valid range is from 0.0 to 1.0 the player.cpp contains a wired in filter curve list (https://github.com/libsidplayfp/sidplayfp/blob/master/src/player.cpp#L184) which contains values above 1.0, and the sound really differs between for instance 1.0 and 1.5.

The values in that list are transformed to fit into the correct range:

https://github.com/libsidplayfp/sidplayfp/blob/7f980a9cf1bd1f74e119d363edee5bed82a88bf4/src/player.cpp#L242

The range is 0.0 - 1.0 as stated, what is really missing is a santity check for the fcurve parameter.

TCH68k commented 1 month ago

I see. Thank you for clarifying.

BTW, i know that the range is specified in the manual, but i think the program also should print it out in it's help text. Currently it only states

 --fcurve=<num>|auto Controls the filter curve in the ReSIDfp emulation

and it should be

 --fcurve=<num>|auto Controls the filter curve in the ReSIDfp emulation (0.0 to 1.0, default: 0.5)

or something like that.