[FFmpeg-devel] [PATCH v2 1/5] avformat/matroskaenc: Ensure that ChapterUID are != 0

[Andreas Rheinhardt]

AVChapters have an int as id field and therefore this value can appear
<= 0. When remuxing from Matroska, this value is actually the lower 32
bits of the original ChapterUID (which can be 64 bits).

In order to ensure that the ChapterUID be always > 0, they were offset
as follows (since 07704c61): First max(0, 1LL - chapter[i].id) was computed
and stored in an uint32_t. And then the IDs were offset using this value.

This has two downsides:
1. It does not ensure that the UID is actually != 0: Namely if there is
a chapter with id == INT_MIN, then the offset will be 2^31 + 1 and a
chapter with id == INT_MAX will become 2^31 - 1 + 2^31 + 1 = 2^32 = 0,
because the actual calculation was performed in 32 bits.
2. As soon as a chapter id appears to be negative, a nontrivial offset
is used, so that not even a ChapterUID that only uses 32 bits is
preserved.

So change this by treating the id as an unsigned value internally and
only offset (by 1) if an id vanishes. The actual offsetting then has to
be performed in 64 bits in order to make sure that no UINT32_MAX wraps
around.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
 libavformat/matroskaenc.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 5c04ec98f6..1d3203c969 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -1500,7 +1500,8 @@ static int mkv_write_chapters(AVFormatContext *s)
         }
 
         chapteratom = start_ebml_master(dyn_cp, MATROSKA_ID_CHAPTERATOM, 0);
-        put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERUID, c->id + mkv->chapter_id_offset);
+        put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERUID,
+                      (uint32_t)c->id + (uint64_t)mkv->chapter_id_offset);
         put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERTIMESTART, chapterstart);
         put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERTIMEEND, chapterend);
         if (mkv->mode != MODE_WEBM) {
@@ -1557,7 +1558,7 @@ static int mkv_write_simpletag(AVIOContext *pb, AVDictionaryEntry *t)
 }
 
 static int mkv_write_tag_targets(AVFormatContext *s, uint32_t elementid,
-                                 unsigned int uid, ebml_master *tag)
+                                 uint64_t uid, ebml_master *tag)
 {
     AVIOContext *pb;
     MatroskaMuxContext *mkv = s->priv_data;
@@ -1595,7 +1596,7 @@ static int mkv_check_tag_name(const char *name, uint32_t elementid)
 }
 
 static int mkv_write_tag(AVFormatContext *s, AVDictionary *m, uint32_t elementid,
-                         unsigned int uid)
+                         uint64_t uid)
 {
     MatroskaMuxContext *mkv = s->priv_data;
     ebml_master tag;
@@ -1686,7 +1687,8 @@ static int mkv_write_tags(AVFormatContext *s)
             if (!mkv_check_tag(ch->metadata, MATROSKA_ID_TAGTARGETS_CHAPTERUID))
                 continue;
 
-            ret = mkv_write_tag(s, ch->metadata, MATROSKA_ID_TAGTARGETS_CHAPTERUID, ch->id + mkv->chapter_id_offset);
+            ret = mkv_write_tag(s, ch->metadata, MATROSKA_ID_TAGTARGETS_CHAPTERUID,
+                                (uint32_t)ch->id + (uint64_t)mkv->chapter_id_offset);
             if (ret < 0)
                 return ret;
         }
@@ -1961,7 +1963,10 @@ static int mkv_write_header(AVFormatContext *s)
         return ret;
 
     for (i = 0; i < s->nb_chapters; i++)
-        mkv->chapter_id_offset = FFMAX(mkv->chapter_id_offset, 1LL - s->chapters[i]->id);
+        if (!s->chapters[i]->id) {
+            mkv->chapter_id_offset = 1;
+            break;
+        }
 
     ret = mkv_write_chapters(s);
     if (ret < 0)
-- 
2.20.1