Hydrogenaudio Forums > Bug in ReplayGain reference code

Full Version: Bug in ReplayGain reference code

Hydrogenaudio Forums > Hydrogenaudio Forum > General Audio

Garf

Sep 5 2003, 09:58

There seems to be a bug in the ReplayGain reference code. The problem came to light because vorbisgain on Linux crashed on 48kHz files.

The bug proved to be pretty elusive, but was found by Simon Hosie.

QUOTE

gain_analysis.c: 111:
#define MAX_SAMPLES_PER_WINDOW (size_t) (MAX_SAMP_FREQ * RMS_WINDOW_TIME)

and gain_analysis.c: 234
sampleWindow = (int) ceil (samplefreq * RMS_WINDOW_TIME);

sampleWindow may be > MAX_SAMPLES_PER_WINDOW because of the ceil() call.
Here's my fix:

#define MAX_SAMPLES_PER_WINDOW (size_t) (MAX_SAMP_FREQ * RMS_WINDOW_TIME + 1)

The bug will be in all ReplayGain implementations, but may not show up due to the sensitivity to rounding. It may cause programs to randomly stop working when compiled with a different compiler or settings.

n68

Sep 5 2003, 12:34

Gday...

68 views.. & not one replie..

well.. what does this mean.. brand new compiles.. patches..
or just something to be aware of..

john33

Sep 5 2003, 15:22

I'll update VorbisGain and WaveGain at Rarewares over the next 24 hours. I'll post when they're done.

kmart

Sep 5 2003, 16:58

I understand how that could cause an error, but I don't see how it would with 48 khz files. Does vorbisgain use an unusual RMS_WINDOW_TIME?

I think the two RMS_WINDOW_TIME values I have seen used (50 msec & 10 msec) would both result in integer values for MAX_SAMPLES_PER_WINDOW and sampleWindow. So how would the ceil call make sampleWindow > MAX_SAMPLES_PER_WINDOW?

Am I missing something obvious?

Garf

Sep 6 2003, 03:35

They're defined as floats, not integers.

You get a buffer overrun that overwrites something random. It happens at 48kHz because that is MAX_SAMP_FREQ, smaller values simply don't overrun the buffer.

Garf

Sep 6 2003, 03:38

QUOTE(n68 @ Sep 5 2003, 08:34 PM)

well.. what does this mean.. brand new compiles.. patches..
or just something to be aware of..

All programmers that use ReplayGain should fix this or they risk their programs to randomly stop working at some point.

kmart

Sep 6 2003, 03:57

I don't think I explained my question very well.

I realize that they are defined as floats. Hopefully this will explain what I'm asking better.

MAX_SAMP_FREQ = 48000.0
RMS_WINDOW_TIME = 0.050

therefore,

MAX_SAMPLES_PER_WINDOW = 48000.0 * 0.050 = 2400.0 = 2400 as an integer

likewise, sampleWindow = ceil (48000.0 * 0.050) = ceil (2400.0) = 2400

What i'm saying is that the ceil call shouldn't make any difference in this case, right?

Garf

Sep 6 2003, 05:04

Floating point arithmetic doesn't work as you assume.

48000.0 when converted to binary float can correspond to something that is a very very very little bit bigger or smaller than 48000.0. Same for 0.050. Multiple the two together and you may have something a very very little bit bigger than 2400. BOOM.

john33

Sep 6 2003, 05:11

QUOTE(john33 @ Sep 5 2003, 09:22 PM)

I'll update VorbisGain and WaveGain at Rarewares over the next 24 hours. I'll post when they're done.

Both have been updated and are available now at Rarewares.

This is a "lo-fi" version of our main content. To view the full version with more information, formatting and images, please click here.