[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:
scummvm-0.11.1-lure-sound-bug-fixes.patch:
------------------------------------------
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.
scummvm-0.11.1-lure-volume-fixes.patch:
---------------------------------------
There are various issues with the current volume handling code in lure's sound
code.
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
volume
-taking sfx / music volume into account in MidiMusic::setVolume(), and only in
MidiMusic::setVolume()
-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.
scummvm-0.11.1-lure-sound-channel-alloc.patch
---------------------------------------------
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.
Regards,
Hans
-------------- 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