diff mbox series

[FFmpeg-devel] avformat/flacenc: Fix memleak when writing attached pictures fails

Message ID 20201120205044.834944-1-andreas.rheinhardt@gmail.com
State Accepted
Commit 029cf6a91ceb849f31f575111070a113c53b29ee
Headers show
Series [FFmpeg-devel] avformat/flacenc: Fix memleak when writing attached pictures fails
Related show

Checks

Context Check Description
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt Nov. 20, 2020, 8:50 p.m. UTC
The FLAC muxer currently stores an attached picture corresponding to an
AVStream in AVStream.priv_data. The AVPacket contained therein is
unreferenced after it has been written. The AVPacket structure itself is
then freed generically as AVStream.priv_data.

And this can lead to memleaks if an attached picture is not written:
It might be because the trailer is never written or because writing
a previous attached picture failed in case error_recognition is set
to explode.

Therefore free the packets properly (i.e. with av_packet_free())
in the muxer's deinit function.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
I intend to make muxers use AVStream.attached_pic in the medium term
[1]; then this code can be removed again. I am just sending this now in
order not to leave a memleak open and to have something easy to
backport.

[1]: https://github.com/mkver/FFmpeg/commit/f4513d613b3abf427d042687ba3ae1d2120f3fa3

 libavformat/flacenc.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Andreas Rheinhardt Nov. 25, 2020, 1:27 a.m. UTC | #1
Andreas Rheinhardt:
> The FLAC muxer currently stores an attached picture corresponding to an
> AVStream in AVStream.priv_data. The AVPacket contained therein is
> unreferenced after it has been written. The AVPacket structure itself is
> then freed generically as AVStream.priv_data.
> 
> And this can lead to memleaks if an attached picture is not written:
> It might be because the trailer is never written or because writing
> a previous attached picture failed in case error_recognition is set
> to explode.
> 
> Therefore free the packets properly (i.e. with av_packet_free())
> in the muxer's deinit function.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> I intend to make muxers use AVStream.attached_pic in the medium term
> [1]; then this code can be removed again. I am just sending this now in
> order not to leave a memleak open and to have something easy to
> backport.
> 
> [1]: https://github.com/mkver/FFmpeg/commit/f4513d613b3abf427d042687ba3ae1d2120f3fa3
> 
>  libavformat/flacenc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c
> index 6b8ce8d7ee..1c983486aa 100644
> --- a/libavformat/flacenc.c
> +++ b/libavformat/flacenc.c
> @@ -347,6 +347,8 @@ static void flac_deinit(struct AVFormatContext *s)
>      FlacMuxerContext *c = s->priv_data;
>  
>      avpriv_packet_list_free(&c->queue, &c->queue_end);
> +    for (unsigned i = 0; i < s->nb_streams; i++)
> +        av_packet_free((AVPacket **)&s->streams[i]->priv_data);
>  }
>  
>  static int flac_write_packet(struct AVFormatContext *s, AVPacket *pkt)
> 
Will apply tomorrow unless there are objections.

- Andreas
diff mbox series

Patch

diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c
index 6b8ce8d7ee..1c983486aa 100644
--- a/libavformat/flacenc.c
+++ b/libavformat/flacenc.c
@@ -347,6 +347,8 @@  static void flac_deinit(struct AVFormatContext *s)
     FlacMuxerContext *c = s->priv_data;
 
     avpriv_packet_list_free(&c->queue, &c->queue_end);
+    for (unsigned i = 0; i < s->nb_streams; i++)
+        av_packet_free((AVPacket **)&s->streams[i]->priv_data);
 }
 
 static int flac_write_packet(struct AVFormatContext *s, AVPacket *pkt)