[PATCH] dev-capture.rst/dev-output.rst: video standards ioctls are optional

[PATCH] dev-capture.rst/dev-output.rst: video standards ioctls are optional

[Hans Verkuil]

The documentation for video capture and output devices claims that the video standard
ioctls are required. This is not the case, they are only required for PAL/NTSC/SECAM
type inputs and outputs. Sensors do not implement this at all and e.g. HDMI inputs
implement the DV Timings ioctls.

Just drop the mention of 'video standard' ioctls.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
diff --git a/Documentation/media/uapi/v4l/dev-capture.rst b/Documentation/media/uapi/v4l/dev-capture.rst
index 32b32055d070..4218742ab5d9 100644
--- a/Documentation/media/uapi/v4l/dev-capture.rst
+++ b/Documentation/media/uapi/v4l/dev-capture.rst
@@ -42,8 +42,8 @@ Video capture devices shall support :ref:`audio input <audio>`,
 :ref:`tuner`, :ref:`controls <control>`,
 :ref:`cropping and scaling <crop>` and
 :ref:`streaming parameter <streaming-par>` ioctls as needed. The
-:ref:`video input <video>` and :ref:`video standard <standard>`
-ioctls must be supported by all video capture devices.
+:ref:`video input <video>` ioctls must be supported by all video
+capture devices.


 Image Format Negotiation
diff --git a/Documentation/media/uapi/v4l/dev-output.rst b/Documentation/media/uapi/v4l/dev-output.rst
index 25ae8ec96fdf..342eb4931f5c 100644
--- a/Documentation/media/uapi/v4l/dev-output.rst
+++ b/Documentation/media/uapi/v4l/dev-output.rst
@@ -40,8 +40,8 @@ Video output devices shall support :ref:`audio output <audio>`,
 :ref:`modulator <tuner>`, :ref:`controls <control>`,
 :ref:`cropping and scaling <crop>` and
 :ref:`streaming parameter <streaming-par>` ioctls as needed. The
-:ref:`video output <video>` and :ref:`video standard <standard>`
-ioctls must be supported by all video output devices.
+:ref:`video output <video>` ioctls must be supported by all video
+output devices.


 Image Format Negotiation

Re: [PATCH] dev-capture.rst/dev-output.rst: video standards ioctls are optional

[Mauro Carvalho Chehab]

Em Wed, 29 Mar 2017 09:56:47 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> The documentation for video capture and output devices claims that the video standard
> ioctls are required. This is not the case, they are only required for PAL/NTSC/SECAM
> type inputs and outputs. Sensors do not implement this at all and e.g. HDMI inputs
> implement the DV Timings ioctls.
> 
> Just drop the mention of 'video standard' ioctls.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

This is an API change that has the potential of breaking userspace.

In the past, several applications were failing if VIDIOC_ENUMSTD ioctl is
not implemented. So, I remember we had this discussion before, but I don't
remember the dirty details anymore.

Yet, looking at the code, it seems that we ended by making VIDIOC_ENUMSTD
mandatory and implemented at the core. So, V4L2 core will make this
ioctl available for all drivers. The core implementattion will, however, 
return -ENODATA  if the driver doesn't set video_device.tvnorms, indicating
that standard video timings are not supported.

So, instead of the enclosed patch, the documentation should mention the
standard ioctls, saying that G_STD/S_STD are optional, and ENUMSTD is
mandatory. 

We could include a note about it may return -ENODATA, although the ENUMSTD
documentation already states that it returns -ENODATA:
	https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/vidioc-enumstd.html

Regards,
Mauro

> ---
> diff --git a/Documentation/media/uapi/v4l/dev-capture.rst b/Documentation/media/uapi/v4l/dev-capture.rst
> index 32b32055d070..4218742ab5d9 100644
> --- a/Documentation/media/uapi/v4l/dev-capture.rst
> +++ b/Documentation/media/uapi/v4l/dev-capture.rst
> @@ -42,8 +42,8 @@ Video capture devices shall support :ref:`audio input <audio>`,
>  :ref:`tuner`, :ref:`controls <control>`,
>  :ref:`cropping and scaling <crop>` and
>  :ref:`streaming parameter <streaming-par>` ioctls as needed. The
> -:ref:`video input <video>` and :ref:`video standard <standard>`
> -ioctls must be supported by all video capture devices.
> +:ref:`video input <video>` ioctls must be supported by all video
> +capture devices.
> 
> 
>  Image Format Negotiation
> diff --git a/Documentation/media/uapi/v4l/dev-output.rst b/Documentation/media/uapi/v4l/dev-output.rst
> index 25ae8ec96fdf..342eb4931f5c 100644
> --- a/Documentation/media/uapi/v4l/dev-output.rst
> +++ b/Documentation/media/uapi/v4l/dev-output.rst
> @@ -40,8 +40,8 @@ Video output devices shall support :ref:`audio output <audio>`,
>  :ref:`modulator <tuner>`, :ref:`controls <control>`,
>  :ref:`cropping and scaling <crop>` and
>  :ref:`streaming parameter <streaming-par>` ioctls as needed. The
> -:ref:`video output <video>` and :ref:`video standard <standard>`
> -ioctls must be supported by all video output devices.
> +:ref:`video output <video>` ioctls must be supported by all video
> +output devices.
> 
> 
>  Image Format Negotiation



Thanks,
Mauro

Re: [PATCH] dev-capture.rst/dev-output.rst: video standards ioctls are optional

[Hans Verkuil]

On 04/10/2017 12:21 PM, Mauro Carvalho Chehab wrote:
> Em Wed, 29 Mar 2017 09:56:47 +0200
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> The documentation for video capture and output devices claims that the video standard
>> ioctls are required. This is not the case, they are only required for PAL/NTSC/SECAM
>> type inputs and outputs. Sensors do not implement this at all and e.g. HDMI inputs
>> implement the DV Timings ioctls.
>>
>> Just drop the mention of 'video standard' ioctls.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This is an API change that has the potential of breaking userspace.
> 
> In the past, several applications were failing if VIDIOC_ENUMSTD ioctl is
> not implemented. So, I remember we had this discussion before, but I don't
> remember the dirty details anymore.
> 
> Yet, looking at the code, it seems that we ended by making VIDIOC_ENUMSTD
> mandatory and implemented at the core. So, V4L2 core will make this
> ioctl available for all drivers. The core implementattion will, however, 
> return -ENODATA  if the driver doesn't set video_device.tvnorms, indicating
> that standard video timings are not supported.
> 
> So, instead of the enclosed patch, the documentation should mention the
> standard ioctls, saying that G_STD/S_STD are optional, and ENUMSTD is
> mandatory. 

I don't think so. In v4l2-dev.c ENUMSTD is only enabled if the driver supports
the s_std ioctl:

        if (is_vid || is_vbi || is_tch) {
                /* ioctls valid for video or vbi */
                if (ops->vidioc_s_std)
                        set_bit(_IOC_NR(VIDIOC_ENUMSTD), valid_ioctls);

And in case you are wondering: if you have two inputs, one SDTV and one HDTV, then
you have both s_std and s_dv_timings ioctls and if you switch to the HDTV input,
then tvnorms is set to 0, causing ENUMSTD to return -ENODATA. If you switch back,
then the driver will fill in tvnorms to something non-0.

Regards,

	Hans

> 
> We could include a note about it may return -ENODATA, although the ENUMSTD
> documentation already states that it returns -ENODATA:
> 	https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/vidioc-enumstd.html
> 
> Regards,
> Mauro
> 
>> ---
>> diff --git a/Documentation/media/uapi/v4l/dev-capture.rst b/Documentation/media/uapi/v4l/dev-capture.rst
>> index 32b32055d070..4218742ab5d9 100644
>> --- a/Documentation/media/uapi/v4l/dev-capture.rst
>> +++ b/Documentation/media/uapi/v4l/dev-capture.rst
>> @@ -42,8 +42,8 @@ Video capture devices shall support :ref:`audio input <audio>`,
>>  :ref:`tuner`, :ref:`controls <control>`,
>>  :ref:`cropping and scaling <crop>` and
>>  :ref:`streaming parameter <streaming-par>` ioctls as needed. The
>> -:ref:`video input <video>` and :ref:`video standard <standard>`
>> -ioctls must be supported by all video capture devices.
>> +:ref:`video input <video>` ioctls must be supported by all video
>> +capture devices.
>>
>>
>>  Image Format Negotiation
>> diff --git a/Documentation/media/uapi/v4l/dev-output.rst b/Documentation/media/uapi/v4l/dev-output.rst
>> index 25ae8ec96fdf..342eb4931f5c 100644
>> --- a/Documentation/media/uapi/v4l/dev-output.rst
>> +++ b/Documentation/media/uapi/v4l/dev-output.rst
>> @@ -40,8 +40,8 @@ Video output devices shall support :ref:`audio output <audio>`,
>>  :ref:`modulator <tuner>`, :ref:`controls <control>`,
>>  :ref:`cropping and scaling <crop>` and
>>  :ref:`streaming parameter <streaming-par>` ioctls as needed. The
>> -:ref:`video output <video>` and :ref:`video standard <standard>`
>> -ioctls must be supported by all video output devices.
>> +:ref:`video output <video>` ioctls must be supported by all video
>> +output devices.
>>
>>
>>  Image Format Negotiation
> 
> 
> 
> Thanks,
> Mauro
> 

Re: [PATCH] dev-capture.rst/dev-output.rst: video standards ioctls are optional

[Hans Verkuil]

On 04/10/2017 12:36 PM, Hans Verkuil wrote:
> On 04/10/2017 12:21 PM, Mauro Carvalho Chehab wrote:
>> Em Wed, 29 Mar 2017 09:56:47 +0200
>> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>>
>>> The documentation for video capture and output devices claims that the video standard
>>> ioctls are required. This is not the case, they are only required for PAL/NTSC/SECAM
>>> type inputs and outputs. Sensors do not implement this at all and e.g. HDMI inputs
>>> implement the DV Timings ioctls.
>>>
>>> Just drop the mention of 'video standard' ioctls.
>>>
>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> This is an API change that has the potential of breaking userspace.
>>
>> In the past, several applications were failing if VIDIOC_ENUMSTD ioctl is
>> not implemented. So, I remember we had this discussion before, but I don't
>> remember the dirty details anymore.
>>
>> Yet, looking at the code, it seems that we ended by making VIDIOC_ENUMSTD
>> mandatory and implemented at the core. So, V4L2 core will make this
>> ioctl available for all drivers. The core implementattion will, however, 
>> return -ENODATA  if the driver doesn't set video_device.tvnorms, indicating
>> that standard video timings are not supported.
>>
>> So, instead of the enclosed patch, the documentation should mention the
>> standard ioctls, saying that G_STD/S_STD are optional, and ENUMSTD is
>> mandatory. 
> 
> I don't think so. In v4l2-dev.c ENUMSTD is only enabled if the driver supports
> the s_std ioctl:
> 
>         if (is_vid || is_vbi || is_tch) {
>                 /* ioctls valid for video or vbi */
>                 if (ops->vidioc_s_std)
>                         set_bit(_IOC_NR(VIDIOC_ENUMSTD), valid_ioctls);
> 
> And in case you are wondering: if you have two inputs, one SDTV and one HDTV, then
> you have both s_std and s_dv_timings ioctls and if you switch to the HDTV input,
> then tvnorms is set to 0, causing ENUMSTD to return -ENODATA. If you switch back,
> then the driver will fill in tvnorms to something non-0.

Note that v4l2-compliance will verify that you can't enumerate standards if the
input/output doesn't indicate STD support. So this patch is really correct.

Regards,

	Hans

> 
> Regards,
> 
> 	Hans
> 
>>
>> We could include a note about it may return -ENODATA, although the ENUMSTD
>> documentation already states that it returns -ENODATA:
>> 	https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/vidioc-enumstd.html
>>
>> Regards,
>> Mauro
>>
>>> ---
>>> diff --git a/Documentation/media/uapi/v4l/dev-capture.rst b/Documentation/media/uapi/v4l/dev-capture.rst
>>> index 32b32055d070..4218742ab5d9 100644
>>> --- a/Documentation/media/uapi/v4l/dev-capture.rst
>>> +++ b/Documentation/media/uapi/v4l/dev-capture.rst
>>> @@ -42,8 +42,8 @@ Video capture devices shall support :ref:`audio input <audio>`,
>>>  :ref:`tuner`, :ref:`controls <control>`,
>>>  :ref:`cropping and scaling <crop>` and
>>>  :ref:`streaming parameter <streaming-par>` ioctls as needed. The
>>> -:ref:`video input <video>` and :ref:`video standard <standard>`
>>> -ioctls must be supported by all video capture devices.
>>> +:ref:`video input <video>` ioctls must be supported by all video
>>> +capture devices.
>>>
>>>
>>>  Image Format Negotiation
>>> diff --git a/Documentation/media/uapi/v4l/dev-output.rst b/Documentation/media/uapi/v4l/dev-output.rst
>>> index 25ae8ec96fdf..342eb4931f5c 100644
>>> --- a/Documentation/media/uapi/v4l/dev-output.rst
>>> +++ b/Documentation/media/uapi/v4l/dev-output.rst
>>> @@ -40,8 +40,8 @@ Video output devices shall support :ref:`audio output <audio>`,
>>>  :ref:`modulator <tuner>`, :ref:`controls <control>`,
>>>  :ref:`cropping and scaling <crop>` and
>>>  :ref:`streaming parameter <streaming-par>` ioctls as needed. The
>>> -:ref:`video output <video>` and :ref:`video standard <standard>`
>>> -ioctls must be supported by all video output devices.
>>> +:ref:`video output <video>` ioctls must be supported by all video
>>> +output devices.
>>>
>>>
>>>  Image Format Negotiation
>>
>>
>>
>> Thanks,
>> Mauro
>>
>