#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)
Change History (10)
by , 20 years ago
| Attachment: | vorbis.diff added |
|---|
comment:1 by , 20 years ago
| Owner: | set to |
|---|
comment:2 by , 20 years ago
| Summary: | Fix for Ogg Vorbis playback in BS2 cutscenes → Fix for Ogg Vorbis buffer overflow |
|---|
comment:3 by , 20 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 , 20 years ago
| Status: | new → closed |
|---|---|
| Summary: | Fix for Ogg Vorbis buffer overflow → Fix for Ogg Vorbis playback in BS2 cutscenes |
comment:5 by , 20 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 , 20 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 , 20 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 , 20 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 , 5 years ago
| Component: | → Engine: Sword2 |
|---|---|
| Game: | → Broken Sword 2 |
Patch against a January 17 CVS snapshot