[FFmpeg-devel] [PATCH/RFC] Per-codec option system

[Michael Niedermayer]

On Wed, Sep 30, 2009 at 10:02:02AM +0200, Robert Swain wrote:
> 2009/9/30 Jason Garrett-Glaser <darkshikari at gmail.com>:
> >> * parameters that are semantically identical between a pair of codecs,
> >> ?must use the same parameter name, they also must use a compatible
> >> ?parameter value system. That makes some sense because, well, its nice if
> >> ?users dont have to releaern a new system for each codec in ffmpeg.
> >
> > IMO this should be handled internally to codecs, not by literally
> > sharing the parameters. ?In other words, it should be a policy: codecs
> > should use the same name for parameters when it makes sense. ?It
> > should be enforced by rules, not code.
> 
> I agree that it makes sense for semantically identical parameters to
> use the same name, but I definitely agree with Jason that it does not
> need to be the same declared variable. Items with the same semantics
> can have different types. Sometimes items may share a (sane) name but
> have different semantics and so should have different descriptions of
> the variables.

i think some example of this could help the discussion? do you maybe have
one where we would want different types?


> 
> >> * parameters that are used by more than 1 codec must be conventional fields
> >> ?in AVCodecContext (this is vastly more efficient in terms of access speed
> >> ?and amount of memory needed)
> >
> > Why does access speed matter for setting parameters? ?This is stupid.
> 
> +1

we all agree but its still not what i meant


> 
> This is for configuration. The long-running restriction on the
> flexibility of configuration needs to be succeeded by something such
> as is being proposed.
> 
> However, my issue with shared options in AVCodecContext is that they

> have shared default values which make no sense in various codecs, that

that can be changed ...


> their types may not be sufficient for reuse in new codecs and they

types can be changed ...


> would then either need a new variant option or need to be moved out of
> AVCC, that their ranges may differ per codec and so on. I _much_
> prefer codecs doing their own option parsing of strings.

i already asked jason, so let me ask you too, do you have some arguments
in favor of strings being parsed by duplicated parsing code in codecs
and inevitably incompatible option value systems?


> 
> >> * as a consequence of above it also must be possible to transparently move
> >> ?a name/value parameter into AVCodecContext, this can be done by extending
> >> ?AVOptions.
> 
> Why is this necessary?

it follows from the other requirements that you seem not to understand


> 
> > I don't see why AVCodecContext even needs to exist in the first place.
> > ?There is not a *single parameter* in the context that applies to all
> > codecs.
> 
> Indeed. It's big, it's awkward to alter (API/ABI issues, strictness of
> reusing variables as mentioned above) without breaking stuff and as
> Jason notes, only parts of it are relevant to each codec.

Please dont forget that API/ABI issues exist with per codec parsing too,
no single option name or value system can change [THIS INCLUDES LIBX264!]
if libx264 internally changes its values or parameters, our libx264 wraper
would have to emulate the old system until the next major version bump!

its part of the "promise" a library like libavcodec makes to its users
in relation to the version numbers


> 
> >> * If there exists an established parameter name/value for something like
> >> ?a paremeter in lame or x264 then these should in addition to the standard
> >> ?system in ffmpeg be supported.
> >
> > So you want to have two redundant systems for passing options to an
> > encoder, each with different naming schemes? ?This is a usability
> > disaster.
> 
> I hope that a sane new string parsing method (or similar) will
> supersede the current system which is difficult to learn (CLI options
> are order dependent which is unusual and confusing) and has a bad
> policy with regard to what it does automatically and what the default
> behaviour is. The default behaviour should be to produce good quality
> output for all codecs, to negotiate mismatches between input and
> output (frame rate, sample rate, resolution, pixel format, sample
> format, channels, etc.) unless no sane solution can be found or unless
> the user specifies otherwise. It should of course inform the user of
> what it is doing in case it is not desired, and it should try not to
> do anything if reasonably possible (e.g. input is 44.1kHz, output
> needs 11025Hz/22050Hz, downsample to 22050Hz; output needs 44.1Khz do
> nothing).

You throw a huge number of totally unrelated things you dont like into
the same pot, but let me say i fear there is no "heal all ill" new NIH
syndrom rewritten parser that will be better. Bugs have to be fixed
not once every 6 month a rewrite be done that ends up with new bugs
that people refuse to work on.

The significance of options ordering follows from ffmpeg.c design, it wont
change if you replace some unrelated code.

The defaults have historically been "everything disabled" for encoders,
I cant see how that is related to have fields in a struct or strings
parsed by codecs.

automatically selecting good fps, sample rate, ... is difficult and wont
happen through a parameter parser rewrite. I suggested many times methods
to improve these but no single person gave a damn, so i assume you want to
work on this now or is it just bickering? For good fps selection we need
to pass some information on what the output supports up to
av_find_stream_info() or pass alot more information down from there, of
course one can split and rename functions as well ...
without that, that is just a fps=29.9 a encoder canot know if
30, 60,30000/1001 is the best value to represent the actual input timestamps

also id like to mention that we do have a supported_samplerates array in
AVCodec, i assume you want to add code to use that?
or do you prefer to throw that away and restart from scratch with a obviously
poorer system? (poorer because doing it by parsing and correcting inside the
codec makes the actually supported rates invisible, it would be like a black
box)


> 
> > I agree that it shouldn't be committed unless it fulfills
> > requirements; the statement was rather that if we agree that it
> > satisfies the requirements but people won't stop bickering over
> > trivial aspects, I will commit it anyways.
> 
> It's unfortunate but sometimes it seems this is the way to get things
> done around here.

for the option parsing, things can be sumarized as 0% of people fixing bugs,
discussing, or coding except me and stefano and once every 6 month someone
coming up with the great idea of a rewrite.


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

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- 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/20090930/c9f03d55/attachment.pgp>