[FFmpeg-devel] [PATCH] avcodec/aaccoder: Limit sf_idx difference for all cases

[Rostislav Pehlivanov]

On 23 August 2016 at 11:27, Michael Niedermayer <michael at niedermayer.cc>
wrote:

> Fixes: assertion failure
> Fixes: 86914558f0a471f038ee1102c02eeb45/signal_sigabrt_7ffff6ae7c37_3051_
> 64ed96a710787ba5d0666746a8562e7d.dee
>
> Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> ---
>  libavcodec/aaccoder.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
> index 284b401..995724b 100644
> --- a/libavcodec/aaccoder.c
> +++ b/libavcodec/aaccoder.c
> @@ -196,7 +196,7 @@ typedef struct TrellisPath {
>  static void set_special_band_scalefactors(AACEncContext *s,
> SingleChannelElement *sce)
>  {
>      int w, g;
> -    int prevscaler_n = -255, prevscaler_i = 0;
> +    int prevscaler_n = -255, prevscaler_i = 0, prevscaler_d = -255;
>      int bands = 0;
>
>      for (w = 0; w < sce->ics.num_windows; w += sce->ics.group_len[w]) {
> @@ -211,6 +211,10 @@ static void set_special_band_scalefactors(AACEncContext
> *s, SingleChannelElement
>                  if (prevscaler_n == -255)
>                      prevscaler_n = sce->sf_idx[w*16+g];
>                  bands++;
> +            } else {
> +                if (prevscaler_d == -255)
> +                    prevscaler_d = sce->sf_idx[w*16+g];
> +                bands++;
>              }
>          }
>      }
> @@ -227,6 +231,8 @@ static void set_special_band_scalefactors(AACEncContext
> *s, SingleChannelElement
>                  sce->sf_idx[w*16+g] = prevscaler_i =
> av_clip(sce->sf_idx[w*16+g], prevscaler_i - SCALE_MAX_DIFF, prevscaler_i +
> SCALE_MAX_DIFF);
>              } else if (sce->band_type[w*16+g] == NOISE_BT) {
>                  sce->sf_idx[w*16+g] = prevscaler_n =
> av_clip(sce->sf_idx[w*16+g], prevscaler_n - SCALE_MAX_DIFF, prevscaler_n +
> SCALE_MAX_DIFF);
> +            } else {
> +                sce->sf_idx[w*16+g] = prevscaler_d =
> av_clip(sce->sf_idx[w*16+g], prevscaler_d - SCALE_MAX_DIFF, prevscaler_d +
> SCALE_MAX_DIFF);
>              }
>          }
>      }
> --
> 2.9.3
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


That fuzzed sample seems to be causing the algorithm which does SF
difference normalization between normal and PNS bands to fail. This commit
masks the problem downstream. IMO that's not the correct way to solve this,
as there's no guarantee that another sample won't trigger the same assert
even when limiting all scalefactors. Fixing a single fuzzed sample with a
hack which doesn't stop other fuzzed samples from triggering the same bug
isn't justified.
I have the time right now and I'll try to fix this properly, but it might
take me a day or two. I think the problem is that when the twoloop coder
does the the normalization it doesn't take into account the fact that IS
and PNS have their scalefactors modified by set_special_band_scalefactors()
later on before encoding.