diff mbox series

[FFmpeg-devel,01/26] avformat/matroskadec: Move AVBufferRef instead of copying, fix memleak

Message ID 20200614223656.21338-1-andreas.rheinhardt@gmail.com
State Accepted
Commit cbe336c9e81e2d9de3a18abef887c9255a9b9da5
Headers show
Series [FFmpeg-devel,01/26] avformat/matroskadec: Move AVBufferRef instead of copying, fix memleak
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt June 14, 2020, 10:36 p.m. UTC
EBML binary elements are already made reference-counted when read;
so when populating the AVStream.attached_pic, one does not need to
allocate a new buffer for the data; instead the current code just
creates a new reference to the underlying AVBuffer. But this can be
improved even further: Just move the already existing reference.

This also fixes a memleak that happens upon error because
matroska_read_close has not been called in this scenario.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
This fixes one of the memleaks that I found during the work on my patchset
[1] that intended to automatically call a demuxer's read_close function
if reading the header fails. Several other of the patches of this
patchset also fix such memleaks.

I intend to apply this patchset soon so that 4.3 can finally be
released.

[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-March/258831.html

 libavformat/matroskadec.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Andreas Rheinhardt June 15, 2020, 1:40 p.m. UTC | #1
Andreas Rheinhardt:
> EBML binary elements are already made reference-counted when read;
> so when populating the AVStream.attached_pic, one does not need to
> allocate a new buffer for the data; instead the current code just
> creates a new reference to the underlying AVBuffer. But this can be
> improved even further: Just move the already existing reference.
> 
> This also fixes a memleak that happens upon error because
> matroska_read_close has not been called in this scenario.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> This fixes one of the memleaks that I found during the work on my patchset
> [1] that intended to automatically call a demuxer's read_close function
> if reading the header fails. Several other of the patches of this
> patchset also fix such memleaks.
> 
> I intend to apply this patchset soon so that 4.3 can finally be
> released.
> 
> [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-March/258831.html
> 
>  libavformat/matroskadec.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index bb3a126c29..b1da40983f 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -2938,9 +2938,8 @@ static int matroska_read_header(AVFormatContext *s)
>                  st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
>  
>                  av_init_packet(pkt);
> -                pkt->buf = av_buffer_ref(attachments[j].bin.buf);
> -                if (!pkt->buf)
> -                    return AVERROR(ENOMEM);
> +                pkt->buf          = attachments[j].bin.buf;
> +                attachments[j].bin.buf = NULL;
>                  pkt->data         = attachments[j].bin.data;
>                  pkt->size         = attachments[j].bin.size;
>                  pkt->stream_index = st->index;
> 
Will apply 1-26.

- Andreas
diff mbox series

Patch

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index bb3a126c29..b1da40983f 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -2938,9 +2938,8 @@  static int matroska_read_header(AVFormatContext *s)
                 st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
 
                 av_init_packet(pkt);
-                pkt->buf = av_buffer_ref(attachments[j].bin.buf);
-                if (!pkt->buf)
-                    return AVERROR(ENOMEM);
+                pkt->buf          = attachments[j].bin.buf;
+                attachments[j].bin.buf = NULL;
                 pkt->data         = attachments[j].bin.data;
                 pkt->size         = attachments[j].bin.size;
                 pkt->stream_index = st->index;