[FFmpeg-devel] [PATCH] ffmpeg.c: copy global metadata by default.

[Michael Niedermayer]

On Wed, Jun 02, 2010 at 11:20:13AM +0200, Anton Khirnov wrote:
> On Wed, Mar 31, 2010 at 11:15:05AM +0200, Michael Niedermayer wrote:
> > On Thu, Mar 18, 2010 at 08:54:14PM +0100, Anton Khirnov wrote:
> > > Metadata is copied from the first input file with at least one tag to
> > > all output files.
> > > ---
> > >  ffmpeg.c |   20 ++++++++++++++++++++
> > >  1 files changed, 20 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/ffmpeg.c b/ffmpeg.c
> > > index 5a22dde..a04f052 100644
> > > --- a/ffmpeg.c
> > > +++ b/ffmpeg.c
> > > @@ -2175,6 +2175,26 @@ static int av_encode(AVFormatContext **output_files,
> > >                                      in_file->iformat->metadata_conv);
> > >      }
> > >  
> > > +    if (!nb_meta_data_maps) {
> > > +        for (i = 0; i < nb_input_files; i++) {
> > > +            is = input_files[i];
> > > +
> > > +            if (!av_metadata_get(is->metadata, "", NULL, AV_METADATA_IGNORE_SUFFIX))
> > > +                continue;
> > > +
> > > +            for (j = 0; j < nb_output_files; j++) {
> > > +                AVMetadataTag *t = NULL;
> > > +
> > > +                os = output_files[j];
> > > +                while ((t = av_metadata_get(is->metadata, "", t,
> > > +                                            AV_METADATA_IGNORE_SUFFIX)))
> > > +                    av_metadata_set2(&os->metadata, t->key, t->value, 0);
> > > +                av_metadata_conv(os, os->oformat->metadata_conv,
> > > +                                 is->iformat->metadata_conv);
> > > +            }
> > 
> > this looks buggy, if there where any metadata tags in os already 
> > av_metadata_conv() would convert them too
> 
> Should be fixed with attached patches.
> 
> Anton Khirnov

>  doc/APIchanges         |    3 +++
>  libavformat/avformat.h |   15 ++++++++++++++-
>  libavformat/metadata.c |   22 ++++++++++++++++++++++
>  3 files changed, 39 insertions(+), 1 deletion(-)
> 29daf3ff685655de670b11e6961b9aa78a1598b1  0001-metadata-add-av_metadata_copy-function.patch
> From 6732f53321e43d5bc097bb86116f6bc09f265972 Mon Sep 17 00:00:00 2001
> From: Anton Khirnov <wyskas at gmail.com>
> Date: Wed, 2 Jun 2010 10:12:01 +0200
> Subject: [PATCH 1/2] metadata: add av_metadata_copy() function.
> 
> ---
>  doc/APIchanges         |    3 +++
>  libavformat/avformat.h |   15 ++++++++++++++-
>  libavformat/metadata.c |   22 ++++++++++++++++++++++
>  3 files changed, 39 insertions(+), 1 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 3bb05c1..0f2d1db 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -12,6 +12,9 @@ libavutil:   2009-03-08
>  
>  API changes, most recent first:
>  
> +2010-xx-xx - rxxxxx - lavf 52.68.0 - metadata API
> +  Add av_metadata_copy().
> +
>  2010-06-01 - r31301 - lsws 0.11.0 - convertPalette API
>    Add sws_convertPalette8ToPacked32 and sws_convertPalette8ToPacked24
>  
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 80dc5c3..6d83c24 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -22,7 +22,7 @@
>  #define AVFORMAT_AVFORMAT_H
>  
>  #define LIBAVFORMAT_VERSION_MAJOR 52
> -#define LIBAVFORMAT_VERSION_MINOR 67
> +#define LIBAVFORMAT_VERSION_MINOR 68
>  #define LIBAVFORMAT_VERSION_MICRO  0
>  
>  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> @@ -173,6 +173,19 @@ void av_metadata_conv(struct AVFormatContext *ctx,const AVMetadataConv *d_conv,
>   */
>  void av_metadata_free(AVMetadata **m);
>  
> +/**
> + * Copy metadata from src to dst, converting it from src_conv format to
> + * dst_conv. Existing metadata in dst is left untouched. dst may be empty
> + * (i.e. a pointer to NULL), in which case it is created.
> + * @param pattern Used to copy only some keys. If non-NULL, it is passed
> + *                directly to av_metadata_get().
> + * @param flags   Passed directly to av_metadata_get/set. If pattern is NULL,


> + *                defaults to AV_METADATA_IGNORE_SUFFIX|AV_METADATA_DONT_OVERWRITE.

iam unsure about this


> + */
> +void av_metadata_copy(AVMetadata  *src, const AVMetadataConv *src_conv,
> +                      AVMetadata **dst, const AVMetadataConv *dst_conv,
> +                      const uint8_t *pattern, int flags);
> +
>  
>  /* packet functions */
>  
> diff --git a/libavformat/metadata.c b/libavformat/metadata.c
> index ff7ffe9..4fea7da 100644
> --- a/libavformat/metadata.c
> +++ b/libavformat/metadata.c
> @@ -151,3 +151,25 @@ void av_metadata_conv(AVFormatContext *ctx, const AVMetadataConv *d_conv,
>      for (i=0; i<ctx->nb_programs; i++)
>          metadata_conv(&ctx->programs[i]->metadata, d_conv, s_conv);
>  }
> +
> +void av_metadata_copy(AVMetadata  *src, const AVMetadataConv *src_conv,
> +                      AVMetadata **dst, const AVMetadataConv *dst_conv,
> +                      const uint8_t *pattern, int flags)
> +{
> +    AVMetadata  *tmp = NULL;
> +    AVMetadataTag *t = NULL;
> +
> +    if (!pattern && !flags)
> +        flags = AV_METADATA_IGNORE_SUFFIX | AV_METADATA_DONT_OVERWRITE;
> +
> +    while ((t = av_metadata_get(src, pattern ? pattern : "", t, flags)))
> +        av_metadata_set2(&tmp, t->key, t->value, flags);
> +
> +    metadata_conv(&tmp, dst_conv, src_conv);

i think metadata_conv() has a misdesinged API and we should drop it in
favor of a _conv that has a src and distinct dst argument but maybe allows
src=dst, though later is not really important

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
-------------- 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/20100604/e8cd478c/attachment.pgp>