[FFmpeg-devel] [PATCH] libavcodec/opus: Add NULL pointer check for incoming data; avoid segfault

[Colin NG]

It still crashes with the following change. It crashes at ff_pvq_search_exact_avx () instead. We should wait for your improvement to fix the crash.


Colin


________________________________
From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> on behalf of Rostislav Pehlivanov <atomnuker at gmail.com>
Sent: November 22, 2017 8:02 PM
To: FFmpeg development discussions and patches
Subject: Re: [FFmpeg-devel] [PATCH] libavcodec/opus: Add NULL pointer check for incoming data; avoid segfault

On 23 November 2017 at 00:53, Colin NG <colin_ng at hotmail.com> wrote:

> ---
> Fix for ticket 7674
>
>  libavcodec/opus_pvq.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/opus_pvq.c b/libavcodec/opus_pvq.c
> index f98b85d..02ccd69 100644
> --- a/libavcodec/opus_pvq.c
> +++ b/libavcodec/opus_pvq.c
> @@ -504,6 +504,9 @@ static av_always_inline uint32_t
> quant_band_template(CeltPVQ *pvq, CeltFrame *f,
>      int longblocks = (B0 == 1);
>      uint32_t cm = 0;
>
> +     if (!X)
> +        return cm;
> +
>      if (N == 1) {
>          float *x = X;
>          for (i = 0; i <= stereo; i++) {
> @@ -795,7 +798,7 @@ static av_always_inline uint32_t
> quant_band_template(CeltPVQ *pvq, CeltFrame *f,
>              f->remaining2 -= curr_bits;
>          }
>
> -        if (q != 0) {
> +        if (X && !q) {
>              /* Finally do the actual (de)quantization */
>              if (quant) {
>                  cm = celt_alg_quant(rc, X, N, (q < 8) ? q : (8 + (q & 7))
> << ((q >> 3) - 1),
> @@ -902,6 +905,9 @@ static float pvq_band_cost(CeltPVQ *pvq, CeltFrame *f,
> OpusRangeCoder *rc, int b
>      float *Y_orig = f->block[1].coeffs + (ff_celt_freq_bands[band] <<
> f->size);
>      OPUS_RC_CHECKPOINT_SPAWN(rc);
>
> +    if (!X && !Y)
> +        return 0.0f;
> +
>      memcpy(X, X_orig, band_size*sizeof(float));
>      if (Y)
>          memcpy(Y, Y_orig, band_size*sizeof(float));
> @@ -911,7 +917,6 @@ static float pvq_band_cost(CeltPVQ *pvq, CeltFrame *f,
> OpusRangeCoder *rc, int b
>          int curr_balance = f->remaining / FFMIN(3, f->coded_bands - band);
>          b = av_clip_uintp2(FFMIN(f->remaining2 + 1, f->pulses[band] +
> curr_balance), 14);
>      }
> -
>      if (f->dual_stereo) {
>          pvq->encode_band(pvq, f, rc, band, X, NULL, band_size, b / 2,
> f->blocks, NULL,
>                           f->size, norm1, 0, 1.0f, lowband_scratch, cm[0]);
> @@ -925,7 +930,12 @@ static float pvq_band_cost(CeltPVQ *pvq, CeltFrame
> *f, OpusRangeCoder *rc, int b
>
>      for (i = 0; i < band_size; i++) {
>          err_x += (X[i] - X_orig[i])*(X[i] - X_orig[i]);
> -        err_y += (Y[i] - Y_orig[i])*(Y[i] - Y_orig[i]);
> +    }
> +
> +   if (Y) {
> +        for (i = 0; i < band_size; i++) {
> +            err_y += (Y[i] - Y_orig[i])*(Y[i] - Y_orig[i]);
> +        }
>      }
>
>      dist = sqrtf(err_x) + sqrtf(err_y);
> --
> 2.7.4
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
ffmpeg-devel Info Page<http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
ffmpeg.org
This list is about FFmpeg development discussions and patches; but not for bug-reports. Please read the Code-of-conduct. To see the collection of prior postings to ...



>


I actually found out about this bug while working on it a few days ago but
wanted to wait until I got the opus_rc and some opusenc_psy improvements
fully done.
Pushed my fix for mono. I disagree with the rest. There should always be a
channel of audio (X) to analyze.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel at ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
ffmpeg-devel Info Page<http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
ffmpeg.org
This list is about FFmpeg development discussions and patches; but not for bug-reports. Please read the Code-of-conduct. To see the collection of prior postings to ...