| Message ID | 20240801021410.5061-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avformat/mov: ensure the IAMF track is the first | expand |
| 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 |
Quoting James Almer (2024-08-01 04:14:09) > Fixes crashes when muxing video tracks alongside IAMF ones. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavformat/movenc.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c > index ae49582a1a..87ec368d52 100644 > --- a/libavformat/movenc.c > +++ b/libavformat/movenc.c > @@ -7536,6 +7536,7 @@ static int mov_init_iamf_track(AVFormatContext *s) > if (!track->iamf) > return AVERROR(ENOMEM); > > + track->first_iamf_idx = INT_MAX; > for (int i = 0; i < s->nb_stream_groups; i++) { > const AVStreamGroup *stg = s->stream_groups[i]; > switch(stg->type) { > @@ -7558,6 +7559,11 @@ static int mov_init_iamf_track(AVFormatContext *s) > return ret; > } > > + if (track->first_iamf_idx != 0) { > + av_log(s, AV_LOG_ERROR, "The IMAF track must be the first track\n"); Why? And is this documented anywhere?
On 8/1/2024 5:59 AM, Anton Khirnov wrote: > Quoting James Almer (2024-08-01 04:14:09) >> Fixes crashes when muxing video tracks alongside IAMF ones. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavformat/movenc.c | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/libavformat/movenc.c b/libavformat/movenc.c >> index ae49582a1a..87ec368d52 100644 >> --- a/libavformat/movenc.c >> +++ b/libavformat/movenc.c >> @@ -7536,6 +7536,7 @@ static int mov_init_iamf_track(AVFormatContext *s) >> if (!track->iamf) >> return AVERROR(ENOMEM); >> >> + track->first_iamf_idx = INT_MAX; >> for (int i = 0; i < s->nb_stream_groups; i++) { >> const AVStreamGroup *stg = s->stream_groups[i]; >> switch(stg->type) { >> @@ -7558,6 +7559,11 @@ static int mov_init_iamf_track(AVFormatContext *s) >> return ret; >> } >> >> + if (track->first_iamf_idx != 0) { >> + av_log(s, AV_LOG_ERROR, "The IMAF track must be the first track\n"); > > Why? And is this documented anywhere? Just comments in the code. The reason i wrote it this way is because i parse the stream groups first, then the remaining streams, and generate the tracks in that order, as it was the simplest, least invasive way (The muxer handled streams and tracks as separate concepts with potentially different count for each of them even before iamf). I could look into changing it, but it may require some restructuring. In the meantime the muxer should not crash when you mix video tracks with iamf tracks. More so considering we need something easy to backport to 7.0.
On 8/1/2024 9:35 AM, James Almer wrote: > On 8/1/2024 5:59 AM, Anton Khirnov wrote: >> Quoting James Almer (2024-08-01 04:14:09) >>> Fixes crashes when muxing video tracks alongside IAMF ones. >>> >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> libavformat/movenc.c | 11 ++++++++++- >>> 1 file changed, 10 insertions(+), 1 deletion(-) >>> >>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c >>> index ae49582a1a..87ec368d52 100644 >>> --- a/libavformat/movenc.c >>> +++ b/libavformat/movenc.c >>> @@ -7536,6 +7536,7 @@ static int mov_init_iamf_track(AVFormatContext *s) >>> if (!track->iamf) >>> return AVERROR(ENOMEM); >>> + track->first_iamf_idx = INT_MAX; >>> for (int i = 0; i < s->nb_stream_groups; i++) { >>> const AVStreamGroup *stg = s->stream_groups[i]; >>> switch(stg->type) { >>> @@ -7558,6 +7559,11 @@ static int mov_init_iamf_track(AVFormatContext >>> *s) >>> return ret; >>> } >>> + if (track->first_iamf_idx != 0) { >>> + av_log(s, AV_LOG_ERROR, "The IMAF track must be the first >>> track\n"); >> >> Why? And is this documented anywhere? > > Just comments in the code. The reason i wrote it this way is because i > parse the stream groups first, then the remaining streams, and generate > the tracks in that order, as it was the simplest, least invasive way > (The muxer handled streams and tracks as separate concepts with > potentially different count for each of them even before iamf). > > I could look into changing it, but it may require some restructuring. Ok, gave it a try and seemingly found out how to get this working without too many changes. Will send an updated patch.
diff --git a/libavformat/movenc.c b/libavformat/movenc.c index ae49582a1a..87ec368d52 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -7536,6 +7536,7 @@ static int mov_init_iamf_track(AVFormatContext *s) if (!track->iamf) return AVERROR(ENOMEM); + track->first_iamf_idx = INT_MAX; for (int i = 0; i < s->nb_stream_groups; i++) { const AVStreamGroup *stg = s->stream_groups[i]; switch(stg->type) { @@ -7558,6 +7559,11 @@ static int mov_init_iamf_track(AVFormatContext *s) return ret; } + if (track->first_iamf_idx != 0) { + av_log(s, AV_LOG_ERROR, "The IMAF track must be the first track\n"); + return AVERROR(EINVAL);; + } + track->tag = MKTAG('i','a','m','f'); ret = avio_open_dyn_buf(&track->iamf_buf); @@ -7830,8 +7836,11 @@ static int mov_init(AVFormatContext *s) for (int j = 0, i = 0; j < s->nb_streams; j++) { AVStream *st = s->streams[j]; - if (st != st->priv_data) + if (st != st->priv_data) { + if (!i) + i++; // IAMF track is the first one continue; + } st->priv_data = &mov->tracks[i++]; }
Fixes crashes when muxing video tracks alongside IAMF ones. Signed-off-by: James Almer <jamrial@gmail.com> --- libavformat/movenc.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)