[PATCH v2 0/4] MR6022: qcap: Delay setting v4l device format.
-- v2: qcap: Delay setting v4l device format until stream start. quartz: Propagate graph start error in MediaControl_Run(). qcap/tests: Add a test for simultaneous video capture usage. qcap/tests: Fix test failure in testsink_Receive() on some hardware. https://gitlab.winehq.org/wine/wine/-/merge_requests/6022
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/qcap/tests/videocapture.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/qcap/tests/videocapture.c b/dlls/qcap/tests/videocapture.c index cb74c20481f..97d29c72338 100644 --- a/dlls/qcap/tests/videocapture.c +++ b/dlls/qcap/tests/videocapture.c @@ -406,7 +406,7 @@ static HRESULT WINAPI testsink_Receive(struct strmbase_sink *iface, IMediaSample if (winetest_debug > 1) trace("Receive()\n"); hr = IMediaSample_GetTime(sample, &start, &end); - todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(hr == S_OK || hr == VFW_E_SAMPLE_TIME_NOT_SET, "Got hr %#lx.\n", hr); SetEvent(filter->got_sample); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6022
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/qcap/tests/videocapture.c | 109 ++++++++++++++++++++++++++++++++- 1 file changed, 108 insertions(+), 1 deletion(-) diff --git a/dlls/qcap/tests/videocapture.c b/dlls/qcap/tests/videocapture.c index 97d29c72338..31c34de7417 100644 --- a/dlls/qcap/tests/videocapture.c +++ b/dlls/qcap/tests/videocapture.c @@ -649,6 +649,113 @@ static void test_connection(IMoniker *moniker) ok(!ref, "Got outstanding refcount %ld.\n", ref); } +static void test_multiple_objects(IMoniker *moniker) +{ + struct testfilter testsink, testsink2; + IAMStreamConfig *config, *config2; + IMediaControl *control, *control2; + IFilterGraph2 *graph, *graph2; + IBaseFilter *filter, *filter2; + IEnumPins *enum_pins; + OAFilterState state; + IPin *pin, *pin2; + HRESULT hr; + ULONG ref; + + hr = IMoniker_BindToObject(moniker, NULL, NULL, &IID_IBaseFilter, (void **)&filter); + ok(hr == S_OK, "got %#lx.\n", hr); + hr = IMoniker_BindToObject(moniker, NULL, NULL, &IID_IBaseFilter, (void **)&filter2); + todo_wine ok(hr == S_OK, "got %#lx.\n", hr); + if (FAILED(hr)) + { + IBaseFilter_Release(filter); + return; + } + ok(filter != filter2, "got same objects.\n"); + + hr = IBaseFilter_EnumPins(filter, &enum_pins); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + pin = NULL; + while (IEnumPins_Next(enum_pins, 1, &pin, NULL) == S_OK) + { + PIN_DIRECTION dir; + IPin_QueryDirection(pin, &dir); + if (dir == PINDIR_OUTPUT) + break; + IPin_Release(pin); + } + IEnumPins_Release(enum_pins); + ok(!!pin, "got NULL.\n"); + + hr = IBaseFilter_EnumPins(filter2, &enum_pins); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + pin2 = NULL; + while (IEnumPins_Next(enum_pins, 1, &pin2, NULL) == S_OK) + { + PIN_DIRECTION dir; + IPin_QueryDirection(pin2, &dir); + if (dir == PINDIR_OUTPUT) + break; + IPin_Release(pin2); + } + IEnumPins_Release(enum_pins); + ok(!!pin2, "got NULL.\n"); + + CoCreateInstance(&CLSID_FilterGraph, NULL, CLSCTX_INPROC_SERVER, + &IID_IFilterGraph2, (void **)&graph); + IFilterGraph2_QueryInterface(graph, &IID_IMediaControl, (void **)&control); + testfilter_init(&testsink); + IFilterGraph2_AddFilter(graph, &testsink.filter.IBaseFilter_iface, L"sink"); + IFilterGraph2_AddFilter(graph, filter, L"source"); + hr = IPin_QueryInterface(pin, &IID_IAMStreamConfig, (void **)&config); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + CoCreateInstance(&CLSID_FilterGraph, NULL, CLSCTX_INPROC_SERVER, + &IID_IFilterGraph2, (void **)&graph2); + IFilterGraph2_QueryInterface(graph2, &IID_IMediaControl, (void **)&control2); + testfilter_init(&testsink2); + IFilterGraph2_AddFilter(graph2, &testsink2.filter.IBaseFilter_iface, L"sink"); + IFilterGraph2_AddFilter(graph2, filter2, L"source"); + hr = IPin_QueryInterface(pin2, &IID_IAMStreamConfig, (void **)&config2); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IFilterGraph2_ConnectDirect(graph, pin, &testsink.sink.pin.IPin_iface, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IFilterGraph2_ConnectDirect(graph2, pin2, &testsink2.sink.pin.IPin_iface, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IMediaControl_Run(control); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IMediaControl_GetState(control, 0, &state); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(state == State_Running, "Got state %lu.\n", state); + + hr = IMediaControl_Run(control2); + /* Depending on device driver Windows has different behaviour on attempt to concurrently use capture + * device in the same process. So far observed variants are: + * - running both graphs succeeds but only the graph started second receives samples, attempt to change + * state of the first returns MF_E_VIDEO_RECORDING_DEVICE_PREEMPTED; + * - attempt to change second filter or graph state fails with HRESULT_FROM_WIN32(ERROR_NO_SYSTEM_RESOURCES). */ + ok(hr == S_OK || hr == HRESULT_FROM_WIN32(ERROR_NO_SYSTEM_RESOURCES), "Got hr %#lx.\n", hr); + + IAMStreamConfig_Release(config); + IMediaControl_Release(control); + ref = IFilterGraph2_Release(graph); + ok(!ref, "Got outstanding refcount %ld.\n", ref); + + IPin_Release(pin); + ref = IBaseFilter_Release(filter); + ok(!ref, "Got outstanding refcount %ld.\n", ref); + + IAMStreamConfig_Release(config2); + IMediaControl_Release(control2); + ref = IFilterGraph2_Release(graph2); + ok(!ref, "Got outstanding refcount %ld.\n", ref); + IPin_Release(pin2); + ref = IBaseFilter_Release(filter2); + ok(!ref, "Got outstanding refcount %ld.\n", ref); +} + START_TEST(videocapture) { ICreateDevEnum *dev_enum; @@ -693,11 +800,11 @@ START_TEST(videocapture) test_pins(filter); test_misc_flags(filter); test_unconnected_filter_state(filter); - ref = IBaseFilter_Release(filter); ok(!ref, "Got outstanding refcount %ld.\n", ref); test_connection(moniker); + test_multiple_objects(moniker); IMoniker_Release(moniker); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6022
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/quartz/filtergraph.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/quartz/filtergraph.c b/dlls/quartz/filtergraph.c index cc66b17c75b..987821f3fb1 100644 --- a/dlls/quartz/filtergraph.c +++ b/dlls/quartz/filtergraph.c @@ -1943,7 +1943,7 @@ static HRESULT WINAPI MediaControl_Run(IMediaControl *iface) } else { - graph_start(graph, 0); + hr = graph_start(graph, 0); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6022
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/qcap/qcap_private.h | 6 ++++++ dlls/qcap/tests/videocapture.c | 7 +------ dlls/qcap/v4l.c | 30 ++++++++++++++++++++++++++---- dlls/qcap/vfwcapture.c | 9 +++++++++ 4 files changed, 42 insertions(+), 10 deletions(-) diff --git a/dlls/qcap/qcap_private.h b/dlls/qcap/qcap_private.h index c9119b6a52e..d0acd9e3bd7 100644 --- a/dlls/qcap/qcap_private.h +++ b/dlls/qcap/qcap_private.h @@ -47,6 +47,11 @@ struct create_params video_capture_device_t *device; }; +struct start_params +{ + video_capture_device_t device; +}; + struct destroy_params { video_capture_device_t device; @@ -130,6 +135,7 @@ struct read_frame_params enum unix_funcs { unix_create, + unix_start, unix_destroy, unix_check_format, unix_set_format, diff --git a/dlls/qcap/tests/videocapture.c b/dlls/qcap/tests/videocapture.c index 31c34de7417..3569bf33da4 100644 --- a/dlls/qcap/tests/videocapture.c +++ b/dlls/qcap/tests/videocapture.c @@ -665,12 +665,7 @@ static void test_multiple_objects(IMoniker *moniker) hr = IMoniker_BindToObject(moniker, NULL, NULL, &IID_IBaseFilter, (void **)&filter); ok(hr == S_OK, "got %#lx.\n", hr); hr = IMoniker_BindToObject(moniker, NULL, NULL, &IID_IBaseFilter, (void **)&filter2); - todo_wine ok(hr == S_OK, "got %#lx.\n", hr); - if (FAILED(hr)) - { - IBaseFilter_Release(filter); - return; - } + ok(hr == S_OK, "got %#lx.\n", hr); ok(filter != filter2, "got same objects.\n"); hr = IBaseFilter_EnumPins(filter, &enum_pins); diff --git a/dlls/qcap/v4l.c b/dlls/qcap/v4l.c index 134c3eca9e1..5ab9613b4e2 100644 --- a/dlls/qcap/v4l.c +++ b/dlls/qcap/v4l.c @@ -93,6 +93,8 @@ struct video_capture_device const struct caps *current_caps; struct caps *caps; LONG caps_count; + struct v4l2_format device_format; + BOOL started; int image_size, image_pitch; BYTE *image_data; @@ -162,7 +164,7 @@ static NTSTATUS v4l_device_check_format( void *args ) return E_FAIL; } -static HRESULT set_caps(struct video_capture_device *device, const struct caps *caps) +static HRESULT set_caps(struct video_capture_device *device, const struct caps *caps, BOOL try) { struct v4l2_format format = {0}; LONG width, height, image_size; @@ -182,7 +184,7 @@ static HRESULT set_caps(struct video_capture_device *device, const struct caps * format.fmt.pix.pixelformat = caps->pixelformat; format.fmt.pix.width = width; format.fmt.pix.height = height; - if (xioctl(device->fd, VIDIOC_S_FMT, &format) == -1 + if (xioctl(device->fd, try ? VIDIOC_TRY_FMT : VIDIOC_S_FMT, &format) == -1 || format.fmt.pix.pixelformat != caps->pixelformat || format.fmt.pix.width != width || format.fmt.pix.height != height) @@ -192,6 +194,8 @@ static HRESULT set_caps(struct video_capture_device *device, const struct caps * return VFW_E_TYPE_NOT_ACCEPTED; } + device->started = !try; + device->device_format = format; device->current_caps = caps; device->image_size = image_size; device->image_pitch = width * caps->video_info.bmiHeader.biBitCount / 8; @@ -213,7 +217,7 @@ static NTSTATUS v4l_device_set_format( void *args ) if (device->current_caps == caps) return S_OK; - return set_caps(device, caps); + return set_caps(device, caps, FALSE); } static NTSTATUS v4l_device_get_format( void *args ) @@ -550,7 +554,7 @@ static NTSTATUS v4l_device_create( void *args ) for (i = 0; i < device->caps_count; ++i) device->caps[i].media_type.pbFormat = (BYTE *)&device->caps[i].video_info; - if (FAILED(hr = set_caps(device, &device->caps[0]))) + if (FAILED(hr = set_caps(device, &device->caps[0], TRUE))) { if (hr == VFW_E_TYPE_NOT_ACCEPTED && !have_libv4l2) ERR_(winediag)("You may need libv4l2 to use this device.\n"); @@ -569,6 +573,22 @@ error: return E_FAIL; } +static NTSTATUS v4l_device_start( void *args ) +{ + const struct start_params *params = args; + struct video_capture_device *device = get_device(params->device); + + if (device->started) return S_OK; + + if (xioctl(device->fd, VIDIOC_S_FMT, &device->device_format) == -1) + { + ERR("Failed to set pixel format: %s.\n", strerror(errno)); + return errno == EBUSY ? HRESULT_FROM_WIN32(ERROR_NO_SYSTEM_RESOURCES) : VFW_E_TYPE_NOT_ACCEPTED; + } + device->started = TRUE; + return S_OK; +} + static NTSTATUS v4l_device_destroy( void *args ) { const struct destroy_params *params = args; @@ -580,6 +600,7 @@ static NTSTATUS v4l_device_destroy( void *args ) const unixlib_entry_t __wine_unix_call_funcs[] = { v4l_device_create, + v4l_device_start, v4l_device_destroy, v4l_device_check_format, v4l_device_set_format, @@ -846,6 +867,7 @@ static NTSTATUS wow64_v4l_device_read_frame( void *args ) const unixlib_entry_t __wine_unix_call_wow64_funcs[] = { wow64_v4l_device_create, + v4l_device_start, v4l_device_destroy, wow64_v4l_device_check_format, wow64_v4l_device_set_format, diff --git a/dlls/qcap/vfwcapture.c b/dlls/qcap/vfwcapture.c index 0899b228ec2..bf3ded9223c 100644 --- a/dlls/qcap/vfwcapture.c +++ b/dlls/qcap/vfwcapture.c @@ -215,10 +215,19 @@ static HRESULT vfw_capture_init_stream(struct strmbase_filter *iface) static HRESULT vfw_capture_start_stream(struct strmbase_filter *iface, REFERENCE_TIME time) { struct vfw_capture *filter = impl_from_strmbase_filter(iface); + struct start_params params; + HRESULT hr; if (!filter->source.pin.peer) return S_OK; + params.device = filter->device; + if (FAILED(hr = V4L_CALL( start, ¶ms ))) + { + ERR("start stream failed.\n"); + return hr; + } + EnterCriticalSection(&filter->state_cs); filter->state = State_Running; LeaveCriticalSection(&filter->state_cs); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6022
v2: - Modify test so it succeeds on different devices we observed so far; - Move v4l device start further down to Run so simultaneous device start fails more like in one variant observed on Windows (also a patch to quartz.dll added for that purpose). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6022#note_76795
In any case I guess the tests should be restricted to something closer to what the application expects.
The only thing I know so far the app expects is creating two filters from the same moniker with IMoniker_BindToObject(), I didn't spot it trying to use those filters simultaneously in graphs so far. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6022#note_76796
To elaborate a bit on moving device start to Run and adding error return to failed graph Run. I also tested that with PlayCap sample [1] when the device is opened by another process. The way it was done in the first patch the sample will be displaying black screen in this case, with the present variant the behaviour matches Windows (at least with USB camera on Windows): it displays message box about graph run failure with the same error code. Also currently in Wine failure in filter_Pause() leads to hang on tear down (the case when some filter failed to start doesn't work very well). But then with the original test with _Pause on the filter tearing down graph was also hanging on Windows for me with USB camera. So while there might be some separate issue with handling failures in filter initialization to Pause state, it looks to me this way is better anyway. 1. https://github.com/microsoft/Windows-classic-samples/blob/main/Samples/Win7S... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6022#note_76804
This merge request was approved by Elizabeth Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6022
participants (3)
-
Elizabeth Figura (＠zfigura) -
Paul Gofman
-
Paul Gofman (＠gofman)