diff mbox series

[FFmpeg-devel,19/20] avformat/matroskaenc: Fix edge case of writing Cues at the beginning

Message ID 20200101005837.11356-20-andreas.rheinhardt@gmail.com
State Superseded
Headers show
Series Matroska muxer patches | expand

Checks

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

Commit Message

Andreas Rheinhardt Jan. 1, 2020, 12:58 a.m. UTC
The Matroska muxer has the ability to write the Cues (the index) at the
beginning of the file (in front of the Cluster): The user inputs the
amount of space that should be reserved at the beginning of the file and
if this is sufficient, the Cues will be written there and the part of the
reserved space not used up by the Cues will be filled with a "Void"
element.

There is just one problem with this: One can not fill a single byte this
way, because said Void element is minimally two bytes long (one byte ID,
one byte length field). Up until now, if one reserved one byte more than
needed, one would run into an assert when writing the Void element.

There are two solutions for this: Error out if it happens. Or adjust the
length field of the Cues in order to ensure that the above situation
can't happen (i.e. write the length on one byte more than necessary).
The first solution is very unsatisfactory, as enough space has been
reserved. Therefore this commit implements the second solution.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/matroskaenc.c | 42 +++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 15 deletions(-)
diff mbox series

Patch

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 4e739d245b..24317d3819 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -339,14 +339,15 @@  static int start_ebml_master_crc32(AVIOContext **dyn_cp, MatroskaMuxContext *mkv
 }
 
 static void end_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp,
-                                  MatroskaMuxContext *mkv, uint32_t id)
+                                  MatroskaMuxContext *mkv, uint32_t id,
+                                  int length_size)
 {
     uint8_t *buf, crc[4];
     int size, skip = 0;
 
     put_ebml_id(pb, id);
     size = avio_close_dyn_buf(*dyn_cp, &buf);
-    put_ebml_num(pb, size, 0);
+    put_ebml_num(pb, size, length_size);
     if (mkv->write_crc) {
         skip = 6; /* Skip reserved 6-byte long void element from the dynamic buffer. */
         AV_WL32(crc, av_crc(av_crc_get_table(AV_CRC_32_IEEE_LE), UINT32_MAX, buf + skip, size - skip) ^ UINT32_MAX);
@@ -467,7 +468,7 @@  static int mkv_write_seekhead(AVIOContext *pb, MatroskaMuxContext *mkv,
         put_ebml_uint(dyn_cp, MATROSKA_ID_SEEKPOSITION, entry->segmentpos);
         end_ebml_master(dyn_cp, seekentry);
     }
-    end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_SEEKHEAD);
+    end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_SEEKHEAD, 0);
 
     remaining = seekhead->filepos + seekhead->reserved_size - avio_tell(pb);
     put_ebml_void(pb, remaining);
@@ -1383,7 +1384,7 @@  static int mkv_write_tracks(AVFormatContext *s)
         end_ebml_master_crc32_preliminary(pb, mkv->tracks_bc,
                                           MATROSKA_ID_TRACKS, &mkv->tracks_pos);
     else
-        end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, MATROSKA_ID_TRACKS);
+        end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, MATROSKA_ID_TRACKS, 0);
 
     return 0;
 }
@@ -1439,7 +1440,7 @@  static int mkv_write_chapters(AVFormatContext *s)
         end_ebml_master(dyn_cp, chapteratom);
     }
     end_ebml_master(dyn_cp, editionentry);
-    end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_CHAPTERS);
+    end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_CHAPTERS, 0);
 
     mkv->wrote_chapters = 1;
     return 0;
@@ -1638,7 +1639,7 @@  static int mkv_write_tags(AVFormatContext *s)
             end_ebml_master_crc32_preliminary(s->pb, mkv->tags_bc,
                                               MATROSKA_ID_TAGS, &mkv->tags_pos);
         else
-            end_ebml_master_crc32(s->pb, &mkv->tags_bc, mkv, MATROSKA_ID_TAGS);
+            end_ebml_master_crc32(s->pb, &mkv->tags_bc, mkv, MATROSKA_ID_TAGS, 0);
     }
     return 0;
 }
@@ -1734,7 +1735,7 @@  static int mkv_write_attachments(AVFormatContext *s)
         mkv->attachments->entries[mkv->attachments->num_entries].stream_idx = i;
         mkv->attachments->entries[mkv->attachments->num_entries++].fileuid  = fileuid;
     }
-    end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_ATTACHMENTS);
+    end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_ATTACHMENTS, 0);
 
     return 0;
 }
@@ -1875,7 +1876,7 @@  static int mkv_write_header(AVFormatContext *s)
         end_ebml_master_crc32_preliminary(s->pb, mkv->info_bc,
                                           MATROSKA_ID_INFO, &mkv->info_pos);
     else
-        end_ebml_master_crc32(s->pb, &mkv->info_bc, mkv, MATROSKA_ID_INFO);
+        end_ebml_master_crc32(s->pb, &mkv->info_bc, mkv, MATROSKA_ID_INFO, 0);
     pb = s->pb;
 
     ret = mkv_write_tracks(s);
@@ -2157,7 +2158,7 @@  static void mkv_end_cluster(AVFormatContext *s)
 {
     MatroskaMuxContext *mkv = s->priv_data;
 
-    end_ebml_master_crc32(s->pb, &mkv->cluster_bc, mkv, MATROSKA_ID_CLUSTER);
+    end_ebml_master_crc32(s->pb, &mkv->cluster_bc, mkv, MATROSKA_ID_CLUSTER, 0);
     mkv->cluster_pos = -1;
     avio_flush(s->pb);
 }
@@ -2462,7 +2463,7 @@  static int mkv_write_trailer(AVFormatContext *s)
     }
 
     if (mkv->cluster_bc) {
-        end_ebml_master_crc32(pb, &mkv->cluster_bc, mkv, MATROSKA_ID_CLUSTER);
+        end_ebml_master_crc32(pb, &mkv->cluster_bc, mkv, MATROSKA_ID_CLUSTER, 0);
     }
 
     ret = mkv_write_chapters(s);
@@ -2476,6 +2477,7 @@  static int mkv_write_trailer(AVFormatContext *s)
         if (mkv->cues.num_entries) {
             AVIOContext *cues;
             uint64_t size;
+            int length_size = 0;
 
             ret = start_ebml_master_crc32(&cues, mkv);
             if (ret < 0)
@@ -2488,7 +2490,8 @@  static int mkv_write_trailer(AVFormatContext *s)
 
             if (mkv->reserve_cues_space) {
                 size  = avio_tell(cues);
-                size += 4 + ebml_num_size(size);
+                length_size = ebml_num_size(size);
+                size += 4 + length_size;
                 if (mkv->reserve_cues_space < size) {
                     av_log(s, AV_LOG_WARNING,
                            "Insufficient space reserved for Cues: %d < "
@@ -2501,10 +2504,19 @@  static int mkv_write_trailer(AVFormatContext *s)
                         ffio_free_dyn_buf(&cues);
                         return ret64;
                     }
+                    if (mkv->reserve_cues_space == size + 1) {
+                        /* There is no way to reserve a single byte because
+                         * the minimal size of an EBML Void element is 2
+                         * (1 byte ID, 1 byte length field). This problem
+                         * is solved by writing the Cues' length field on
+                         * one byte more than necessary. */
+                        length_size++;
+                        size++;
+                    }
                 }
             }
             mkv_add_seekhead_entry(mkv, MATROSKA_ID_CUES, avio_tell(pb));
-            end_ebml_master_crc32(pb, &cues, mkv, MATROSKA_ID_CUES);
+            end_ebml_master_crc32(pb, &cues, mkv, MATROSKA_ID_CUES, length_size);
             if (mkv->reserve_cues_space) {
                 if (size < mkv->reserve_cues_space)
                     put_ebml_void(pb, mkv->reserve_cues_space - size);
@@ -2522,11 +2534,11 @@  static int mkv_write_trailer(AVFormatContext *s)
         av_log(s, AV_LOG_DEBUG, "end duration = %" PRIu64 "\n", mkv->duration);
         avio_seek(mkv->info_bc, mkv->duration_offset, SEEK_SET);
         put_ebml_float(mkv->info_bc, MATROSKA_ID_DURATION, mkv->duration);
-        end_ebml_master_crc32(pb, &mkv->info_bc, mkv, MATROSKA_ID_INFO);
+        end_ebml_master_crc32(pb, &mkv->info_bc, mkv, MATROSKA_ID_INFO, 0);
 
         // write tracks master
         avio_seek(pb, mkv->tracks_pos, SEEK_SET);
-        end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, MATROSKA_ID_TRACKS);
+        end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, MATROSKA_ID_TRACKS, 0);
 
         // update stream durations
         if (!mkv->is_live) {
@@ -2556,7 +2568,7 @@  static int mkv_write_trailer(AVFormatContext *s)
         }
         if (mkv->tags_bc && !mkv->is_live) {
             avio_seek(pb, mkv->tags_pos, SEEK_SET);
-            end_ebml_master_crc32(pb, &mkv->tags_bc, mkv, MATROSKA_ID_TAGS);
+            end_ebml_master_crc32(pb, &mkv->tags_bc, mkv, MATROSKA_ID_TAGS, 0);
         }
 
         avio_seek(pb, currentpos, SEEK_SET);