We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 5464 - SDL_sound midi code is depending on timidity config, and broken
Summary: SDL_sound midi code is depending on timidity config, and broken
Status: RESOLVED FIXED
Alias: None
Product: SDL_sound
Classification: Unclassified
Component: everything (show other bugs)
Version: unspecified
Hardware: x86_64 Linux
: P2 normal
Assignee: Ryan C. Gordon
QA Contact: Ryan C. Gordon
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-01-13 20:22 UTC by rose.garcia-eggl2fk
Modified: 2021-01-14 22:18 UTC (History)
1 user (show)

See Also:

Attachments
fixes all mentioned issues and more. (2.49 KB, patch)
2021-01-13 21:38 UTC, rose.garcia-eggl2fk 		Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description rose.garcia-eggl2fk 2021-01-13 20:22:56 UTC 
i was trying to use a midi-playing application with SDL(2)_sound. at first the stdout is spammed with message about timidity.cfg not found, and after that is fixed via the proposed environment variable, i get messages like this:

DEBUG: Couldn't open /usr/share/timidity/instruments/.pat
DEBUG: Couldn't open /usr/share/timidity/instruments/.pat
DEBUG: Couldn't open /usr/share/timidity/instruments/.pat
DEBUG: Couldn't open /usr/share/timidity/instruments/.pat

the /usr/share/timidity path is also hardcoded into the SDL2_sound library.

my timidity config contains only one line:

soundfont /share/sounds/sf2/OPL-3_FM_128M.sf2

and apparently the parser code doesn't expect to find a soundfont.

i also tried with GUS patches (this config works with e.g. wildmidi):

source /etc/propatches/propatches.cfg
dir /share/midi/propatches

but it seems the "dir" directive isn't known either.

interestingly SDL2_sound still produces midi sound, but the tones sound all wrong, like every note is delayed by 10x.
Comment 1 rose.garcia-eggl2fk 2021-01-13 21:00: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.
Comment 2 Ozkan Sezer 2021-01-13 21:04:05 UTC 
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.
Comment 3 rose.garcia-eggl2fk 2021-01-13 21:38:54 UTC 
Created attachment 4663 [details]
fixes all mentioned issues and more.

above patch fixes the issues, apart from _mm_fgets vs _mm_feof.
Comment 4 rose.garcia-eggl2fk 2021-01-13 21:43:17 UTC 
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).
Comment 5 Ozkan Sezer 2021-01-14 22:18:04 UTC 
Your patch is in (in a slightly modified form):
https://hg.icculus.org/icculus/SDL_sound/rev/f8de63ad5832

Closing as fixed.