| 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 | expand |
| Context | Check | Description |
|---|---|---|
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
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 --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)
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(+)