diff mbox series

[FFmpeg-devel,06/21] avformat/mov: Fix memleaks when adding stream side-data fails

Message ID 20200322034756.29907-6-andreas.rheinhardt@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,01/21] avformat/nsvdec: Use av_packet_move_ref() for packet ownership transfer | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Andreas Rheinhardt March 22, 2020, 3:47 a.m. UTC 
By default, a demuxer's read_close function is not called automatically
if an error happens when reading the header; instead it is up to the
demuxer to clean up after itself in this case. The mov demuxer did this
by calling its read_close function when it encountered some errors when
reading the header. Yet for other errors (namely adding side-data to
streams) this has been forgotten, so that all the internal structures
of the demuxer leak.

This commit fixes this by setting the FF_INPUTFORMAT_HEADER_CLEANUP flag
so that mov_read_close() is automatically called when an error happens
when reading the header.

(Btw: mov_read_close() is not idempotent: Calling it twice is
dangerouos, because MOVContext.frag_index.item will be av_freep'ed,
yet MOVContext.frag_index.nb_items won't be reset. So the calls to
mov_read_close() have to be removed before the switch to freeing
generically.)

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/mov.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
diff mbox series

Patch

diff --git a/libavformat/mov.c b/libavformat/mov.c
index f280f360b6..61fdb45d9a 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -7494,13 +7494,11 @@  static int mov_read_header(AVFormatContext *s)
             avio_seek(pb, 0, SEEK_SET);
         if ((err = mov_read_default(mov, pb, atom)) < 0) {
             av_log(s, AV_LOG_ERROR, "error reading header\n");
-            mov_read_close(s);
             return err;
         }
     } while ((pb->seekable & AVIO_SEEKABLE_NORMAL) && !mov->found_moov && !mov->moov_retry++);
     if (!mov->found_moov) {
         av_log(s, AV_LOG_ERROR, "moov atom not found\n");
-        mov_read_close(s);
         return AVERROR_INVALIDDATA;
     }
     av_log(mov->fc, AV_LOG_TRACE, "on_parse_exit_offset=%"PRId64"\n", avio_tell(pb));
@@ -7574,7 +7572,6 @@  static int mov_read_header(AVFormatContext *s)
                 if (sc->data_size > INT64_MAX / sc->time_scale / 8) {
                     av_log(s, AV_LOG_ERROR, "Overflow during bit rate calculation %"PRId64" * 8 * %d\n",
                            sc->data_size, sc->time_scale);
-                    mov_read_close(s);
                     return AVERROR_INVALIDDATA;
                 }
                 st->codecpar->bit_rate = sc->data_size * 8 * sc->time_scale / st->duration;
@@ -7590,7 +7587,6 @@  static int mov_read_header(AVFormatContext *s)
                 if (sc->data_size > INT64_MAX / sc->time_scale / 8) {
                     av_log(s, AV_LOG_ERROR, "Overflow during bit rate calculation %"PRId64" * 8 * %d\n",
                            sc->data_size, sc->time_scale);
-                    mov_read_close(s);
                     return AVERROR_INVALIDDATA;
                 }
                 st->codecpar->bit_rate = sc->data_size * 8 * sc->time_scale /
@@ -7614,10 +7610,8 @@  static int mov_read_header(AVFormatContext *s)
         switch (st->codecpar->codec_type) {
         case AVMEDIA_TYPE_AUDIO:
             err = ff_replaygain_export(st, s->metadata);
-            if (err < 0) {
-                mov_read_close(s);
+            if (err < 0)
                 return err;
-            }
             break;
         case AVMEDIA_TYPE_VIDEO:
             if (sc->display_matrix) {
@@ -8098,4 +8092,5 @@  AVInputFormat ff_mov_demuxer = {
     .read_close     = mov_read_close,
     .read_seek      = mov_read_seek,
     .flags          = AVFMT_NO_BYTE_SEEK | AVFMT_SEEK_TO_PTS,
+    .flags_internal = FF_INPUTFORMAT_HEADER_CLEANUP,
 };