diff mbox series

[FFmpeg-devel] avcodec/proresenc_kostya: Remove harmful check

Message ID HE1PR0301MB21545219D027AE7524A601348F759@HE1PR0301MB2154.eurprd03.prod.outlook.com
State Accepted
Commit 7c109cb92381c4895a95cf706527c0d0ff656ee7
Headers show
Series [FFmpeg-devel] avcodec/proresenc_kostya: Remove harmful check
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt April 7, 2021, 9:15 p.m. UTC
The ProRes encoder allocates huge worst-case buffers just to be safe;
and for huge resolutions (8k in this case) these can be so big that the
number of bits does no longer fit into a (signed 32-bit) int; this means
that one must no longer use the parts of the PutBits API that deal with
bit counters. Yet proresenc_kostya did it, namely for a check about
whether we are already beyond the end. Yet this check is unnecessary
nowadays, because the PutBits API comes with automatic checks (with
a log message and a av_assert2() in put_bits() and an av_assert0() in
flush_put_bits()), so this is unnecessary. So simply remove the check.

Fixes ticket #9173.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
I actually pondered removing said check when I wrote
f9d1528fc900dac4975ce785dd95004daeacec39 (which stopped using
intermediate bitcounts).

 libavcodec/proresenc_kostya.c | 5 -----
 1 file changed, 5 deletions(-)

Comments

Andreas Rheinhardt April 9, 2021, 12:06 p.m. UTC | #1
Andreas Rheinhardt:
> The ProRes encoder allocates huge worst-case buffers just to be safe;
> and for huge resolutions (8k in this case) these can be so big that the
> number of bits does no longer fit into a (signed 32-bit) int; this means
> that one must no longer use the parts of the PutBits API that deal with
> bit counters. Yet proresenc_kostya did it, namely for a check about
> whether we are already beyond the end. Yet this check is unnecessary
> nowadays, because the PutBits API comes with automatic checks (with
> a log message and a av_assert2() in put_bits() and an av_assert0() in
> flush_put_bits()), so this is unnecessary. So simply remove the check.
> 
> Fixes ticket #9173.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> I actually pondered removing said check when I wrote
> f9d1528fc900dac4975ce785dd95004daeacec39 (which stopped using
> intermediate bitcounts).
> 
>  libavcodec/proresenc_kostya.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/libavcodec/proresenc_kostya.c b/libavcodec/proresenc_kostya.c
> index d8edd12f34..54fc6707a1 100644
> --- a/libavcodec/proresenc_kostya.c
> +++ b/libavcodec/proresenc_kostya.c
> @@ -619,11 +619,6 @@ static int encode_slice(AVCodecContext *avctx, const AVFrame *pic,
>          flush_put_bits(pb);
>          sizes[i]   = put_bytes_output(pb) - total_size;
>          total_size = put_bytes_output(pb);
> -        if (put_bits_left(pb) < 0) {
> -            av_log(avctx, AV_LOG_ERROR,
> -                   "Underestimated required buffer size.\n");
> -            return AVERROR_BUG;
> -        }
>      }
>      return total_size;
>  }
> 
Will apply tomorrow unless there are objections.

- Andreas
diff mbox series

Patch

diff --git a/libavcodec/proresenc_kostya.c b/libavcodec/proresenc_kostya.c
index d8edd12f34..54fc6707a1 100644
--- a/libavcodec/proresenc_kostya.c
+++ b/libavcodec/proresenc_kostya.c
@@ -619,11 +619,6 @@  static int encode_slice(AVCodecContext *avctx, const AVFrame *pic,
         flush_put_bits(pb);
         sizes[i]   = put_bytes_output(pb) - total_size;
         total_size = put_bytes_output(pb);
-        if (put_bits_left(pb) < 0) {
-            av_log(avctx, AV_LOG_ERROR,
-                   "Underestimated required buffer size.\n");
-            return AVERROR_BUG;
-        }
     }
     return total_size;
 }