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

[Michael Niedermayer]

On Wed, Sep 30, 2009 at 01:48:43AM -0700, Jason Garrett-Glaser wrote:
> On Wed, Sep 30, 2009 at 1:42 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Wed, Sep 30, 2009 at 01:24:10AM -0700, Jason Garrett-Glaser wrote:
> >> On Wed, Sep 30, 2009 at 1:17 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >> > On Tue, Sep 29, 2009 at 05:13:24PM -0700, Jason Garrett-Glaser wrote:
> >> >> > As Mans noted maybe this is not necessary.
> >> >>
> >> >> How are we supposed to know whether to set the per-codec defaults, or
> >> >> whether to use the avcodec global defaults?
> >> >
> >> > The obvious way is to use per codec defaults where available and global
> >> > defaults where not.
> >> >
> >> > avcodec_get_context_defaults2() could trvially do that if it was given the
> >> > codec id, that is no matter which APIs and implementations are used ...
> >> >
> >> >
> >> >>
> >> >> We need some way of signalling which API we are using.
> >> >
> >> > there should be just one API
> >> >
> >> > Note, ive not read the other replies in this thread, this one just looked
> >> > small and easy to awnser
> >>
> >> There should be just one API, but I think completely rewriting the
> >> current API all at once will be very difficult, especially since only
> >> the authors of codecs will really be able to accurately set defaults
> >> and option sets. ?The best approach IMO is to do it gradually or else
> >> it will never happen at all.
> >
> > the current public API, that is
> >
> > av_set_string(codec_context, "bitrate", "123kb");
> >
> > does not need a rewrite or change, its implementation might need changes
> > and so might the interface between codecs and AVOptions
> > but none of that would be vissible to an end user application ...
> 
> Ah, so you propose changing the set_string option to set per-codec
> options instead of global options... I like this idea.

approximately yes ...


> 
> Still, while the public API doesn't need a rewrite, the internals
> will,

the internals will need changes


> and it will be all-or-nothing; 

i dont see that as needed


>every codec will have to have a
> help(), an option handler, and so forth.  How do you propose that we
> implement this new system for only *some* codecs so that it can be
> incrementally added?

i need to reread the mails from the past [someone got the $subs?]
but without that i can think of an obvious solution to a few of the
issues ...
We currently have the AVCodecContext context and a private context
(AVCodecContext->priv_data that being X264Context in x264s case)
if we now add an AVOptions array for X264Context and make its first
element an AVClass then av_set_string() could be called on priv_data
and libx264.c would have full control over, help, default, min, max
and type of each of these parameters.
if we further could allocate priv_data in avcodec_get_context_defaults2()
then we trivially could also initialize it to defaults with a single
av_opt_set_defaults2() call
that would look like

void avcodec_get_context_defaults2(AVCodecContext *s, enum CodecId codec_id){
    ...
    AVCodec codec= find_codec(codec_id);
    s->codec= codec;
    s->priv_data= av_mallocz(codec->priv_data_size);
    if(codec->priv_class){
        s->priv_data->av_class= codec->priv_class;
        av_opt_set_defaults2(s->priv_data, flags, flags);
    }
}

(above would need a priv_class be addeded to AVCodec)


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato
-------------- 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/76f6e7b3/attachment.pgp>