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
>From 21bf9c94e4fb3aaacfc122eacb581c7504d2183a Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <[email protected]>
Date: Tue, 21 Apr 2015 00:13:16 +0200
Subject: [PATCH] aacpsy: clip coeffs[g].ath at 1e-30f
This ensures that band->thr is always positive.
Signed-off-by: Andreas Cadhalpun <[email protected]>
---
libavcodec/aacpsy.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libavcodec/aacpsy.c b/libavcodec/aacpsy.c
index 7205ee3..bd3068f 100644
--- a/libavcodec/aacpsy.c
+++ b/libavcodec/aacpsy.c
@@ -350,6 +350,9 @@ static av_cold int psy_3gpp_init(FFPsyContext *ctx) {
for (i = 1; i < band_sizes[g]; i++)
minscale = FFMIN(minscale, ath((start + i) * line_to_frequency, ATH_ADD));
coeffs[g].ath = minscale - minath;
+ if (coeffs[g].ath < 1e-30f) {
+ coeffs[g].ath = 1e-30f;
+ }
start += band_sizes[g];
}
}
--
2.1.4
_______________________________________________
ffmpeg-devel mailing list
[email protected]
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
