On 27/03/17 20:09, Mauro Carvalho Chehab wrote:
> Em Mon, 27 Mar 2017 12:19:51 -0300
> Helen Koike <helen.ko...@collabora.co.uk> escreveu:
> 
>> Hi Sakari,
>>
>> On 2017-03-26 10:31 AM, Sakari Ailus wrote:
>>> Hi Helen,
>>>
>>> ...  
>>>> +static int vimc_cap_enum_input(struct file *file, void *priv,
>>>> +                         struct v4l2_input *i)
>>>> +{
>>>> +  /* We only have one input */
>>>> +  if (i->index > 0)
>>>> +          return -EINVAL;
>>>> +
>>>> +  i->type = V4L2_INPUT_TYPE_CAMERA;
>>>> +  strlcpy(i->name, "VIMC capture", sizeof(i->name));
>>>> +
>>>> +  return 0;
>>>> +}
>>>> +
>>>> +static int vimc_cap_g_input(struct file *file, void *priv, unsigned int 
>>>> *i)
>>>> +{
>>>> +  /* We only have one input */
>>>> +  *i = 0;
>>>> +  return 0;
>>>> +}
>>>> +
>>>> +static int vimc_cap_s_input(struct file *file, void *priv, unsigned int i)
>>>> +{
>>>> +  /* We only have one input */
>>>> +  return i ? -EINVAL : 0;
>>>> +}  
>>>
>>> You can drop the input IOCTLs altogether here. If you had e.g. a TV
>>> tuner, it'd be the TV tuner driver's responsibility to implement them.
>>>  
>>
>> input IOCTLs seems to be mandatory from v4l2-compliance when capability 
>> V4L2_CAP_VIDEO_CAPTURE is set (which is the case):
>>
>> https://git.linuxtv.org/v4l-utils.git/tree/utils/v4l2-compliance/v4l2-test-input-output.cpp#n418
>>
>> https://git.linuxtv.org/v4l-utils.git/tree/utils/v4l2-compliance/v4l2-compliance.cpp#n989
> 
> The V4L2 spec doesn't actually define what's mandatory and what's
> optional. The idea that was agreed on one of the media summits
> were to define a set of profiles for different device types,
> matching the features required by existing applications to work,
> but this was never materialized.
> 
> So, my understanding is that any driver can implement
> any V4L2 ioctl.
> 
> Yet, some applications require enum/get/set inputs, or otherwise
> they wouldn't work. It is too late to change this behavior. 
> So, either the driver or the core should implement those
> ioctls, in order to avoid breaking backward-compatibility.

The closest we have to determining which ioctls are mandatory or not is
v4l2-compliance. That said, v4l2-compliance is actually a bit more strict
in some cases than the spec since some ioctls are optional in the spec, but
required in v4l2-compliance for the simple reason that there is no reason
for drivers NOT to implement those ioctls.

However, the v4l2-compliance test was never written for MC devices. It turns
out that it works reasonably well as long as a working pipeline is configured
first, but these input ioctls are a bit iffy.

There are really two options: don't implement them, or implement it as a single
input. Multiple inputs make no sense for MC devices: the video node is the
endpoint of a video pipeline, you never switch 'inputs' there.

The way the input ioctls are implemented here would fit nicely for an MC
device IMHO.

So should we define these ioctls or not?

I am inclined to define them for the following reasons:

- Some applications expect them, so adding them to the driver costs little but
  allows these applications to work, provided the correct pipeline is configured
  first.

- If a plugin is needed, then that plugin can always override these ioctls and
  for different 'inputs' reconfigure the pipeline.

I really don't see implementing this as a problem. Reporting that an MC video 
node
has a "VIMC capture" input seems perfectly reasonable to me.

Regards,

        Hans

Reply via email to