• All submissions to this site are governed by Second Life Project Contribution Agreement. By submitting patches and other information using this site, you acknowledge that you have read, understood, and agreed to those terms.
Issue Details (XML | Word | Printable)

Key: VWR-6154
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: Normal Normal
Assignee: Tofu Linden
Reporter: Carjay McGinnis
Votes: 2
Watchers: 3
Operations

If you were logged in you would be able to see more operations.
1. Second Life Viewer - VWR

Decoded audio WAV files have 8 bytes missing at the end (llaudiodecodemgr.cpp)

Created: 06/Apr/08 02:22 PM   Updated: 17/Oct/08 10:57 AM
Return to search
Component/s: Sound, Source Code
Affects Version/s: Source code
Fix Version/s: 1.21

File Attachments: 1. File audiodecodemgr_fix.diff (0.6 kB)

Environment: Kubuntu Linux x86_64
Issue Links:
Relates
 

Last Triaged: 10/Jul/08 09:18 AM
Linden Lab Issue ID: DEV-13485
Patch attached: Patch attached
Linden Lab Internal Branch: maint-viewer-9


 Description  « Hide
llaudiodecodemgr.cpp decodes the incoming OGG Vorbis files to WAV files (used for all sounds in-world and for the UI).

But the size that is used when writing the data out to the ".dsf"-files is not including the WAVE-header so 8 bytes are missing.

This is obviously not an issue for FMOD but the routine used in the OpenAL patches (actually part of ALUT) barfs because it expects that the amount of data that is stated in the "data"-header is actually present in the file.

I looked at older sources from around December which still worked with OpenAL and it seems that somebody wanted to clean up unnecessary calculations with data_length so this bug must have crept in in between.

The attached patch fixes this (it writes out the complete mWAVBuffer as intended).



 All   Comments   Change History      Sort Order: Ascending order - Click to sort in descending order
Soft Linden added a comment - 07/Apr/08 09:12 AM
I want to make sure a Linden doesn't take this in without understanding - can you point to an authoritative source that says the header size should be included in the total?

Lex Neva added a comment - 07/Apr/08 10:01 AM
Let me see if I understand you properly. You're saying the header in a WAVE file is 8 bytes long, and it contains the amount of data that will be in the rest of the file. Say there's 512 bytes of actual sound data. You're saying that the .dsf file being written is going to be exactly 512 bytes long, INCLUDING the header? That sounds to me like it'll leave off 8 bytes of data, which is definitely a problem!

This could explain why I sometimes get little pop sounds in looping sound files even when I carefully cut my sound samples at zero-crossings.

Soft, if I'm correct in my understanding, Carjay is saying the exact opposite – that the header size shouldn't be included in the data_length in the header, but the current code is essentially treating it as if it is.


Carjay McGinnis added a comment - 08/Apr/08 06:38 PM - edited
Ah, no, Soft, you misunderstood. It's pretty simple and the way Lex assumed.

The generated header is correct, it's the amount of data that is written which is wrong.

Original code snippet:

S32 data_length = mWAVBuffer.size() - WAV_HEADER_SIZE;
mWAVBuffer[40] = (data_length) & 0x000000FF;
mWAVBuffer[41] = (data_length >> 8) & 0x000000FF;
mWAVBuffer[42] = (data_length >> 16) & 0x000000FF;
mWAVBuffer[43] = (data_length >> 24) & 0x000000FF;
data_length += 36;
mWAVBuffer[4] = (data_length) & 0x000000FF;
mWAVBuffer[5] = (data_length >> 8) & 0x000000FF;
mWAVBuffer[6] = (data_length >> 16) & 0x000000FF;
mWAVBuffer[7] = (data_length >> 24) & 0x000000FF;

mFileHandle = LLLFSThread::sLocal->write(mOutFilename, &mWAVBuffer[0], 0, data_length,
new WriteResponder(this));

1. mWAVBuffer holds all data (the total 44 byte header and the pcm data).

2. data_length is initialized with that size minus WAV_HEADER_SIZE (the 44 bytes) so by now it is the length of the actual pcm sample data.
This value is written to the "data" section header and is correct.

2. Next, 36 is added (which is the length of the "fmt" section header). This value is now written to the RIFF header. This is correct, too.

So all the header data is indeed correct.

But we want to write the whole mWAVBuffer and data_length is still missing the 8 bytes from the RIFF header. Either add 8 or just use mWAVBuffer.size() (as I did).


Tofu Linden added a comment - 10/Jul/08 07:01 PM
Thanks Carjay!