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

[Robert Swain]

2009/9/30 Michael Niedermayer <michaelni at gmx.at>:
> 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?

Aside from what others have mentioned:

'-me_range          <int>   E.V.. limit motion vectors range (1023 for
DivX player)'

This variable has different meaning for x264:

'--merange <integer>     Maximum motion vector search range [16]'

The quality option was another of which I was thinking. And then
things which are flags which get extended to have multiple levels like
trellis and so on.

>> >> * 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

OK...*

>> 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 ...

Indeed.

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

Indeed. But, for example, when adding new codecs which have
multiple-level (e.g. 0 - off, 1 - low, 2 - medium, 3 - high ...)
variables versus existing parameters that are flags, changing the type
of the variable means API breakage unless one keeps both methods in
place and translates one to the other. If the variables are defined
within the codec scope, it could be added without having to alter the
variable type for other codecs and so no breakage would occur.

>> 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?

The code need not be duplicated. The string parsing could be coded
such that it can be shared between all codecs.

Passing a string simplifies the API. Rather than the external caller
requiring lots of lines of assignment to individual API variables, one
can create a single string and pass the pointer to it. This seems to
work very neatly for x264, it seems to be very flexible and so it
seems like a good idea to me.

What do you mean by inevitably incompatible? Variables with the same
meaning but lots of different names? That shouldn't happen if we're
anything like as strict as we have been for variables under the
current regime.

>> >> * 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

* so what did you mean?

>> > 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

Yes, noted.

>> >> * 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.

Of course. Is it really once every six months that a different rewrite
is proposed though? Or is it rather that there is distaste for the
current method, no significant change has been enacted and so once
every six months the same problem rears its head for another round of
discussion?

You don't need to answer that, I completely agree that there's no
point reinventing the wheel repeatedly when no one is bothered to work
on improving just one idea to a usable state.

I apologise for throwing in a lot of unrelated things to the
discussion, they're just things that bug me and when considering
fixing up per-codec defaults, we were also talking about making the
CLI ffmpeg generally more usable.

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

Of course.

> 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.

It isn't, but I would request a policy change if this is considered policy.

> 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?

Do you have any links to or pointers to find your suggestions? I would
like to see them as I expect your suggestions are good. I apologise if
I did not see them or pick up on them in the past.

> 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

FPS negotiation is indeed a tough one. Especially if we might consider
that an output codec does not support interlaced encoding and
deinterlacing is desirable. Dealing with telecine and interlacing may
not be feasible.

> 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?

I want some sample rate conversion code that works for any number of
channels and that has SIMD for cases that can be optimised. Why isn't
there single channel resampling that can be iterated for N channels?
Or is there but it's just not hooked up in ffmpeg.c?

I'm also interested in channel mixing with preset coefficients for
Dolby Pro Logic II and similar.

> 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)

The comments were, as noted, unrelated. Option parsing won't fix these
ills and I didn't mean to suggest it should or could. They're just
issues that are on my mind and I wanted to voice them. This was the
thread that was born from discussions on IRC so it was relevant in my
mind, but I see it is not so in this thread.

>> > 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.

Again, if the great idea of a rewrite is the same idea that recurs and
it does solve the issues of contention, then maybe it's good that it
keeps reappearing.

Regards,
Rob