On Wed, 16 Nov 2016 13:21:34 +0100
Michael Niedermayer <mich...@niedermayer.cc> wrote:
> On Tue, Nov 15, 2016 at 09:56:16PM +0100, Andreas Cadhalpun wrote:
> > 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.
>
> id say decoders which crash should be marked as
> AV_CODEC_CAP_EXPERIMENTAL
Experimental implies it's early work in progress which will stabilize
later.
libschroedinger is completely dead, and it's entirely possible that
you're the person who cares most about it on this planet.
Should we introduce a AV_CODEC_CAP_TRASH flag?
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
