[PATCH] qcap: fix recent regression that limited Wine's v4l2 support to devices that read()

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] qcap: fix recent regression that limited Wine's v4l2 support to devices that read()

Damjan Jovanovic-2
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 <[hidden email]>
---
 dlls/qcap/v4l.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)



0001-qcap-fix-recent-regression-that-limited-Wine-s-v4l2-su.txt (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] qcap: fix recent regression that limited Wine's v4l2 support to devices that read()

Zebediah Figura
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 <[hidden email]>
> ---
>   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>.


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] qcap: fix recent regression that limited Wine's v4l2 support to devices that read()

Damjan Jovanovic-2


On Sat, Apr 20, 2019 at 4:52 PM Zebediah Figura <[hidden email]> 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 <[hidden email]>
> ---
>   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.



Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] qcap: fix recent regression that limited Wine's v4l2 support to devices that read()

Zebediah Figura
On 4/20/19 10:31 AM, Damjan Jovanovic wrote:

>
>
> On Sat, Apr 20, 2019 at 4:52 PM Zebediah Figura <[hidden email]
> <mailto:[hidden email]>> 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 <[hidden email]
>     <mailto:[hidden email]>>
>      > ---
>      >   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?


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] qcap: fix recent regression that limited Wine's v4l2 support to devices that read()

Damjan Jovanovic-2


On Sat, Apr 20, 2019 at 5:44 PM Zebediah Figura <[hidden email]> wrote:
On 4/20/19 10:31 AM, Damjan Jovanovic wrote:
>
>
> On Sat, Apr 20, 2019 at 4:52 PM Zebediah Figura <[hidden email]
> <mailto:[hidden email]>> 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 <[hidden email]
>     <mailto:[hidden email]>>
>      > ---
>      >   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?


$ grep v4l /proc/74345/map
0x64ed6000 0x64ede000 8 11 0xfffff80239c004b0 r-x 2 1 0x1000 COW NC vnode /compat/freebsd-i386/usr/local/lib/libv4l2.so.0.0.0 NCH -1
0x64ede000 0x64ee2000 4 0 0xfffff80260dcdd20 rw- 1 0 0x3000 COW NNC vnode /compat/freebsd-i386/usr/local/lib/libv4l2.so.0.0.0 CH 1002
0x64ee2000 0x64f01000 31 33 0xfffff803549822d0 r-x 2 1 0x1000 COW NC vnode /compat/freebsd-i386/usr/local/lib/libv4lconvert.so.0.0.0 NCH -1
0x64f01000 0x64f03000 2 0 0xfffff8038cc0c1e0 rw- 1 0 0x3000 COW NNC vnode /compat/freebsd-i386/usr/local/lib/libv4lconvert.so.0.0.0 CH 1002

caps.capabilities are 0x84a00001
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




Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] qcap: fix recent regression that limited Wine's v4l2 support to devices that read()

Damjan Jovanovic-2
In reply to this post by Zebediah Figura


On Sat, Apr 20, 2019 at 5:44 PM Zebediah Figura <[hidden email]> wrote:
On 4/20/19 10:31 AM, Damjan Jovanovic wrote:
>
>
> On Sat, Apr 20, 2019 at 4:52 PM Zebediah Figura <[hidden email]
> <mailto:[hidden email]>> 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 <[hidden email]
>     <mailto:[hidden email]>>
>      > ---
>      >   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?



Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] qcap: fix recent regression that limited Wine's v4l2 support to devices that read()

Zebediah Figura
On 4/20/19 10:56 AM, Damjan Jovanovic wrote:

>
>
> On Sat, Apr 20, 2019 at 5:44 PM Zebediah Figura <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     On 4/20/19 10:31 AM, Damjan Jovanovic wrote:
>      >
>      >
>      > On Sat, Apr 20, 2019 at 4:52 PM Zebediah Figura
>     <[hidden email] <mailto:[hidden email]>
>      > <mailto:[hidden email] <mailto:[hidden email]>>> 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 <[hidden email]
>     <mailto:[hidden email]>
>      >     <mailto:[hidden email] <mailto:[hidden email]>>>
>      >      > ---
>      >      >   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?


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] qcap: fix recent regression that limited Wine's v4l2 support to devices that read()

Damjan Jovanovic-2


On Sat, Apr 20, 2019 at 6:07 PM Zebediah Figura <[hidden email]> wrote:
On 4/20/19 10:56 AM, Damjan Jovanovic wrote:
>
>
> On Sat, Apr 20, 2019 at 5:44 PM Zebediah Figura <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     On 4/20/19 10:31 AM, Damjan Jovanovic wrote:
>      >
>      >
>      > On Sat, Apr 20, 2019 at 4:52 PM Zebediah Figura
>     <[hidden email] <mailto:[hidden email]>
>      > <mailto:[hidden email] <mailto:[hidden email]>>> 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 <[hidden email]
>     <mailto:[hidden email]>
>      >     <mailto:[hidden email] <mailto:[hidden email]>>>
>      >      > ---
>      >      >   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.

 


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] qcap: fix recent regression that limited Wine's v4l2 support to devices that read()

Zebediah Figura
On 4/20/19 11:55 AM, Damjan Jovanovic wrote:

>
>
> On Sat, Apr 20, 2019 at 6:07 PM Zebediah Figura <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     On 4/20/19 10:56 AM, Damjan Jovanovic wrote:
>      >
>      >
>      > On Sat, Apr 20, 2019 at 5:44 PM Zebediah Figura
>     <[hidden email] <mailto:[hidden email]>
>      > <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>      >
>      >     On 4/20/19 10:31 AM, Damjan Jovanovic wrote:
>      >      >
>      >      >
>      >      > On Sat, Apr 20, 2019 at 4:52 PM Zebediah Figura
>      >     <[hidden email] <mailto:[hidden email]>
>     <mailto:[hidden email] <mailto:[hidden email]>>
>      >      > <mailto:[hidden email] <mailto:[hidden email]>
>     <mailto:[hidden email] <mailto:[hidden email]>>>> 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
>     <[hidden email] <mailto:[hidden email]>
>      >     <mailto:[hidden email] <mailto:[hidden email]>>
>      >      >     <mailto:[hidden email]
>     <mailto:[hidden email]> <mailto:[hidden email]
>     <mailto:[hidden email]>>>>
>      >      >      > ---
>      >      >      >   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.


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] qcap: fix recent regression that limited Wine's v4l2 support to devices that read()

Damjan Jovanovic-2


On Sat, Apr 20, 2019 at 7:08 PM Zebediah Figura <[hidden email]> wrote:
On 4/20/19 11:55 AM, Damjan Jovanovic wrote:
>
>
> On Sat, Apr 20, 2019 at 6:07 PM Zebediah Figura <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     On 4/20/19 10:56 AM, Damjan Jovanovic wrote:
>      >
>      >
>      > On Sat, Apr 20, 2019 at 5:44 PM Zebediah Figura
>     <[hidden email] <mailto:[hidden email]>
>      > <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>      >
>      >     On 4/20/19 10:31 AM, Damjan Jovanovic wrote:
>      >      >
>      >      >
>      >      > On Sat, Apr 20, 2019 at 4:52 PM Zebediah Figura
>      >     <[hidden email] <mailto:[hidden email]>
>     <mailto:[hidden email] <mailto:[hidden email]>>
>      >      > <mailto:[hidden email] <mailto:[hidden email]>
>     <mailto:[hidden email] <mailto:[hidden email]>>>> 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
>     <[hidden email] <mailto:[hidden email]>
>      >     <mailto:[hidden email] <mailto:[hidden email]>>
>      >      >     <mailto:[hidden email]
>     <mailto:[hidden email]> <mailto:[hidden email]
>     <mailto:[hidden email]>>>>
>      >      >      > ---
>      >      >      >   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!