public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* Re: Skype and libv4
  2009-03-24  9:59 ` Hans de Goede
@ 2009-03-24 22:09   ` Lamarque Vieira Souza
  2009-03-24 22:29     ` Lamarque Vieira Souza
  2009-03-25  9:03     ` Hans de Goede
  0 siblings, 2 replies; 7+ messages in thread
From: Lamarque Vieira Souza @ 2009-03-24 22:09 UTC (permalink / raw)
  To: Hans de Goede; +Cc: video4linux-list

[-- Attachment #1: Type: text/plain, Size: 238 bytes --]

	Hi,

	Applying this patch to libv4l makes Skype works with my webcam without 
changing the driver. Do you think the patch is ok?

-- 
Lamarque V. Souza
http://www.geographicguide.com/brazil.htm
Linux User #57137 - http://counter.li.org/

[-- Attachment #2: libv4l-0.5.9.patch --]
[-- Type: text/x-patch, Size: 588 bytes --]

*** libv4l-0.5.9/libv4lconvert/libv4lconvert.c	2009-03-13 08:05:56.000000000 -0300
--- libv4l-0.5.9-lvs/libv4lconvert/libv4lconvert.c	2009-03-24 19:07:14.000000000 -0300
*************** static int v4lconvert_do_try_format(stru
*** 307,312 ****
--- 307,315 ----
      try_fmt = *dest_fmt;
      try_fmt.fmt.pix.pixelformat = supported_src_pixfmts[i].fmt;
  
+     /* Lamarque 24/03/2009 */
+     try_fmt.fmt.pix.field = V4L2_FIELD_ANY;
+ 
      if (!syscall(SYS_ioctl, data->fd, VIDIOC_TRY_FMT, &try_fmt))
      {
        if (try_fmt.fmt.pix.pixelformat == supported_src_pixfmts[i].fmt) {

[-- Attachment #3: Type: text/plain, Size: 164 bytes --]

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Skype and libv4
  2009-03-24 22:09   ` Skype and libv4 Lamarque Vieira Souza
@ 2009-03-24 22:29     ` Lamarque Vieira Souza
  2009-03-25  9:03     ` Hans de Goede
  1 sibling, 0 replies; 7+ messages in thread
From: Lamarque Vieira Souza @ 2009-03-24 22:29 UTC (permalink / raw)
  To: Hans de Goede; +Cc: video4linux-list


	By the way, the message "Skype Xv: No suitable overlay format found" was gone 
when I apply this patch, it was only one problem, not two. That makes the 
think that this message is misleading. It is not a Xv overlay problem, it is 
just Skype that does not recognize the format returned by the v4l2's driver 
(or libv4l in my case).

Em Tuesday 24 March 2009, Lamarque Vieira Souza escreveu:
> 	Hi,
>
> 	Applying this patch to libv4l makes Skype works with my webcam without
> changing the driver. Do you think the patch is ok?


-- 
Lamarque V. Souza
http://www.geographicguide.com/brazil.htm
Linux User #57137 - http://counter.li.org/

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Skype and libv4
  2009-03-24 22:09   ` Skype and libv4 Lamarque Vieira Souza
  2009-03-24 22:29     ` Lamarque Vieira Souza
@ 2009-03-25  9:03     ` Hans de Goede
  2009-03-25 14:17       ` Lamarque Vieira Souza
  1 sibling, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2009-03-25  9:03 UTC (permalink / raw)
  To: Lamarque Vieira Souza; +Cc: video4linux-list

On 03/24/2009 11:09 PM, Lamarque Vieira Souza wrote:
> 	Hi,
>
> 	Applying this patch to libv4l makes Skype works with my webcam without
> changing the driver. Do you think the patch is ok?
>

No it is not ok, luckily I've also read the rest of this thread, where you write:

 > 	I have found the problem. The vidioc_try_fmt_vid_cap function in the driver
 > return -EINVAL if the fmt.pix.field is different from V4L2_FIELD_ANY or
 > V4L2_FIELD_NONE. Skype seems to set this field as V4L2_FIELD_INTERLACED.
 > Because of that libv4l assumes that all destination formats (YUV420 included)
 > are invalid. Commenting this part of the driver makes Skype work and it is
 > showing pictures. YES!!! :-)
 >

What you are seeing is a bug in the driver. VIDIOC_TRY_FMT should *never*
return -EINVAL, except, and that is the only exception when it does not
support the passed in type, so v4l2_format.type is something which is not
supported, note that when vidioc_try_fmt_vid_cap is called the type is
already checked (hence the _vid_ in the function name).

When any member of fmt.pix. is not supported it should set it to something
which it does support (and the app should check what it got) so the proper
fix is to always set fmt.pix.field to V4L2_FIELD_NONE in the driver
(V4L2_FIELD_ANY is an input only value, a format returned by a driver
should never have V4L2_FIELD_ANY).

Regards,

Hans


--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Skype and libv4
  2009-03-25  9:03     ` Hans de Goede
@ 2009-03-25 14:17       ` Lamarque Vieira Souza
       [not found]         ` <49CB4D4E.6030901@redhat.com>
  0 siblings, 1 reply; 7+ messages in thread
From: Lamarque Vieira Souza @ 2009-03-25 14:17 UTC (permalink / raw)
  To: Hans de Goede; +Cc: video4linux-list

Em Wednesday 25 March 2009, Hans de Goede escreveu:
> On 03/24/2009 11:09 PM, Lamarque Vieira Souza wrote:
> > 	Hi,
> >
> > 	Applying this patch to libv4l makes Skype works with my webcam without
> > changing the driver. Do you think the patch is ok?
>
> No it is not ok, luckily I've also read the rest of this thread, where you 
write:
>  > 	I have found the problem. The vidioc_try_fmt_vid_cap function in the
>  > driver return -EINVAL if the fmt.pix.field is different from
>  > V4L2_FIELD_ANY or V4L2_FIELD_NONE. Skype seems to set this field as
>  > V4L2_FIELD_INTERLACED. Because of that libv4l assumes that all
>  > destination formats (YUV420 included) are invalid. Commenting this part
>  > of the driver makes Skype work and it is showing pictures. YES!!! :-)
>
> What you are seeing is a bug in the driver. VIDIOC_TRY_FMT should *never*
> return -EINVAL, except, and that is the only exception when it does not
> support the passed in type, so v4l2_format.type is something which is not
> supported, note that when vidioc_try_fmt_vid_cap is called the type is
> already checked (hence the _vid_ in the function name).

	Well, so there are some drivers in the kernel with this bug too (all in 
linux/drivers/media/video/ directory): meye.c and soc_camera.c (those two does 
exactly as zr364xx.c does); bt8xx/bttv-driver.c (this one returns -EINVAL if a 
combination of field (V4L2_FIELD_SEQ_TB) and flags (FORMAT_FLAGS_PLANAR) is 
invalid) or when field value is unknow, if it is unknow it really should 
return -EINVAL. Some drivers (cx18/cx18-ioctl.c, em28xx/em28xx-video.c, 
ivtv/ivtv-ioctl.c and stk-webcam.c) just set the field value to one they 
accept without even reading what value was passed to them and some 
(cx23885/cx23885-417.c and gspca/gspca.c) just ignores the field value 
returning what as passed to them. cx23885/cx23885-video.c, cx88/cx88-video.c 
and saa7134/saa7134-video.c will return -EINVAL if you pass 
V4L2_FIELD_ALTERNATE or V4L2_FIELD_SEQ_TB in the field field, if you pass 
V4L2_FIELD_ANY they will try to find one field value that is ok for them. 
s2255drv.c does more effort to validate the field value, in some cases it will 
return -EINVAL if it does not find a good value for the field field.

	Besides, vivi.c does something similar to zr364xx.c. It returns -EINVAL if 
the field value is different from V4L2_FIELD_ANY or V4L2_FIELD_INTERLACED. So 
if you pass V4L2_FIELD_NONE to vivi.c's try_fmt it will return -EINVAL too.

	What I am seeing here is that each driver behaves differently. So what is the 
correct behaviour?:

1. ignores the field value.
2. set the field value inconditionally to one value that the driver accepts.
3. only set the field value to one value that the driver accepts if 
V4L2_FIELD_ANY is passed to the driver (this one is what zr364xx.c and vivi.c 
do).

> When any member of fmt.pix. is not supported it should set it to something
> which it does support (and the app should check what it got) so the proper
> fix is to always set fmt.pix.field to V4L2_FIELD_NONE in the driver
> (V4L2_FIELD_ANY is an input only value, a format returned by a driver
> should never have V4L2_FIELD_ANY).

	So the behaviour number two is the correct one. So someone must change vivi.c 
as it is used as a guide to create new v4l2 drivers, although I still thinks 
the zr364xx.c and vivi.c do the logical thing: only changes the field value if 
the application allowed them to (when passing V4L2_FIELD_ANY).

-- 
Lamarque V. Souza
http://www.geographicguide.com/brazil.htm
Linux User #57137 - http://counter.li.org/

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Skype and libv4
       [not found]         ` <49CB4D4E.6030901@redhat.com>
@ 2009-03-26 13:36           ` Lamarque Vieira Souza
  0 siblings, 0 replies; 7+ messages in thread
From: Lamarque Vieira Souza @ 2009-03-26 13:36 UTC (permalink / raw)
  To: Hans de Goede; +Cc: video4linux-list

Em Thursday 26 March 2009, Hans de Goede escreveu:
> Hi all,
>
> Not quite, the correct behaviour is:
> "If the field value is set to an unsupported value, then set the field
> value to *a* value that the driver accepts"

	Now I get it. This webcam only accepts V4L2_FIELD_NONE, so commenting that 
part of try_fmt makes it compliant with v4l2 standard. Thank you for helping 
me with this. The zr364xx's maintainer contacted me yesterday, he is busy 
theses days, when he has more time he is going to take a look at my changes. 
With lucky the changes will be in 2.6.30. At least 2.6.29 sets the 
compat_ioctl32 automatically for all drivers, in 2.6.28.8 I had to set it in 
the driver to make Skype and mplayer (32-bit) work, one less change for the 
driver :-)

> This takes in to account certain devices can support multiple field types,
> which is the whole purpose of the field value.
>
> And yes unfortunately many many v4l drivers have various bugs in their
> implementation, in some cases I do work around driver bugs in libv4l, but
> it this case that would hurt proper use of the field value, and that is not
> acceptable, so fixing the driver is the only solution.

	Have you tried to contact the drivers' maintainers for fixing those bugs?

> Note, that the v4l2 API is pretty well documented, and the correct
> behaviour as I describe it can be found in the docs too:
> http://www.linuxtv.org/downloads/video4linux/API/V4L2_API/spec/r10944.htm
>
> And the "Return Value" section, note how EINVAL is only supposed to be
> returned up on an invalid, or unsupported type value. And also from the
> description: "Drivers should not return an error code unless the input is
> ambiguous"


-- 
Lamarque V. Souza
http://www.geographicguide.com/brazil.htm
Linux User #57137 - http://counter.li.org/

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Re: Skype and libv4
       [not found] <20090326160017.048668E03F1@hormel.redhat.com>
@ 2009-03-26 16:42 ` dean
  2009-03-27  1:31   ` Lamarque Vieira Souza
  0 siblings, 1 reply; 7+ messages in thread
From: dean @ 2009-03-26 16:42 UTC (permalink / raw)
  To: Lamarque Vieira Souza; +Cc: hdegoede, video4linux-list, Greg KH

Hi, Please see comments below:

> Subject:
> Re: Skype and libv4
> From:
> Lamarque Vieira Souza <lamarque@gmail.com>
> Date:
> Thu, 26 Mar 2009 10:36:56 -0300
> To:
> Hans de Goede <hdegoede@redhat.com>
> 
> To:
> Hans de Goede <hdegoede@redhat.com>
> CC:
> video4linux-list@redhat.com
> 
> 
> 
> Em Thursday 26 March 2009, Hans de Goede escreveu:
>> Hi all,
>>
>> Not quite, the correct behaviour is:
>> "If the field value is set to an unsupported value, then set the field
>> value to *a* value that the driver accepts"
> 
> 	Now I get it. This webcam only accepts V4L2_FIELD_NONE, so commenting that 
> part of try_fmt makes it compliant with v4l2 standard. Thank you for helping 
> me with this. The zr364xx's maintainer contacted me yesterday, he is busy 
> theses days, when he has more time he is going to take a look at my changes. 
> With lucky the changes will be in 2.6.30. At least 2.6.29 sets the 
> compat_ioctl32 automatically for all drivers, in 2.6.28.8 I had to set it in 
> the driver to make Skype and mplayer (32-bit) work, one less change for the 
> driver :-)

The lack of V4L2_FIELD_NONE caused what sort of problems in these 
applications/drivers?  Did you the driver recover without it?

> 
>> This takes in to account certain devices can support multiple field types,
>> which is the whole purpose of the field value.
>>
>> And yes unfortunately many many v4l drivers have various bugs in their
>> implementation, in some cases I do work around driver bugs in libv4l, but
>> it this case that would hurt proper use of the field value, and that is not
>> acceptable, so fixing the driver is the only solution.

Can you elaborate on the V4L drivers with bugs?  If they aren't 
identified, they won't be fixed.

> 
> 	Have you tried to contact the drivers' maintainers for fixing those bugs?
> 
>> Note, that the v4l2 API is pretty well documented, and the correct
>> behaviour as I describe it can be found in the docs too:
>> http://www.linuxtv.org/downloads/video4linux/API/V4L2_API/spec/r10944.htm
>>
>> And the "Return Value" section, note how EINVAL is only supposed to be
>> returned up on an invalid, or unsupported type value. And also from the
>> description: "Drivers should not return an error code unless the input is
>> ambiguous"
> 
> 
> 
> ------------------------------------------------------------------------
> 
> --
> video4linux-list mailing list
> Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
> https://www.redhat.com/mailman/listinfo/video4linux-list


I also have a question about the removal of videobuf_waiton in the 
patch.  Only 3 other drivers are using videobuf_waiton. Should 
videobuf_waiton be removed from them also?  I believe it was in vivi.c 
at some point, but I'll have to double check...




--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Skype and libv4
  2009-03-26 16:42 ` Re: Skype and libv4 dean
@ 2009-03-27  1:31   ` Lamarque Vieira Souza
  0 siblings, 0 replies; 7+ messages in thread
From: Lamarque Vieira Souza @ 2009-03-27  1:31 UTC (permalink / raw)
  To: dean; +Cc: hdegoede, video4linux-list, Greg KH

Em Thursday 26 March 2009, dean escreveu:
> Hi, Please see comments below:
> > Subject:
> > Re: Skype and libv4
> > From:
> > Lamarque Vieira Souza <lamarque@gmail.com>
> > Date:
> > Thu, 26 Mar 2009 10:36:56 -0300
> > To:
> > Hans de Goede <hdegoede@redhat.com>
> >
> > To:
> > Hans de Goede <hdegoede@redhat.com>
> > CC:
> > video4linux-list@redhat.com
> >
> > Em Thursday 26 March 2009, Hans de Goede escreveu:
> >> Hi all,
> >>
> >> Not quite, the correct behaviour is:
> >> "If the field value is set to an unsupported value, then set the field
> >> value to *a* value that the driver accepts"
> >
> > 	Now I get it. This webcam only accepts V4L2_FIELD_NONE, so commenting
> > that part of try_fmt makes it compliant with v4l2 standard. Thank you for
> > helping me with this. The zr364xx's maintainer contacted me yesterday, he
> > is busy theses days, when he has more time he is going to take a look at
> > my changes. With lucky the changes will be in 2.6.30. At least 2.6.29
> > sets the compat_ioctl32 automatically for all drivers, in 2.6.28.8 I had
> > to set it in the driver to make Skype and mplayer (32-bit) work, one less
> > change for the driver :-)
>
> The lack of V4L2_FIELD_NONE caused what sort of problems in these
> applications/drivers?  Did you the driver recover without it?

	Skype+libv4l did not work and let the driver in unsable state (no application 
worked until I reloaded the driver). mplayer and Kopete+libv4l work without 
changing this part of the driver because they probably passed V4L2_FIELD_ANY, 
which makes the driver return V4L2_FIELD_NONE. Skypes passes 
V4L2_FIELD_INTERLACED, which this card does not support. 

	One strange thing is that Skype returns two error messages: one about the 
v4l2 format and this one "Skype Xv: No suitable overlay format found". I think 
this message is misleading because there is no problem with Xv's overlay, it 
just the stream format from the v4l2's driver that was not recognized. I have 
seen several people on the Internet with problem with Skype and this error 
message, maybe the drivers they are using is doing the wrong thing in try_fmt 
too. meye.c is another one which probably has this problem with Skype.

> >> This takes in to account certain devices can support multiple field
> >> types, which is the whole purpose of the field value.
> >>
> >> And yes unfortunately many many v4l drivers have various bugs in their
> >> implementation, in some cases I do work around driver bugs in libv4l,
> >> but it this case that would hurt proper use of the field value, and that
> >> is not acceptable, so fixing the driver is the only solution.
>
> Can you elaborate on the V4L drivers with bugs?  If they aren't
> identified, they won't be fixed.
>
> > 	Have you tried to contact the drivers' maintainers for fixing those
> > bugs?
> >
> >> Note, that the v4l2 API is pretty well documented, and the correct
> >> behaviour as I describe it can be found in the docs too:
> >> http://www.linuxtv.org/downloads/video4linux/API/V4L2_API/spec/r10944.ht
> >>m
> >>
> >> And the "Return Value" section, note how EINVAL is only supposed to be
> >> returned up on an invalid, or unsupported type value. And also from the
> >> description: "Drivers should not return an error code unless the input
> >> is ambiguous"
> >
> > ------------------------------------------------------------------------
> >
> > --
> > video4linux-list mailing list
> > Unsubscribe
> > mailto:video4linux-list-request@redhat.com?subject=unsubscribe
> > https://www.redhat.com/mailman/listinfo/video4linux-list
>
> I also have a question about the removal of videobuf_waiton in the
> patch.  Only 3 other drivers are using videobuf_waiton. Should
> videobuf_waiton be removed from them also?  I believe it was in vivi.c
> at some point, but I'll have to double check...

	I think it should be removed. As far as I understand videobuf_waiton(&buf-
>vb, 0, 0) will block forever until the video buffer list is emptied, but if 
the driver is an unload module state nobody will empty the list. If the 
application works as expected it will empty the list before closing the device 
and everything works, but Skype was not working properly because of the 
V4L2_FIELD_INTERLACED problem, it issued some vidioc_qbuf calls but no 
vidioc_dqbuf calls reached the driver accordingly to my logs, so the dead lock 
when trying to unload the driver's module.

-- 
Lamarque V. Souza
http://www.geographicguide.com/brazil.htm
Linux User #57137 - http://counter.li.org/

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2009-03-27  1:31 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20090326160017.048668E03F1@hormel.redhat.com>
2009-03-26 16:42 ` Re: Skype and libv4 dean
2009-03-27  1:31   ` Lamarque Vieira Souza
2009-03-23 20:08 Skype and libv4l Lamarque Vieira Souza
2009-03-24  9:59 ` Hans de Goede
2009-03-24 22:09   ` Skype and libv4 Lamarque Vieira Souza
2009-03-24 22:29     ` Lamarque Vieira Souza
2009-03-25  9:03     ` Hans de Goede
2009-03-25 14:17       ` Lamarque Vieira Souza
     [not found]         ` <49CB4D4E.6030901@redhat.com>
2009-03-26 13:36           ` Lamarque Vieira Souza

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox