diff mbox series

[FFmpeg-devel] avformat/movenc: use stream indexes when generating track ids

Message ID 20240803191900.41511-1-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel] avformat/movenc: use stream indexes when generating track ids | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

James Almer Aug. 3, 2024, 7:19 p.m. UTC
In some scenarios nb_tracks isn't the same as nb_streams, so a given id may end
up being used for two separate streams.

e.g. when muxing an IAMF track followed by a video track, if the IAMF track
consists of several streams, the video track would end up having an id of 2,
which may also be used by one of the IAMF streams.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/movenc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Marton Balint Aug. 11, 2024, 7:05 p.m. UTC | #1
On Sat, 3 Aug 2024, James Almer wrote:

> In some scenarios nb_tracks isn't the same as nb_streams, so a given id may end
> up being used for two separate streams.
>
> e.g. when muxing an IAMF track followed by a video track, if the IAMF track
> consists of several streams, the video track would end up having an id of 2,
> which may also be used by one of the IAMF streams.
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> libavformat/movenc.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 8e4a037e46..8d91059081 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -4944,11 +4944,15 @@ static int mov_setup_track_ids(MOVMuxContext *mov, AVFormatContext *s)
>             mov->tracks[i].track_id = i >= mov->nb_streams ? ++next_generated_track_id : mov->tracks[i].st->id;
>         }
>     } else {
> +        int last_track_id = 0;
>         for (i = 0; i < mov->nb_tracks; i++) {
>             if (mov->tracks[i].entry <= 0 && !(mov->flags & FF_MOV_FLAG_FRAGMENT))
>                 continue;
>
> -            mov->tracks[i].track_id = i + 1;
> +            last_track_id =
> +            mov->tracks[i].track_id = (mov->tracks[i].st
> +                                       ? FFMAX(mov->tracks[i].st->index, last_track_id)
> +                                       : FFMAX((i ? mov->tracks[i - 1].track_id : i), last_track_id)) + 1;

Are you sure mov->tracks[i-1].track_id is always initialized? Because 
there is a condition above which can skip certain tracks. Maybe it would 
be safer to use last_track_id instead of mov->tracks[i-1].track_id ?

Thanks,
Marton
diff mbox series

Patch

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 8e4a037e46..8d91059081 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -4944,11 +4944,15 @@  static int mov_setup_track_ids(MOVMuxContext *mov, AVFormatContext *s)
             mov->tracks[i].track_id = i >= mov->nb_streams ? ++next_generated_track_id : mov->tracks[i].st->id;
         }
     } else {
+        int last_track_id = 0;
         for (i = 0; i < mov->nb_tracks; i++) {
             if (mov->tracks[i].entry <= 0 && !(mov->flags & FF_MOV_FLAG_FRAGMENT))
                 continue;
 
-            mov->tracks[i].track_id = i + 1;
+            last_track_id =
+            mov->tracks[i].track_id = (mov->tracks[i].st
+                                       ? FFMAX(mov->tracks[i].st->index, last_track_id)
+                                       : FFMAX((i ? mov->tracks[i - 1].track_id : i), last_track_id)) + 1;
         }
     }