https://moral.net.au/misc/pq1sci.006 - example save game, tested with ScummVM's builtin MT-32 support

• Take the ticket from your inventory
• Hand it to the woman in the car
• Confirm that you want to give a ticket
• After the first bit of dialogue, SCI will load in a new song and attempt to allocate channels
• For chan.number == 9 , the song will first set pSnd->_chan[chan.number]._dontRemap to true, then to false.
• Later, the MIDI driver will attempt to remap the channels. As _dontRemap is false for chan.number == 9, the percussion channel will get assigned to a normal instrument channel, and sound very broken.

I have checked LSL6 disasm. The original code will only make changes to the flag if it is set in the resource. So this fix is correct. I have added a commit which also extends this to other flags and settings, since the original also does that...

@moralrecordings thanks for figuring this out. And impressive! SCI audio is... intense. Can you test these latest changes to make sure you're still hearing what you expect? Then we can merge.

I think the changes have introduced some problems in pq1sci.

• Music in the intro no longer fades out to the news dialogue
• Music no longer stops when you move the mouse to the menu bar when the police car pulls into the station
• Office sounds get stuck on when you walk into the briefing room

My guess is the mute flag, but will try and figure out which ones it is.

Okay, _dontMap is responsible for all the glitches; our sound mapper uses it as a cue to just skip the track entirely, which could be quite different from the original sound mapper. With everything else left in, music tracks now fade out properly when they didn't before.

There is one sound glitch left that I could spot in pq1sci; https://moral.net.au/misc/pq1sci.009 - when you pull into the jail and you arrive on the next screen, the driving music is meant to fade out and be replaced with another track. From what I can tell:

• the old song fades down
• ScummVM maps in the new song
• the old song continues playing at the original volume

This might be more invasive to fix and is less jarring a glitch compared to the percussion stuff, so maybe that's best left alone for now.

Thanks for your tests and for the fix. If this gets rid of the glitches I guess we could merge the PR now as its is. The revert is kind of masking a bug with another bug, but it allows fixing the dontMap flag without any pressure... The dontMap flag is rarely used. I have seen it only in KQV so far. So it wouldn't come as a big surprise if the handling is bugged in our code.

You're right that _dontMap is probably masking a more integral issue with the feature... that said, given it's rarely used and not a regression per se, I'm happy with merging it now.

Hmm, now I did a little debugging and I actually did not find a single use case of the _dontMap flag in any track. My impression is that the '|=' just revealed the fact that we did not initialize these values. So, for _dontMap I had a random value of true (which would then remain true due to the '|=' and for _dontRemap I had a random value of false.

Could you please check what happens to the glitches that you noticed if you re-add the '|=' for the dontMap, but add
pSnd->_chan[i]._dontMap = false;
pSnd->_chan[i]._dontRemap = false;
pSnd->_chan[i]._mute = 0;

after line 501?
I will not make a commit for now, so we don't add and remove the same thing over and over ;-)

Hah, yep it turned out to be uninitialized values (isn't C++ great??). All good in pq1sci. Good catch, I've reworked my commit to include the changes.

@moralrecordings Let's squash this thrilling drama into one commit and merge.

@athrxx It's Been 0xCCCCCCCC Days Since Our Last Accident

All done, feel free to merge.

Great, thanks!