diff mbox series

[FFmpeg-devel] avformat/matroskaenc: Sort cues entries by pts

Message ID AM7PR03MB666073074B9D9E14C96733BC8F649@AM7PR03MB6660.eurprd03.prod.outlook.com
State Accepted
Commit 8a18db3ec6af13c0a9889f1ef61d38cabc714813
Headers show
Series [FFmpeg-devel] avformat/matroskaenc: Sort cues entries by pts | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Andreas Rheinhardt Nov. 27, 2021, 3:16 p.m. UTC 
Currently they are ordered as-written (i.e. by increasing position);
in case av_interleaved_write_frame() is used, this is (mostly)
the same as ordered by increasing dts.
Yet the Matroska specification strongly recommends (SHOULD) that
the CuePoints be sorted by CueTime. mkvalidator warns when they are
not. Therefore this commit sorts them accordingly.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/matroskaenc.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

Comments

Andreas Rheinhardt Nov. 30, 2021, 10:52 a.m. UTC | #1 
Andreas Rheinhardt:
> Currently they are ordered as-written (i.e. by increasing position);
> in case av_interleaved_write_frame() is used, this is (mostly)
> the same as ordered by increasing dts.
> Yet the Matroska specification strongly recommends (SHOULD) that
> the CuePoints be sorted by CueTime. mkvalidator warns when they are
> not. Therefore this commit sorts them accordingly.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavformat/matroskaenc.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index f08ead0a96..8c4cf4024a 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -533,6 +533,7 @@ static int mkv_add_cuepoint(MatroskaMuxContext *mkv, int stream, int64_t ts,
>  {
>      mkv_cues *cues = &mkv->cues;
>      mkv_cuepoint *entries = cues->entries;
> +    unsigned idx = cues->num_entries;
>  
>      if (ts < 0)
>          return 0;
> @@ -542,11 +543,19 @@ static int mkv_add_cuepoint(MatroskaMuxContext *mkv, int stream, int64_t ts,
>          return AVERROR(ENOMEM);
>      cues->entries = entries;
>  
> -    cues->entries[cues->num_entries].pts           = ts;
> -    cues->entries[cues->num_entries].stream_idx    = stream;
> -    cues->entries[cues->num_entries].cluster_pos   = cluster_pos - mkv->segment_offset;
> -    cues->entries[cues->num_entries].relative_pos  = relative_pos;
> -    cues->entries[cues->num_entries++].duration    = duration;
> +    /* Make sure the cues entries are sorted by pts. */
> +    while (idx > 0 && entries[idx - 1].pts > ts)
> +        idx--;
> +    memmove(&entries[idx + 1], &entries[idx],
> +            (cues->num_entries - idx) * sizeof(entries[0]));
> +
> +    entries[idx].pts           = ts;
> +    entries[idx].stream_idx    = stream;
> +    entries[idx].cluster_pos   = cluster_pos - mkv->segment_offset;
> +    entries[idx].relative_pos  = relative_pos;
> +    entries[idx].duration      = duration;
> +
> +    cues->num_entries++;
>  
>      return 0;
>  }
> 
Will apply tonight unless there are objections.

- Andreas
diff mbox series

Patch

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index f08ead0a96..8c4cf4024a 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -533,6 +533,7 @@  static int mkv_add_cuepoint(MatroskaMuxContext *mkv, int stream, int64_t ts,
 {
     mkv_cues *cues = &mkv->cues;
     mkv_cuepoint *entries = cues->entries;
+    unsigned idx = cues->num_entries;
 
     if (ts < 0)
         return 0;
@@ -542,11 +543,19 @@  static int mkv_add_cuepoint(MatroskaMuxContext *mkv, int stream, int64_t ts,
         return AVERROR(ENOMEM);
     cues->entries = entries;
 
-    cues->entries[cues->num_entries].pts           = ts;
-    cues->entries[cues->num_entries].stream_idx    = stream;
-    cues->entries[cues->num_entries].cluster_pos   = cluster_pos - mkv->segment_offset;
-    cues->entries[cues->num_entries].relative_pos  = relative_pos;
-    cues->entries[cues->num_entries++].duration    = duration;
+    /* Make sure the cues entries are sorted by pts. */
+    while (idx > 0 && entries[idx - 1].pts > ts)
+        idx--;
+    memmove(&entries[idx + 1], &entries[idx],
+            (cues->num_entries - idx) * sizeof(entries[0]));
+
+    entries[idx].pts           = ts;
+    entries[idx].stream_idx    = stream;
+    entries[idx].cluster_pos   = cluster_pos - mkv->segment_offset;
+    entries[idx].relative_pos  = relative_pos;
+    entries[idx].duration      = duration;
+
+    cues->num_entries++;
 
     return 0;
 }