[Scummvm-devel] PATCH: several patches to imporves lure's sound

Hans de Goede j.w.r.degoede at hhs.nl
Sat Mar 8 22:41:44 CET 2008

Hi All,

Thanks for getting a new lure.zip up with a license text included! As a result 
of this I've been working on packaging lure for Fedora.

During the packaging I've noticed several issues with Lure's sound and I'm 
happy that after a day worth of hacking I'm able to offer you patches fixing 
most of these issues:


The current code has 2 issues best described as bugs, for some unknown reason 
the channel alloc code works with something called outerchannels, of which 
there are 8, and which are allocated 2 at a time. Which are mapped to 
innerchannels (the real channels), of which there are 16 and 2 outerchannels 
correspond to 4 innerchannels, so channels are mapped 4 at a time, but with 
some strange bookkeeping added.

The problem with this code is that the activeSounds list, contains 
SoundDescResource structures, who's channel member is an outerchannel, but in 
various places, for example in setVolume(uint8 soundNumber, uint8 volume) and 
in restoreSounds() it is interpreted as an innerchannel.

An other issue is that the current channel alloc loop, tries to be able to 
handle channel allocs in other sizes then 2 at a time, but fails due to various 
logic errors in that loop.

Both of these are fixed by these patch by removing the difference between outer 
and innerchannels, and by always allocating 4 channels at a time.

Another issue this patch fixes is that all the musicInterface_xxxx functions 
expect MidiMusic instances to hold an 8 bit soundnummer with the highest bit 
clear to 0, but musicInterface_Play creates the MidiMusic instances with the 
full 8 bit soundNumber var instead of passing in the 7 bit (highest bit 
cleared) soundNum var. The same mistake was made in the check if this was the 
last sound for the size calculation.


There are various issues with the current volume handling code in lure's sound 

1) The volume member of the 16 channels gets set to either the scummvm sfx or
music volume levels which is 192 by default. Then in musicInterface_Play
MidiMusic::setVolume() gets called, which scales the channel volume by passed 
volume/255, but this gets passed the scummvm sfx or music volume level again by
musicInterface_Play, resulting in the channel volume becoming: 192x192/255 = 
144.  (And if musicInterface_Play is called from addSound, the 
MidiMusic::setVolume() gets called again immediately with the volume specified 
for the playedsound in its SoundDescResource).

2) The values 192 / 142 are not valid values for midi channel volumes. Midi
channel volume go from 0-127. In the case of the adlib driver this leads to
an array being read out of bounds (which should really be fixed by clamping
the input in the volume() method in sound/softsynth/adlib.c).

3) Storing the sfx / music volume in a midi channel volume doesn't make any 
sense a midi channel volume should according to the midi standard start at a 
volume of 90.

This patch fixes all of the above by:
-storing the volume set by midi commands / as default (90) in the midi channel
-taking sfx / music volume into account in MidiMusic::setVolume(), and only in
-still passing the volume specified for the playedsound in its
  SoundDescResource to MidiMusic::setVolume(), where it gets scaled by the
  sfx / music volume and then gets applied as a multiplier to the channel volume
  of all channels used by this sound/song. Note that the passing of the volume
  from the SoundDescResource now is the only use of MidiMusic::setVolume(), its
  no longer also used for the sfx/music volume, as that really is another
  volume. As already described the sfx/music volume is no taken into account
  in MidiMusic::setVolume() internally.

On top of this, it:
-no longer uses the volume from the SoundDescResource when playing through
  adlib, as those volumes do need seem to make much send with our adlib midi
  emulation. Some sounds which are correctly made softer by the
  SoundDescResource as they are quite loud when using fluidsynth, are actually
  pretty quiet on the adlib. And the other way around.


This patch improves the sound channel allocation routines to only alloc as many 
channels as really needed. This fixes some missing sounds in the first room 
(some sounds go missing if you time dropping the torch in a certain way).

Note: these patches should be applied in the above order.



-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: scummvm-0.11.1-lure-sound-bug-fixes.patch
URL: <http://lists.scummvm.org/pipermail/scummvm-devel/attachments/20080308/d79caabc/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: scummvm-0.11.1-lure-sound-channel-alloc.patch
URL: <http://lists.scummvm.org/pipermail/scummvm-devel/attachments/20080308/d79caabc/attachment-0001.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: scummvm-0.11.1-lure-volume-fixes.patch
URL: <http://lists.scummvm.org/pipermail/scummvm-devel/attachments/20080308/d79caabc/attachment-0002.ksh>

More information about the Scummvm-devel mailing list