[FFmpeg-devel] [PATCH] avformat/mov: export xml metadata

[Paul B Mahol]

On 2/1/17, wm4 <nfxjfg at googlemail.com> wrote:
> On Wed, 1 Feb 2017 13:31:46 +0100
> Robert Krueger <krueger at lesspain.software> wrote:
>
>> On Wed, Feb 1, 2017 at 1:07 PM, wm4 <nfxjfg at googlemail.com> wrote:
>>
>> > On Wed,  1 Feb 2017 12:50:51 +0100
>> > Paul B Mahol <onemda at gmail.com> wrote:
>> >
>> > > Signed-off-by: Paul B Mahol <onemda at gmail.com>
>> > > ---
>> > >  libavformat/mov.c | 25 +++++++++++++++++++++++++
>> > >  1 file changed, 25 insertions(+)
>> > >
>> > > diff --git a/libavformat/mov.c b/libavformat/mov.c
>> > > index 9ae4f8c..75e1c9c60 100644
>> > > --- a/libavformat/mov.c
>> > > +++ b/libavformat/mov.c
>> > > @@ -3764,6 +3764,25 @@ static int mov_read_keys(MOVContext *c,
>> > AVIOContext *pb, MOVAtom atom)
>> > >      return 0;
>> > >  }
>> > >
>> > > +static int mov_read_xml(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>> > > +{
>> > > +    uint8_t *xml;
>> > > +
>> > > +    if (atom.size < 5)
>> > > +        return 0;
>> > > +
>> > > +    avio_skip(pb, 4);
>> > > +    xml = av_calloc(atom.size - 4 + 1, sizeof(uint8_t));
>> > > +    if (!xml)
>> > > +        return AVERROR(ENOMEM);
>> > > +
>> > > +    avio_read(pb, xml, atom.size - 4);
>> > > +    av_dict_set(&c->fc->metadata, "xml", xml, 0);
>> > > +    av_free(xml);
>> > > +
>> > > +    return 0;
>> > > +}
>> > > +
>> > >  static int mov_read_custom(MOVContext *c, AVIOContext *pb, MOVAtom
>> > > atom)
>> > >  {
>> > >      int64_t end = avio_tell(pb) + atom.size;
>> > > @@ -5280,6 +5299,12 @@ static int mov_read_default(MOVContext *c,
>> > AVIOContext *pb, MOVAtom atom)
>> > >              parse = mov_read_keys;
>> > >          }
>> > >
>> > > +        if (!parse &&
>> > > +            atom.type == MKTAG('m','e','t','a') &&
>> > > +            a.type == MKTAG('x','m','l',' ')) {
>> > > +            parse = mov_read_xml;
>> > > +        }
>> > > +
>> > >          if (!parse) { /* skip leaf atoms data */
>> > >              avio_skip(pb, a.size);
>> > >          } else {
>> >
>> > We had this discussion recently: the metadata AVDictionary should
>> > probably only contain real tags, not other information that's contained
>> > in the format but for which libavformat has no good way to export yet.
>> >
>> > There needs to be a concept for distinguishing user-visible tags and
>> > other data.
>> >
>> > (You don't want to show a xml dump in a GUI that happens to use ffmpeg,
>> > do you?)
>> >
>> >
>> I thought that was already the case for XMP (i.e. xml) metadata embedded
>> in
>> mov. I must look up how that was handled. I seem to remember there was a
>> flag to suppress this on the command line. With the policy you propose,
>> libavformat will be of a lot less use for api users. Not that I had a vote
>> here. It's just 2c of an api user.
>
> I'm not voting for removing it, just that there should be a separate
> mechanism for this.

Can you define/explain such thing in detail?
If not I will use current approach.

>
> And I as API user are very bothered by irrelevant crap being dumped
> in what should return user tags (that can be displayed to a user), or
> the fact that these can even show up in remuxes.
>
> Example: just what is the use of "crap" like MAJOR_BRAND or
> COMPATIBLE_BRANDS in Matroska tags? Yeah, let's add some
> meaningless XML garbage to produced files as well.