diff mbox series

[FFmpeg-devel,1/2] avformat/matroskaenc: Check chapter ids for duplicates

Message ID 20210316082953.273550-1-andreas.rheinhardt@gmail.com
State Accepted
Commit e1e6a5c8a515afa488faee3200666ce0f14ea5eb
Headers show
Series [FFmpeg-devel,1/2] avformat/matroskaenc: Check chapter ids for duplicates | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt March 16, 2021, 8:29 a.m. UTC
Up until now, there has been no check that each chapter has a unique id;
there was only a check for whether a chapter id is zero (this happens
often when the chapters originated from a format that lacks the concept
of chapter id and simply counts from zero) which is invalid in Matroska.
In this case the chapter ids are offset by 1 to make them nonnegative.
Yet offsetting won't fix duplicate ids, therefore this is changed to
simply create new chapter uids when the input chapter uids don't conform
to the requirements of Matroska (in which case it can be presumed that
they did not originate from Matroska, so that we don't need to bother
to preserve them).

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

Patch

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 1749b7fd37..8f29d64e72 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -1625,24 +1625,29 @@  static int mkv_write_tags(AVFormatContext *s)
     return 0;
 }
 
+static int mkv_new_chapter_ids_needed(const AVFormatContext *s)
+{
+    for (unsigned i = 0; i < s->nb_chapters; i++) {
+        if (!s->chapters[i]->id)
+            return 1;
+        for (unsigned j = 0; j < i; j++)
+            if (s->chapters[j]->id == s->chapters[i]->id)
+                return 1;
+    }
+    return 0;
+}
+
 static int mkv_write_chapters(AVFormatContext *s)
 {
     MatroskaMuxContext *mkv = s->priv_data;
     AVIOContext *dyn_cp = NULL, *dyn_tags = NULL, **tags, *pb = s->pb;
     ebml_master editionentry;
-    uint64_t chapter_id_offset = 0;
     AVRational scale = {1, 1E9};
-    int i, ret;
+    int ret, create_new_ids;
 
     if (!s->nb_chapters || mkv->wrote_chapters)
         return 0;
 
-    for (i = 0; i < s->nb_chapters; i++)
-        if (!s->chapters[i]->id) {
-            chapter_id_offset = 1;
-            break;
-        }
-
     ret = start_ebml_master_crc32(&dyn_cp, mkv);
     if (ret < 0)
         return ret;
@@ -1656,12 +1661,15 @@  static int mkv_write_chapters(AVFormatContext *s)
     } else
         tags = NULL;
 
-    for (i = 0; i < s->nb_chapters; i++) {
+    create_new_ids = mkv_new_chapter_ids_needed(s);
+
+    for (unsigned i = 0; i < s->nb_chapters; i++) {
         ebml_master chapteratom, chapterdisplay;
         const AVChapter *c   = s->chapters[i];
         int64_t chapterstart = av_rescale_q(c->start, c->time_base, scale);
         int64_t chapterend   = av_rescale_q(c->end,   c->time_base, scale);
         const AVDictionaryEntry *t;
+        uint64_t uid = create_new_ids ? i + 1ULL : (uint32_t)c->id;
         if (chapterstart < 0 || chapterstart > chapterend || chapterend < 0) {
             av_log(s, AV_LOG_ERROR,
                    "Invalid chapter start (%"PRId64") or end (%"PRId64").\n",
@@ -1671,8 +1679,7 @@  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,
-                      (uint32_t)c->id + chapter_id_offset);
+        put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERUID, uid);
         put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERTIMESTART, chapterstart);
         put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERTIMEEND, chapterend);
         if ((t = av_dict_get(c->metadata, "title", NULL, 0))) {
@@ -1685,8 +1692,7 @@  static int mkv_write_chapters(AVFormatContext *s)
 
         if (tags && mkv_check_tag(c->metadata, MATROSKA_ID_TAGTARGETS_CHAPTERUID)) {
             ret = mkv_write_tag(mkv, c->metadata, tags, NULL,
-                                MATROSKA_ID_TAGTARGETS_CHAPTERUID,
-                                (uint32_t)c->id + chapter_id_offset);
+                                MATROSKA_ID_TAGTARGETS_CHAPTERUID, uid);
             if (ret < 0)
                 goto fail;
         }