Opened 10 years ago

Closed 10 years ago

#4693 closed defect (fixed)

FT: Unknown VOC block type 128

Reported by: (none) Owned by: fingolfin
Priority: normal Component: Engine: SCUMM
Keywords: Cc:
Game: Full Throttle

Description

ScummVM versions: 1.0.0rc143760 and 1.0.0pre045515
Bug: Crashes whenever inventory is brought up in the Corley Motors factory and adjacent areas
Language: English
Game version: Macintosh
Platform: OSX 10.5.8

Ticket imported from: #2890038. Ticket imported from: bugs/4693.

Attachments (1)

ft-mac.s04 (17.6 KB ) - added by SF/*anonymous 10 years ago.
corley_factory

Download all attachments as: .zip

Change History (17)

by SF/*anonymous, 10 years ago

Attachment: ft-mac.s04 added

corley_factory

comment:1 by fingolfin, 10 years ago

Priority: normalblocker

comment:2 by fingolfin, 10 years ago

I cannot reproduce this issue on trunk, but could reproduce it on the 1.0.0 branch. In both cases, I saw this warning when opening the inventory:

WARNING: Unhandled code in VOC file : 128!

Raising to release criticial, as this is a clear regression.

comment:3 by fingolfin, 10 years ago

It seems to hang in an endless loop in Audio::LinearDiskStream<false, false, true, false>::readBuffer(). I suspect this is related to the unknown VOC type.

Here's a backtrace from the VOC warning; I think it crashed on my branch build but not on my trunk build because I built the former in release mode and the latter in debug mode.

#0 warning (s=0xacdb4c "Unhandled code in VOC file : %d") at common/util.cpp:442
#1 0x009bcee9 in loadVOCFromStream (stream=@0x15b9a00, size=@0xbfffd1c8, rate=@0xbfffd1c4, loops=@0xbfffd17c, begin_loop=@0xbfffd178, end_loop=@0xbfffd174) at sound/voc.cpp:156
#2 0x009bcf58 in Audio::loadVOCFromStream (stream=@0x15b9a00, size=@0xbfffd1c8, rate=@0xbfffd1c4) at sound/voc.cpp:166
#3 0x009bcf92 in Audio::makeVOCStream (stream=@0x15b9a00, flags=1 '\001', loopStart=0, loopEnd=0, takeOwnershipOfStream=false) at sound/voc.cpp:311
#4 0x0010c6fa in Scumm::Sound::startTalkSound (this=0x1872000, offset=15794451, b=10, mode=2, handle=0x1872334) at engines/scumm/sound.cpp:654
#5 0x0011104d in Scumm::Sound::processSfxQueues (this=0x1872000) at engines/scumm/sound.cpp:470
#6 0x00111423 in Scumm::Sound::processSound (this=0x1872000) at engines/scumm/sound.cpp:115
#7 0x00100f3c in Scumm::ScummEngine::scummLoop_handleSound (this=0x16be8000) at engines/scumm/scumm.cpp:2235
#8 0x00100f4f in Scumm::ScummEngine_v7::scummLoop_handleSound (this=0x16be8000) at engines/scumm/scumm.cpp:2240
#9 0x0010a832 in Scumm::ScummEngine::scummLoop (this=0x16be8000, delta=6) at engines/scumm/scumm.cpp:2030
#10 0x001007f0 in Scumm::ScummEngine::go (this=0x16be8000) at engines/scumm/scumm.cpp:1846
#11 0x0003c067 in Scumm::ScummEngine::run (this=0x16be8000) at scumm.h:457

The "length" of the unknown block is 0x560080 (so it probably isn't a length, but rather something else is wrong). All in all, the VOC data the decoder "sees" is first a block of type 1, len 30340; followed by the mysterious block of type 128, len 0x560080 = 5636224

Lowering priority a bit, as I am no longer sure whether this is really a regression; I see nothing in older ScummVM releases that makes me believe that we ever supported this correctly.

comment:4 by fingolfin, 10 years ago

Please disregards the last paragraph in my previous comment (I forgot to remove it before posting m comment).

comment:5 by eriktorbjorn, 10 years ago

Does that mean that STREAM_AUDIO_FROM_DISK is not defined on OS X? Because that seems to be the major difference between voc.cpp in 0.13.1 and the 1.0.0 branch. (Well, that and case 8, "Extended".) But if that's not used... does the unhandled code appear in 0.13.1 as well?

comment:6 by fingolfin, 10 years ago

STREAM_AUDIO_FROM_DISK is enabled in common/scummsys.h for all UNIX-like targets, including Mac OS X.

comment:7 by eriktorbjorn, 10 years ago

That's what I would have guessed, but your backtrace looked like it was refering to parts of the code that would only be compiled when STREAM_AUDIO_FROM_DISK isn't defined. For instance, line 311 of sound/voc.cpp.

comment:8 by eriktorbjorn, 10 years ago

At least in the DOS version of the game, there's a sound effect while the inventory is open. Is that the one that's causing problems? It seems odd because surely it should be the same sound effect throughout the game?

Maybe there's more than one sound being streamed simultaneously from the same file. That could cause problems, but the LinearDiskStream class looks like it was written to handle that case.

comment:9 by fingolfin, 10 years ago

I commited a fix (resp. workaround) for this. The problem was that due to the invalid chunk in the VOC data, parseVOCFormat() returned that there are 0 blocks. But makeVOCDiskStream() nevertheless created a LinearDiskStream. The code in LinearDiskStream was not prepared for that, though, e.g. its endOfData() method checks
(_currentBlock == _audioBlockCount - 1)
but _audioBlockCount is 0 in the situation we are in, and _currentBlock is always >= 0. Boom.

I made two changes: I added an assert(numBlocks>0) to LinearDiskStream, and I changed makeVOCDiskStream() to return 0 if parsing the VOC data failed.

So the crash is gone and this is no longer release critical. Phew :). Drawback of this is that the sound is never played. We could add a workaround (by at least returning the sound data that was already decoded). Even better would be to figure out why this invalid chunk type pops up, and how to handle it...

comment:10 by fingolfin, 10 years ago

Owner: set to fingolfin
Priority: blockernormal

comment:11 by sev-, 10 years ago

Raising priority. This is a release-critical bug.

comment:12 by sev-, 10 years ago

Priority: normalblocker

comment:13 by fingolfin, 10 years ago

Summary: FT: Crashes when entering inventoryFT: Unknown VOC block type 128

comment:14 by fingolfin, 10 years ago

I don't think this is release critical: The crash has already been fixed (see my previous comment). The only remaining issue is an unknown block type. This might be a simple bug in the data, though. Maybe we can add a workaround for it, if somebody spent some time on analyzing the VOC data in question. But so far, nobody complained about missing sounds in FT :-).

Looking at the data once more, it seems that the cause for this is indeed a bug in the VOC encoding: It looks to me as if the VOC data is simply two bytes longer than specified. Given that the VOC block length is always two more than the audio data size (because it also counts the two bytes which encode the sample rate and encoding type), this sounds like a very plausible bug. And indeed, the next two bytes after the audio data are 0x80 and again 0x80 (which is also where the mysterious block type 128 = 0x80 comes from). Forcing the block sizes to be 2 bigger fixes the issue, although I still don't really notice a particular sound :).

comment:15 by fingolfin, 10 years ago

Priority: blockernormal
Resolution: fixed
Status: newclosed

comment:16 by fingolfin, 10 years ago

Summary: An ugly workaround would be possible, but it has no noticeable effect. Conclusion: I am closing this bug as resolved, as the crash is gone, and the warning can be safely ignored.

Note: See TracTickets for help on using tickets.