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

[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.

-- 
Anton Khirnov