#1669 closed defect (fixed)
VORBIS: Crash when using Ogg Vorbis CD tracks (regression)
| Reported by: | eriktorbjorn | Owned by: | fingolfin |
|---|---|---|---|
| Priority: | normal | Component: | Audio |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
Since the recent "refcount" changes in the File class, ScummVM crashes the second time the CD Audio manager tries to play a particular Vorbis file.
For instance, this happens with Loom CD (crashes after you select difficulty level) and MI1 CD (crashes when reaching the "lookout" part of the intro).
As far as I can tell, the crash happens in VorbisTrackInfo::play() because the file it's about to play isn't open. Which probably is because the track was found in the cache, so it was assumed to be open already.
Right now, the only simple way of fixing it that I can think of would be to remove the entire caching mechanism completely. Does it really serve any useful purpose?
Ticket imported from: #981991. Ticket imported from: bugs/1669.
Change History (5)
comment:1 by , 20 years ago
| Owner: | set to |
|---|---|
| Summary: | VORBIS: Crash when using Ogg Vorbis CD tracks → VORBIS: Crash when using Ogg Vorbis CD tracks (regression) |
comment:2 by , 20 years ago
comment:3 by , 20 years ago
| Status: | new → closed |
|---|
comment:4 by , 20 years ago
| Resolution: | → fixed |
|---|
comment:5 by , 6 years ago
| Component: | --Unset-- → Audio |
|---|
Note:
See TracTickets
for help on using tickets.
The problem is not caused by the refcount changes; rather it is caused by a bug fix: VorbisInputStream didn't call ov_clear in the past, as it should have. I corrected that, thus fixing some potential leaks. However, the Vorbis CD track code relied on that buggy behaviour..
Personally, I don't see why we need the caching code. Maybe keeping the file handle open does help the disk cache of some operating systems (i.e. closing a file might cause it to be flushed from disk buffers; if we are reusing an audio track file, that could, hypothetically, increase stutter. But that's just wild guessing).