| Message ID | 20201026101150.425151-1-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Commit | c72a29df6b4ec2858575bd8b6c3874784209c7b2 |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avcodec/cllc: Don't unnecessarily free VLC | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
Andreas Rheinhardt: > The Canopus Lossless decoder uses several VLCs and if initializing the > ith VLC fails, all the VLCs 0..i have been freed; the ith VLC's table is > initialized to NULL for this purpose. Yet it is totally unnecessary to > free the ith VLC table at all: ff_init_vlc_sparse() cleans up after > itself on error and if an error happens before ff_init_vlc_sparse(), > the ith VLC hasn't been touched yet and doesn't need freeing. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavcodec/cllc.c | 11 +++-------- > 1 file changed, 3 insertions(+), 8 deletions(-) > > diff --git a/libavcodec/cllc.c b/libavcodec/cllc.c > index 1f2c98ef73..8b1bc75faa 100644 > --- a/libavcodec/cllc.c > +++ b/libavcodec/cllc.c > @@ -57,8 +57,6 @@ static int read_code_table(CLLCContext *ctx, GetBitContext *gb, VLC *vlc) > num_lens = get_bits(gb, 5); > > if (num_lens > VLC_BITS * VLC_DEPTH) { > - vlc->table = NULL; > - > av_log(ctx->avctx, AV_LOG_ERROR, "To long VLCs %d\n", num_lens); > return AVERROR_INVALIDDATA; > } > @@ -68,8 +66,6 @@ static int read_code_table(CLLCContext *ctx, GetBitContext *gb, VLC *vlc) > num_codes_sum += num_codes; > > if (num_codes_sum > 256) { > - vlc->table = NULL; > - > av_log(ctx->avctx, AV_LOG_ERROR, > "Too many VLCs (%d) to be read.\n", num_codes_sum); > return AVERROR_INVALIDDATA; > @@ -83,7 +79,6 @@ static int read_code_table(CLLCContext *ctx, GetBitContext *gb, VLC *vlc) > count++; > } > if (prefix > (65535 - 256)/2) { > - vlc->table = NULL; > return AVERROR_INVALIDDATA; > } > > @@ -247,7 +242,7 @@ static int decode_argb_frame(CLLCContext *ctx, GetBitContext *gb, AVFrame *pic) > for (i = 0; i < 4; i++) { > ret = read_code_table(ctx, gb, &vlc[i]); > if (ret < 0) { > - for (j = 0; j <= i; j++) > + for (j = 0; j < i; j++) > ff_free_vlc(&vlc[j]); > > av_log(ctx->avctx, AV_LOG_ERROR, > @@ -290,7 +285,7 @@ static int decode_rgb24_frame(CLLCContext *ctx, GetBitContext *gb, AVFrame *pic) > for (i = 0; i < 3; i++) { > ret = read_code_table(ctx, gb, &vlc[i]); > if (ret < 0) { > - for (j = 0; j <= i; j++) > + for (j = 0; j < i; j++) > ff_free_vlc(&vlc[j]); > > av_log(ctx->avctx, AV_LOG_ERROR, > @@ -343,7 +338,7 @@ static int decode_yuv_frame(CLLCContext *ctx, GetBitContext *gb, AVFrame *pic) > for (i = 0; i < 2; i++) { > ret = read_code_table(ctx, gb, &vlc[i]); > if (ret < 0) { > - for (j = 0; j <= i; j++) > + for (j = 0; j < i; j++) > ff_free_vlc(&vlc[j]); > > av_log(ctx->avctx, AV_LOG_ERROR, > Will apply the rest of this patchset tomorrow unless there are objections. - Andreas
diff --git a/libavcodec/cllc.c b/libavcodec/cllc.c index 1f2c98ef73..8b1bc75faa 100644 --- a/libavcodec/cllc.c +++ b/libavcodec/cllc.c @@ -57,8 +57,6 @@ static int read_code_table(CLLCContext *ctx, GetBitContext *gb, VLC *vlc) num_lens = get_bits(gb, 5); if (num_lens > VLC_BITS * VLC_DEPTH) { - vlc->table = NULL; - av_log(ctx->avctx, AV_LOG_ERROR, "To long VLCs %d\n", num_lens); return AVERROR_INVALIDDATA; } @@ -68,8 +66,6 @@ static int read_code_table(CLLCContext *ctx, GetBitContext *gb, VLC *vlc) num_codes_sum += num_codes; if (num_codes_sum > 256) { - vlc->table = NULL; - av_log(ctx->avctx, AV_LOG_ERROR, "Too many VLCs (%d) to be read.\n", num_codes_sum); return AVERROR_INVALIDDATA; @@ -83,7 +79,6 @@ static int read_code_table(CLLCContext *ctx, GetBitContext *gb, VLC *vlc) count++; } if (prefix > (65535 - 256)/2) { - vlc->table = NULL; return AVERROR_INVALIDDATA; } @@ -247,7 +242,7 @@ static int decode_argb_frame(CLLCContext *ctx, GetBitContext *gb, AVFrame *pic) for (i = 0; i < 4; i++) { ret = read_code_table(ctx, gb, &vlc[i]); if (ret < 0) { - for (j = 0; j <= i; j++) + for (j = 0; j < i; j++) ff_free_vlc(&vlc[j]); av_log(ctx->avctx, AV_LOG_ERROR, @@ -290,7 +285,7 @@ static int decode_rgb24_frame(CLLCContext *ctx, GetBitContext *gb, AVFrame *pic) for (i = 0; i < 3; i++) { ret = read_code_table(ctx, gb, &vlc[i]); if (ret < 0) { - for (j = 0; j <= i; j++) + for (j = 0; j < i; j++) ff_free_vlc(&vlc[j]); av_log(ctx->avctx, AV_LOG_ERROR, @@ -343,7 +338,7 @@ static int decode_yuv_frame(CLLCContext *ctx, GetBitContext *gb, AVFrame *pic) for (i = 0; i < 2; i++) { ret = read_code_table(ctx, gb, &vlc[i]); if (ret < 0) { - for (j = 0; j <= i; j++) + for (j = 0; j < i; j++) ff_free_vlc(&vlc[j]); av_log(ctx->avctx, AV_LOG_ERROR,
The Canopus Lossless decoder uses several VLCs and if initializing the ith VLC fails, all the VLCs 0..i have been freed; the ith VLC's table is initialized to NULL for this purpose. Yet it is totally unnecessary to free the ith VLC table at all: ff_init_vlc_sparse() cleans up after itself on error and if an error happens before ff_init_vlc_sparse(), the ith VLC hasn't been touched yet and doesn't need freeing. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavcodec/cllc.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)