[FFmpeg-devel] [PATCH 11/19] avformat/matroskadec: Support FlagOriginal

[Andreas Rheinhardt]

Anton Khirnov:
> Quoting Andreas Rheinhardt (2021-02-22 05:05:29)
>> Andreas Rheinhardt:
>>> Needs a CountedElement in order to distinguish the case of the element
>>> not being present and the element being present with a value of zero.
>>>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>>> ---
>>>  libavformat/matroska.h    | 1 +
>>>  libavformat/matroskadec.c | 7 ++++++-
>>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/matroska.h b/libavformat/matroska.h
>>> index 191c4f6149..8ab87eff20 100644
>>> --- a/libavformat/matroska.h
>>> +++ b/libavformat/matroska.h
>>> @@ -100,6 +100,7 @@
>>>  #define MATROSKA_ID_TRACKFLAGDEFAULT 0x88
>>>  #define MATROSKA_ID_TRACKFLAGFORCED 0x55AA
>>>  #define MATROSKA_ID_TRACKFLAGLACING 0x9C
>>> +#define MATROSKA_ID_TRACKFLAGORIGINAL 0x55AE
>>>  #define MATROSKA_ID_TRACKMINCACHE 0x6DE7
>>>  #define MATROSKA_ID_TRACKMAXCACHE 0x6DF8
>>>  #define MATROSKA_ID_TRACKDEFAULTDURATION 0x23E383
>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>>> index fa266fcaec..f15bf8f9d2 100644
>>> --- a/libavformat/matroskadec.c
>>> +++ b/libavformat/matroskadec.c
>>> @@ -251,6 +251,7 @@ typedef struct MatroskaTrack {
>>>      uint64_t flag_default;
>>>      uint64_t flag_forced;
>>>      uint64_t flag_comment;
>>> +    CountedElement flag_original;
>>>      uint64_t seek_preroll;
>>>      MatroskaTrackVideo video;
>>>      MatroskaTrackAudio audio;
>>> @@ -410,7 +411,7 @@ typedef struct MatroskaDemuxContext {
>>>  // incomplete type (6.7.2 in C90, 6.9.2 in C99).
>>>  // Removing the sizes breaks MSVC.
>>>  static EbmlSyntax ebml_syntax[3], matroska_segment[9], matroska_track_video_color[15], matroska_track_video[19],
>>> -                  matroska_track[28], matroska_track_encoding[6], matroska_track_encodings[2],
>>> +                  matroska_track[29], matroska_track_encoding[6], matroska_track_encodings[2],
>>>                    matroska_track_combine_planes[2], matroska_track_operation[2], matroska_tracks[2],
>>>                    matroska_attachments[2], matroska_chapter_entry[9], matroska_chapter[6], matroska_chapters[2],
>>>                    matroska_index_entry[3], matroska_index[2], matroska_tag[3], matroska_tags[2], matroska_seekhead[2],
>>> @@ -575,6 +576,7 @@ static EbmlSyntax matroska_track[] = {
>>>      { MATROSKA_ID_TRACKFLAGCOMMENTARY,   EBML_UINT,  0, 0, offsetof(MatroskaTrack, flag_comment), { .u = 0 } },
>>>      { MATROSKA_ID_TRACKFLAGDEFAULT,      EBML_UINT,  0, 0, offsetof(MatroskaTrack, flag_default), { .u = 1 } },
>>>      { MATROSKA_ID_TRACKFLAGFORCED,       EBML_UINT,  0, 0, offsetof(MatroskaTrack, flag_forced),  { .u = 0 } },
>>> +    { MATROSKA_ID_TRACKFLAGORIGINAL,     EBML_UINT,  1, 0, offsetof(MatroskaTrack, flag_original), {.u = 0 } },
>>>      { MATROSKA_ID_TRACKVIDEO,            EBML_NEST,  0, 0, offsetof(MatroskaTrack, video),        { .n = matroska_track_video } },
>>>      { MATROSKA_ID_TRACKAUDIO,            EBML_NEST,  0, 0, offsetof(MatroskaTrack, audio),        { .n = matroska_track_audio } },
>>>      { MATROSKA_ID_TRACKOPERATION,        EBML_NEST,  0, 0, offsetof(MatroskaTrack, operation),    { .n = matroska_track_operation } },
>>> @@ -2746,6 +2748,9 @@ static int matroska_parse_tracks(AVFormatContext *s)
>>>              st->disposition |= AV_DISPOSITION_FORCED;
>>>          if (track->flag_comment)
>>>              st->disposition |= AV_DISPOSITION_COMMENT;
>>> +        if (track->flag_original.count > 0)
>>> +            st->disposition |= track->flag_original.el.u ? AV_DISPOSITION_ORIGINAL
>>> +                                                         : AV_DISPOSITION_DUB;
>>>  
>>>          if (!st->codecpar->extradata) {
>>>              if (extradata) {
>>>
>> Ridley Combs reviewed this set via IRC and approved it with one
>> exception: It makes no sense to use AV_DISPOSITION_DUB for something
>> else than an audio track, so if FlagOriginal is set to zero (meaning the
>> track is not in the content's original language), it should not be
>> exported at all. And the muxer should ignore AV_DISPOSITION_DUB for
>> non-audio-tracks. How do others think about this?
> 
> AV_DISPOSITION_DUB is not documented, so you can interpret it pretty
> widely. One interpretation that I can think of is:
> - there are two audio tracks - let's say japanese original and english
>   dub
> - there are also two english subtitle tracks:
>     * one that translates everything, meant to be used with the original
>       audio;
>     * the other one translating just the text in the video (and possibly
>       things that were meant to be subtitled in the original, like
>       speech in Klingon), meant to be used with the english dub
> 
> Then it can make sense to tag the second subtitle track with
> AV_DISPOSITION_DUB to indicate it is supposed to be used with the dubbed
> audio track.
> 
That is also pretty much what I thought; furthermore, I'd really hate if
something was lost during a remuxing. I will therefore apply this as-is
tomorrow unless more comes up.
(I will leave the matroska-wtv-remux fate test out of this for reasons
explained here:
https://ffmpeg.org/pipermail/ffmpeg-devel/2021-February/276986.html)

- Andreas