diff mbox series

[FFmpeg-devel] avformat/utils: Fix memleak when decoding subtitle in find_stream_info

Message ID 20200418195426.25674-1-andreas.rheinhardt@gmail.com
State Accepted
Commit d026fef999f7e2491721b3878bea6e6da8b9f92e
Headers show
Series [FFmpeg-devel] avformat/utils: Fix memleak when decoding subtitle in find_stream_info | expand

Checks

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

Commit Message

Andreas Rheinhardt April 18, 2020, 7:54 p.m. UTC
avformat_find_stream_info() may decode some frames to get stream
information. And when it does this for subtitles, the decoded subtitles
leak.

(Decoding subtitles was added in b1511e00f6fefde6cb31b2e17f7812cfac1c8bd6
for PGS subtitles. When PGS subtitles originate from a container that
exports every segment as a packet of its own, no output will be
generated when decoding a packet, because not enough input is available.
Yet when used with PGS subtitles in the Matroska form a single packet
contains enough data to generate output. Yet said output is not freed,
hence this leak.)

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
As soon as the new pgs_frame_merge bsf gets merged, we can add a test
that remuxes the sub/pgs_sub.sup file from the fate suite to Matroska
and reads/decodes the resulting file. Such a test would have caught
this.

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

Comments

Anton Khirnov April 20, 2020, 1:01 p.m. UTC | #1
Quoting Andreas Rheinhardt (2020-04-18 21:54:26)
> avformat_find_stream_info() may decode some frames to get stream
> information. And when it does this for subtitles, the decoded subtitles
> leak.
> 
> (Decoding subtitles was added in b1511e00f6fefde6cb31b2e17f7812cfac1c8bd6
> for PGS subtitles. When PGS subtitles originate from a container that
> exports every segment as a packet of its own, no output will be
> generated when decoding a packet, because not enough input is available.
> Yet when used with PGS subtitles in the Matroska form a single packet
> contains enough data to generate output. Yet said output is not freed,
> hence this leak.)
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---

Looks good.
Andreas Rheinhardt April 20, 2020, 4:37 p.m. UTC | #2
Anton Khirnov:
> Quoting Andreas Rheinhardt (2020-04-18 21:54:26)
>> avformat_find_stream_info() may decode some frames to get stream
>> information. And when it does this for subtitles, the decoded subtitles
>> leak.
>>
>> (Decoding subtitles was added in b1511e00f6fefde6cb31b2e17f7812cfac1c8bd6
>> for PGS subtitles. When PGS subtitles originate from a container that
>> exports every segment as a packet of its own, no output will be
>> generated when decoding a packet, because not enough input is available.
>> Yet when used with PGS subtitles in the Matroska form a single packet
>> contains enough data to generate output. Yet said output is not freed,
>> hence this leak.)
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
>> ---
> 
> Looks good.
> 
Applied, thanks.

- Andreas
diff mbox series

Patch

diff --git a/libavformat/utils.c b/libavformat/utils.c
index a58e47fabc..6b72c657d8 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -3125,6 +3125,8 @@  static int try_decode_frame(AVFormatContext *s, AVStream *st,
         } else if (avctx->codec_type == AVMEDIA_TYPE_SUBTITLE) {
             ret = avcodec_decode_subtitle2(avctx, &subtitle,
                                            &got_picture, &pkt);
+            if (got_picture)
+                avsubtitle_free(&subtitle);
             if (ret >= 0)
                 pkt.size = 0;
         }