| Summary: | SDL_sound midi code is depending on timidity config, and broken | ||
|---|---|---|---|
| Product: | SDL_sound | Reporter: | rose.garcia-eggl2fk |
| Component: | everything | Assignee: | Ryan C. Gordon <icculus> |
| Status: | RESOLVED FIXED | QA Contact: | Ryan C. Gordon <icculus> |
| Severity: | normal | ||
| Priority: | P2 | CC: | sezeroz |
| Version: | unspecified | ||
| Hardware: | x86_64 | ||
| OS: | Linux | ||
| Attachments: | fixes all mentioned issues and more. | ||
|
Description
rose.garcia-eggl2fk
2021-01-13 20:22:56 UTC
the code in pat_init_patnames() looks wrong in many regards.
for starters, the first line is read with _mm_fgets, and a loop entered that checks for feof(). if mm_fgets behaviour is identical to fgets, if the config contains only one line feof() would already be true after the first line has been picked.
a typical read loop looks like
while(fgets(buf, bufsize, f)) {
// buf contains the line
}
// feof has been reached.
since _mm_fgets has a prototype with void, this kind of check can't be used.
it really should return char* to indicate success as the fgets() in libc does.
the next issue is that leading whitespace isn't skipped here:
if( SDL_isdigit(line[0]) || (IsBlank(line[0]) && SDL_isdigit(line[1])) ) {
only if this conditions isn't true the whitespace is skipped but then it's already to late to enter this condition.
my propatches.cfg looks like this:
bank 0
000 ACPIANO.PAT
001 BRITEPNO.PAT
...
drumset 0
027 HIGHQ.PAT
028 SLAP.PAT
029 SCRATCH1.PAT
so none of those enter the isnumeric condition above.
the next issue is that the "bank" directive isn't recognized, and "dir" neither.
I am guessing that you built SDL2_sound from the 'default' branch? If that is the case, that version of SDL_sound relies on libmodplug to decode midi files which indeed is broken. icculus wants a zlib-compatible licensed midi decoder, so he gave up on the old timidity code in favor of public-domain libmodplug, and that's the result. If you have patches though, do send them. Created attachment 4663 [details]
fixes all mentioned issues and more.
above patch fixes the issues, apart from _mm_fgets vs _mm_feof.
yes sezero, this is against latest default branch. btw i used SDL_loginfo to display a DEBUG warning for "soundfont" directive, but that might not be the right choice (there was no other example logging func in that file). Your patch is in (in a slightly modified form): https://hg.icculus.org/icculus/SDL_sound/rev/f8de63ad5832 Closing as fixed. |