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 2909

Summary: Mix_PauseMusic and Mix_ResumeMusic not working on MIDI (.mid)
Product: SDL_mixer Reporter: Anton <anton.synytsia>
Component: miscAssignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2 CC: philipp.wiesemann, vikas.ag
Version: 2.0.0   
Hardware: x86_64   
OS: Windows 7   
Attachments: Patch for pause and resume music with Windows native MIDI decoder
Patch for pause and resume music with Windows native MIDI decoder, 2nd version

Description Anton 2015-03-15 08:44:35 UTC
Mix_PauseMusic and Mix_ResumeMusic have no effect when MIDI (.mid) music is playing. These two functions work with .wav, .mp3, .pat, and .ogg, but not on .mid music format.
Comment 1 Vikas 2015-07-24 11:50:42 UTC
Any solution for this?
Comment 2 Sam Lantinga 2015-08-02 05:08:47 UTC
No, do you have a proposed patch?
Comment 3 Philipp Wiesemann 2015-08-16 20:32:33 UTC
Created attachment 2245 [details]
Patch for pause and resume music with Windows native MIDI decoder

This is a patch to make Mix_PauseMusic() and Mix_ResumeMusic() work with the Windows native MIDI decoder.

I only tested it with "playmus.c" and it may break something else.

Also, there should be a better and less fragile way to implement this.
Comment 4 Vikas 2015-08-17 14:36:13 UTC
I think there can be a better way

In proposed scenario:
1. If Mixer is built with MID support, then for every music format mix_music(music_mixer) will be called(unnecessarily) even music is in pause state. However there are no side effect but since this method will be called so many time, we should avoid this call, if possible. 

2. If MID format is being played, then  native_midi_resume will be called(Unnecessarily) for each music chunk.

Solution:
I think it will be better If We call native_midi_resume from Mix_ResumeMusic

void Mix_ResumeMusic(void)
{
    music_active = 1;

#ifdef MID_MUSIC
#	ifdef USE_NATIVE_MIDI
	// Native midi is handled asynchronously, so it needs to be resumed separately
	if (native_midi_ok && native_midi_active())
	{
		native_midi_resume();
	}
#	endif
#endif
}

Similarly We can make changes for Mix_PauseMusic
Also We need to make some changes in native_midi_active to avoid crash

int native_midi_active()
{
	return currentsong && currentsong->MusicPlaying;
}
Comment 5 Philipp Wiesemann 2015-08-17 18:07:31 UTC
The variable music_active is also changed in Mix_FadeInMusicPos(). If the functionality to pause/resume MIDI would be in Mix_PauseMusic() and Mix_ResumeMusic() only then this would have no effect.
Comment 6 Vikas 2015-08-21 07:13:52 UTC
1. We can play music using Mix_PlayMusic/Mix_FadeInMusic/Mix_FadeInMusicPos but ultimately Mix_FadeInMusicPoswill be called in these cases. If this method is not called then there is no music to pause/resume. 

2. Now music_active is being set to 1 in Mix_FadeInMusicPos. This is initialization of this variable, if this is not set to 1, music will not even start. If music does not gets started, there is nothing to pause.
check this code snippet

void music_mixer(void *udata, Uint8 *stream, int len)
{
    int left = 0;

    if ( music_playing && music_active ) {
	
      /* Logic to play next chunk of music for every format except mid, because mid is being played in a seperate thread */
	}
}

Now by default also music_active is 1, consider the following call sequence
	Mix_PlayMusic(music, -1);	/* music_active  = 1 */ 
	Mix_PauseMusic();	/* music_active  = 0 */ 
	Mix_HaltMusic();   /* music_active  still 0 */ 
	Mix_PlayMusic(music, -1); /* Since value is 0, it must be set to 1, so that next chunk of music gets played */

3. As for as mid format is concerned, it is not much dependent on music_active. native_midi_start gets called irrespective of value of music_active. Once this method gets called, it will be played in seperate thread. 

void music_mixer(void *udata, Uint8 *stream, int len)
{
    int left = 0;

    if ( music_playing && music_active ) {
	......
	#ifdef MID_MUSIC
            case MUS_MID:
#ifdef USE_NATIVE_MIDI
                if ( native_midi_ok ) {
                    /* Native midi is handled asynchronously */
                    goto skip;
                }
#endif
.....
}

So as per my understanding functionality to pause/resume MIDI should be in Mix_PauseMusic() and Mix_ResumeMusic().
It will fix the problem as well as perfornce wise(which I already explained in prevous post) it will be a better approch.
Comment 7 Philipp Wiesemann 2015-08-21 19:52:15 UTC
Created attachment 2251 [details]
Patch for pause and resume music with Windows native MIDI decoder, 2nd version

Thank you for looking into this problem. I was confused about music_active (but still think that it is also needed for fading volume of MIDI).

I attached a different version of the patch. Now it also has locking and a switch statement to add pause/resume for CMD_MUSIC later.

I am still not sure if the implementation is generally correct and does not break something else (e.g. also because the two functions now lock the audio thread which they did not before).
Comment 8 Sam Lantinga 2017-10-18 05:29:53 UTC
Fixed, thanks!
https://hg.libsdl.org/SDL_mixer/rev/5cb894f15335