Opened 15 years ago

Closed 15 years ago

Last modified 7 months ago

#8312 closed patch

Fix for Ogg Vorbis playback in BS2 cutscenes

Reported by: eriktorbjorn Owned by: fingolfin
Priority: normal Component: Engine: Sword2
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 15 years ago.
Patch against a January 17 CVS snapshot

Download all attachments as: .zip

Change History (10)

Changed 15 years ago by eriktorbjorn

Attachment: vorbis.diff added

Patch against a January 17 CVS snapshot

comment:1 Changed 15 years ago by eriktorbjorn

Owner: set to fingolfin

comment:2 Changed 15 years ago by eriktorbjorn

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

comment:3 Changed 15 years ago by eriktorbjorn

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 Changed 15 years ago by fingolfin

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

comment:5 Changed 15 years ago by eriktorbjorn

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

comment:6 Changed 15 years ago by fingolfin

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 Changed 15 years ago by eriktorbjorn

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 Changed 15 years ago by fingolfin

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

comment:9 Changed 7 months ago by digitall

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