[FFmpeg-devel] [PATCH 4/8] lavu/opt: extend AVOptionRange by second value

[Michael Niedermayer]

On Fri, Feb 28, 2014 at 02:59:41AM +0100, Lukasz Marek wrote:
> On 26.02.2014 17:22, Michael Niedermayer wrote:
> >On Wed, Feb 26, 2014 at 01:55:53PM +0100, Lukasz M wrote:
> >>On 26 February 2014 12:41, Nicolas George <george at nsup.org> wrote:
> >>
> >>>L'octidi 8 ventôse, an CCXXII, Lukasz M a écrit :
> >>>>Hmm, I treated it a bit different, but I think your proposition is
> >>>correct.
> >>>>So for example for size opt value would be number of pixels. components
> >>>>would define ranges of width and height?
> >>>
> >>>Maybe I missed something, but I suspect you should completely ignore the
> >>>component_min/max part.
> >>>
> >>
> >>I'm not sure how it is supposed to be extended.
> >>It was a question to Michael, because it is not clear and both options make
> >>sense - depending how you interpret them.
> >>But my first guess was also to ignore component part
> >>
> >>
> >>>>I share your opinion. But technically this is api break. At major bump
> >>>>extra_value_min can be removed and value_min/max can be array as in my
> >>>>first patch.
> >>>>On the other hand I believe in practice it would be better to change it
> >>>>now. I doubt query_ranges is commonly used so far outside ffmpeg. When
> >>>dev
> >>>>cap api starts relying on it, users may start to use it more.
> >>>
> >>>Is there really a benefit in having a real array, i.e. being able to handle
> >>>both values at once? If not, you could just add value2_min/max and be done
> >>>with it. That has the advantage of not polluting the normal code with
> >>>"[0]".
> >>>
> >>
> >>OK, I can change it this way.
> >>
> >>
> >
> >>>Also, is it worth the complexity? IIRC, you did not respond to the option
> >>>of
> >>>leaving width and height separate, and just set width to get the
> >>>corresponding heights.
> >>>
> >>
> >>No, I haven't. Old version would work as you described (query width, set
> >>width, query height),
> >>Michael gave a hints about extending AVOptionRange struct and I think it is
> >>better than previous solution.
> >> From client point of view it seems to have less complexity.
> >
> >yes, i think keeping the client complexity in mind is important as
> >one developer had complained about the API
> >
> >the alternative to changing AVOptionRange would possibly be to add
> >a function which can take 2 or more AVOptions and returns a list of
> >their allowed values or value ranges
> >This function would then querry the "One component" style API
> >to create a list of allowed pairs/pair ranges
> >
> >That could then also be used to create widthxheight fps triplets
> >if these depend on each other
> >
> >also i just realized that AVOptionRange probably should have a
> >"multiple_of" field so that for example even width can be specified
> >without having to specify each individual value in a list
> 
> I promise it is the last one. No more from me for this thread unless
> you decide what you want :P

I think a big question is if we want to directly support vectors or
not.
Both choices are possible and i dont realy know which is better nor
do i have a real preferrance
One question in case of supporting vectors is if a 2 element limit
will ever be a problem, because its hard to raise later with this
API

also the question of how to handle other combinations matters
for example widthxheight might depend on fps or pix_fmt
It may be that the API we would need to conveniently create a
list (for exampe for presentation to the user) of the supported
combinations would end up rendering the 2 component vector support
obsoleet

and for AV_OPT_TYPE_IMAGE_SIZE, with a non vector API how would one
get the 2nd components range

and theres AV_OPT_TYPE_COLOR which is 3 component



[...]


> +    double extra_value_min1, extra_value_max1; ///< Dimensions use it to store min width.
> +    double extra_value_min2, extra_value_max2; ///< Dimensions use it to store max height.

i think if they are together in the struct, they should be arrays



> +    double multiple_of;                        ///< Valid values have to be multiple of multiple_of. Ignore if multiple_of is 0.

this can differ for each component of a vector
yuv422 generally needs to have width to be a multiple of 2 but
height is possibly odd

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140301/adc67c09/attachment.asc>