| Message ID | 20191019141108.1434-1-jamrial@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Sat, Oct 19, 2019 at 11:11:07AM -0300, James Almer wrote: > Despite the doxy stating that it's called when the muxer is destroyed, > this was not true in practice. It's only called by av_write_trailer() > and on init() failure. > > An AVFormatContext may be closed without writing the trailer if errors > ocurred while muxing packets, so in order to prevent memory leaks, it > should effectively be called when freeing the muxer. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavformat/mux.c | 4 +++- > libavformat/utils.c | 3 +++ > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/libavformat/mux.c b/libavformat/mux.c > index 0227c0dadc..719dec8346 100644 > --- a/libavformat/mux.c > +++ b/libavformat/mux.c > @@ -538,6 +538,8 @@ int avformat_write_header(AVFormatContext *s, AVDictionary **options) > fail: > if (s->oformat->deinit) > s->oformat->deinit(s); > + s->internal->initialized = > + s->internal->streams_initialized = 0; > return ret; > } > > @@ -1286,7 +1288,7 @@ fail: > } > } > > - if (s->oformat->deinit) > + if (s->oformat->deinit && s->internal->initialized) > s->oformat->deinit(s); > > s->internal->initialized = > diff --git a/libavformat/utils.c b/libavformat/utils.c > index 60f0229adc..cfb6d03397 100644 > --- a/libavformat/utils.c > +++ b/libavformat/utils.c > @@ -4437,6 +4437,9 @@ void avformat_free_context(AVFormatContext *s) > if (!s) > return; > > + if (s->oformat && s->oformat->deinit && s->internal->initialized) > + s->oformat->deinit(s); > + maybe this whole set of checks and clears could be factored in a function ? something like: deinit() { if (s->oformat && s->oformat->deinit && s->internal->initialized) s->oformat->deinit(s); s->internal->initialized = s->internal->streams_initialized = 0; } [...]
diff --git a/libavformat/mux.c b/libavformat/mux.c index 0227c0dadc..719dec8346 100644 --- a/libavformat/mux.c +++ b/libavformat/mux.c @@ -538,6 +538,8 @@ int avformat_write_header(AVFormatContext *s, AVDictionary **options) fail: if (s->oformat->deinit) s->oformat->deinit(s); + s->internal->initialized = + s->internal->streams_initialized = 0; return ret; } @@ -1286,7 +1288,7 @@ fail: } } - if (s->oformat->deinit) + if (s->oformat->deinit && s->internal->initialized) s->oformat->deinit(s); s->internal->initialized = diff --git a/libavformat/utils.c b/libavformat/utils.c index 60f0229adc..cfb6d03397 100644 --- a/libavformat/utils.c +++ b/libavformat/utils.c @@ -4437,6 +4437,9 @@ void avformat_free_context(AVFormatContext *s) if (!s) return; + if (s->oformat && s->oformat->deinit && s->internal->initialized) + s->oformat->deinit(s); + av_opt_free(s); if (s->iformat && s->iformat->priv_class && s->priv_data) av_opt_free(s->priv_data);
Despite the doxy stating that it's called when the muxer is destroyed, this was not true in practice. It's only called by av_write_trailer() and on init() failure. An AVFormatContext may be closed without writing the trailer if errors ocurred while muxing packets, so in order to prevent memory leaks, it should effectively be called when freeing the muxer. Signed-off-by: James Almer <jamrial@gmail.com> --- libavformat/mux.c | 4 +++- libavformat/utils.c | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-)