| 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 | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
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 --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;
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(-)