[FFmpeg-devel] [PATCH] new generic metadata API

Aurelien Jacobs aurel
Mon Sep 29 00:46:54 CEST 2008


On Sun, 28 Sep 2008 00:27:40 +0200
Michael Niedermayer <michaelni at gmx.at> wrote:

> 
> On Sat, Sep 27, 2008 at 04:03:53PM +0200, Aurelien Jacobs wrote:
> > On Tue, 23 Sep 2008 15:21:43 +0200
> > Michael Niedermayer <michaelni at gmx.at> wrote:
> [...]
> 
> > > [note, also see the nut-devel ML for some related change suggestion i just
> > >  posted]
> > 
> > Seen it.
> > I've added generic flattening and un-flattening support, following your
> > proposed nut scheme. The exact formatting could be changed latter if
> > a different scheme is accepted.
> 
> What does it do with 2
> Author tags? does it convert to Author / Author2?

Yes.

> Similarly, does it remove the '2' postfix when unflattening?

No. I'm not sure if it's a good idea to remove it...
Anyway, that could be changed later without touching the API.

> does it prefix non standard parts with 'X-' when muxing nut?

It didn't. Now it does.

> [...]
> > Index: libavformat/avformat.h
> > ===================================================================
> > --- libavformat/avformat.h	(revision 15392)
> > +++ libavformat/avformat.h	(working copy)
> > @@ -22,8 +22,8 @@
> >  #define AVFORMAT_AVFORMAT_H
> >  
> >  #define LIBAVFORMAT_VERSION_MAJOR 52
> > -#define LIBAVFORMAT_VERSION_MINOR 22
> > -#define LIBAVFORMAT_VERSION_MICRO  1
> > +#define LIBAVFORMAT_VERSION_MINOR 23
> > +#define LIBAVFORMAT_VERSION_MICRO  0
> >  
> >  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> >                                                 LIBAVFORMAT_VERSION_MINOR, \
> 
> > @@ -314,6 +314,20 @@
> >      struct AVInputFormat *next;
> >  } AVInputFormat;
> >  
> > +typedef struct AVMetadataTag AVMetadataTag;
> > +
> > +typedef struct AVMetadata {
> > +    unsigned int nb;
> > +    AVMetadataTag **list;
> > +} AVMetadata;
> > +
> > +struct AVMetadataTag {
> > +    char *name;
> > +    char *value;
> > +    char *lang;
> > +    AVMetadata nested;
> > +};
> > +
> >  enum AVStreamParseType {
> >      AVSTREAM_PARSE_NONE,
> >      AVSTREAM_PARSE_FULL,       /**< full parsing and repack */
> 
> Is it needed to have these structs in a public header?

Yes. Applications have to be able to parse the full tree to
display it for example. See ffplay.c:dump_metadata() for what
an application may require.

> Making the structs less public would allow more changes without breaking
> the API/ABI, we for example might one day want to use AVTree for it, and or
> might even want to switch to a flat internal format.

I understand your concern, but I think the price to pay would be too
high. It would require some tree iterator functions and some individual
fields accessor functions, etc...
Moreover, this wouldn't be in the spirit of other libav* APIs...

> [..]
> > +/**
> > + * Count the number of tags present in a metadata set among a list of names.
> > + * @param metadata metadata set
> > + * @param names tag name list (NULL terminated)
> > + * @return the count of available tags
> > + */
> > +int av_metadata_has_tags(AVMetadata *metadata, const char **names);
> > +
> 
> i would suggest "count" instead of "has"

OK.

> > +/**
> > + * Copy the content of a full metadata set.
> > + * @param out destination of the copy
> > + * @param in source of the copy
> > + * @return 0 if OK. AVERROR_xxx if error.
> > + */
> > +int av_metadata_copy(AVMetadata *out, AVMetadata *in);
> 
> clone is maybe a better term than copy

OK.

> > [...metadata implementation in utils.c...]
> 
> maybe all that stuff could be put in a metadata.c instead of utils.c

OK.

Updated patch attached.

Aurel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: metadata2.diff
Type: text/x-diff
Size: 108723 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080929/85cf9862/attachment.diff>



More information about the ffmpeg-devel mailing list