Originally posted by zZzZzZz
ok, no prob.
btw, i've seen Garf's source; may i ask what's the bloody deal with rc3 ov_read_float problems ? (i'm too lazy to look for answer myself; new in_vorbis uses ov_read_float all time).
ov_read_float is a new undocumented function in rc3 (which only you and me are using AFAIK). As you probably have noticed, it's got a weird API. In the normal ov_read, you can specify how many samples you want to get back at most. ov_read_float doesn't allow that, and you have no idea how large a thing you're going to get back. That's a bug (which makes some things more complicated than they should be), and it's going to be fixed in rc4.
rc3 ov_read_float:
ret = ov_read_float(&vf, &pcm, &iforgotwhatthiswas);
rc4 (and later) ov_read_float:
ret = ov_read_float(&vf, &pcm, max_number_of_samples_you_want, &iforgotwhatthiswas);
If your code can handle the rc3 behaviour, it'll handle the rc4 behaviour too, you'll just have to add a parameter in the calling sequence.
This wasn't discovered until now because nearly noone used ov_read float. I stumbled over it when implementing ReplayGain in XMMS, and Stan Seibert stumbled over it when implementing ReplayGain in ogg123. xiphmont agreed the current behaviour wasn't what was intended, and decided to change it while still not too many people are using it.
As for ReplayGain support in the vorbis plugin:
if you add a switch for Radio vs Audiophile gain, make sure that if Audiophile gain is selected, and no RG_AUDIOPHILE tag is present, you use the RG_RADIO one. You might want to make the switches read Radio/Track Gain and Audiophile/Album Gain for extra clarity.
Also, if it's not too much trouble, add a seperate switch (seperate from the ReplayGain stuff) 'Clipping prevention'
Psuedocode for impelementation:
if (there_is_a_rg_peak_tag())
peak = to_float(get_rg_peak_tag());
else
peak = 0.99;
if (cfg->use_replaygain)
replaygain = to_float(get_right_tag_here_depending_on_user_settings());
else
replaygain = 0.0;
scale = pow(10. , replaygain/20);
if (cfg->use_clippingprevention)
{
if (scale * peak > 1.0)
{
scale = 1.0 / peak;
}
}
Lastly, if you still have time, perhaps make the RG_* tags in the file info window appear a bit nicer, and perhaps allow adjustment of RG_RADIO and RG_AUDIOPHILE (not of RG_PEAK) via a simple spin button or so.
--
GCP