[Scummvm-tracker] [ScummVM :: Bugs] #12932: Grim Fandango crash after solving the signpost in the Petrified Forest

[ScummVM :: Bugs]

#12932: Grim Fandango crash after solving the signpost in the Petrified Forest
---------------------+----------------------------
Reporter:  Die4Ever  |       Owner:  (none)
    Type:  defect    |      Status:  new
Priority:  blocker   |   Component:  Engine: Grim
 Version:            |  Resolution:
Keywords:  crash     |        Game:  Grim Fandango
---------------------+----------------------------
Comment (by antoniou79):

 As far as I can tell, when moving tracks to fadeout
 (moveToFadeOutTrack()), the fadeout track maintains the same handle,
 whereas when cloning to fadeout, a new handle is created. Why not always
 use the clone to fadeout, which is safer?

 Also for some reason the "used" field of the source position of the moved
 track remains true (same thing happens when cloning). The comment says
 "Mark as used for now so the track won't be reused again this frame", but
 it seems it keeps the used value longer than it should. It's possible that
 the "used" field has ambiguous usage here.

 And, the clear() method, does not clear the handle field of the source
 track (there's a FIXME note about that), so in this case, trying to move
 the track back from fadeout to normal, the stopHandle is called on the
 (common) handle of the track, which is still in the "handle" field of the
 target position.

 I guess that the "if clause" in startSound() is there for the case when
 the track in fadeout is a different one than the normal one is replacing?
 But there's no check for the (very likely) case that it is the same track
 with the same handle (if moveToFadeOutTrack() was used).

 We could compare the sound handle we're stopping with the one that we need
 to keep working.
 Ie. this works:


 {{{
                         if (track->used) {
                                 flushTrack(track);
                                 if (memcmp(&(track->handle),
 &(fadeTrack->handle), sizeof(Audio::SoundHandle)) != 0) {
 g_system->getMixer()->stopHandle(track->handle);
                                 }
                         }
 }}}

 Not sure how we feel about memcmp on soundHandles, though.
 And if we do this here, it probably would make sense to do this at least
 in moveToFadeOutTrack() (maybe in cloneToFadeOutTrack() for consistency)
 just in case we don't obliviously stop a handle that is supposed to keep
 playing.
-- 
Ticket URL: <https://bugs.scummvm.org/ticket/12932#comment:9>
ScummVM :: Bugs <https://bugs.scummvm.org>
ScummVM