diff mbox series

[FFmpeg-devel] avcodec/dstdec: Replace AC overread check by sample rate check

Message ID 20200701200324.4936-1-michael@niedermayer.cc
State Accepted
Headers show
Series [FFmpeg-devel] avcodec/dstdec: Replace AC overread check by sample rate check | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Michael Niedermayer July 1, 2020, 8:03 p.m. UTC
Real files do skip coding 0 bits at the end, thus this kind of check
does not work reliable.

Fixes: Ticket 8770
Fixes: dst-256fs44-6ch-refdstencoder.dff

The samplerate is specified in ISO/IEC 14496-3:2005(E) as one of 3 fixed
values, this also can be used to limit the duration and avoid the timeout

This reverts commit f6df99dba1ae64b05d08fba8160d13eb9795042f.
---
 libavcodec/dstdec.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

Comments

Paul B Mahol July 1, 2020, 8:57 p.m. UTC | #1
NAK

There is DSD 512 variant

On 7/1/20, Michael Niedermayer <michael@niedermayer.cc> wrote:
> Real files do skip coding 0 bits at the end, thus this kind of check
> does not work reliable.
>
> Fixes: Ticket 8770
> Fixes: dst-256fs44-6ch-refdstencoder.dff
>
> The samplerate is specified in ISO/IEC 14496-3:2005(E) as one of 3 fixed
> values, this also can be used to limit the duration and avoid the timeout
>
> This reverts commit f6df99dba1ae64b05d08fba8160d13eb9795042f.
> ---
>  libavcodec/dstdec.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/libavcodec/dstdec.c b/libavcodec/dstdec.c
> index 771887faf9..f0926d6c09 100644
> --- a/libavcodec/dstdec.c
> +++ b/libavcodec/dstdec.c
> @@ -56,7 +56,6 @@ static const int8_t probs_code_pred_coeff[3][3] = {
>  typedef struct ArithCoder {
>      unsigned int a;
>      unsigned int c;
> -    int overread;
>  } ArithCoder;
>
>  typedef struct Table {
> @@ -86,6 +85,12 @@ static av_cold int decode_init(AVCodecContext *avctx)
>          return AVERROR_PATCHWELCOME;
>      }
>
> +    // the sample rate is only allowed to be 64,128,256 * 44100 by ISO/IEC
> 14496-3:2005(E)
> +    // We are a bit more tolerant here, but this check is needed to bound
> the size and duration
> +    if (avctx->sample_rate > 256 * 44100)
> +        return AVERROR_INVALIDDATA;
> +
> +
>      if (DST_SAMPLES_PER_FRAME(avctx->sample_rate) & 7) {
>          return AVERROR_PATCHWELCOME;
>      }
> @@ -181,7 +186,6 @@ static void ac_init(ArithCoder *ac, GetBitContext *gb)
>  {
>      ac->a = 4095;
>      ac->c = get_bits(gb, 12);
> -    ac->overread = 0;
>  }
>
>  static av_always_inline void ac_get(ArithCoder *ac, GetBitContext *gb, int
> p, int *e)
> @@ -201,8 +205,6 @@ static av_always_inline void ac_get(ArithCoder *ac,
> GetBitContext *gb, int p, in
>      if (ac->a < 2048) {
>          int n = 11 - av_log2(ac->a);
>          ac->a <<= n;
> -        if (get_bits_left(gb) < n)
> -            ac->overread ++;
>          ac->c = (ac->c << n) | get_bits(gb, n);
>      }
>  }
> @@ -355,9 +357,6 @@ static int decode_frame(AVCodecContext *avctx, void
> *data,
>                  prob = 128;
>              }
>
> -            if (ac->overread > 16)
> -                return AVERROR_INVALIDDATA;
> -
>              ac_get(ac, gb, prob, &residual);
>              v = ((predict >> 15) ^ residual) & 1;
>              dsd[((i >> 3) * channels + ch) << 2] |= v << (7 - (i & 0x7 ));
> --
> 2.17.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Michael Niedermayer July 2, 2020, 8:41 a.m. UTC | #2
On Wed, Jul 01, 2020 at 10:57:53PM +0200, Paul B Mahol wrote:
> NAK
> 
> There is DSD 512 variant

will apply with the limit raised to 44100 * 512 then

thanks

[...]
diff mbox series

Patch

diff --git a/libavcodec/dstdec.c b/libavcodec/dstdec.c
index 771887faf9..f0926d6c09 100644
--- a/libavcodec/dstdec.c
+++ b/libavcodec/dstdec.c
@@ -56,7 +56,6 @@  static const int8_t probs_code_pred_coeff[3][3] = {
 typedef struct ArithCoder {
     unsigned int a;
     unsigned int c;
-    int overread;
 } ArithCoder;
 
 typedef struct Table {
@@ -86,6 +85,12 @@  static av_cold int decode_init(AVCodecContext *avctx)
         return AVERROR_PATCHWELCOME;
     }
 
+    // the sample rate is only allowed to be 64,128,256 * 44100 by ISO/IEC 14496-3:2005(E)
+    // We are a bit more tolerant here, but this check is needed to bound the size and duration
+    if (avctx->sample_rate > 256 * 44100)
+        return AVERROR_INVALIDDATA;
+
+
     if (DST_SAMPLES_PER_FRAME(avctx->sample_rate) & 7) {
         return AVERROR_PATCHWELCOME;
     }
@@ -181,7 +186,6 @@  static void ac_init(ArithCoder *ac, GetBitContext *gb)
 {
     ac->a = 4095;
     ac->c = get_bits(gb, 12);
-    ac->overread = 0;
 }
 
 static av_always_inline void ac_get(ArithCoder *ac, GetBitContext *gb, int p, int *e)
@@ -201,8 +205,6 @@  static av_always_inline void ac_get(ArithCoder *ac, GetBitContext *gb, int p, in
     if (ac->a < 2048) {
         int n = 11 - av_log2(ac->a);
         ac->a <<= n;
-        if (get_bits_left(gb) < n)
-            ac->overread ++;
         ac->c = (ac->c << n) | get_bits(gb, n);
     }
 }
@@ -355,9 +357,6 @@  static int decode_frame(AVCodecContext *avctx, void *data,
                 prob = 128;
             }
 
-            if (ac->overread > 16)
-                return AVERROR_INVALIDDATA;
-
             ac_get(ac, gb, prob, &residual);
             v = ((predict >> 15) ^ residual) & 1;
             dsd[((i >> 3) * channels + ch) << 2] |= v << (7 - (i & 0x7 ));