Opened 10 years ago

Closed 9 years ago

#4437 closed defect (fixed)

DC: Drascula (Italian) Intro Movie Crash

Reported by: SF/who-sthere Owned by: sev-
Priority: normal Component: Engine: Drascula
Keywords: Cc:
Game: Drascula

Description

Platform: Dreamcast
Engines: Drascula
Version: 1.0.0svn42599 (latest)

Used Drascula freeware version, with other langauge pack from ScummVM downloads page, audio compressed with mp3.

During the movie intro, the game will crash/freeze at the scene where it says "Transilvania, 1993 D.C. DOPO CENA", necessitating a system reset. This crash can be avoided by skipping the movie.

Bug non-existant in Windows SVN build (latest version).

Ticket imported from: #2824291. Ticket imported from: bugs/4437.

Change History (10)

comment:1 by zeldin, 10 years ago

I've investigated the problem, and the crash is caused by ArjFile::open() not checking for NULL return values from malloc(). So we get an out of memory condition, and ArjFile doesn't notice and start trashing memory.

Now, a bit more interresting is the fact why we get the out-of-memory condition in the first place. ArjFile actually allocates two buffers, one with the size of the entire compressed data, and one with the size of the entire uncompressed data, and then proceeds to perform the decompression all at once, in RAM. (To add insult to injury, the Drascula engine also has code which allocates another RAM buffer with the size of the entire uncompressed data, and copies the data from the ArjFile's RAM buffer with the uncompressed data to it's own. Gah!) This is obviously mental if the data in question is a multi-megabyte movie. It should never have to keep more than one chunk of data (max 64K) in RAM at each time. Any takers for improving this class?

comment:2 by wjp, 10 years ago

It seems storing the entire compressed data was added in r32472 with the note "Speed up decoding by memory caching". Maybe using a BufferedReadStream instead of a MemoryReadStream would be a good compromise between memory usage and performance on the input side.

comment:3 by wjp, 10 years ago

On the output side, the full memcpy() in drascula's PlayFLI() is already optional. The TryInMem() function handles memory allocation failure.

The full decoding in ArjFile is less easy to avoid, though, but should be quite doable, as long as the output doesn't have to be seekable.

comment:4 by zeldin, 10 years ago

A side note regarding performance: Decompressing the whole thing at once gives not only worse memory usage, but also worse performance; it makes the it impossible to start movie playback before the whole stream has been uncompressed, which leads to a noticable pause at the beginning of the movie.

comment:5 by fingolfin, 10 years ago

The ARJ code is not very nice, aye. In several ways... ;)

Might be a good idea to post about this particular issue on scummvm-devel, to make people aware of it.

comment:6 by wjp, 10 years ago

I committed a change to make ArjFile use a BufferedReadStream instead of a MemoryReadStream for its input. Marcus tested it and it reduced memory usage enough for at least the intro movie to play.

comment:7 by fingolfin, 10 years ago

So, does this mean this is issue is resolved? If not, what is left?

comment:8 by wjp, 10 years ago

Ideally, the movie playing code should still be changed to do the decompression in a streaming mode rather than decompress the entire movie before playback. I believe the direct problem is solved, though.

comment:9 by sev-, 9 years ago

Owner: set to sev-
Resolution: fixed
Status: newclosed

comment:10 by sev-, 9 years ago

This has been fixed with switch to ArchiveMan

Note: See TracTickets for help on using tickets.