[PATCH] qcap: fix recent regression that limited Wine's v4l2 support to devices that read()
Damjan Jovanovic
damjan.jov at gmail.com
Sat Apr 20 12:29:28 CDT 2019
On Sat, Apr 20, 2019 at 7:08 PM Zebediah Figura <z.figura12 at gmail.com>
wrote:
> On 4/20/19 11:55 AM, Damjan Jovanovic wrote:
> >
> >
> > On Sat, Apr 20, 2019 at 6:07 PM Zebediah Figura <z.figura12 at gmail.com
> > <mailto:z.figura12 at gmail.com>> wrote:
> >
> > On 4/20/19 10:56 AM, Damjan Jovanovic wrote:
> > >
> > >
> > > On Sat, Apr 20, 2019 at 5:44 PM Zebediah Figura
> > <z.figura12 at gmail.com <mailto:z.figura12 at gmail.com>
> > > <mailto:z.figura12 at gmail.com <mailto:z.figura12 at gmail.com>>>
> wrote:
> > >
> > > On 4/20/19 10:31 AM, Damjan Jovanovic wrote:
> > > >
> > > >
> > > > On Sat, Apr 20, 2019 at 4:52 PM Zebediah Figura
> > > <z.figura12 at gmail.com <mailto:z.figura12 at gmail.com>
> > <mailto:z.figura12 at gmail.com <mailto:z.figura12 at gmail.com>>
> > > > <mailto:z.figura12 at gmail.com <mailto:z.figura12 at gmail.com>
> > <mailto:z.figura12 at gmail.com <mailto:z.figura12 at gmail.com>>>> wrote:
> > > >
> > > > On 4/20/19 8:15 AM, Damjan Jovanovic wrote:
> > > > > Recent changes to qcap/v4l.c resulted in the
> > removal of mmap()
> > > > > for v4l2 devices, but read() is optional for device
> > > drivers and
> > > > > some don't support it. This isn't a major problem
> > with the
> > > > > presense of libv4l2 though, as it can emulate
> > read() on top of
> > > > > mmap(). However the code checks whether the device
> > can read(),
> > > > > and if not, doesn't even try to use it, even though
> > it can.
> > > > >
> > > > > Fix this by only warning that read() is being
> > emulated, and
> > > > > continuing with libv4l2 if available.
> > > > >
> > > > > Also clarifies the logging.
> > > > >
> > > > > Signed-off-by: Damjan Jovanovic
> > <damjan.jov at gmail.com <mailto:damjan.jov at gmail.com>
> > > <mailto:damjan.jov at gmail.com <mailto:damjan.jov at gmail.com>>
> > > > <mailto:damjan.jov at gmail.com
> > <mailto:damjan.jov at gmail.com> <mailto:damjan.jov at gmail.com
> > <mailto:damjan.jov at gmail.com>>>>
> > > > > ---
> > > > > dlls/qcap/v4l.c | 13 ++++++++-----
> > > > > 1 file changed, 8 insertions(+), 5 deletions(-)
> > > > >
> > > > >
> > > > >
> > > >
> > > > This doesn't seem right; libv4l2 should massage the
> > result of
> > > > VIDIOC_QUERYCAP to include V4L2_CAP_READWRITE. See
> > > >
> > >
> > <
> https://git.linuxtv.org/v4l-utils.git/tree/lib/libv4l2/libv4l2.c#n1210>.
> > > >
> > > >
> > > > On FreeBSD 11.2 with libv4l-1.6.3_4, that is certainly NOT
> the
> > > case. It
> > > > could be a newer or not yet released feature.
> > > >
> > > >
> > > >
> > >
> > > It's been the case since libv4l2 0.5:
> > >
> > <
> https://git.linuxtv.org/v4l-utils.git/commit/lib/libv4l2/libv4l2.c?id=0215f2ac08c3ad0dc66ad8036f4e186a5e8f56d6
> >
> > >
> > > If FreeBSD is using 1.6.3, then I think something else is
> > wrong. Are
> > > you
> > > sure that libv4l2 is actually being loaded?
> > >
> > >
> > > int v4l2_ioctl(int fd, unsigned long int request, ...)
> > > {
> > > ...
> > > if (devices[index].convert == NULL)
> > > goto no_capture_request;
> > >
> > > <your code nippet from above>
> > >
> > > no_capture_request:
> > >
> > >
> > >
> > > So the code that would set that flag, is skipped if the .convert
> > field
> > > is NULL.
> > >
> > > When is it NULL?
> > >
> > >
> > >
> >
> > If my reading of the code is correct, only when v4l2_fd_open() is
> > called
> > manually with V4L2_DISABLE_CONVERSION. Obviously we don't do that.
> >
> > Perhaps setting LIBV4L2_LOG_FILENAME might give some hints as to
> what's
> > going wrong?
> >
> >
> >
> > int v4l2_fd_open(int fd, int v4l2_flags)
> > {
> > ...
> > <The actual ioctl() returns cap.capabilities = 0x84a00001,
> > cap.device_caps = 0x04a00000>
> >
> > if (cap.capabilities & V4L2_CAP_DEVICE_CAPS)
> > cap.capabilities = cap.device_caps;
> > if (!(cap.capabilities & V4L2_CAP_VIDEO_CAPTURE) ||
> > !(cap.capabilities & (V4L2_CAP_STREAMING |
> > V4L2_CAP_READWRITE))) {
> > goto no_capture;
> > }
> >
> > 0x000000001 = V4L2_CAP_VIDEO_CAPTURE
> > 0x00200000 = V4L2_CAP_EXT_PIX_FORMAT
> > 0x00800000 = V4L2_CAP_META_CAPTURE
> > 0x040000000 = V4L2_CAP_STREAMING
> > 0x800000000 = V4L2_CAP_DEVICE_CAPS
> >
> > So cap.capabilities have all those flags set, but cap.device_caps lacks
> > V4L2_CAP_VIDEO_CAPTURE.
> > cap.capabilities is overwritten by cap.device_caps, thus
> > V4L2_CAP_VIDEO_CAPTURE is cleared.
> > Since libv4l2 only manages devices with V4L2_CAP_VIDEO_CAPTURE set, and
> > that flag is now missing, the first condition in the next "if" takes the
> > goto, and .convert remains NULL.
> > With .convert NULL, libv4l2 no longer manages the device, so
> > communication is directly between Wine and kernel.
> > Therefore the V4L2_CAP_READWRITE flag is not going to be set in
> > v4l2_ioctl(), and libv4l2 cannot be made to do any conversions for us.
> > But then again we can't use a device without V4L2_CAP_VIDEO_CAPTURE
> > either, libv4l2 or not.
> >
> > I think the fundamental problem is that Wine is trying to use
> > /dev/video1 which is broken, due to a bug in devenum that overwrites the
> > working /dev/video0 registry entry with it, because their descriptions
> > are the same.
> >
> >
> >
>
> Thanks, I've just sent patches to resolve both of these bugs.
>
>
Wow, so quickly!
Thank you!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20190420/12efb1b7/attachment-0001.html>
More information about the wine-devel
mailing list