Opened 15 years ago

Closed 14 years ago

#1951 closed defect (fixed)

COMI: cloneTofadeTrackId() Can't find free fade track

Reported by: SF/ed-hed Owned by: aquadran
Priority: high Component: Engine: SCUMM
Keywords: Cc:
Game: Monkey Island 3

Description

Yesterday night I fell asleep at the keys, playing Monkey3. Two or three hours later I wake up, save my game and go to bed. Next afternoon I go to play again and when I load my savegame (or the autosave) it starts up with the console window open. Console displays the following text:

ScummVM 0.7.0 (Dec 23 2004 23:23:39) Console is ready Debugger started, type 'exit' to return to game. Type 'help' to see a little list of commands and variables. ERROR: IMuseDigital::cloneTofadeTrackId() Can't find free fade track

Attached you may find my savegame.

I play it on Windows XP Home. The game is in English and on CD.

Grreetzz, - Eddie

Ticket imported from: #1161787. Ticket imported from: bugs/1951.

Attachments (2)

comi.s01 (62.6 KB ) - added by SF/ed-hed 15 years ago.
comi.s43 (62.5 KB ) - added by eriktorbjorn 15 years ago.
Experimental save game

Download all attachments as: .zip

Change History (27)

by SF/ed-hed, 15 years ago

Attachment: comi.s01 added

comment:1 by fingolfin, 15 years ago

Owner: set to aquadran
Summary: Can't get rid of consoleCOMI: cloneTofadeTrackId() Can't find free fade track

comment:2 by fingolfin, 15 years ago

Aquadran?

comment:3 by aquadran, 15 years ago

I can't reproduce this even with cvs head, release and later branch 0.7.0 builds.

comment:4 by SF/tbcarey, 15 years ago

I also cannot reproduce the error under latest CVS build on win32.

comment:5 by SF/ed-hed, 15 years ago

Is there perhaps any more information you'd need from me? I'm somewhat of a PC-retard, so I can't figure out much from the followups. One thing it could be is that I may have attached the wrong savegame. My girlfriend's savegame was in there too and I had to more or less guess which one it was. In game it should be called "EdGuy is back!" Any which way, knowing my computer, the problem could well be at my end, but I hope I can get it fixed. I'd prefer not to start over.

comment:6 by eriktorbjorn, 15 years ago

That appears to be the right savegame. After loading it, Guybrush is in the Goodsoup hotel. (It doesn't crash for me either.)

comment:7 by SF/ed-hed, 15 years ago

If I may be so blunt then, could I ask a favor of you, or anyone willing. Load the save and then save it under a different name and somehow get that new save to me. You see, I could start over, but I'd prefer to avoid it.

Thanx anyway.

by eriktorbjorn, 15 years ago

Attachment: comi.s43 added

Experimental save game

comment:8 by eriktorbjorn, 15 years ago

I've loaded and re-saved the game with ScummVM 0.7.0. Unless I made a mistake, it should appear in save slot 43 (though you can easily change that by renaming the file) as "EdGuy Is Back Again!".

Of course, I can't say whether or not it'll make any difference.

I wonder what'd make it crash on your system, but not mine though... some uninitialized variable, perhaps? I'll try to remember looking for that later.

comment:9 by SF/ed-hed, 15 years ago

I gave it a go, but I get the same thing again. The clonetrack business. Mind you, it's not a crash as in *poof* back to windows. All it does is open the console window without allowing me to do anything at all. I can see the game behind it.

Anyway, thanx for trying.

comment:10 by eriktorbjorn, 15 years ago

Strangely enough I can reproduce this error on my Linux box, but only if I run it under Valgrind. (A memory debugging tool that slows down everything enormously.) I did get a few warning messages, but nothing that looked relevant to the problem at hand.

Adding a few debug messages showed that switchToNextRegion() was repeatedly calling cloneToFadeOutTrack() with the same parameters until running out of fade track slots. I don't know if it's significant, but all the calls were made from the code path where sampleHookId was zero.

If I run the game without Valgrind it still called cloneToFadeOutTrack() several times with the same parameters, but not enough to fill up all the slots. I guess there's something timing-dependent going on here...

comment:11 by SF/ed-hed, 15 years ago

For the record, I did finish the game now. I started over and nothing went wrong this time. Guess it's a one-off, my girlfriend finished the game before me (which really hurt my manliness ;o) and didn't have the problem either. If you guys wanna be done with it, fine by me.

comment:12 by lavosspawn, 15 years ago

This seems to be some strange timing related issue. I just got the same error message while stepping though the code in debug mode with MSVC++.

ComI deadlocks on the Playstation 2 after selecting the difficulty level, with the message (in debug level 6): IMuseDigital::fadeOutMusic IMuseDigital::cloneToFadeOutTrack(0, 120) Locking mutex IMuseDigital::cloneToFadeOutTrack()

which is why I was stepping through the code at all... I wonder if that's related...

comment:13 by aquadran, 15 years ago

unfornately I can't find workaround for that deadlock and proper solution need a lot of changes

comment:14 by sev-, 15 years ago

What is the status of this item?

comment:15 by sev-, 15 years ago

So what is the resolution?

comment:16 by aquadran, 15 years ago

it can deadlock, when i last tried, imuse hasnt touched, so problem exist

comment:17 by fingolfin, 15 years ago

Priority: normalhigh

comment:18 by lavosspawn, 15 years ago

This seems to be what happens: iMuse's timer callback gets called and does something that takes a lot of time, like doing some file I/O from the cddrive. it returns after, say, 1-2 seconds, so afterwards our Timer implementation will call iMuse digital's timer handler e.g. 50 times in a row to make up for the delay earlier. And that's apparently what makes it run out of fade tracks or free sound slots... I was able to work around that by changing the Timer function so that it keeps calling at the specified rate without doing lots of continous calls after a delay. It doesn't crash then, but obviously it makes all things go out of sync...

comment:19 by fingolfin, 15 years ago

Thanks for analyzing this!

How about this alternate solution: We still do multiple calls in order to "catch up", but limit these to e.g. 10 calls "in a row" at most. This is *of course* just a horrible hack/workaround to the real issue, but might be a viable compromise (the idea is that the Timer still "catches up", after some iterations). Of course this only makes sense if those extremly slow CD accesses do not happen too frequently...

The real solution of course is to not have very slow disk I/O in a synchronous timer... It seems to me that we need a way to handle slow disk I/O async in order to fix this properly! Some possibilities that come to mind:

1) Reintroduce proper threads. (Drawbacks: Portability are a problem, much harder to debug in general)

2) Introduce async timers; see also the relevant patch. However, this will again require a lot of code to be changed

3) Introduce special generic code (to OSystem or File, or so) which provides a "generic" interface to do async disk I/O, and which internally can be implemented in any way we like (using threads; or synchronous; or using callback APIs of the OS; etc.).

I think I prefer 3 (my explanations are already biased, so you may have guessed it :-). I know several OSes which do not provide full threads, but which do provide an API for async disk I/O anyway, so portability is better. Plus it's much easier to debug one specific kind of async (possible threaded) code/API than to do so with the full (complicated) iMuseDigital code base...

The problem there of course is to design such an API, and to modify iMuseDigital to use it. The former isn't that hard, though (e.g. I already have ideas for that), the latter is the hard part. In particular it would be necessary to find out where the criticial spots are... The iMuseDigital code seems to do file reading in only three places (unless I missed something):

ImuseDigiSndMgr::prepareSoundFromRMAP BundleDirCache::matchFile BundleMgr::decompressSampleByIndex

I think decompressSampleByIndex is the main place to worry about... I'll think a bit about it...

comment:20 by aquadran, 14 years ago

does it still deadlock ? (playing from non-cd)

comment:21 by sev-, 14 years ago

What is the status of this item?

comment:22 by aquadran, 14 years ago

it should be fixed, i found bug and fixed it. thought my comments about deadlocks are still valid but it's not related with that bug report.

comment:23 by aquadran, 14 years ago

Status: newpending

comment:24 by sev-, 14 years ago

So, Pending or closed?

comment:25 by aquadran, 14 years ago

Resolution: fixed
Status: pendingclosed
Note: See TracTickets for help on using tickets.