Re: [PATCH 5/6] [media] V4L: Add VP8 encoder controls

[Sylwester Nawrocki <s.nawrocki@samsung.com>]

Hi Arun,

On 06/14/2013 11:26 AM, Arun Kumar K wrote:
> Hi Sylwester,
> 
>>> +     static const char * const vpx_num_partitions[] = {
>>> +             "1 partition",
>>> +             "2 partitions",
>>> +             "4 partitions",
>>> +             "8 partitions",
>>> +             NULL,
>>> +     };
>>> +     static const char * const vpx_num_ref_frames[] = {
>>> +             "1 reference frame",
>>> +             "2 reference frame",
>>> +             NULL,
>>> +     };
>>
>> Have you considered using V4L2_CTRL_TYPE_INTEGER_MENU control type for this ?
>> One example is V4L2_CID_ISO_SENSITIVITY control.
>>
> 
> If I understand correctly, V4L2_CTRL_TYPE_INTEGER_MENU is used for
> controls where
> the driver / IP can support different values depending on its capabilities.

No, not really, it just happens there is no INTEGER_MENU control with standard
values yet. I think there are some (minor) changes needed in the v4l2-ctrls
code to support INTEGER_MENU control with standard menu items.

> But here VP8 standard supports only 4 options for no. of partitions
> that is 1, 2, 4 and 8.

I think such a standard menu list should be defined in v4l2-ctrls.c then.

> Also for number of ref frames, the standard allows only the options 1,
> 2 and 3 which
> cannot be extended more. So is it correct to use INTEGER_MENU control here and
> let the driver define the values?

If this is standard then the core should define available menu items. But
it seems more appropriate for me to use INTEGER_MENU. I'd like to hear other
opinions though.

Regards,
Sylwester