PR #20971 opened by xobust
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20971
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20971.patch

This lays the groundwork for ordered-chapter playback in Matroska, fixing 
ticket [#3123](https://trac.ffmpeg.org/ticket/3123).

To stay compatible with the existing logic by fetching the chapters from all 
editions thereby flattening the structure.
I think It would be a good idea to add an option that only fetches the default 
or first edition staying compliant to the matroska spec.
Maybe adding --edition X and --default-edition in a future patch is appropriate.

Next steps would be dealing with external chapter files.

This is my first pr let me know if I can improve anything, my C is a little 
rusty. 
I manually added the [video from the 
ticket](https://archive.org/details/chapters_test) as a fate test just to make 
sure it was read correctly.
Should it be added to the test suite? 



>From 2d760b92bea9d8125f6e2150c28fa9d5b4d3dff8 Mon Sep 17 00:00:00 2001
From: Alexander Westberg-Bladh <[email protected]>
Date: Tue, 18 Nov 2025 11:42:21 +0100
Subject: [PATCH 1/3] avformat/matroska: Add element IDs for ordered chapters

Add MATROSKA_ID_CHAPTERSEGMENTUID and MATROSKA_ID_CHAPTERSEGMENTEDITIONUID
element IDs to support parsing of ordered chapters segment linking.

Signed-off-by: Alexander Westberg-Bladh <[email protected]>
---
 libavformat/matroska.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavformat/matroska.h b/libavformat/matroska.h
index 719f2ef796..9118dc216c 100644
--- a/libavformat/matroska.h
+++ b/libavformat/matroska.h
@@ -274,6 +274,8 @@
 #define MATROSKA_ID_CHAPTERFLAGHIDDEN   0x98
 #define MATROSKA_ID_CHAPTERFLAGENABLED  0x4598
 #define MATROSKA_ID_CHAPTERPHYSEQUIV    0x63C3
+#define MATROSKA_ID_CHAPTERSEGMENTUID          0x6E67
+#define MATROSKA_ID_CHAPTERSEGMENTEDITIONUID   0x6EBC
 
 typedef enum {
   MATROSKA_TRACK_TYPE_NONE     = 0x0,
-- 
2.49.1


>From b9803b4c966859e3492199802689a41d63d564b1 Mon Sep 17 00:00:00 2001
From: Alexander Westberg-Bladh <[email protected]>
Date: Tue, 18 Nov 2025 13:38:42 +0100
Subject: [PATCH 2/3] avformat/matroskadec: Add data structures for ordered
 chapters

Add MatroskaEdition structure and extend MatroskaChapter and 
MatroskaDemuxContext
with fields needed for ordered chapters support.

Signed-off-by: Alexander Westberg-Bladh <[email protected]>
---
 libavformat/matroskadec.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 8b5eda8213..0b120bb43e 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -307,11 +307,21 @@ typedef struct MatroskaAttachment {
     AVStream *stream;
 } MatroskaAttachment;
 
+typedef struct MatroskaEdition {
+    uint64_t uid;
+    uint64_t flag_hidden;
+    uint64_t flag_default;
+    uint64_t flag_ordered;
+    EbmlList chapters;
+} MatroskaEdition;
+
 typedef struct MatroskaChapter {
     uint64_t start;
     uint64_t end;
     uint64_t uid;
     char    *title;
+    EbmlBin  segment_uid;
+    uint64_t segment_edition_uid;
 
     AVChapter *chapter;
 } MatroskaChapter;
@@ -438,6 +448,10 @@ typedef struct MatroskaDemuxContext {
 
     /* Bandwidth value for WebM DASH Manifest */
     int bandwidth;
+
+    /* Editions and ordered chapters support */
+    EbmlList editions;
+    EbmlBin  segment_uid;
 } MatroskaDemuxContext;
 
 #define CHILD_OF(parent) { .def = { .n = parent } }
-- 
2.49.1


>From 97e6eca06390c54c5ee189f567a19468cecf16fc Mon Sep 17 00:00:00 2001
From: Alexander Westberg-Bladh <[email protected]>
Date: Tue, 18 Nov 2025 15:26:00 +0100
Subject: [PATCH 3/3] avformat/matroskadec: Update EBML syntax to parse
 EditionEntry

Update matroska_chapters[] syntax to parse EditionEntry elements into
matroska->editions instead of the flat matroska->chapters list.
update chapter finding code to look through all editions.

Signed-off-by: Alexander Westberg-Bladh <[email protected]>
---
 libavformat/matroskadec.c | 85 ++++++++++++++++++++++-----------------
 1 file changed, 48 insertions(+), 37 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 0b120bb43e..c771f87a06 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -463,7 +463,7 @@ typedef struct MatroskaDemuxContext {
 static EbmlSyntax ebml_syntax[3], matroska_segment[9], 
matroska_track_video_color[15], matroska_track_video[19],
                   matroska_track[33], matroska_track_encoding[6], 
matroska_track_encodings[2],
                   matroska_track_combine_planes[2], 
matroska_track_operation[2], matroska_block_addition_mapping[5], 
matroska_tracks[2],
-                  matroska_attachments[2], matroska_chapter_entry[9], 
matroska_chapter[6], matroska_chapters[2],
+                  matroska_attachments[2], matroska_chapter_entry[10], 
matroska_edition[6], matroska_chapter[6], matroska_chapters[2],
                   matroska_index_entry[3], matroska_index[2], matroska_tag[3], 
matroska_tags[2], matroska_seekhead[2],
                   matroska_blockadditions[2], matroska_blockgroup[8], 
matroska_cluster_parsing[8];
 
@@ -489,9 +489,9 @@ static EbmlSyntax matroska_info[] = {
     { MATROSKA_ID_DURATION,      EBML_FLOAT, 0, 0, 
offsetof(MatroskaDemuxContext, duration) },
     { MATROSKA_ID_TITLE,         EBML_UTF8,  0, 0, 
offsetof(MatroskaDemuxContext, title) },
     { MATROSKA_ID_WRITINGAPP,    EBML_NONE },
-    { MATROSKA_ID_MUXINGAPP,     EBML_UTF8, 0, 0, 
offsetof(MatroskaDemuxContext, muxingapp) },
-    { MATROSKA_ID_DATEUTC,       EBML_BIN,  0, 0, 
offsetof(MatroskaDemuxContext, date_utc) },
-    { MATROSKA_ID_SEGMENTUID,    EBML_NONE },
+    { MATROSKA_ID_MUXINGAPP,     EBML_UTF8,  0, 0, 
offsetof(MatroskaDemuxContext, muxingapp) },
+    { MATROSKA_ID_DATEUTC,       EBML_BIN,   0, 0, 
offsetof(MatroskaDemuxContext, date_utc) },
+    { MATROSKA_ID_SEGMENTUID,    EBML_BIN,   0, 0, 
offsetof(MatroskaDemuxContext, segment_uid) },
     CHILD_OF(matroska_segment)
 };
 
@@ -683,28 +683,29 @@ static EbmlSyntax matroska_chapter_display[] = {
 };
 
 static EbmlSyntax matroska_chapter_entry[] = {
-    { MATROSKA_ID_CHAPTERTIMESTART,   EBML_UINT, 0, 0, 
offsetof(MatroskaChapter, start), { .u = AV_NOPTS_VALUE } },
-    { MATROSKA_ID_CHAPTERTIMEEND,     EBML_UINT, 0, 0, 
offsetof(MatroskaChapter, end),   { .u = AV_NOPTS_VALUE } },
-    { MATROSKA_ID_CHAPTERUID,         EBML_UINT, 0, 0, 
offsetof(MatroskaChapter, uid) },
-    { MATROSKA_ID_CHAPTERDISPLAY,     EBML_NEST, 0, 0,                        
0,         { .n = matroska_chapter_display } },
-    { MATROSKA_ID_CHAPTERFLAGHIDDEN,  EBML_NONE },
-    { MATROSKA_ID_CHAPTERFLAGENABLED, EBML_NONE },
-    { MATROSKA_ID_CHAPTERPHYSEQUIV,   EBML_NONE },
-    { MATROSKA_ID_CHAPTERATOM,        EBML_NONE },
-    CHILD_OF(matroska_chapter)
+    { MATROSKA_ID_CHAPTERTIMESTART,          EBML_UINT, 0, 0, 
offsetof(MatroskaChapter, start), { .u = AV_NOPTS_VALUE } },
+    { MATROSKA_ID_CHAPTERTIMEEND,            EBML_UINT, 0, 0, 
offsetof(MatroskaChapter, end),   { .u = AV_NOPTS_VALUE } },
+    { MATROSKA_ID_CHAPTERUID,                EBML_UINT, 0, 0, 
offsetof(MatroskaChapter, uid) },
+    { MATROSKA_ID_CHAPTERSEGMENTUID,         EBML_BIN,  0, 0, 
offsetof(MatroskaChapter, segment_uid) },
+    { MATROSKA_ID_CHAPTERSEGMENTEDITIONUID,  EBML_UINT, 0, 0, 
offsetof(MatroskaChapter, segment_edition_uid) },
+    { MATROSKA_ID_CHAPTERDISPLAY,            EBML_NEST, 0, 0,                  
      0,         { .n = matroska_chapter_display } },
+    { MATROSKA_ID_CHAPTERFLAGHIDDEN,         EBML_NONE },
+    { MATROSKA_ID_CHAPTERFLAGENABLED,        EBML_NONE },
+    { MATROSKA_ID_CHAPTERPHYSEQUIV,          EBML_NONE },
+    CHILD_OF(matroska_edition)
 };
 
-static EbmlSyntax matroska_chapter[] = {
-    { MATROSKA_ID_CHAPTERATOM,        EBML_NEST, 0, sizeof(MatroskaChapter), 
offsetof(MatroskaDemuxContext, chapters), { .n = matroska_chapter_entry } },
-    { MATROSKA_ID_EDITIONUID,         EBML_NONE },
-    { MATROSKA_ID_EDITIONFLAGHIDDEN,  EBML_NONE },
-    { MATROSKA_ID_EDITIONFLAGDEFAULT, EBML_NONE },
-    { MATROSKA_ID_EDITIONFLAGORDERED, EBML_NONE },
+static EbmlSyntax matroska_edition[] = {
+    { MATROSKA_ID_EDITIONUID,         EBML_UINT, 0, 0, 
offsetof(MatroskaEdition, uid) },
+    { MATROSKA_ID_EDITIONFLAGHIDDEN,  EBML_UINT, 0, 0, 
offsetof(MatroskaEdition, flag_hidden) },
+    { MATROSKA_ID_EDITIONFLAGDEFAULT, EBML_UINT, 0, 0, 
offsetof(MatroskaEdition, flag_default) },
+    { MATROSKA_ID_EDITIONFLAGORDERED, EBML_UINT, 0, 0, 
offsetof(MatroskaEdition, flag_ordered) },
+    { MATROSKA_ID_CHAPTERATOM,        EBML_NEST, 0, sizeof(MatroskaChapter), 
offsetof(MatroskaEdition, chapters), { .n = matroska_chapter_entry } },
     CHILD_OF(matroska_chapters)
 };
 
 static EbmlSyntax matroska_chapters[] = {
-    { MATROSKA_ID_EDITIONENTRY, EBML_NEST, 0, 0, 0, { .n = matroska_chapter } 
},
+    { MATROSKA_ID_EDITIONENTRY, EBML_NEST, 0, sizeof(MatroskaEdition), 
offsetof(MatroskaDemuxContext, editions), { .n = matroska_edition } },
     CHILD_OF(matroska_segment)
 };
 
@@ -1884,14 +1885,18 @@ static void matroska_convert_tags(AVFormatContext *s)
                        i, tags[i].target.attachuid);
             }
         } else if (tags[i].target.chapteruid) {
-            MatroskaChapter *chapter = matroska->chapters.elem;
+            MatroskaEdition *editions = matroska->editions.elem;
+            MatroskaChapter *chapter;
             int found = 0;
-            for (j = 0; j < matroska->chapters.nb_elem; j++) {
-                if (chapter[j].uid == tags[i].target.chapteruid &&
-                    chapter[j].chapter) {
+            for (j = 0; j < matroska->editions.nb_elem && !found; j++) {
+                chapter = editions[j].chapters.elem;
+                for (int k = 0; k < editions[j].chapters.nb_elem; k++) {
+                    if (chapter[k].uid == tags[i].target.chapteruid &&
+                        chapter[k].chapter) {
                     matroska_convert_tag(s, &tags[i].tag,
-                                         &chapter[j].chapter->metadata, NULL);
+                                             &chapter[k].chapter->metadata, 
NULL);
                     found = 1;
+                    }
                 }
             }
             if (!found) {
@@ -3328,9 +3333,10 @@ static int matroska_read_header(AVFormatContext *s)
     FFFormatContext *const si = ffformatcontext(s);
     MatroskaDemuxContext *matroska = s->priv_data;
     EbmlList *attachments_list = &matroska->attachments;
-    EbmlList *chapters_list    = &matroska->chapters;
     MatroskaAttachment *attachments;
-    MatroskaChapter *chapters;
+    EbmlList *editions_list = &matroska->editions;
+    MatroskaEdition *editions;
+    MatroskaChapter *chapters = NULL;
     uint64_t max_start = 0;
     int64_t pos;
     Ebml ebml = { 0 };
@@ -3456,17 +3462,22 @@ static int matroska_read_header(AVFormatContext *s)
         }
     }
 
-    chapters = chapters_list->elem;
-    for (i = 0; i < chapters_list->nb_elem; i++)
-        if (chapters[i].start != AV_NOPTS_VALUE && chapters[i].uid &&
-            (max_start == 0 || chapters[i].start > max_start)) {
-            chapters[i].chapter =
-                avpriv_new_chapter(s, chapters[i].uid,
-                                   (AVRational) { 1, 1000000000 },
-                                   chapters[i].start, chapters[i].end,
-                                   chapters[i].title);
-            max_start = chapters[i].start;
+    editions = editions_list->elem;
+    for (i = 0; i < editions_list->nb_elem; i++) {
+        chapters = editions[i].chapters.elem;
+
+        for (j = 0; j < editions[i].chapters.nb_elem; j++) {
+            if (chapters[j].start != AV_NOPTS_VALUE && chapters[j].uid &&
+                (max_start == 0 || chapters[j].start > max_start)) {
+                chapters[j].chapter =
+                    avpriv_new_chapter(s, chapters[j].uid,
+                                    (AVRational) { 1, 1000000000 },
+                                    chapters[j].start, chapters[j].end,
+                                    chapters[j].title);
+                max_start = chapters[j].start;
+            }
         }
+    }
 
     matroska_add_index_entries(matroska);
 
-- 
2.49.1

_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to