kcat / alure

Alure is a utility library for OpenAL, providing a C++ API and managing common tasks that include file loading, caching, and streaming
zlib License
70 stars 20 forks source link

Clicks when decoding files (at the end of playback) #20

Closed LAGonauta closed 6 years ago

LAGonauta commented 6 years ago

Hello, and thank you for doing this amazing library. It is really helpful!

Anyway, I am using it on a Half-Life plugin (https://github.com/LAGonauta/metahook/tree/alure_simple/Plugins/Audio) but when playing the data files from Half-Life there are clicks and pops in the sound at the end. Maybe the decoding is not working as it should?

Most files show this clicking behaviour, some examples are "delayagain.wav" (from the scientists) and "pl_step1.wav". This also happens on the code examples, compiled with MSVC or on MSYS2. On Linux this doesn't seem to happen and every file is decoded as it should. I can send some samples if you do not have them.

EDIT: Just tested with libsndfile. I disabled the native wav support and compile Alure with libsndfile support using MSVC2017. Works fine now, so there must be a bug hidden in the native wave decoder used by Alure that shows up only on Windows.

Thanks for your time.

LAGonauta commented 6 years ago

Just found out where the problem is. In line 413 of wave.cpp: std::istream::pos_type end = start + std::istream::pos_type(size - (size % framesize));

Would need to be changed to: std::istream::pos_type end = start + std::istream::pos_type(size - (size % framesize)) - std::istream::pos_type(framesize);

But how correct is this? This seems more like a workaround than a fix. According to Audacity "delayagain.wav" has 35695 samples, but Alure2 decodes it to 35696. With this workaround Alure2 decodes 35695 samples as it should.

I noticed that actually the default behaviour plays back fine on OpenAL Soft, but on Creative OpenAL and on Rapture3D it has the click at the end, so maybe OpenAL Soft does not play the last sample? (another bug?)

kcat commented 6 years ago

Most files show this clicking behaviour, some examples are "delayagain.wav" (from the scientists) and "pl_step1.wav". This also happens on the code examples, compiled with MSVC or on MSYS2. On Linux this doesn't seem to happen and every file is decoded as it should.

That's really odd. If it only happens with Windows, that would suggest something's up with the file I/O routines, either misdetecting some size/offset or misreading some value. Do you override the default file I/O with your own methods?

But how correct is this? This seems more like a workaround than a fix.

That's removing the last (supposedly) valid sample from the detected data chunk, it's not correct. Though why it thinks there's one more sample than there actually is, I'm not sure.

I noticed that actually the default behaviour plays back fine on OpenAL Soft, but on Creative OpenAL and on Rapture3D it has the click at the end, so maybe OpenAL Soft does not play the last sample? (another bug?)

If it's decoding and giving 35696 samples for all three drivers, any difference in output would suggest some uninitialized memory. Perhaps the extra sample value just happened to be at or around 0 when you were trying OpenAL Soft, and farther from 0 with the others. You can make a new MessageHandler class and override the bufferLoading method to see what data it read in and is about to give OpenAL.

LAGonauta commented 6 years ago

Actually my initial assumptions from the first post seems to be invalid. My new assumptions are as following:

What I noticed is that in the second pass of the while loop the value of "size" (which would be the number of bytes in the data) is correctly set, but after line 226 where size = std::min((size+1) & ~1u, totalsize) increases the value of size by 1. So 35695 8-bit mono frames turns into 35696 8-bit mono frames here.

The workaround would be just changing it to size = std::min(size, totalsize), but I admit I do not know what this operation is used for and there is a reason for it to be like it is now.

EDIT: I just imported the "delayagain.wav" file into Audacity and exported as an unsigned 8-bit mono file. Alure's WAV decoder could decode it correctly.

It seems that Half-Life audio files have a zero at the last byte, but this byte is removed on the Audacity export. So on lines 188 and 189:

mFile->read(reinterpret_cast<char*>(ptr), len);
ALuint got = static_cast<ALuint>(mFile->gcount());

Where len is 35696 bytes for both files, the value of got is set to 35695 for the audacity file and 35696 for the original file. This last unexpected byte at the end of the file is what causes all the problems...

kcat commented 6 years ago

The last frame is for some reason not played by OpenAL Soft, thus this issue does not show up on Linux. Also does not show on Windows with OpenAL Soft as driver.

That has me stumped. OpenAL Soft should be using all the data in the buffer. This happens with all sounds that have the click with Creative and Rapture3D drivers? And the reported buffer size is the same for all three?

What I noticed is that in the second pass of the while loop the value of "size" (which would be the number of bytes in the data) is correctly set, but after line 226 where size = std::min((size+1) & ~1u, totalsize) increases the value of size by 1. So 35695 8-bit mono frames turns into 35696 8-bit mono frames here.

The workaround would be just changing it to size = std::min(size, totalsize), but I admit I do not know what this operation is used for and there is a reason for it to be like it is now.

According to the RIFF format spec, chunk sizes need to be multiples of 2, and if the specified size is odd it needs to be rounded up to the next even value. That's what the (size+1) & ~1u does. It seems, however, that the amount of valid data should be used as specified for the chunk size, with the extra byte just being left as unused padding.

kcat commented 6 years ago

Commit 3ced8b9d9c31b5192319f010dcf0ad58dc1b7e62 should fix the extra sample from the wave reader. Still not sure about OpenAL Soft skipping the last sample, though.

LAGonauta commented 6 years ago

Thanks @kcat, your fix worked :)

EDIT: About what got you stumped. Yes, this happens (happened, as the click is fixed now) with all sounds that have the click with those drivers. I just tested it again with "OpenAL Soft" and noticed that I was actually misled as I was testing with "alure-play.exe". The click was not audible with "OpenAL Soft" and "Rapture3D" if the file with the click is the last file played. Sorry.