The check to see if background music should be started was implemented as part
of the input processing. As a result, background music would not start if the
game was not getting any input. F.e. on the main menu after starting the game,
if you let the music play until it finishes without doing anything, the game
will go silent. Only after moving the mouse will the background music track
start. The original interpreter starts the background music without needing
user input.
I've fixed this by moving the check to start background music to the timer
loop of the MusicPlayer. I've added a check to prevent background music from
starting while the game is not accepting input.
When loading a savegame, the current background music would play for a few
seconds before the game would set the music for the room that just loaded.
This change stops the music when loading a savegame to prevent this.
When fading out a music track, the original interpreter would keep playing the
track at volume 0. ScummVM would instead stop playing the track, which would
cause the background music track to start playing, while there would be no
music in the original interpreter. Also, when starting playback of a new track,
the original interpreter would restore the volume first, then start playback of
the new track, causing a short part of the old track to be played before the
new track starts. ScummVM would turn the volume back up, but not actually apply
this to the MIDI device, causing new tracks to sometimes play at volume 0.
This change fixes these issues. After fading out, actual playback on the MIDI
device is stopped, but the state of the music player keeps indicating that
music is playing. This prevents the background music from starting. Because
playback is actually stopped, the original interpreter issue of playing a short
bit of the old track when the new track starts is prevented. When resetting
volume when starting a new track, the new volume is now applied to the MIDI
device, which fixes the issue of the new track playing at volume 0.
The 7th Guest has MIDI initialization files for GM and the MT-32 that need to
run for these devices to sound correctly. When loading a game from the ScummVM
main menu, MIDI initialization was not performed.
This change adds this. It runs a slightly changed copy of the game's MIDI init
script when loading a game using the ScummVM menus. It also tracks if MIDI init
has been performed, so it doesn't run a second time when loading a game.
I don't know if the script works properly for the Mac and iOS versions of the
game (or if it is even needed), so I've only enabled it for the DOS versions of
the game for now.
The MIDI music would keep playing while the ScummVM menu was open. This would
cause the music for cutscenes to go out of sync. I've added pausing to the
MusicPlayerMidi to fix this.
The MIDI parser would send All Notes Off events on all MIDI channels as a
safety measure when stopping playback in various situations without first
checking if a track was actually playing. Depending on the specifics of the
code driving the MIDI parser, this could generate many duplicate All Notes Off,
Sustain off and/or Pitch Bend center messages. This is a problem for slow MIDI
hardware like the MT-32, which needs some time to process all these messages.
As a result, the timing at the beginning of the next track could be wrong.
I've introduced checks in stopPlaying, setTrack, unloadMusic and the MIDI
parser destructor so that allNotesOff is called once when a track is playing,
and not a second time when playback has already stopped. Similarly, unloadMusic
does not execute when no music is loaded to prevent duplicate Pitch Bend
messages.
When the MIDI parser wanted to stop all notes on a MIDI device, it would send
an All Notes Off message and, optionally, a Sustain off message on all 16 MIDI
channels. This is inefficient for devices like the MT-32, which uses only 9
channels. The MT-32 hardware is also quite slow, so it is more responsive if it
has to process less messages.
I've added a generic stopAllNotes message to the MIDI driver using the old
method, which can be overriden by more specific implementations in subclasses.
The Miles MIDI driver implementation sends All Notes Off only on the channels
used by the MT-32. There are probably also more efficient implementations
possible for f.e. AdLib.
The Groovie engine implemented some functionality which was later reimplemented
in the generic Miles drivers. These are now used by the engine, but the old
code is still present. I've removed this to clean up the audio code.
XMIDI timbre chunk processing was implemented using a callback. This
reimplements this using an interface for the necessary driver method, which is
a somewhat cleaner solution.
XMIDI data for the MT-32 can contain a timbre chunk which contains the custom
timbre numbers used for a track. Before starting playback of the track, the
timbres must be loaded into the MT-32 using SysEx messages. Each SysEx message
needs a delay before sending the next message to give the MT-32 enough time to
process it.
The delays between the messages were generated using the OSystem::delayMillis
function. This throws off the timing of the MidiParser, which causes MIDI
messages to "pile up", in this case at the start of the track. This change adds
a queue to the Miles MIDI driver, which can store the SysEx messages that must
be executed. The MidiParser suspends playback until the driver has sent all
messages in the queue.
This fixes the start of tracks in The 7th Guest, which had incorrect timing.
Because of the new SysEx queue, MT-32 initialization of The 7th Guest takes a
bit longer than it did before. The game plays an animation of a fixed length
and aborts the initialization if it is not done by the end of the animation.
This problem was already fixed for the GM initialization, so I've applied the
same fix for the MT-32.
This adds using the updated Miles MIDI driver for GM music and using the driver
for source volume scaling.
This reduces the amount of MIDI volume messages when fading, because they are
now only sent on active MIDI channels and if they are different from the
current channel volume. This fixes problems with hardware MIDI devices like the
MT-32 and SC-55, which have trouble dealing with the high number of messages.
The GM music for The 7th Guest contains several drumkit selection
messages which use an incorrect drumkit number. This relied on a
correction mechanism of the Roland SC-55, which would correct these
numbers to valid selections. This mechanism was not present in later
GS and GM devices, causing error messages and wrong drumkits.
I've fixed this by adding code which simulates the correction
mechanism of the SC-55.
The 7th Guest sends several SysEx messages for Roland GS devices during
startup. ScummVM did not send these messages.
The Groovie MusicPlayerMidi class encapsulates the MidiDriver which is
actually used for sending MIDI events. However, it did not implement the
sysEx method, so the empty implementation of MidiDriver_BASE was used.
I've added a sysEx method which forwards the messages to the actual
MidiDriver.
This is now uneeded as the GUI debugger superclass implements the same
functionality and this removes a bunch of complexity from the Groovie
engine debug calls. Also, removed groovie prefix from the debug flag
naming as unecessary as these are within the Groovie namespace.
This tries to make our code a bit more compliant with our code formatting
conventions. For future use, this is the command I used:
git ls-files "*.cpp" "*.h" | xargs sed -i -e 's/[ \t]*$//'
MKID_BE relied on unspecified behavior of the C++ compiler,
and as such was always a bit unsafe. The new MKTAG macro
is slightly less elegant, but does no longer depend on the
behavior of the compiler.
Inspired by FFmpeg, which has an almost identical macro.
The regression affected AGOS and maybe some others; specifically,
the real MidiDriver would have been deleted twice -- I previously
missed that the Engine instances takes care of freeing the real
MidiDriver, not the MidiPlayer wrapping it.
This commit should clarify the ownership of the real MidiDriver for
most pseudo MidiDrivers.
Many engines follow the advice in audio/midiparser.h and create a
"pseudo-MidiDriver" subclass. But MidiParser really only needs a tiny
subset of the MidiDriver capabilities, namely those found in
MidiDriver_BASE. So we try to subclass from that whenever possible; this
allows us to remove many stub methods, and enables further future
simplifications.
This in turn enables modifying MidiDriver_MPU401::close() to allow
it to be called on a midi driver that has not yet been opened.
The specific issue that triggered me to make these changes was a
crash-upon-quit in HUGO, caused by it instantiating a midi driver,
then encountering an error (missing hugo.dat) *before* having
opened the new midi driver; the general cleanup code then tries
to close the (not yet opened) midi driver -> kaboom
Also fixed some engines which were leaking MidiDriver instances.
This includes an rather hacky attempt to merge all the recent gp2x backend
changes into the branch. I suppose the gp2x backend and probably all new
backends, i.e. gph, dingux etc., might not compile anymore.
Since I have no way of testing those it would be nice if porters could look
into getting those up to speed in this branch.
svn-id: r53399
NMIError
Eugene Sandulenko
Matthew Hoops
Martin Kiewitz
D G Turner
Johannes Schickel
Max Horn
Scott Thomas
strangerke
Ori Avtalion