diff mbox series

[FFmpeg-devel,18/20] avformat/matroskaenc: Don't fail if reserved Cues space doesn't suffice

Message ID 20200101005837.11356-19-andreas.rheinhardt@gmail.com
State Superseded
Headers show
Series Matroska muxer patches
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Andreas Rheinhardt Jan. 1, 2020, 12:58 a.m. UTC
When the user opted to write the Cues at the beginning, the Cues were
simply written without checking in advance whether enough space has been
reserved for them. If it wasn't enough, the data following the space
reserved for the Cues was simply overwritten, corrupting the file.

This commit changes this by checking whether enough space has been
reserved for the Cues before outputting anything. If it isn't enough,
the Cues will be output at the end as usual.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 doc/muxers.texi           |  4 +-
 libavformat/matroskaenc.c | 79 ++++++++++++++++++++-------------------
 2 files changed, 43 insertions(+), 40 deletions(-)
diff mbox series

Patch

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 5d7ff1ab3b..9eca32c1ca 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -1314,8 +1314,8 @@  index at the beginning of the file.
 
 If this option is set to a non-zero value, the muxer will reserve a given amount
 of space in the file header and then try to write the cues there when the muxing
-finishes. If the available space does not suffice, muxing will fail. A safe size
-for most use cases should be about 50kB per hour of video.
+finishes. If the reserved space does not suffice, the Cues will be written at
+the end. A safe size for most use cases should be about 50kB per hour of video.
 
 Note that cues are only written if the output is seekable and this option will
 have no effect if it is not.
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index d7aa832eb3..4e739d245b 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -503,23 +503,15 @@  static int mkv_add_cuepoint(MatroskaMuxContext *mkv, int stream, int tracknum, i
     return 0;
 }
 
-static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track *tracks, int num_tracks)
+static int mkv_assemble_cues(AVStream **streams, AVIOContext *dyn_cp,
+                             mkv_cues *cues, mkv_track *tracks, int num_tracks)
 {
-    MatroskaMuxContext *mkv = s->priv_data;
-    AVIOContext *dyn_cp, *pb = s->pb, *cuepoint;
-    int64_t currentpos;
+    AVIOContext *cuepoint;
     int ret;
 
-    currentpos = avio_tell(pb);
-    ret = start_ebml_master_crc32(&dyn_cp, mkv);
-    if (ret < 0)
-        return ret;
-
     ret = avio_open_dyn_buf(&cuepoint);
-    if (ret < 0) {
-        ffio_free_dyn_buf(&dyn_cp);
+    if (ret < 0)
         return ret;
-    }
 
     for (mkv_cuepoint *entry = cues->entries, *end = entry + cues->num_entries;
          entry < end;) {
@@ -538,7 +530,7 @@  static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track *tra
             int idx = entry->stream_idx;
 
             av_assert0(idx >= 0 && idx < num_tracks);
-            if (tracks[idx].has_cue && s->streams[idx]->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE)
+            if (tracks[idx].has_cue && streams[idx]->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE)
                 continue;
             tracks[idx].has_cue = 1;
             track_positions = start_ebml_master(cuepoint, MATROSKA_ID_CUETRACKPOSITION, MAX_CUETRACKPOS_SIZE);
@@ -554,9 +546,8 @@  static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track *tra
         ffio_reset_dyn_buf(cuepoint);
     }
     ffio_free_dyn_buf(&cuepoint);
-    end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_CUES);
 
-    return currentpos;
+    return 0;
 }
 
 static int put_xiph_codecpriv(AVFormatContext *s, AVIOContext *pb, AVCodecParameters *par)
@@ -2457,7 +2448,7 @@  static int mkv_write_trailer(AVFormatContext *s)
 {
     MatroskaMuxContext *mkv = s->priv_data;
     AVIOContext *pb = s->pb;
-    int64_t currentpos, cuespos;
+    int64_t currentpos;
     int ret;
 
     // check if we have an audio packet cached
@@ -2480,33 +2471,45 @@  static int mkv_write_trailer(AVFormatContext *s)
 
 
     if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && !mkv->is_live) {
+        int64_t ret64;
+
         if (mkv->cues.num_entries) {
-            if (mkv->reserve_cues_space) {
-                int64_t cues_end;
-
-                currentpos = avio_tell(pb);
-                avio_seek(pb, mkv->cues_pos, SEEK_SET);
-
-                cuespos  = mkv_write_cues(s, &mkv->cues, mkv->tracks, s->nb_streams);
-                cues_end = avio_tell(pb);
-                if (cues_end > cuespos + mkv->reserve_cues_space) {
-                    av_log(s, AV_LOG_ERROR,
-                           "Insufficient space reserved for cues: %d "
-                           "(needed: %" PRId64 ").\n",
-                           mkv->reserve_cues_space, cues_end - cuespos);
-                    return AVERROR(EINVAL);
-                }
+            AVIOContext *cues;
+            uint64_t size;
 
-                if (cues_end < cuespos + mkv->reserve_cues_space)
-                    put_ebml_void(pb, mkv->reserve_cues_space -
-                                  (cues_end - cuespos));
+            ret = start_ebml_master_crc32(&cues, mkv);
+            if (ret < 0)
+                return ret;
 
+            ret = mkv_assemble_cues(s->streams, cues, &mkv->cues,
+                                    mkv->tracks, s->nb_streams);
+            if (ret < 0)
+                return ret;
+
+            if (mkv->reserve_cues_space) {
+                size  = avio_tell(cues);
+                size += 4 + ebml_num_size(size);
+                if (mkv->reserve_cues_space < size) {
+                    av_log(s, AV_LOG_WARNING,
+                           "Insufficient space reserved for Cues: %d < "
+                           "%"PRIu64". Writing Cues at the end of the file.\n",
+                           mkv->reserve_cues_space, size);
+                    mkv->reserve_cues_space = 0;
+                } else {
+                    currentpos = avio_tell(pb);
+                    if ((ret64 = avio_seek(pb, mkv->cues_pos, SEEK_SET)) < 0) {
+                        ffio_free_dyn_buf(&cues);
+                        return ret64;
+                    }
+                }
+            }
+            mkv_add_seekhead_entry(mkv, MATROSKA_ID_CUES, avio_tell(pb));
+            end_ebml_master_crc32(pb, &cues, mkv, MATROSKA_ID_CUES);
+            if (mkv->reserve_cues_space) {
+                if (size < mkv->reserve_cues_space)
+                    put_ebml_void(pb, mkv->reserve_cues_space - size);
                 avio_seek(pb, currentpos, SEEK_SET);
-            } else {
-                cuespos = mkv_write_cues(s, &mkv->cues, mkv->tracks, s->nb_streams);
             }
-
-            mkv_add_seekhead_entry(mkv, MATROSKA_ID_CUES, cuespos);
         }
 
         currentpos = avio_tell(pb);