Re: partial revert of "uvcvideo: set error_idx properly"

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 01/27/2013 03:06 PM, Mauro Carvalho Chehab wrote:
> Em Fri, 25 Jan 2013 11:40:13 +0100
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>
>> On Fri January 25 2013 10:51:57 Hans de Goede wrote:
>>> <modified the CC list to be more appropriate>
>>>
>>> Hi,
>>>
>>> On 12/25/2012 05:56 AM, Mauro Carvalho Chehab wrote:
>>>
>>>> The pwc driver can currently return -ENOENT at VIDIOC_S_FMT ioctl. This
>>>> doesn't seem right. Instead, it should be getting the closest format to
>>>> the requested one and return 0, passing the selected format back to
>>>> userspace, just like the other drivers do. I'm c/c Hans de Goede for him
>>>> to take a look on it.
>>>
>>> I've been looking into this today, and the ENOENT gets returned by
>>> pwc_set_video_mode and through that by:
>>> 1) Device init
>>> 2) VIDIOC_STREAMON
>>> 3) VIDIOC_S_PARM
>>> 4) VIDIOC_S_FMT
>>>
>>> But only when the requested width + height + pixelformat is an
>>> unsupported combination, and it being a supported combination
>>> already gets enforced by a call to pwc_get_size in
>>> pwc_vidioc_try_fmt, which also gets called from pwc_s_fmt_vid_cap
>>> before it does anything else.
>>>
>>> So the ENOENT can only happen on some internal driver error,
>>> I'm open for suggestions for a better error code to return in
>>> this case.
>>
>> Perhaps returning EINVAL but adding a WARN_ON would be a good compromise.
>>
>>> What I did notice is that pwc_vidioc_try_fmt returns EINVAL when
>>> an unsupported pixelformat is requested. IIRC we agreed that the
>>> correct behavior in this case is to instead just change the
>>> pixelformat to a default format, so I'll write a patch fixing
>>> this.
>>
>> There are issues with that idea in the case of TV capture cards, since
>> some important apps (tvtime and mythtv to a lesser extent) assume -EINVAL
>> in the case of unsupported pixelformats.
>>
>> Webcam apps can't assume that since gspca never returned -EINVAL, so I
>> think it should be OK to fix this in pwc, but Mauro may disagree.
>
> It is known that both MythTV and tvtime have issues.
>
> Well, I don't think that MythTV has webcam support. So, it will likely
> fail with pwc anyway, as it doesn't have a tuner. So, webcam drivers don't
> need to care with breaking anything on it.
>
> Tvtime can work with webcams, if they provide a resolution that it is
> compatible with it and if it supports UVYV or YUYV. This is not the case
> of pwc, that seems to support only pwc proprietary formats and yuv420.
>
> So, neither tvtime or MythTV currently works with pwc cameras.
>
> However, the issue is a little more complex, as we don't really know if
> there aren't any other applications that use a code similar to tvtime
> or MythYV.

I understand. A patch to change pwc to the behavior discussed in Barcelona
(so changing the format to a supported one rather then returning -EINVAL),
is part of my last pull-req, I'll leave it up to you whether you will take
it or not :)

If you don't take it I'll drop it from my tree, so that it does not
show up again in my next pull-req.

Regards,

Hans



>
> Regards,
> Mauro
>