[FFmpeg-devel] [PATCH 3/3] Export metadata in the generic format.

[Anton Khirnov]

On Tue, Oct 12, 2010 at 11:54:12PM +0200, Aurelien Jacobs wrote:
> On Tue, Oct 12, 2010 at 09:37:32PM +0200, Anton Khirnov wrote:
> > On Tue, Oct 12, 2010 at 09:12:31PM +0200, Michael Niedermayer wrote:
> > > On Tue, Oct 12, 2010 at 07:35:17PM +0200, Anton Khirnov wrote:
> > > > On Mon, Oct 11, 2010 at 10:33:21PM +0200, Michael Niedermayer wrote:
> > > > > On Tue, Oct 05, 2010 at 07:57:06PM +0200, Anton Khirnov wrote:
> > > > > 
> > > > > thats a strange place for it one would expect it at the end of the header
> > > > > reading code
> > > > > 
> > > > > 
> > > > [...]
> > > > > 
> > > > > same as asf odd place, there could be multipl info tags
> > > > > and the same applies to other changes too
> > > > > 
> > > > Better now?
> > > > 
> > > > I've also removed the conv tables from (de)muxers completely instead of
> > > > just ifdefing them, since NULL is also a perfeclty valid conv table.
> > > [...]
> > > > diff --git a/libavformat/metadata.c b/libavformat/metadata.c
> > > > index 8fc1771..713304b 100644
> > > > --- a/libavformat/metadata.c
> > > > +++ b/libavformat/metadata.c
> > > > @@ -140,8 +140,8 @@ void metadata_conv(AVMetadata **pm, const AVMetadataConv *d_conv,
> > > >      *pm = dst;
> > > >  }
> > > >  
> > > > -void av_metadata_conv(AVFormatContext *ctx, const AVMetadataConv *d_conv,
> > > > -                                            const AVMetadataConv *s_conv)
> > > > +void metadata_conv_ctx(AVFormatContext *ctx, const AVMetadataConv *d_conv,
> > > 
> > > ff_ prefix
> > done
> > 
> > i guess metadata_conv() should get a ff_ prefix too, i'll send a patch
> > for this later
> 
> Sure ! Patch welcome.
> 
> > [...]
> > @@ -147,7 +146,9 @@ typedef struct {
> >  }AVMetadataTag;
> >  
> >  typedef struct AVMetadata AVMetadata;
> > +#if FF_API_OLD_METADATA
> >  typedef struct AVMetadataConv AVMetadataConv;
> > +#endif
> 
> AFAICT this typedef is still used with the new API (but don't need to be
> public anymore). Did you tried to compile with:
>   --extra-cflags=-DFF_API_OLD_METADATA=0
fixed
> 
> > [...]
> > @@ -152,3 +152,9 @@ 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_conv(AVFormatContext *ctx, const AVMetadataConv *d_conv,
> > +                                            const AVMetadataConv *s_conv)
> > +{
> > +    return;
> > +}
> 
> Should be inside #if FF_API_OLD_METADATA.
> 
fixed
> Overall, I have a slightly bad feeling about exporting normalized
> metadata tag instead of the native one, but I don't have any objective
> reason for this, just a feeling. So if everyone is happy with this, I
> won't object.
> 
Well it seems to be the simplest solution. And in all the time the new
metadata API existed i didn't see any program use the native tags.

-- 
Anton Khirnov
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Export-metadata-in-the-generic-format.patch
Type: text/x-diff
Size: 17827 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101013/b074b0eb/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101013/b074b0eb/attachment.pgp>