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@xxxxxxxxx> 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@xxxxxxxxx>
>>
>> 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
>>
>
