On 23.11.2014 11:42, Michael Niedermayer wrote:
On Sun, Nov 23, 2014 at 12:58:30AM +0100, Lukasz Marek wrote:
Signed-off-by: Lukasz Marek <lukasz.m.lu...@gmail.com>
---
libavcodec/huffyuvdec.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/libavcodec/huffyuvdec.c b/libavcodec/huffyuvdec.c
index 3b2b0f7..5535323 100644
--- a/libavcodec/huffyuvdec.c
+++ b/libavcodec/huffyuvdec.c
@@ -275,7 +275,7 @@ static int read_old_huffman_tables(HYuvContext *s)
static av_cold int decode_init(AVCodecContext *avctx)
{
HYuvContext *s = avctx->priv_data;
- int ret;
+ int ret, i;
ff_huffyuvdsp_init(&s->hdsp);
memset(s->vlc, 0, 4 * sizeof(VLC));
@@ -327,7 +327,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
if ((ret = read_huffman_tables(s, avctx->extradata + 4,
avctx->extradata_size - 4)) < 0)
- return ret;
+ goto error;
} else {
switch (avctx->bits_per_coded_sample & 7) {
case 1:
@@ -355,7 +355,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
s->context = 0;
if ((ret = read_old_huffman_tables(s)) < 0)
- return ret;
+ goto error;
}
if (s->version <= 2) {
@@ -383,7 +383,8 @@ static av_cold int decode_init(AVCodecContext *avctx)
s->alpha = 1;
break;
default:
- return AVERROR_INVALIDDATA;
+ ret = AVERROR_INVALIDDATA;
+ goto error;
}
av_pix_fmt_get_chroma_sub_sample(avctx->pix_fmt,
&s->chroma_h_shift,
@@ -520,7 +521,8 @@ static av_cold int decode_init(AVCodecContext *avctx)
avctx->pix_fmt = AV_PIX_FMT_YUVA420P16;
break;
default:
- return AVERROR_INVALIDDATA;
+ ret = AVERROR_INVALIDDATA;
+ goto error;
}
}
@@ -528,21 +530,27 @@ static av_cold int decode_init(AVCodecContext *avctx)
if ((avctx->pix_fmt == AV_PIX_FMT_YUV422P || avctx->pix_fmt == AV_PIX_FMT_YUV420P)
&& avctx->width & 1) {
av_log(avctx, AV_LOG_ERROR, "width must be even for this
colorspace\n");
- return AVERROR_INVALIDDATA;
+ ret = AVERROR_INVALIDDATA;
+ goto error;
}
if (s->predictor == MEDIAN && avctx->pix_fmt == AV_PIX_FMT_YUV422P &&
avctx->width % 4) {
av_log(avctx, AV_LOG_ERROR, "width must be a multiple of 4 "
"for this combination of colorspace and predictor type.\n");
- return AVERROR_INVALIDDATA;
+ ret = AVERROR_INVALIDDATA;
+ goto error;
}
if ((ret = ff_huffyuv_alloc_temp(s)) < 0) {
ff_huffyuv_common_end(s);
- return ret;
+ goto error;
}
return 0;
+ error:
+ for (i = 0; i < 8; i++)
+ ff_free_vlc(&s->vlc[i]);
i think calling decode_end() is better than duplicating what it does
if it works
I have no opinion about that, but changed. I had to move decode_end above.
I could add prototype but from these 2 I prefer move it as it is not long.
Perfectly decode end callback could be called in avcodec_open2 on fail
but I'm not sure every codec under any circumstances is secure to call
it twice.
>From 4b4ea1105b54165b654e738bd27bb76cdc7e0db1 Mon Sep 17 00:00:00 2001
From: Lukasz Marek <lukasz.m.lu...@gmail.com>
Date: Sun, 23 Nov 2014 00:57:33 +0100
Subject: [PATCH 1/2] lavc/huffyuvdec: fix mem leak in case of init failure
Signed-off-by: Lukasz Marek <lukasz.m.lu...@gmail.com>
---
libavcodec/huffyuvdec.c | 49 ++++++++++++++++++++++++++++---------------------
1 file changed, 28 insertions(+), 21 deletions(-)
diff --git a/libavcodec/huffyuvdec.c b/libavcodec/huffyuvdec.c
index 3b2b0f7..62ed0f9 100644
--- a/libavcodec/huffyuvdec.c
+++ b/libavcodec/huffyuvdec.c
@@ -272,6 +272,20 @@ static int read_old_huffman_tables(HYuvContext *s)
return 0;
}
+static av_cold int decode_end(AVCodecContext *avctx)
+{
+ HYuvContext *s = avctx->priv_data;
+ int i;
+
+ ff_huffyuv_common_end(s);
+ av_freep(&s->bitstream_buffer);
+
+ for (i = 0; i < 8; i++)
+ ff_free_vlc(&s->vlc[i]);
+
+ return 0;
+}
+
static av_cold int decode_init(AVCodecContext *avctx)
{
HYuvContext *s = avctx->priv_data;
@@ -327,7 +341,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
if ((ret = read_huffman_tables(s, avctx->extradata + 4,
avctx->extradata_size - 4)) < 0)
- return ret;
+ goto error;
} else {
switch (avctx->bits_per_coded_sample & 7) {
case 1:
@@ -355,7 +369,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
s->context = 0;
if ((ret = read_old_huffman_tables(s)) < 0)
- return ret;
+ goto error;
}
if (s->version <= 2) {
@@ -383,7 +397,8 @@ static av_cold int decode_init(AVCodecContext *avctx)
s->alpha = 1;
break;
default:
- return AVERROR_INVALIDDATA;
+ ret = AVERROR_INVALIDDATA;
+ goto error;
}
av_pix_fmt_get_chroma_sub_sample(avctx->pix_fmt,
&s->chroma_h_shift,
@@ -520,7 +535,8 @@ static av_cold int decode_init(AVCodecContext *avctx)
avctx->pix_fmt = AV_PIX_FMT_YUVA420P16;
break;
default:
- return AVERROR_INVALIDDATA;
+ ret = AVERROR_INVALIDDATA;
+ goto error;
}
}
@@ -528,21 +544,26 @@ static av_cold int decode_init(AVCodecContext *avctx)
if ((avctx->pix_fmt == AV_PIX_FMT_YUV422P || avctx->pix_fmt == AV_PIX_FMT_YUV420P) && avctx->width & 1) {
av_log(avctx, AV_LOG_ERROR, "width must be even for this colorspace\n");
- return AVERROR_INVALIDDATA;
+ ret = AVERROR_INVALIDDATA;
+ goto error;
}
if (s->predictor == MEDIAN && avctx->pix_fmt == AV_PIX_FMT_YUV422P &&
avctx->width % 4) {
av_log(avctx, AV_LOG_ERROR, "width must be a multiple of 4 "
"for this combination of colorspace and predictor type.\n");
- return AVERROR_INVALIDDATA;
+ ret = AVERROR_INVALIDDATA;
+ goto error;
}
if ((ret = ff_huffyuv_alloc_temp(s)) < 0) {
ff_huffyuv_common_end(s);
- return ret;
+ goto error;
}
return 0;
+ error:
+ decode_end(avctx);
+ return ret;
}
static av_cold int decode_init_thread_copy(AVCodecContext *avctx)
@@ -1209,20 +1230,6 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
return (get_bits_count(&s->gb) + 31) / 32 * 4 + table_size;
}
-static av_cold int decode_end(AVCodecContext *avctx)
-{
- HYuvContext *s = avctx->priv_data;
- int i;
-
- ff_huffyuv_common_end(s);
- av_freep(&s->bitstream_buffer);
-
- for (i = 0; i < 8; i++)
- ff_free_vlc(&s->vlc[i]);
-
- return 0;
-}
-
AVCodec ff_huffyuv_decoder = {
.name = "huffyuv",
.long_name = NULL_IF_CONFIG_SMALL("Huffyuv / HuffYUV"),
--
1.9.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
