Opened 14 years ago
Closed 13 years ago
#4693 closed defect (fixed)
FT: Unknown VOC block type 128
|Reported by:||(none)||Owned by:||fingolfin|
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.
Change History (17)
by , 14 years ago
comment:1 by , 14 years ago
|Priority:||normal → blocker|
comment:2 by , 14 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 , 14 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 , 14 years ago
Please disregards the last paragraph in my previous comment (I forgot to remove it before posting m comment).
comment:5 by , 14 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 , 14 years ago
STREAM_AUDIO_FROM_DISK is enabled in common/scummsys.h for all UNIX-like targets, including Mac OS X.
comment:7 by , 14 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 , 14 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 , 14 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 , 14 years ago
|Priority:||blocker → normal|
comment:11 by , 13 years ago
Raising priority. This is a release-critical bug.
comment:12 by , 13 years ago
|Priority:||normal → blocker|
comment:13 by , 13 years ago
|Summary:||FT: Crashes when entering inventory → FT: Unknown VOC block type 128|
comment:14 by , 13 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 , 13 years ago
|Priority:||blocker → normal|
|Status:||new → closed|
comment:16 by , 13 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.