| Message ID | cf370a39-1405-1296-0c22-fb05cc8d2f19@googlemail.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Fri, Dec 09, 2016 at 11:42:59PM +0100, Andreas Cadhalpun wrote: > On 09.12.2016 15:23, Michael Niedermayer wrote: > > On Fri, Dec 09, 2016 at 12:08:10AM +0100, Andreas Cadhalpun wrote: > >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > >> --- > >> libavcodec/opus_parser.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/libavcodec/opus_parser.c b/libavcodec/opus_parser.c > >> index c30fd7b..21a73ee 100644 > >> --- a/libavcodec/opus_parser.c > >> +++ b/libavcodec/opus_parser.c > >> @@ -116,11 +116,11 @@ static int opus_find_frame_end(AVCodecParserContext *ctx, AVCodecContext *avctx, > >> > >> if (avctx->extradata && !s->extradata_parsed) { > >> ret = ff_opus_parse_extradata(avctx, &s->ctx); > >> + av_freep(&s->ctx.channel_maps); > >> if (ret < 0) { > >> av_log(avctx, AV_LOG_ERROR, "Error parsing Ogg extradata.\n"); > >> return AVERROR_INVALIDDATA; > >> } > >> - av_freep(&s->ctx.channel_maps); > >> s->extradata_parsed = 1; > >> } > > > > isnt it more correct for ff_opus_parse_extradata() to cleanup what > > it allocated on error ? > > Probably, but opusdec already did it the other way around. > I'm fine with changing that, though. See attached patch. > > Best regards, > Andreas > > opus.c | 1 + > opusdec.c | 1 - > 2 files changed, 1 insertion(+), 1 deletion(-) > 3e534ddbb39e05f8c7a5cfeba4b65ed587c3a519 0001-opus_parser-fix-leaking-channel_maps-on-error.patch > From 2edf5f9a571d483858928dd6bb9e84031ef8a391 Mon Sep 17 00:00:00 2001 > From: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > Date: Fri, 9 Dec 2016 00:00:18 +0100 > Subject: [PATCH 1/2] opus_parser: fix leaking channel_maps on error > > Make ff_opus_parse_extradata free allocated memory on error instead of > expecting callers to free it in that case. > > Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > --- > libavcodec/opus.c | 1 + > libavcodec/opusdec.c | 1 - > 2 files changed, 1 insertion(+), 1 deletion(-) LGTM thx [...]
On 10.12.2016 04:19, Michael Niedermayer wrote: > On Fri, Dec 09, 2016 at 11:42:59PM +0100, Andreas Cadhalpun wrote: >> opus.c | 1 + >> opusdec.c | 1 - >> 2 files changed, 1 insertion(+), 1 deletion(-) >> 3e534ddbb39e05f8c7a5cfeba4b65ed587c3a519 0001-opus_parser-fix-leaking-channel_maps-on-error.patch >> From 2edf5f9a571d483858928dd6bb9e84031ef8a391 Mon Sep 17 00:00:00 2001 >> From: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> >> Date: Fri, 9 Dec 2016 00:00:18 +0100 >> Subject: [PATCH 1/2] opus_parser: fix leaking channel_maps on error >> >> Make ff_opus_parse_extradata free allocated memory on error instead of >> expecting callers to free it in that case. >> >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> >> --- >> libavcodec/opus.c | 1 + >> libavcodec/opusdec.c | 1 - >> 2 files changed, 1 insertion(+), 1 deletion(-) > > LGTM Pushed. Best regards, Andreas
From 2edf5f9a571d483858928dd6bb9e84031ef8a391 Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> Date: Fri, 9 Dec 2016 00:00:18 +0100 Subject: [PATCH 1/2] opus_parser: fix leaking channel_maps on error Make ff_opus_parse_extradata free allocated memory on error instead of expecting callers to free it in that case. Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> --- libavcodec/opus.c | 1 + libavcodec/opusdec.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/opus.c b/libavcodec/opus.c index 08d94b6..1eeb92c 100644 --- a/libavcodec/opus.c +++ b/libavcodec/opus.c @@ -403,6 +403,7 @@ av_cold int ff_opus_parse_extradata(AVCodecContext *avctx, } else if (idx >= streams + stereo_streams) { av_log(avctx, AV_LOG_ERROR, "Invalid channel map for output channel %d: %d\n", i, idx); + av_freep(&s->channel_maps); return AVERROR_INVALIDDATA; } diff --git a/libavcodec/opusdec.c b/libavcodec/opusdec.c index ec793c6..329f784 100644 --- a/libavcodec/opusdec.c +++ b/libavcodec/opusdec.c @@ -646,7 +646,6 @@ static av_cold int opus_decode_init(AVCodecContext *avctx) /* find out the channel configuration */ ret = ff_opus_parse_extradata(avctx, c); if (ret < 0) { - av_freep(&c->channel_maps); av_freep(&c->fdsp); return ret; } -- 2.10.2