[PATCH] qcap:fix the bug which camera device can't be released correctly.

From: Keming Liang <liangkeming(a)kylinos.cn>

Signed-off-by: Keming Liang <liangkeming(a)kylinos.cn>
---
 dlls/qcap/qcap_private.h | 1 +
 dlls/qcap/v4l.c          | 8 ++++++++
 dlls/qcap/vfwcapture.c   | 3 +++
 3 files changed, 12 insertions(+)

diff --git a/dlls/qcap/qcap_private.h b/dlls/qcap/qcap_private.h
index 104dba6a649..22dcecf40fd 100644
--- a/dlls/qcap/qcap_private.h
+++ b/dlls/qcap/qcap_private.h
@@ -44,6 +44,7 @@ struct video_capture_funcs
 {
     struct video_capture_device *(CDECL *create)(USHORT index);
     void (CDECL *destroy)(struct video_capture_device *device);
+    void (CDECL *close)(struct video_capture_device *device);
     HRESULT (CDECL *check_format)(struct video_capture_device *device, const AM_MEDIA_TYPE *mt);
     HRESULT (CDECL *set_format)(struct video_capture_device *device, const AM_MEDIA_TYPE *mt);
     void (CDECL *get_format)(struct video_capture_device *device, AM_MEDIA_TYPE *mt, VIDEOINFOHEADER *format);
diff --git a/dlls/qcap/v4l.c b/dlls/qcap/v4l.c
index ccc06194b99..8102be50aec 100644
--- a/dlls/qcap/v4l.c
+++ b/dlls/qcap/v4l.c
@@ -118,6 +118,13 @@ static int xioctl(int fd, int request, void * arg)
     return r;
 }
 
+static void CDECL v4l_device_close(struct video_capture_device *device)
+{
+    if (device->fd != -1)
+        video_close(device->fd);
+    device->fd = -1;
+}
+
 static void CDECL v4l_device_destroy(struct video_capture_device *device)
 {
     if (device->fd != -1)
@@ -552,6 +559,7 @@ error:
 const struct video_capture_funcs v4l_funcs =
 {
     .create = v4l_device_create,
+    .close = v4l_device_close,
     .destroy = v4l_device_destroy,
     .check_format = v4l_device_check_format,
     .set_format = v4l_device_set_format,
diff --git a/dlls/qcap/vfwcapture.c b/dlls/qcap/vfwcapture.c
index 48401344ac4..155cfda20ba 100644
--- a/dlls/qcap/vfwcapture.c
+++ b/dlls/qcap/vfwcapture.c
@@ -254,6 +254,9 @@ static HRESULT vfw_capture_cleanup_stream(struct strmbase_filter *iface)
     if (hr != S_OK && hr != VFW_E_NOT_COMMITTED)
         ERR("Failed to decommit allocator, hr %#x.\n", hr);
 
+    if (filter->init)
+        capture_funcs->close(filter->device);
+
     return S_OK;
 }
 
-- 
2.25.1

[Zebediah Figura (she/her)]

Hello Keming, thanks for the patch!

Unfortunately this can't work as-is; we need to be able to access the fd 
even after IBaseFilter::Stop() [e.g. to query or set properties, or to 
restart the stream.]

Can you perhaps provide more details about the problem you are trying to 
solve?

ἔρρωσο,
Zebediah

[梁珂铭]

Dear Zebediah,



it is a Wechat problem, when we open video call and open camera, at first time  it is normal , but the second time we open video call ,It is abnormal. and report open fd failed,  and the fd is busy. we analyze this problem and think it should close fd in cleanup_stream function ,and not in destroy function.




--------------------------------------------------------------------------------------------------------

04d0:trace:qcap:vfw_capture_create Created VFW capture filter 007D1A30.

04d0:trace:qcap:PPB_Load filter 007D1A30, bag 007D1A24, error_log 00000000.

04d0:trace:qcap:v4l_device_create Opening device /dev/video0.

libv4l2: error setting pixformat: Device or resource busy

04d0:err:qcap:set_caps Failed to set pixel format: Device or resource busy.

04d0:trace:qcap:DllCanUnloadNow .




-----------------------------------------------------------------------------------

we need to be able to access the fd
> even after IBaseFilter::Stop() [e.g. to query or set properties, or to 
> restart the stream.]  ---->maybe we need more changes

在 2021-06-05 05:57:40，"Zebediah Figura (she/her)" <zfigura(a)codeweavers.com> 写道：
> Hello Keming, thanks for the patch!
>
> Unfortunately this can't work as-is; we need to be able to access the fd 
> even after IBaseFilter::Stop() [e.g. to query or set properties, or to 
> restart the stream.]
>
> Can you perhaps provide more details about the problem you are trying to 
> solve?
>
> ἔρρωσο,
> Zebediah

[Zebediah Figura (she/her)]

Hello Keming,

It isn't clear from the log, but it looks like the application may be 
trying to create a second filter on the same device without fully 
releasing the first one.

Although I haven't tested very fully, I think Windows is more lenient 
about opening multiple capture filters than we currently allow for. It 
may be that there's a leak somewhere that we're to blame for; it's 
probably worth checking carefully to make sure that isn't the case.

But we may need to find ways to avoid holding a file descriptor open 
when not streaming. When doing that you will need to account for the 
fact that most APIs remain usable even after a stream is stopped. You 
may want to defer VIDIOC_S_FMT until streaming starts, and find a way to 
release ownership of the device when streaming stops (I don't know 
offhand if v4l2 provides this; you may need to close and reopen the 
descriptor.)

Tests replicating the application behaviour would also be useful.

ἔρρωσο,
Zebediah