[FFmpeg-devel] [PATCH] new generic metadata API
Michael Niedermayer
michaelni
Mon Oct 13 19:27:29 CEST 2008
On Mon, Sep 29, 2008 at 12:46:54AM +0200, Aurelien Jacobs wrote:
> 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...
mkv->nut-mkv should generally end up with equivalent metadata, similarly
nut->mkv->nut should too.
Also what the user of libavcodec sees from both should be roughly the same.
> 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.
"X-" really means free form user data vs. the lack of it means that the
data is exactly in the format as written in the spec.
simply adding and removing 'X-' depending on the name being on a list
likely does not achive this.
Also this patch does not seem to convert time formats, language code and
country code formats. I think its a rather strong assumtation that every
2 containers use compatible ones.
This also is a rather important issue as it would lead to creation of broken
files.
Also some fields from info packets in nut like "Uses" and "Replaces" refer to
streams, its unlikely that blindly copying them will still have them refer
to the correct streams.
>
> > [...]
> > > 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...
Iam not insisting on this, id just like to understand better if it really
would be more complex to avoid depeding on the tree structure. And what
relation there is between such iterator function and the code in the end
user app, it may very well be that the lack of such iterator would make
accessing the data harder. After all the programmer accessing it at least
needs to understand the structure.
is there more needed than:
void enumerate(const AVMetadata *m, int indent, void *opaque, func)
{
int i;
for (i=0; i<m->nb; i++) {
func(opaque, m->list[i], indent);
if (m->list[i]->nested.nb)
enumerate(&m->list[i]->nested, indent+1, opaque, func);
}
}
?
> Moreover, this wouldn't be in the spirit of other libav* APIs...
hmm, i dont know, the "other" things generally are just flat structs of
fields where there isnt much one might want to change in the future.
[...]
> Index: libavformat/metadata.c
> ===================================================================
> --- libavformat/metadata.c (revision 0)
> +++ libavformat/metadata.c (revision 0)
> @@ -0,0 +1,389 @@
> +/*
> + * metadata utility functions for use within FFmpeg
> + * Copyright (c) 2008 Aurelien Jacobs <aurel at gnuage.org>
> + *
> + * This file is part of FFmpeg.
[...]
> +
> +#include <stdio.h>
> +#include <string.h>
> +#include <strings.h>
> +#include <ctype.h>
> +#include "avformat.h"
> +#include "internal.h"
> +#include "libavutil/avstring.h"
> +
> +#if LIBAVFORMAT_VERSION_MAJOR < 53
[...]
> +#endif
> +
> +static const char const *metadata_mapping[][6] = {
the 2nd const should be after the *
> + { "author", "artist", "creator", "written_by", "lead_performer" },
> + { "comment", "description" },
> + { "album", "albumtitle" },
> + { "year", "date_written", "date_released" },
> + { "track", "tracknumber", "part_number" },
> +};
> +
> +static AVMetadataTag *find_metadata_tag(AVMetadata *metadata,
> + const char *name, const char *lang,
> + int strict_lang, int name_len)
> +{
> + int i;
> + for (i=0; i<metadata->nb; i++)
> + if (!strncasecmp(metadata->list[i]->name, name, name_len))
> + if ((!lang && (!strict_lang || !metadata->list[i]->lang)) ||
> + (lang && metadata->list[i]->lang &&
> + !strncasecmp(metadata->list[i]->lang, lang, 3)))
> + return metadata->list[i];
> + return NULL;
> +}
In general id expect the user to want to see
tag matching his language&country if no such tag exists then one in a
language he understands, failing that too the default tag.
It seems this is not supported ...
> +
> +static const char *extract_metadata_lang(const char *lang,
> + const char *name, int *len)
> +{
> + if (!lang && *len>4 && name[*len-4] == '-' && islower(name[*len-3])
> + && islower(name[*len-2]) && islower(name[*len-1])) {
> + *len -= 4;
> + return name + *len + 1;
> + }
> + return lang;
> +}
i dont think this will work with country codes
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081013/a886a137/attachment.pgp>
More information about the ffmpeg-devel
mailing list