Note: Of course I mean that the tempo is correct if MUSIC_CMD=timidity is added.

Furthermore, fluidsynth support is built-in, in case it helps.

File 

timidity/readmidi.c

Line 677.


Old code (from my copy of SDL 1.3):

if (meep->event.type==ME_TEMPO)
{
	  tempo=
	    meep->event.channel + meep->event.b * 256 + meep->event.a * 65536;
	  compute_sample_increment(tempo, divisions);
	  skip_this_event=1;
}


New Code (SDL 2.0):

if (meep->event.channel >= MAXCHAN)
	skip_this_event=1;
else if (meep->event.type==ME_TEMPO)
{
	  tempo=
	    meep->event.channel + meep->event.b * 256 + meep->event.a * 65536;
	  compute_sample_increment(tempo, divisions);
	  skip_this_event=1;
}



Changing back to old code fixes EDuke32 music, obviously it is hitting the >= MAXCHAN test, don't know enough about timidity to know why this was added or if it is needed.

The original changeset for reference:

https://hg.libsdl.org/SDL_mixer/diff/0ff9b7b8ba7b/timidity/readmidi.c

What is meep->event.channel in this case?  That test was added to prevent memory corruption in timidity code.

Maybe we can just increase MAXCHAN?

Testing from a less-than-two-weeks old build of SDL_mixer 2.0.0, playmus included (which seems to be ok since there has been just one Android-specific commit today):

After starting playmus with castles.mid, gdb catches this event first (meep->event):

{time = 0, channel = 255 '\377', type = 0 '\000', a = 0 '\000', 
  b = 0 '\000'}

Afterwards, there even is caught:

{time = 0, channel = 45 '-', type = 10 '\n', a = 10 '\n', b = 230 '\346'}

Finally, the MIDI is played back faster than intended.


Having practically no real knowledge of MIDI..

It seems like the test against MAXCHAN can safely be relocated after the check if (meep->event.type==ME_TEMPO), with no risk of memory corruption, at least in this function.

You're right, it looks like that field is reused for another purpose for the tempo event.

Thanks!  This is now fixed:
https://hg.libsdl.org/SDL_mixer/rev/8ef083375857

Just a few comments to add:

- Looks, or rather, sounds like it is fixed. Thanks to Belanger for spotting the cause of this!
- On a side note, is it expected behavior that for every email notification that I receive about a new comment added here, the mail's body has no text?

(In reply to ny00 from comment #7)
> - On a side note, is it expected behavior that for every email notification
> that I receive about a new comment added here, the mail's body has no text?

Can you go here:

    https://bugzilla.libsdl.org/userprefs.cgi?tab=settings

And change "Preferred email format" from "HTML" to "text"? If that fixes it, it's either a bug in Bugzilla or your email app.

(HTML mail is working fine here with Thunderbird, fwiw.)

--ryan.

(In reply to Ryan C. Gordon from comment #8)
> Can you go here:
> 
>     https://bugzilla.libsdl.org/userprefs.cgi?tab=settings
> 
> And change "Preferred email format" from "HTML" to "text"? If that fixes it,
> it's either a bug in Bugzilla or your email app.
> 
> (HTML mail is working fine here with Thunderbird, fwiw.)
> 
> --ryan.

Using Thundebird 24.1.0 here (on Ubuntu 12.04), as expected I have received an empty mail as a result of your last reply.

I have just changed the format to "text". Let's see if it fixes anything, at least for reports about SDL_mixer (if it makes any difference at all).

(In reply to ny00 from comment #9)
> I have just changed the format to "text". Let's see if it fixes anything, at
> least for reports about SDL_mixer (if it makes any difference at all).

Did it fix it?   :)

(I'm wondering if Outlook.com is stripping HTML from incoming email or something strange like that.)

--ryan.

(In reply to Ryan C. Gordon from comment #10)
> Did it fix it?   :)

Yeah, seen the message via e-mail. Thanks for the assistance!

In addition, thanks to Sam for committing the bug fix, and further thanks to Ryan, Sam and Emile for participating!