avformat/matroskaenc: Don't fail if reserved Cues space doesn't suffice

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, no Cues will be output at all and the file will be finalized normally, yet writing the trailer will nevertheless return an error to notify the user that his wish of having Cues at the front of the file hasn't been fulfilled. This change opens new usecases for this option: It is now safe to use this option to e.g. record live streams or to use it when muxing the output of an expensive encoding, because when the reserved space turns out to be insufficient, one ends up with a file that just lacks Cues but is otherwise fine.

Andreas Rheinhardt committed 6 years ago

Commit 06f10891 parent 8e1354c2 files · +50 −41Browse files

Unified · Split

2 files

doc/muxers.texi +3 −2
libavformat/matroskaenc.c +47 −39

doc/muxers.texi+3−2

@@ -1352,8 +1352,9 @@ index at the beginning of the file. · 1351 unmodified lines
1352		1352
1353	If this option is set to a non-zero value, the muxer will reserve a given amount	1353	If this option is set to a non-zero value, the muxer will reserve a given amount
1354	of space in the file header and then try to write the cues there when the muxing	1354	of space in the file header and then try to write the cues there when the muxing
1355	finishes. If the available space does not suffice, muxing will fail. A safe size	1355	finishes. If the reserved space does not suffice, no Cues will be written, the
1356	for most use cases should be about 50kB per hour of video.	1356	file will be finalized and writing the trailer will return an error.
		1357	A safe size for most use cases should be about 50kB per hour of video.
1357		1358
1358	Note that cues are only written if the output is seekable and this option will	1359	Note that cues are only written if the output is seekable and this option will
1359	have no effect if it is not.	1360	have no effect if it is not.

libavformat/matroskaenc.c+47−39

     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 *cuepoint;
-    AVIOContext *dyn_cp, *pb = s->pb, *cuepoint;
-    int64_t currentpos;
     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) {
+    if (ret < 0)
-        ffio_free_dyn_buf(&dyn_cp);
         return ret;
-    }
     for (mkv_cuepoint *entry = cues->entries, *end = entry + cues->num_entries;
          entry < end;) {
             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);
         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)
 {
     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
     if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && !mkv->is_live) {
+        int64_t ret64;
         if (mkv->cues.num_entries) {
-            if (mkv->reserve_cues_space) {
+            AVIOContext *cues;
-                int64_t cues_end;
+            uint64_t size;
-                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);
-                }
-                if (cues_end < cuespos + mkv->reserve_cues_space)
+            ret = start_ebml_master_crc32(&cues, mkv);
-                    put_ebml_void(pb, mkv->reserve_cues_space -
+            if (ret < 0)
-                                  (cues_end - cuespos));
+                return ret;
-                avio_seek(pb, currentpos, SEEK_SET);
+            ret = mkv_assemble_cues(s->streams, cues, &mkv->cues,
-            } else {
+                                    mkv->tracks, s->nb_streams);
-                cuespos = mkv_write_cues(s, &mkv->cues, mkv->tracks, s->nb_streams);
+            if (ret < 0) {
+                ffio_free_dyn_buf(&cues);
+                return ret;
             }
-            mkv_add_seekhead_entry(mkv, MATROSKA_ID_CUES, cuespos);
+            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". No Cues will be output.\n",
+                           mkv->reserve_cues_space, size);
+                    mkv->reserve_cues_space = -1;
+                    ffio_free_dyn_buf(&cues);
+                    goto after_cues;
+                } 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);
+            }
         }
+    after_cues:
         currentpos = avio_tell(pb);
         ret = mkv_write_seekhead(pb, mkv, 1, mkv->info_pos);
         end_ebml_master(pb, mkv->segment);
     }
-    return 0;
+    return mkv->reserve_cues_space < 0 ? AVERROR(EINVAL) : 0;
 }
 static int mkv_query_codec(enum AVCodecID codec_id, int std_compliance)

avformat/matroskaenc: Don't fail if reserved Cues space doesn't suffice

Keyboard shortcuts

General

Issues and pull requests

Issues, pull requests and Inbox

Repository code