[FFmpeg-devel] [PATCH] metadata conversion API

[Michael Niedermayer]

On Sat, Feb 28, 2009 at 04:06:55PM -0800, Baptiste Coudurier wrote:
> On 2/28/2009 3:30 PM, Michael Niedermayer wrote:
> > On Sat, Feb 28, 2009 at 02:24:45PM -0800, Baptiste Coudurier wrote:
> >> On 2/28/2009 1:43 PM, Michael Niedermayer wrote:
> >>> On Sat, Feb 28, 2009 at 01:24:41PM -0800, Baptiste Coudurier wrote:
> >>>> On 2/28/2009 5:57 AM, Michael Niedermayer wrote:
> >>> [...]
> >>>>>>> * more compatibility for apps, apps already can through 
> >>>>>>> AVOptions set and get by name and enumerate fields.
> >>>>>> AVOptions uses OPT_<type> isn't it ? Why don't you want to
> >>>>>> apply this to AVMetadata ?
> >>>>> i explained it already above:
> >>>>>>> [...] This has the advantage that it can be muxed in
> >>>>>>> containers that do not support storing such information.
> >>>>> [...]
> >>>>>>> Or how would you store these types? If they are lost on
> >>>>>>> remuxing or their types are randomized then they arent
> >>>>>>> particularely usefull IMHO
> >>>> Well, they are useful to gather information, print metadata and 
> >>>> debugging, maybe less useful for remuxing inter-container, however,
> >>>> mov to mov could end in a pretty accurate way.
> >>>>
> >>>> Exporting all information using AVFormatContext fields will lead to
> >>>> an huge struct.
> >>> exporting all fields as name value pairs will consume more memory 
> >>> what i mean is, an int needs 4 bytes a av_malloc() will need at least
> >>> 16 byte due to alignment alone but i would not be surprised if it
> >>> needs twice that to keep track of things so malloc& free work now
> >>> each metadata tag contains 2 strings, if we assume both fit in the 16
> >>> byte and no additional byte is needed to keep track of things then 
> >>> theres 16 bytes for ther struct (2 8byte pointers on 64bit archs) 
> >>> +16*2 byte for the 2 strings thats 48 byte in the very best case, in
> >>> reality it will need more
> >>>
> >>> that makes it 4 byte for a field in a struct and that could be
> >>> reduced to 1 byte for many things and 48 byte for a name-value tag
> >>>
> >>> that means you need 12 times more unused fields than fields in the 
> >>> struct just to make the name-value tags need the same amount of
> >>> memory. and if its about 1 byte fields the factor is 48 instead of
> >>> 12.
> >>>
> >>> maybe this explains why i disslike them so much * slow to access *
> >>> memory hungry
> >>>
> >>> also there union{} and one can place structs in structs to make the
> >>> source clearer than a monolithic struct.
> >> I believe the code to print everything would be a pain in the ass.
> >> You will have to iterate over all fields. I think it is worth to
> >> consider easy access.
> >>
> > 
> >> It's like to code handling lang mechanism in new metadata API, it is
> >> _ugly_, implementation prove it.
> > 
> > huh?
> > if i compare the current metadata API with what was originally proposed
> > then the original with seperate lang field was much uglier and much bigger
> > loc wise
> 
> That is your opinion, when I see the code splitting lang from the tag
> name in the muxer, and the code to append the lang to the tag, I find it
> ugly.

with -lang it has to be split and appended in some (de)muxers

with seperate lang field to has to be meregd and splited in other (de)muxers and
theres more mess in the core and the tags need more memory

you are mov maintainer and yes -lang is a bigger mess for .mov but overall
its less mess IMHO


> 
> >> Also I believe this would simplify adding support for libx264
> >> commandline switches in libx264 wrapper, since you do not needed an API
> >> extension in AVCodecContext, for it due to x264_parm_parse which takes
> >> exactly 2 char *. This is what I call generic API.
> > 
> > the question is if we _WANT_ to let a encoder bypass the normal way to
> > pass parameters. 
> 
> I do. Is anybody against this ?

Iam not particularely happy about it, "against" is maybe too strong of a
word ...
I also think that allowing to pass opaque string(s) should be limited to
wrapers around external libs and not allowed for native encoders


[...]
> >> On the other end, you could explicitly instruct the demuxer to not
> >> populate metadata, this would save more bytes than your solution, since
> >> the context will be smaller.
> > 
> > that assumes that the fields are all useless for the users task, this is
> > extreemly unlikely
> > if its about true metadata like Author, that exists in a name-value list
> > alraedy and could, would someone actually care be disabled
> 
> Needed values will be always exported because values are needed, obviously.
> 
> > the other fields that we talk about (everything that exists as a field in
> > AVCodecContext currently) arent that useless that you would achive the
> > 12-48 times more unused than used factor IMHO.
> 
> Scenario for encoding and muxing is a lot simpler IMHO.
> 
> - smaller AVCodecContext and AVFormatContext.
> - Generic API which can be extended easier. (code in muxer/encoder and
> API bump)
> 

> Also, why not using fields for "metadata" (author, genre, track, comment
> ...) instead of "tag" strings,  if you claim that this would be more
> efficient speed and memory wise ?

With true metadata, the file can contain any value-name pair, its not
practical to have a field for every ascii name of lets say up to 10 chars
it would need more space than any computer today has
the difference to he existing fields is simply the percentage of used fields

with true metadata (10 char ascii names) its ~1e21 fields from which maybe
0-20 or so are used
with encoder/decoder/muxer/demuxer parameters (width, duration, bitrate,...)
its maybe a few hundread fields from which 10-50% are used (thats just a
estimation from feeling! but i think iam not too far off)

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

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- 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/20090301/cb88fbbf/attachment.pgp>