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 2140 - Track is played back with the wrong tempo
Summary: Track is played back with the wrong tempo
Status: RESOLVED FIXED
Alias: None
Product: SDL_mixer
Classification: Unclassified
Component: misc (show other bugs)
Version: unspecified
Hardware: x86_64 Linux
: P2 major
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-08 09:45 UTC by ny00
Modified: 2013-11-04 17:38 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description ny00 2013-10-08 09:45:41 UTC
Some musical tracks are played back with the wrong tempo (faster or slower than intended).

For a simple example using the 'playmus' application, you can grab castles.mid (Castles Made Of Sand) from http://www.angelfire.com/fl/herky/sounds.html

Then you can try playing either with

playmus castle.mid

or, say,

MUSIC_CMD=timidity playmus castles.mid

P.S. How to specify version 2.0.0?
Comment 1 ny00 2013-10-08 09:58:51 UTC
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.
Comment 2 Emile Belanger 2013-10-29 21:46:27 UTC
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.
Comment 3 ny00 2013-10-29 21:49:52 UTC
The original changeset for reference:

https://hg.libsdl.org/SDL_mixer/diff/0ff9b7b8ba7b/timidity/readmidi.c
Comment 4 Sam Lantinga 2013-11-03 19:24:53 UTC
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?
Comment 5 ny00 2013-11-03 20:33:21 UTC
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.
Comment 6 Sam Lantinga 2013-11-03 21:35:18 UTC
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
Comment 7 ny00 2013-11-03 21:58:57 UTC
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?
Comment 8 Ryan C. Gordon 2013-11-04 06:25:19 UTC
(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.
Comment 9 ny00 2013-11-04 07:32:05 UTC
(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).
Comment 10 Ryan C. Gordon 2013-11-04 15:18:06 UTC
(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.
Comment 11 ny00 2013-11-04 17:38:31 UTC
(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!