Opened 21 years ago

Closed 21 years ago

Last modified 6 years ago

#8312 closed patch

Fix for Ogg Vorbis playback in BS2 cutscenes

Reported by: eriktorbjorn Owned by: fingolfin
Priority: normal Component: Engine: Sword2
Version: Keywords:
Cc: Game: Broken Sword 2

Description

I've been having trouble with the Ogg Vorbis playback in the BS2 cutscenes. I finally sat down and tried to figure out what happened:

readBuffer() is called with 11024 being the desired number of samples.

On the first iteration, samples is 0 and _bufferEnd - _pos is 4096. Since this is less than 11024, len gets set to 4096.

On the second iteration, samples is 4096, and _bufferEnd - _pos is still 4096. Since 4096 + 4096 = 8192 is still less than 11024, len gets set to 8192.

At this stage, we have copied 4096 + 8192 = 12288 samples, which seems like a major whoops to me.

I don't know if this patch is correct (I'm still fighting off the onset of a cold, which doesn't help my thinking :-), but it does fix the crashing problem for me.

Ticket imported from: #878883. Ticket imported from: patches/417.

Attachments (1)

vorbis.diff (619 bytes ) - added by eriktorbjorn 21 years ago.
Patch against a January 17 CVS snapshot

Download all attachments as: .zip

Change History (10)

by eriktorbjorn, 21 years ago

Attachment: vorbis.diff added

Patch against a January 17 CVS snapshot

comment:1 by eriktorbjorn, 21 years ago

Owner: set to fingolfin

comment:2 by eriktorbjorn, 21 years ago

Summary: Fix for Ogg Vorbis playback in BS2 cutscenesFix for Ogg Vorbis buffer overflow

comment:3 by eriktorbjorn, 21 years ago

Actually, the bug almost certainly happen with any Ogg Vorbis playback. But I've only seen it happen with the BS2 cutscenes, and even then only under Windows.

comment:4 by fingolfin, 21 years ago

Status: newclosed
Summary: Fix for Ogg Vorbis buffer overflowFix for Ogg Vorbis playback in BS2 cutscenes

comment:5 by eriktorbjorn, 21 years ago

It looks like mp3.cpp might have the same problem, but I don't have any test case for it.

comment:6 by fingolfin, 21 years ago

I am pretty sure that the MP3 code doesn't have the same problem. Nor that AppendableMemoryStream has the same problem.

These two classes were created first, the Vorbis class last, based on the source of the previous two. If you look, the "len" computation looks very similar in all of them. However, for Ogg we use len to copy memory data, while for MP3 it is a loop end marker, which is compared to the value of "samples".

comment:7 by eriktorbjorn, 21 years ago

Ah, I see. I was looking only at the mp3.cpp len calculation, and assumed the rest of the code was similar to the one in vorbis.cpp. I see now that I was mistaken.

Thanks for clearing that up!

comment:8 by fingolfin, 21 years ago

No no, you got it the wrong way around, I gotta thank *you* for catching the goof in the vorbis code :-)

comment:9 by digitall, 6 years ago

Component: Engine: Sword2
Game: Broken Sword 2
Note: See TracTickets for help on using tickets.