[FFmpeg-devel] [PATCH] metadata conversion API

Baptiste Coudurier baptiste.coudurier
Wed Mar 4 02:16:33 CET 2009


On 3/3/2009 3:02 PM, Michael Niedermayer wrote:
> On Tue, Mar 03, 2009 at 02:20:47PM -0800, Baptiste Coudurier wrote:
>> On 3/3/2009 2:05 PM, Michael Niedermayer wrote:
>>> On Tue, Mar 03, 2009 at 09:35:09PM +0000, Robert Swain wrote:
>>>> 2009/3/3 Michael Niedermayer <michaelni at gmx.at>:
>>>>> On Mon, Mar 02, 2009 at 06:25:30PM -0800, Baptiste Coudurier wrote:
>>>>>> 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.
>>>> OK, so maybe this is off topic for this thread.
>>>>
>>>>> 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. And
>>>>> that you prefer not to export the croping information in the existing
>>>>> fields for this puprose.
>>>>> And you want to export a field (whichs name you have not revealed yet)
>>>>> that only has 2, 4 and 8 as valid values.
>>>>> Have i missed some? I remember alot of flaming and trolling and some
>>>>> unsuccessfull attempts at provocating me but little actual discussion
>>>>> about it.
>>>>>
>>>>> 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 CPU loss should be minimal and it's only a set up cost. How many
>>>> cycles are really spent on option parsing?
>>> we directly access fields from AVCodecContext currently just try
>>> grep avctx libavcodec/*.c | grep -v av_log
>>> there are over 5000 matches
>>> i do not want to replace 5000 single instruction reads with 5000 function
>>> calls doing a search through a table of strings.
>>> Especially considering that i dont see what we would gain by this
>> I don't see how this relates to using char */char * as options, you can
>> set AVCodecContext fields if you want.
> 
> thats what we have currently
> av_set_string() takes a name & value char * and sets the corresponding field
> on AVCodecContext

This does not work with libx264 native options, does it ?

>> Although almost every codec has its own context anyway, point is to
>> factorize when it's actually needed and not by principle.
> 
> So would you agree that the char * are only allowed to be used for things
> that are used by just one muxer/demuxer or encoder/decoder ?

Yes, this rule would apply to any of them, even native ones.

>>>> Also, we can still enforce
>>>> some sort of option naming consistency.
>>> I cant even enforce some decission about where to store the croping
>>> information now. And i am listed as maintainer of mov
>> Well you share maintainership, and please stop saying this, it is not
>> true, if you want to store croping into something actually used, I
>> already said you were free to do so.
> 
> then stop phonily offering to store it in some wrong place

Why ? If you want to store it somewhere you can, if I want to have
access to it in a more easy way, why shouldn't I ?

I won't stop, currently I have no way to retrieve the height and width
stored in the mov.

What you propose is not adequate: I don't want to parse any structure to
be able to print the values. Besides, codec might override it, so we
need a field in AVFormatContext, you will end up duplicating every
field, like sample_aspect_ratio.

AVMetadata is the easiest and simplest method.

This applies to decoders as well, "aac/<special feature only existing in
aac>", "eac3/....", you will add a field in AVCodecContext for
everything, while having char */char * will simplify printing and
retrieving a lot.

>> Please don't focus on this "width" thing, this might apply to any
>> information people find useful.
> 
> so far people have not found anythig else though,
> external libs and mov/width being the only.
> 

mov/height, mov/bits per sample, everything that might differ from the
codec respective field, like height in mxf, stored height, display height.

If I export "DisplayHeight" from mxf I don't see why I shouldn't export
mov height in the same way, ie AVMetadata.

Also native encoders using a field in AVCodecContext only for one
purpose is useless.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no
FFmpeg maintainer                                  http://www.ffmpeg.org




More information about the ffmpeg-devel mailing list