[FFmpeg-devel] [PATCH v3] [GSoC] [AAC] aaccoder: Implement Perceptual Noise Substitution for AAC

[Claudio Freire]

It's close...

On Tue, Apr 14, 2015 at 11:17 PM, Rostislav Pehlivanov
<atomnuker at gmail.com> wrote:
> @@ -711,9 +748,11 @@ static void search_for_quantizers_twoloop(AVCodecContext *avctx,
>  {
>      int start = 0, i, w, w2, g;
>      int destbits = avctx->bit_rate * 1024.0 / avctx->sample_rate / avctx->channels * (lambda / 120.f);
> -    float dists[128] = { 0 }, uplims[128];
> +    const int freq_mul = avctx->sample_rate/(1024/sce->ics.num_windows)/2;

Storing it in a const can induce a huge rounding error.

Think sample_rate 7350, 1 window. That's freq_mult = 3.588, but
rounding it will put 3, which is a huge rounding error.

Just put the whole expression when needed, that will avoid the
rounding error. Like:

> +            if (s->options.pns && start*freq_mul > NOISE_LOW_LIMIT && energy < uplim * 1.2f) {

start*avctx->sample_rate/(1024/sce->ics.num_windows)/2 =
start*7350/1024/2 = implicitly ((start*7350)/1024)/2, avoiding the
rounding error completely.

Always consider that when doing integer math, and the possibility of
intermediate results overflowing (not possible here given the maximum
start and sample_rate).

>          for (w = 0; w < sce->ics.num_windows; w += sce->ics.group_len[w]) {
>              for (g = 0; g < sce->ics.num_swb; g++) {
>                  int prevsc = sce->sf_idx[w*16+g];
> +                if (sce->band_type[w*16+g] == NOISE_BT) {
> +                    sce->sf_idx[w*16+g] = av_clip(noise_sf[w*16+g], minscaler_n, minscaler_n + SCALE_MAX_DIFF);
> +                    continue;
> +                }
>                  if (dists[w*16+g] > uplims[w*16+g] && sce->sf_idx[w*16+g] > 60) {
>                      if (find_min_book(maxvals[w*16+g], sce->sf_idx[w*16+g]-1))
>                          sce->sf_idx[w*16+g]--;

Good, but you don't need to apply the clipping on each iteration. Do
it outside the do { ... } while (fflag && its < 10), which is just
once, at the very end.

With that fixed, I believe it will be committable.

:)