[Ffmpeg-devel] [PATCH] x264 interface update

[Robert Swain]

Hello,
On 12/18/05, Michael Niedermayer <michaelni at gmx.at> wrote:
> Hi
> On Sun, Dec 18, 2005 at 04:36:00AM -0000, Robert Swain wrote:
> > Hello,

[...]

> > - Why do 'flags2' flags require an argument despite the argument not
> > appearing to have a function? (I tried -brdo 0 and -brdo 1, and both enabled
> > brdo)
>
> because the option parser isnt perfect, send patch, furthermore flags1 need an
> argument too currently unless they havnt been ported to the new system

What do you mean about flags1 being ported?

> [...]

> > - Why does setting a float in the default value parameter of the
> > libavcodec/utils.c options array not actually set the default value to that
> > value?
> > I used GDB to print the value of avctx->ratetol for example. In the value in
> > options[] (libavcodec/utils.c) was 1, but in AVCodecContext it was 0. Should
> > avcodec_get_context_defaults() be used for setting default values?
>
> avcodec_get_context_defaults() could _maybe_ be changed to extract and set all
> the defaults for AVCodecContext (patch welcome assuming this has no sideeffects)

I wasn't suggesting that this be done. I just meant to enquire as to
whether putting s->ratetol = 1.0; in avcodec_get_context_defaults()
would produce the correct default value. I did some further testing
and in fact the default values in options[] are completely ignored.
Why are they there? Legacy? A hint at the default? Not updated to use
them yet?

> > - What is the accepted method for altering the default value of a flag? A
>
> the default of the "flags" or "flags2" entry should do, for example:
> {"flags", NULL, OFFSET(flags), FF_OPT_TYPE_FLAGS, CODEC_FLAG_4MV | ... , INT_MIN, INT_MAX, V|A|E|D, "flags"},

OK. As mentioned above, it didn't work. I put all the significant
defaults in avcodec_get_context_defaults() and it works fine.

> > couple of the flags I added should be enabled by default. Furthermore, what
> > is the accepted method of disabling a flag? Once a flag is enabled by
> > default, how can it be disabled?
>
> enable: -4mv foobar or -flags +4mv
> disable: -flags -4mv

OK. I see also that one can chain flags together.

> you could also look at opt.c and ad -no4mv style support

Maybe in a later patch depending on which you deem more desirable:
-<flag> 0/1 or -<flag> -no<flag>. Just give me the nod and if no one
else does, when I get bored I'll do it. ;)

> [...]

> >@@ -834,10 +844,6 @@
> >      */
> >     int delay;
> >
> >-    /* - encoding parameters */
> >-    float qcompress;  ///< amount of qscale change between easy & hard scenes (0.0-1.0)
> >-    float qblur;      ///< amount of qscale smoothing over time (0.0-1.0)
> >-
> >     /**
> >      * minimum quantizer.
> >      * - encoding: set by user.
>
> /me looks left, looks right where are the ABI trolls? only trolling like
> always but not looking at submitted patches but a month later they crawl
> out and complain again

I didn't mean to remove the encoding parameters line and I moved the
others to improve homogeneity within the options. I've returned them
to their original state.

> CODEC_FLAG2_CHROMA_ME is a flag in the cmp functions in lavc

I didn't want to mess with the cmp stuff but I've changed it so that
it uses the current chroma.

> cqp looks a little redundant (CODEC_FLAG_QSCALE, ...) or does this have more
> then on/off?

qscale has a different meaning to qp so passing qps into qscale is off
the books. Supplying a qscale when you want a qp is horrible for a
user in my opinion. That leaves a new variable. I thought a new
variable would be more acceptable than using qscale for passing
through qp values, if you disagree then give me a poke.

> loglevel and ratetol seems redundant too

OK. Reusing variables instead.

> i dont like the char* style variables at all, they should be flags or enums
> IMHO

OK. directpred is now an int. partitions is a flag and has
accompanying flag variables such as -partitions +parti4x4+partp8x8
etc. I think I've dealt with everything even if I've neglected to
mention it here.

The cqm* variables don't work. I don't know why, but all the strings
are coming through as null. The -cqm option is the most important in
my opinion, because JM format cqm file input is the most convenient.
Otherwise I think these have to be char * variables. :/

Hopefully that's everything sorted apart from the mentioned problems.
Thanks for your assistance.

[...]

> --
> Michael

Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpeg_x264_update.1.diff
Type: application/octet-stream
Size: 18985 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20051220/7d78b492/attachment.obj>