On 15.11.2016 03:18, Michael Niedermayer wrote:
> On Sun, Nov 13, 2016 at 11:25:32PM +0100, Andreas Cadhalpun wrote:
>> Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
>> ---
>> libavcodec/libschroedingerdec.c | 26 +++++++++++++++++---------
>> 1 file changed, 17 insertions(+), 9 deletions(-)
>>
>> diff --git a/libavcodec/libschroedingerdec.c
>> b/libavcodec/libschroedingerdec.c
>> index 1e392b3..83c790c 100644
>> --- a/libavcodec/libschroedingerdec.c
>> +++ b/libavcodec/libschroedingerdec.c
>> @@ -218,6 +218,7 @@ static int libschroedinger_decode_frame(AVCodecContext
>> *avctx,
>> int outer = 1;
>> SchroParseUnitContext parse_ctx;
>> LibSchroFrameContext *framewithpts = NULL;
>> + int ret;
>>
>> *got_frame = 0;
>>
>> @@ -236,7 +237,8 @@ static int libschroedinger_decode_frame(AVCodecContext
>> *avctx,
>> enc_buf->tag = schro_tag_new(av_malloc(sizeof(int64_t)),
>> av_free);
>> if (!enc_buf->tag->value) {
>> av_log(avctx, AV_LOG_ERROR, "Unable to allocate
>> SchroTag\n");
>> - return AVERROR(ENOMEM);
>> + ret = AVERROR(ENOMEM);
>> + goto end;
>> }
>> AV_WN(64, enc_buf->tag->value, pts);
>> /* Push buffer into decoder. */
>> @@ -267,8 +269,10 @@ static int libschroedinger_decode_frame(AVCodecContext
>> *avctx,
>> /* Decoder needs a frame - create one and push it in. */
>> frame = ff_create_schro_frame(avctx,
>> p_schro_params->frame_format);
>> - if (!frame)
>> - return AVERROR(ENOMEM);
>> + if (!frame) {
>> + ret = AVERROR(ENOMEM);
>> + goto end;
>> + }
>> schro_decoder_add_output_picture(decoder, frame);
>> break;
>>
>
> this looks a bit strange
> framewithpts is set to newly allocated memory below which is injected
> into the que and IIUC that can occur multiple times
> the free at the end for one of multiple such que entries feels wrong
Indeed, only the framewithpts returned from ff_schro_queue_pop needs to
be freed. New patch is attached.
However, considering the sheer amount of crashes in libschroedinger and
that it's apparently not maintained anymore, it might be better to
simply remove this decoder.
Best regards,
Andreas
>From 9cf7226543e25e494bed768b73f39e67d89f25d1 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
Date: Sun, 13 Nov 2016 23:10:06 +0100
Subject: [PATCH] libschroedingerdec: fix leaking of framewithpts
Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
---
libavcodec/libschroedingerdec.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/libavcodec/libschroedingerdec.c b/libavcodec/libschroedingerdec.c
index 1e392b3..02cbe57 100644
--- a/libavcodec/libschroedingerdec.c
+++ b/libavcodec/libschroedingerdec.c
@@ -218,6 +218,7 @@ static int libschroedinger_decode_frame(AVCodecContext *avctx,
int outer = 1;
SchroParseUnitContext parse_ctx;
LibSchroFrameContext *framewithpts = NULL;
+ int ret;
*got_frame = 0;
@@ -308,10 +309,9 @@ static int libschroedinger_decode_frame(AVCodecContext *avctx,
framewithpts = ff_schro_queue_pop(&p_schro_params->dec_frame_queue);
if (framewithpts && framewithpts->frame && framewithpts->frame->components[0].stride) {
- int ret;
if ((ret = ff_get_buffer(avctx, avframe, 0)) < 0)
- return ret;
+ goto end;
memcpy(avframe->data[0],
framewithpts->frame->components[0].data,
@@ -337,15 +337,17 @@ FF_ENABLE_DEPRECATION_WARNINGS
avframe->linesize[2] = framewithpts->frame->components[2].stride;
*got_frame = 1;
-
- /* Now free the frame resources. */
- libschroedinger_decode_frame_free(framewithpts->frame);
- av_free(framewithpts);
} else {
data = NULL;
*got_frame = 0;
}
- return buf_size;
+ ret = buf_size;
+end:
+ /* Now free the frame resources. */
+ if (framewithpts && framewithpts->frame)
+ libschroedinger_decode_frame_free(framewithpts->frame);
+ av_freep(&framewithpts);
+ return ret;
}
--
2.10.2
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
