[FFmpeg-devel] [PATCH] metadata conversion API

Tue Mar 3 23:25:24 CET 2009

On Tue, Mar 03, 2009 at 01:13:10PM -0800, Baptiste Coudurier wrote:
> On 3/3/2009 12:22 PM, Michael Niedermayer wrote:
> > On Mon, Mar 02, 2009 at 06:25:30PM -0800, Baptiste Coudurier wrote:
> >> On 3/2/2009 5:36 PM, Michael Niedermayer wrote:
> >>> On Mon, Mar 02, 2009 at 03:41:04PM -0800, Baptiste Coudurier wrote:
> >>>> On 3/2/2009 3:32 PM, Michael Niedermayer wrote:
> >>>>> On Mon, Mar 02, 2009 at 10:51:00PM +0100, Peter Eszlari wrote:
> >>> [...]
> >>>>>> Of course manpower is a different matter...suppose we had this
> >>>>>> list with all options&ranges, how would we actually proceed?
> >>>>>> Checking them one by one like it is done atm (see attached
> >>>>>> patch for -aq/mp2 case)? This seems not practical, especially
> >>>>>> if we want to prevent the user from doing really stupid things
> >>>>>> like:
> >>>>> The way to go (if we had the volunteers for it ...) would be to
> >>>>> at the simplest level add an array of const char* name, float
> >>>>> max, float min to AVCodec that can then include what options and
> >>>>> what ranges each codec supports its easy to check then if
> >>>>> everything is within range and if any other option has been
> >>>>> changed from its default (there are also other ways then checking
> >>>>> against the default from AVOptions to detect changed stuff.
> >>>> I propose to add an array of tag/value pairs as "char*" in 
> >>>> AVCodecContext, then encoder_init can check these values.
> >>>>
> >>>> char* is definitely more flexible, and permits more advanced
> >>>> treatment. Furthermore, it can be passed "as is" to libx264.
> >>>>
> >>> [...]
> >>>
> >>> Also we are lacking volunteers to make such lists, are you
> >>> volunteering?
> >> Sorry, nope, I don't find this solution elegant nor flexible.
> >> I need an option which only supports 2, 4 or 8 as param.
> > 
> > its trivial to add a callback to AVOption to do such specific validity
> > checks.
> > Besides whats next? Should i add a fast primality check because some
> > option might want to allow just prime numbers?
> > 
> > 
> >> I however volunteer to add key/value as char* options to AVFormatContext
> >> and AVCodecContext and use them.
> > 
> > You said that already but there is no relation between that and such validity
> > checks, its not going to solve this problem.
> > 
> > Things added to svn have to be justified and their advantages and
> > disadavatanges weighted against each other. Also once a technical discussion
> > happened its the decission of the maintainer(s). Or in exceptional
> > circumstances a vote (or consensus for thouse prefering the terminology)
> > overriding the maintainer.
> > 
> > The arguments brought forth so far in favor of it, where that you want to
> > export ".mov/width" with a value that could be scaling or croping. And that
> > some advanced users may be able to make more sense out of it then we.
> 
> Well, I want to export ".mov/width" with a value, this value can be used
> for whatever the user want.

but it is incomplete information, the user needs more to use it.
it seems you want to
* skip reading and understandig the mov spec
* skip designing a interface that can be used by more than just mov

and that is where i disagree with you, not about exporting that as char*
vs. a field in AVCodecContext
char* just means you can export it without my approval and it seems to
me this is the only reason why you want char*

> 
> > And that you prefer not to export the croping information in the existing
> > fields for this puprose.
> 
> "Feel free to implement this if it works, I'm only proposing to export
> "width" so I can automatically crop in ffmpeg.c, not anything else."
> 
> I'm perfectly fine with this also, I just don't volunteer to do it.

Neither do i volunteer

> 
> > And you want to export a field (whichs name you have not revealed yet)
> > that only has 2, 4 and 8 as valid values.
> 
> Like audio channels for a muxer.

understood

so instead of
if(AVCodecContext.channels != 2,4,8)
    av_log("sorry pal")

you would have to write
if(atoi(get_option_from_string(AVCodecContext.stings)) != 2,4,8)
    av_log("sorry pal")

and where in this did we improve anything?
I mean you say this as if it would be an argument in favor of these
value-name lists but it just makes no difference at all

> 
> > Have i missed some? I remember alot of flaming and trolling and some
> > unsuccessfull attempts at provocating me but little actual discussion
> > about it.
> 
> Sorry, but it seems to me that everytime you disagree, you call flaming
> or trolling, this really doesn't help the discussion.

what discussion?

> 
> > The arguments against, being a roughly 10x increase in memory consumtion,
> > similar or worse increase in cpu consumtion for access and the loss of a
> > common set of parameters for codecs. Each could have their own flag to
> > enable b frames ...
> 
> The advantage being a lot simpler interface of users, and 

Not true

its just:
for(all things on the command line)
    av_set_string3(avctx, name, value, NULL);

> having the
> possibility to declare these options only were are are actually used
> (encoders or muxers)
yes but it will need more memory, so what is the point?

> and with more appropriate names.

If the common names are poorly choosen, they can be changed, this has no
relation to name-value lists.
If OTOH you want different names for the same things for 2 muxers then this
breaks the common interface

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

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- 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/20090303/31bde344/attachment.pgp>