| Message ID | AM7PR03MB6660CB68CDD21232D75256FE8FC19@AM7PR03MB6660.eurprd03.prod.outlook.com |
|---|---|
| State | Accepted |
| Commit | beb60abab5b1b3ff51f49c0959d6d21f866b0802 |
| Headers | show |
| Series | [FFmpeg-devel,1/4] avformat/matroskaenc: Allow to set multiple streams as default | expand |
| 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 |
Andreas Rheinhardt: > The Matroska specifications have evolved and now allow to mark > multiple tracks of the same kind as default (whether this was legal or > not before was dubious; e.g. mkvmerge disallowed it). Yet when the > Matroska muxer is set to infer default dispositions if absent, it also > enforced the now outdated restriction. So update this. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > doc/muxers.texi | 12 ++++++------ > libavformat/matroskaenc.c | 16 ++++++---------- > tests/ref/fate/matroska-flac-extradata-update | 4 ++-- > 3 files changed, 14 insertions(+), 18 deletions(-) > > diff --git a/doc/muxers.texi b/doc/muxers.texi > index 0972bbfd5c..0f8efabab9 100644 > --- a/doc/muxers.texi > +++ b/doc/muxers.texi > @@ -1567,12 +1567,12 @@ It influences which tracks players should play by default. The default mode > is @samp{infer}. > @table @samp > @item infer > -In this mode, for each type of track (audio, video or subtitle), if there is > -a track with disposition default of this type, then the first such track > -(i.e. the one with the lowest index) will be marked as default; if no such > -track exists, the first track of this type will be marked as default instead > -(if existing). This ensures that the default flag is set in a sensible way even > -if the input originated from containers that lack the concept of default tracks. > +Every track with disposition default will have the FlagDefault set. > +Additionally, for each type of track (audio, video or subtitle), if no track > +with disposition default of this type exists, then the first track of this type > +will be marked as default (if existing). This ensures that the default flag > +is set in a sensible way even if the input originated from containers that > +lack the concept of default tracks. > @item infer_no_subs > This mode is the same as infer except that if no subtitle track with > disposition default exists, no subtitle track will be marked as default. > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c > index 899a3388cd..e2d9159e2c 100644 > --- a/libavformat/matroskaenc.c > +++ b/libavformat/matroskaenc.c > @@ -1395,7 +1395,8 @@ static int mkv_write_tracks(AVFormatContext *s) > { > MatroskaMuxContext *mkv = s->priv_data; > AVIOContext *pb = s->pb; > - int i, ret, video_default_idx, audio_default_idx, subtitle_default_idx; > + int video_default_idx = -1, audio_default_idx = -1, subtitle_default_idx = -1; > + int i, ret; > > if (mkv->nb_attachments == s->nb_streams) > return 0; > @@ -1405,11 +1406,7 @@ static int mkv_write_tracks(AVFormatContext *s) > return ret; > > if (mkv->default_mode != DEFAULT_MODE_PASSTHROUGH) { > - int video_idx, audio_idx, subtitle_idx; > - > - video_idx = video_default_idx = > - audio_idx = audio_default_idx = > - subtitle_idx = subtitle_default_idx = -1; > + int video_idx = -1, audio_idx = -1, subtitle_idx = -1; > > for (i = s->nb_streams - 1; i >= 0; i--) { > AVStream *st = s->streams[i]; > @@ -1435,8 +1432,7 @@ static int mkv_write_tracks(AVFormatContext *s) > } > for (i = 0; i < s->nb_streams; i++) { > AVStream *st = s->streams[i]; > - int is_default = mkv->default_mode == DEFAULT_MODE_PASSTHROUGH ? > - st->disposition & AV_DISPOSITION_DEFAULT : > + int is_default = st->disposition & AV_DISPOSITION_DEFAULT || > i == video_default_idx || i == audio_default_idx || > i == subtitle_default_idx; > ret = mkv_write_track(s, mkv, st, &mkv->tracks[i], > @@ -2823,8 +2819,8 @@ static const AVOption options[] = { > { "flipped_raw_rgb", "Raw RGB bitmaps in VFW mode are stored bottom-up", OFFSET(flipped_raw_rgb), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS }, > { "write_crc32", "write a CRC32 element inside every Level 1 element", OFFSET(write_crc), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, FLAGS }, > { "default_mode", "Controls how a track's FlagDefault is inferred", OFFSET(default_mode), AV_OPT_TYPE_INT, { .i64 = DEFAULT_MODE_INFER }, DEFAULT_MODE_INFER, DEFAULT_MODE_PASSTHROUGH, FLAGS, "default_mode" }, > - { "infer", "For each track type, mark the first track of disposition default as default; if none exists, mark the first track as default.", 0, AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER }, 0, 0, FLAGS, "default_mode" }, > - { "infer_no_subs", "For each track type, mark the first track of disposition default as default; for audio and video: if none exists, mark the first track as default.", 0, AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER_NO_SUBS }, 0, 0, FLAGS, "default_mode" }, > + { "infer", "For each track type, mark each track of disposition default as default; if none exists, mark the first track as default.", 0, AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER }, 0, 0, FLAGS, "default_mode" }, > + { "infer_no_subs", "For each track type, mark each track of disposition default as default; for audio and video: if none exists, mark the first track as default.", 0, AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER_NO_SUBS }, 0, 0, FLAGS, "default_mode" }, > { "passthrough", "Use the disposition flag as-is", 0, AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_PASSTHROUGH }, 0, 0, FLAGS, "default_mode" }, > { NULL }, > }; > diff --git a/tests/ref/fate/matroska-flac-extradata-update b/tests/ref/fate/matroska-flac-extradata-update > index fb17bfea0b..1a8add7736 100644 > --- a/tests/ref/fate/matroska-flac-extradata-update > +++ b/tests/ref/fate/matroska-flac-extradata-update > @@ -1,5 +1,5 @@ > -3c721898cf2cf3e2e6c43ad58952bd2d *tests/data/fate/matroska-flac-extradata-update.matroska > -2032 tests/data/fate/matroska-flac-extradata-update.matroska > +c2b76d47a9f0e9626a4999bd395cae08 *tests/data/fate/matroska-flac-extradata-update.matroska > +2029 tests/data/fate/matroska-flac-extradata-update.matroska > #extradata 0: 34, 0x7acb09e7 > #extradata 1: 34, 0x7acb09e7 > #extradata 2: 34, 0x443402dd > Will apply this patchset tonight unless there are objections. - Andreas
diff --git a/doc/muxers.texi b/doc/muxers.texi index 0972bbfd5c..0f8efabab9 100644 --- a/doc/muxers.texi +++ b/doc/muxers.texi @@ -1567,12 +1567,12 @@ It influences which tracks players should play by default. The default mode is @samp{infer}. @table @samp @item infer -In this mode, for each type of track (audio, video or subtitle), if there is -a track with disposition default of this type, then the first such track -(i.e. the one with the lowest index) will be marked as default; if no such -track exists, the first track of this type will be marked as default instead -(if existing). This ensures that the default flag is set in a sensible way even -if the input originated from containers that lack the concept of default tracks. +Every track with disposition default will have the FlagDefault set. +Additionally, for each type of track (audio, video or subtitle), if no track +with disposition default of this type exists, then the first track of this type +will be marked as default (if existing). This ensures that the default flag +is set in a sensible way even if the input originated from containers that +lack the concept of default tracks. @item infer_no_subs This mode is the same as infer except that if no subtitle track with disposition default exists, no subtitle track will be marked as default. diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 899a3388cd..e2d9159e2c 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -1395,7 +1395,8 @@ static int mkv_write_tracks(AVFormatContext *s) { MatroskaMuxContext *mkv = s->priv_data; AVIOContext *pb = s->pb; - int i, ret, video_default_idx, audio_default_idx, subtitle_default_idx; + int video_default_idx = -1, audio_default_idx = -1, subtitle_default_idx = -1; + int i, ret; if (mkv->nb_attachments == s->nb_streams) return 0; @@ -1405,11 +1406,7 @@ static int mkv_write_tracks(AVFormatContext *s) return ret; if (mkv->default_mode != DEFAULT_MODE_PASSTHROUGH) { - int video_idx, audio_idx, subtitle_idx; - - video_idx = video_default_idx = - audio_idx = audio_default_idx = - subtitle_idx = subtitle_default_idx = -1; + int video_idx = -1, audio_idx = -1, subtitle_idx = -1; for (i = s->nb_streams - 1; i >= 0; i--) { AVStream *st = s->streams[i]; @@ -1435,8 +1432,7 @@ static int mkv_write_tracks(AVFormatContext *s) } for (i = 0; i < s->nb_streams; i++) { AVStream *st = s->streams[i]; - int is_default = mkv->default_mode == DEFAULT_MODE_PASSTHROUGH ? - st->disposition & AV_DISPOSITION_DEFAULT : + int is_default = st->disposition & AV_DISPOSITION_DEFAULT || i == video_default_idx || i == audio_default_idx || i == subtitle_default_idx; ret = mkv_write_track(s, mkv, st, &mkv->tracks[i], @@ -2823,8 +2819,8 @@ static const AVOption options[] = { { "flipped_raw_rgb", "Raw RGB bitmaps in VFW mode are stored bottom-up", OFFSET(flipped_raw_rgb), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS }, { "write_crc32", "write a CRC32 element inside every Level 1 element", OFFSET(write_crc), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, FLAGS }, { "default_mode", "Controls how a track's FlagDefault is inferred", OFFSET(default_mode), AV_OPT_TYPE_INT, { .i64 = DEFAULT_MODE_INFER }, DEFAULT_MODE_INFER, DEFAULT_MODE_PASSTHROUGH, FLAGS, "default_mode" }, - { "infer", "For each track type, mark the first track of disposition default as default; if none exists, mark the first track as default.", 0, AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER }, 0, 0, FLAGS, "default_mode" }, - { "infer_no_subs", "For each track type, mark the first track of disposition default as default; for audio and video: if none exists, mark the first track as default.", 0, AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER_NO_SUBS }, 0, 0, FLAGS, "default_mode" }, + { "infer", "For each track type, mark each track of disposition default as default; if none exists, mark the first track as default.", 0, AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER }, 0, 0, FLAGS, "default_mode" }, + { "infer_no_subs", "For each track type, mark each track of disposition default as default; for audio and video: if none exists, mark the first track as default.", 0, AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER_NO_SUBS }, 0, 0, FLAGS, "default_mode" }, { "passthrough", "Use the disposition flag as-is", 0, AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_PASSTHROUGH }, 0, 0, FLAGS, "default_mode" }, { NULL }, }; diff --git a/tests/ref/fate/matroska-flac-extradata-update b/tests/ref/fate/matroska-flac-extradata-update index fb17bfea0b..1a8add7736 100644 --- a/tests/ref/fate/matroska-flac-extradata-update +++ b/tests/ref/fate/matroska-flac-extradata-update @@ -1,5 +1,5 @@ -3c721898cf2cf3e2e6c43ad58952bd2d *tests/data/fate/matroska-flac-extradata-update.matroska -2032 tests/data/fate/matroska-flac-extradata-update.matroska +c2b76d47a9f0e9626a4999bd395cae08 *tests/data/fate/matroska-flac-extradata-update.matroska +2029 tests/data/fate/matroska-flac-extradata-update.matroska #extradata 0: 34, 0x7acb09e7 #extradata 1: 34, 0x7acb09e7 #extradata 2: 34, 0x443402dd
The Matroska specifications have evolved and now allow to mark multiple tracks of the same kind as default (whether this was legal or not before was dubious; e.g. mkvmerge disallowed it). Yet when the Matroska muxer is set to infer default dispositions if absent, it also enforced the now outdated restriction. So update this. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- doc/muxers.texi | 12 ++++++------ libavformat/matroskaenc.c | 16 ++++++---------- tests/ref/fate/matroska-flac-extradata-update | 4 ++-- 3 files changed, 14 insertions(+), 18 deletions(-)