[FFmpeg-devel] [libav-devel] [PATCH] aacpsy: avoid norm_fac becoming NaN

[Andreas Cadhalpun]

On 20.04.2015 22:15, Claudio Freire wrote:
> TL;DR, band->thr should not be negative ever, band->thr == 0.0f would
> cause lots of issues on its own if band->energy != 0.0f in such a case
> (though I don't see how band->thr can be 0.0f if band->energy is not),

This could happen in pathological cases:
    band->energy = 1e-37f
    band->thr    = band->energy * 0.001258925f = 0.0f

> and ath <= 0.0f can happen and should be no trouble if it does.

The trouble begins if band->thr is also 0.0f.

> The long version:
> 
> ath should approximate the shape of the absolute hearing threshold, so
> yes, it's best if it really uses the minimum, since that will prevent
> clipping of the ath curve and result in a more accurate threshold
> computation.

So you agree with my patch fixing minath?
Or would you prefer a version with:
    minath = ath(3410 - 0.733 * ATH_ADD, ATH_ADD)

> Still, when
> 
> band->thr_quiet = band->thr = FFMAX(band->thr, coeffs[g].ath);
> 
> Is computed, correct me if I'm wrong, but band->thr is the band's
> energy (sum of squares), so I see how that can be zero, but not how it
> can be negative.

Yes, band->thr is always >= 0.0f.

> Thus, if ath became negative, its effective shape would be clipped by band->thr.

Indeed. That's why I wrote that ath = 0.0f is just as bad as a negative one.

> The whole point of ath is to avoid spending lots of bits for signals
> normally too faint to be heard. The case of band->energy==0 is already
> handled by zero flags, but faint noise in higher frequencies needs an
> absolute hearing threshold curve to properly decide when not to waste
> bits in those bands. But since people can adjust the volume and you
> never know the final SPL at which the signal will be played, a precise
> calculation of said threshold is pointless, what I try to do in the
> patch series in issue #2686, is to attempt to shift it to the
> equivalent power of a 16-bit signal's quantization noise, which one
> would assume should be below the absolute hearing threshold in any
> sane reproduction environment - so it's a conservative estimate.

Thanks for the explanation.

> That said, ath should be > 0, not >= 0.

Good to know.

> But it's hard to enforce that
> without clipping it, and it's not worth the trouble attempting it.

Why would clipping be a problem?
What about the attached patch?

> I don't believe accurate computation of ath to that point would improve
> encoding that much. Any reasonable approximation will do.

It's more about avoiding nasty problems with division by 0.

Best regards,
Andreas

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-aacpsy-clip-coeffs-g-.ath-at-1e-30f.patch
Type: text/x-diff
Size: 973 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150421/a00e8b67/attachment.bin>