[PATCH v3 1/4] qcap/tests: Test effect of setting stream format on media type enumeration.
Signed-off-by: Jeff Smith <whydoubt(a)gmail.com> --- v3: - Fixup patches failing to apply. dlls/qcap/tests/videocapture.c | 59 ++++++++++++++++++++++++++++------ 1 file changed, 50 insertions(+), 9 deletions(-) diff --git a/dlls/qcap/tests/videocapture.c b/dlls/qcap/tests/videocapture.c index 326601a398..dcce3a7869 100644 --- a/dlls/qcap/tests/videocapture.c +++ b/dlls/qcap/tests/videocapture.c @@ -23,6 +23,12 @@ #include "wine/test.h" #include "wine/strmbase.h" +static BOOL compare_media_types(const AM_MEDIA_TYPE *a, const AM_MEDIA_TYPE *b) +{ + return !memcmp(a, b, offsetof(AM_MEDIA_TYPE, pbFormat)) + && !memcmp(a->pbFormat, b->pbFormat, a->cbFormat); +} + #define check_interface(a, b, c) check_interface_(__LINE__, a, b, c) static void check_interface_(unsigned int line, void *iface_ptr, REFIID iid, BOOL supported) { @@ -81,8 +87,9 @@ static void test_stream_config(IPin *pin) AM_MEDIA_TYPE *format, *format2; VIDEO_STREAM_CONFIG_CAPS vscc; IAMStreamConfig *stream_config; + IEnumMediaTypes *enum_media_types; LONG depth, compression; - LONG count, size; + LONG count, size, i; HRESULT hr; hr = IPin_QueryInterface(pin, &IID_IAMStreamConfig, (void **)&stream_config); @@ -96,6 +103,16 @@ static void test_stream_config(IPin *pin) hr = IAMStreamConfig_SetFormat(stream_config, format); ok(hr == S_OK, "Got hr %#x.\n", hr); + /* After setting format, a single media type is enumerated. + * This persists until the filter is released. */ + IPin_EnumMediaTypes(pin, &enum_media_types); + hr = IEnumMediaTypes_Next(enum_media_types, 1, &format2, NULL); + ok(hr == S_OK, "Got hr %#x.\n", hr); + DeleteMediaType(format2); + hr = IEnumMediaTypes_Next(enum_media_types, 1, &format2, NULL); + todo_wine ok(hr == S_FALSE, "Got hr %#x.\n", hr); + IEnumMediaTypes_Release(enum_media_types); + format->majortype = MEDIATYPE_Audio; hr = IAMStreamConfig_SetFormat(stream_config, format); ok(hr == E_FAIL, "Got hr %#x.\n", hr); @@ -162,14 +179,38 @@ static void test_stream_config(IPin *pin) hr = IAMStreamConfig_GetStreamCaps(stream_config, 100000, &format, (BYTE *)&vscc); ok(hr == S_FALSE, "Got hr %#x.\n", hr); - hr = IAMStreamConfig_GetStreamCaps(stream_config, 0, &format, (BYTE *)&vscc); - ok(hr == S_OK, "Got hr %#x.\n", hr); - ok(IsEqualGUID(&format->majortype, &MEDIATYPE_Video), "Got wrong majortype: %s.\n", - debugstr_guid(&MEDIATYPE_Video)); - ok(IsEqualGUID(&vscc.guid, &FORMAT_VideoInfo) - || IsEqualGUID(&vscc.guid, &FORMAT_VideoInfo2), "Got wrong guid: %s.\n", - debugstr_guid(&vscc.guid)); - FreeMediaType(format); + for (i = 0; i < count; i++) + { + hr = IAMStreamConfig_GetStreamCaps(stream_config, i, &format, (BYTE *)&vscc); + ok(hr == S_OK, "Got hr %#x.\n", hr); + ok(IsEqualGUID(&format->majortype, &MEDIATYPE_Video), "Got wrong majortype: %s.\n", + debugstr_guid(&MEDIATYPE_Video)); + ok(IsEqualGUID(&vscc.guid, &FORMAT_VideoInfo) + || IsEqualGUID(&vscc.guid, &FORMAT_VideoInfo2), "Got wrong guid: %s.\n", + debugstr_guid(&vscc.guid)); + + hr = IAMStreamConfig_SetFormat(stream_config, format); + ok(hr == S_OK, "Got hr %#x.\n", hr); + + hr = IAMStreamConfig_GetFormat(stream_config, &format2); + ok(hr == S_OK, "Got hr %#x.\n", hr); + ok(compare_media_types(format, format2), "Media types didn't match.\n"); + DeleteMediaType(format2); + + hr = IPin_EnumMediaTypes(pin, &enum_media_types); + ok(hr == S_OK, "Got hr %#x.\n", hr); + hr = IEnumMediaTypes_Next(enum_media_types, 1, &format2, NULL); + ok(hr == S_OK, "Got hr %#x.\n", hr); + + /* Which media types will match varies in wine depending on the attached webcam */ + todo_wine_if(compare_media_types(format, format2) == FALSE) + ok(compare_media_types(format, format2), "Media types didn't match.\n"); + + DeleteMediaType(format2); + IEnumMediaTypes_Release(enum_media_types); + + DeleteMediaType(format); + } IAMStreamConfig_Release(stream_config); } -- 2.23.0
Signed-off-by: Jeff Smith <whydoubt(a)gmail.com> --- By calculating and recording the image size when the V4L format is set, the current_caps field can be ignored by functions that are used with streaming. Looking ahead to later patches, this avoids most of the cases where we would need to check whether current_caps has been set. dlls/qcap/v4l.c | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/dlls/qcap/v4l.c b/dlls/qcap/v4l.c index d8c5e168e1..dade5a51e0 100644 --- a/dlls/qcap/v4l.c +++ b/dlls/qcap/v4l.c @@ -96,6 +96,9 @@ struct v4l_device struct caps *caps; LONG caps_count; + int image_size; + int image_pitch; + struct strmbase_source *pin; int fd, mmap; @@ -194,6 +197,8 @@ static BOOL set_caps(struct v4l_device *device, const struct caps *caps) } device->current_caps = caps; + device->image_size = width * height * caps->video_info.bmiHeader.biBitCount / 8; + device->image_pitch = width * caps->video_info.bmiHeader.biBitCount / 8; return TRUE; } @@ -304,15 +309,11 @@ static HRESULT v4l_device_set_prop(struct video_capture_device *iface, static void reverse_image(struct v4l_device *device, LPBYTE output, const BYTE *input) { int inoffset, outoffset, pitch; - UINT width, height, depth; - width = device->current_caps->video_info.bmiHeader.biWidth; - height = device->current_caps->video_info.bmiHeader.biHeight; - depth = device->current_caps->video_info.bmiHeader.biBitCount / 8; /* the whole image needs to be reversed, because the dibs are messed up in windows */ - outoffset = width * height * depth; - pitch = width * depth; + outoffset = device->image_size; + pitch = device->image_pitch; inoffset = 0; while (outoffset > 0) { @@ -330,14 +331,8 @@ static DWORD WINAPI ReadThread(void *arg) HRESULT hr; IMediaSample *pSample = NULL; unsigned char *pTarget, *image_data; - unsigned int image_size; - UINT width, height, depth; - - width = device->current_caps->video_info.bmiHeader.biWidth; - height = device->current_caps->video_info.bmiHeader.biHeight; - depth = device->current_caps->video_info.bmiHeader.biBitCount / 8; - image_size = width * height * depth; - if (!(image_data = heap_alloc(image_size))) + + if (!(image_data = heap_alloc(device->image_size))) { ERR("Failed to allocate memory.\n"); return 0; @@ -363,15 +358,14 @@ static DWORD WINAPI ReadThread(void *arg) { int len; - len = width * height * depth; - IMediaSample_SetActualDataLength(pSample, len); + IMediaSample_SetActualDataLength(pSample, device->image_size); len = IMediaSample_GetActualDataLength(pSample); TRACE("Data length: %d KB\n", len / 1024); IMediaSample_GetPointer(pSample, &pTarget); - while (video_read(device->fd, image_data, image_size) == -1) + while (video_read(device->fd, image_data, device->image_size) == -1) { if (errno != EAGAIN) { @@ -403,8 +397,7 @@ static void v4l_device_init_stream(struct video_capture_device *iface) HRESULT hr; req_props.cBuffers = 3; - req_props.cbBuffer = device->current_caps->video_info.bmiHeader.biWidth * device->current_caps->video_info.bmiHeader.biHeight; - req_props.cbBuffer = (req_props.cbBuffer * device->current_caps->video_info.bmiHeader.biBitCount) / 8; + req_props.cbBuffer = device->image_size; req_props.cbAlign = 1; req_props.cbPrefix = 0; -- 2.23.0
Signed-off-by: Jeff Smith <whydoubt(a)gmail.com> --- Set the V4L format during stream initialization only, and only use the VIDIOC_TRY_FMT ioctl elsewhere. This, along with the previous patch, allows for using current_caps exclusively to indicate the latest format passed to SetFormat (or NULL if it has not been called). dlls/qcap/v4l.c | 45 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/dlls/qcap/v4l.c b/dlls/qcap/v4l.c index dade5a51e0..f151cc66a3 100644 --- a/dlls/qcap/v4l.c +++ b/dlls/qcap/v4l.c @@ -196,13 +196,36 @@ static BOOL set_caps(struct v4l_device *device, const struct caps *caps) return FALSE; } - device->current_caps = caps; device->image_size = width * height * caps->video_info.bmiHeader.biBitCount / 8; device->image_pitch = width * caps->video_info.bmiHeader.biBitCount / 8; return TRUE; } +static BOOL try_caps(struct v4l_device *device, const struct caps *caps) +{ + struct v4l2_format format = {0}; + LONG width = caps->video_info.bmiHeader.biWidth; + LONG height = caps->video_info.bmiHeader.biHeight; + + TRACE("device %p, width %d, height %d, pixelformat %#x.\n", device, width, height, caps->pixelformat); + + format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; + format.fmt.pix.pixelformat = caps->pixelformat; + format.fmt.pix.width = width; + format.fmt.pix.height = height; + if (xioctl(device->fd, VIDIOC_TRY_FMT, &format) == -1 + || format.fmt.pix.pixelformat != caps->pixelformat + || format.fmt.pix.width != width + || format.fmt.pix.height != height) + { + ERR("Failed trying pixel format: %s.\n", strerror(errno)); + return FALSE; + } + + return TRUE; +} + static HRESULT v4l_device_set_format(struct video_capture_device *iface, const AM_MEDIA_TYPE *mt) { struct v4l_device *device = v4l_device(iface); @@ -215,9 +238,11 @@ static HRESULT v4l_device_set_format(struct video_capture_device *iface, const A if (device->current_caps == caps) return S_OK; - if (!set_caps(device, caps)) + if (!try_caps(device, caps)) return VFW_E_TYPE_NOT_ACCEPTED; + device->current_caps = caps; + return S_OK; } @@ -225,7 +250,10 @@ static HRESULT v4l_device_get_format(struct video_capture_device *iface, AM_MEDI { struct v4l_device *device = v4l_device(iface); - return CopyMediaType(mt, &device->current_caps->media_type); + if (device->current_caps) + return CopyMediaType(mt, &device->current_caps->media_type); + + return CopyMediaType(mt, &device->caps[0].media_type); } static __u32 v4l2_cid_from_qcap_property(VideoProcAmpProperty property) @@ -396,6 +424,10 @@ static void v4l_device_init_stream(struct video_capture_device *iface) ALLOCATOR_PROPERTIES req_props, ret_props; HRESULT hr; + /* We must commit to a particular format before reading from device. */ + if (!set_caps(device, (device->current_caps) ? device->current_caps : &device->caps[0])) + ERR("Failed to set format.\n"); + req_props.cBuffers = 3; req_props.cbBuffer = device->image_size; req_props.cbAlign = 1; @@ -656,9 +688,8 @@ struct video_capture_device *v4l_device_create(struct strmbase_source *pin, USHO for (i = 0; i < device->caps_count; ++i) device->caps[i].media_type.pbFormat = (BYTE *)&device->caps[i].video_info; - if (!set_caps(device, &device->caps[0])) + if (!try_caps(device, &device->caps[0])) { - ERR("Failed to set pixel format: %s\n", strerror(errno)); if (!have_libv4l2) ERR_(winediag)("You may need libv4l2 to use this device.\n"); goto error; @@ -671,10 +702,6 @@ struct video_capture_device *v4l_device_create(struct strmbase_source *pin, USHO InitializeCriticalSection(&device->state_cs); device->state_cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": v4l_device.state_cs"); - TRACE("Format: %d bpp - %dx%d.\n", device->current_caps->video_info.bmiHeader.biBitCount, - device->current_caps->video_info.bmiHeader.biWidth, - device->current_caps->video_info.bmiHeader.biHeight); - return &device->d; error: -- 2.23.0
On 9/7/20 3:17 PM, Jeff Smith wrote:
Signed-off-by: Jeff Smith <whydoubt(a)gmail.com> --- Set the V4L format during stream initialization only, and only use the VIDIOC_TRY_FMT ioctl elsewhere. This, along with the previous patch, allows for using current_caps exclusively to indicate the latest format passed to SetFormat (or NULL if it has not been called).
dlls/qcap/v4l.c | 45 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 9 deletions(-)
My point is that if I didn't have access to CodeWeavers' internal bug tracker, I wouldn't understand why this is necessary. E.g. "Some applications opens two filters simultaneously, although they only read from of them. Some v4l2 drivers forbid setting the format on more than one open descriptor at a time."
diff --git a/dlls/qcap/v4l.c b/dlls/qcap/v4l.c index dade5a51e0..f151cc66a3 100644 --- a/dlls/qcap/v4l.c +++ b/dlls/qcap/v4l.c @@ -196,13 +196,36 @@ static BOOL set_caps(struct v4l_device *device, const struct caps *caps) return FALSE; }
- device->current_caps = caps; device->image_size = width * height * caps->video_info.bmiHeader.biBitCount / 8; device->image_pitch = width * caps->video_info.bmiHeader.biBitCount / 8;
return TRUE; }
+static BOOL try_caps(struct v4l_device *device, const struct caps *caps) +{ + struct v4l2_format format = {0}; + LONG width = caps->video_info.bmiHeader.biWidth; + LONG height = caps->video_info.bmiHeader.biHeight; + + TRACE("device %p, width %d, height %d, pixelformat %#x.\n", device, width, height, caps->pixelformat); + + format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; + format.fmt.pix.pixelformat = caps->pixelformat; + format.fmt.pix.width = width; + format.fmt.pix.height = height; + if (xioctl(device->fd, VIDIOC_TRY_FMT, &format) == -1 + || format.fmt.pix.pixelformat != caps->pixelformat + || format.fmt.pix.width != width + || format.fmt.pix.height != height) + { + ERR("Failed trying pixel format: %s.\n", strerror(errno)); + return FALSE; + } + + return TRUE; +} + static HRESULT v4l_device_set_format(struct video_capture_device *iface, const AM_MEDIA_TYPE *mt) { struct v4l_device *device = v4l_device(iface); @@ -215,9 +238,11 @@ static HRESULT v4l_device_set_format(struct video_capture_device *iface, const A if (device->current_caps == caps) return S_OK;
- if (!set_caps(device, caps)) + if (!try_caps(device, caps)) return VFW_E_TYPE_NOT_ACCEPTED;
+ device->current_caps = caps; + return S_OK; }
@@ -225,7 +250,10 @@ static HRESULT v4l_device_get_format(struct video_capture_device *iface, AM_MEDI { struct v4l_device *device = v4l_device(iface);
- return CopyMediaType(mt, &device->current_caps->media_type); + if (device->current_caps) + return CopyMediaType(mt, &device->current_caps->media_type); + + return CopyMediaType(mt, &device->caps[0].media_type); }
static __u32 v4l2_cid_from_qcap_property(VideoProcAmpProperty property) @@ -396,6 +424,10 @@ static void v4l_device_init_stream(struct video_capture_device *iface) ALLOCATOR_PROPERTIES req_props, ret_props; HRESULT hr;
+ /* We must commit to a particular format before reading from device. */ + if (!set_caps(device, (device->current_caps) ? device->current_caps : &device->caps[0])) + ERR("Failed to set format.\n"); + req_props.cBuffers = 3; req_props.cbBuffer = device->image_size; req_props.cbAlign = 1; @@ -656,9 +688,8 @@ struct video_capture_device *v4l_device_create(struct strmbase_source *pin, USHO for (i = 0; i < device->caps_count; ++i) device->caps[i].media_type.pbFormat = (BYTE *)&device->caps[i].video_info;
- if (!set_caps(device, &device->caps[0])) + if (!try_caps(device, &device->caps[0])) { - ERR("Failed to set pixel format: %s\n", strerror(errno)); if (!have_libv4l2) ERR_(winediag)("You may need libv4l2 to use this device.\n"); goto error; @@ -671,10 +702,6 @@ struct video_capture_device *v4l_device_create(struct strmbase_source *pin, USHO InitializeCriticalSection(&device->state_cs); device->state_cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": v4l_device.state_cs");
- TRACE("Format: %d bpp - %dx%d.\n", device->current_caps->video_info.bmiHeader.biBitCount, - device->current_caps->video_info.bmiHeader.biWidth, - device->current_caps->video_info.bmiHeader.biHeight); - return &device->d;
error:
Signed-off-by: Jeff Smith <whydoubt(a)gmail.com> --- dlls/qcap/qcap_private.h | 1 + dlls/qcap/tests/videocapture.c | 6 +----- dlls/qcap/v4l.c | 16 ++++++++++++++++ dlls/qcap/vfwcapture.c | 13 +------------ 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/dlls/qcap/qcap_private.h b/dlls/qcap/qcap_private.h index 70a8c85ad4..a5946ecfd9 100644 --- a/dlls/qcap/qcap_private.h +++ b/dlls/qcap/qcap_private.h @@ -51,6 +51,7 @@ struct video_capture_device_ops HRESULT (*check_format)(struct video_capture_device *device, const AM_MEDIA_TYPE *mt); HRESULT (*set_format)(struct video_capture_device *device, const AM_MEDIA_TYPE *mt); HRESULT (*get_format)(struct video_capture_device *device, AM_MEDIA_TYPE *mt); + HRESULT (*get_media_type)(struct video_capture_device *device, LONG index, AM_MEDIA_TYPE *mt); HRESULT (*get_caps)(struct video_capture_device *device, LONG index, AM_MEDIA_TYPE **mt, VIDEO_STREAM_CONFIG_CAPS *caps); LONG (*get_caps_count)(struct video_capture_device *device); HRESULT (*get_prop_range)(struct video_capture_device *device, VideoProcAmpProperty property, diff --git a/dlls/qcap/tests/videocapture.c b/dlls/qcap/tests/videocapture.c index dcce3a7869..99175c0b3f 100644 --- a/dlls/qcap/tests/videocapture.c +++ b/dlls/qcap/tests/videocapture.c @@ -110,7 +110,7 @@ static void test_stream_config(IPin *pin) ok(hr == S_OK, "Got hr %#x.\n", hr); DeleteMediaType(format2); hr = IEnumMediaTypes_Next(enum_media_types, 1, &format2, NULL); - todo_wine ok(hr == S_FALSE, "Got hr %#x.\n", hr); + ok(hr == S_FALSE, "Got hr %#x.\n", hr); IEnumMediaTypes_Release(enum_media_types); format->majortype = MEDIATYPE_Audio; @@ -201,11 +201,7 @@ static void test_stream_config(IPin *pin) ok(hr == S_OK, "Got hr %#x.\n", hr); hr = IEnumMediaTypes_Next(enum_media_types, 1, &format2, NULL); ok(hr == S_OK, "Got hr %#x.\n", hr); - - /* Which media types will match varies in wine depending on the attached webcam */ - todo_wine_if(compare_media_types(format, format2) == FALSE) ok(compare_media_types(format, format2), "Media types didn't match.\n"); - DeleteMediaType(format2); IEnumMediaTypes_Release(enum_media_types); diff --git a/dlls/qcap/v4l.c b/dlls/qcap/v4l.c index f151cc66a3..d9e17f075b 100644 --- a/dlls/qcap/v4l.c +++ b/dlls/qcap/v4l.c @@ -256,6 +256,21 @@ static HRESULT v4l_device_get_format(struct video_capture_device *iface, AM_MEDI return CopyMediaType(mt, &device->caps[0].media_type); } +static HRESULT v4l_device_get_media_type(struct video_capture_device *iface, LONG index, + AM_MEDIA_TYPE *mt) +{ + struct v4l_device *device = v4l_device(iface); + LONG caps_count = (device->current_caps) ? 1 : device->caps_count; + + if (index >= caps_count) + return VFW_S_NO_MORE_ITEMS; + + if (device->current_caps) + return CopyMediaType(mt, &device->current_caps->media_type); + + return CopyMediaType(mt, &device->caps[index].media_type); +} + static __u32 v4l2_cid_from_qcap_property(VideoProcAmpProperty property) { switch (property) @@ -548,6 +563,7 @@ static const struct video_capture_device_ops v4l_device_ops = .check_format = v4l_device_check_format, .set_format = v4l_device_set_format, .get_format = v4l_device_get_format, + .get_media_type = v4l_device_get_media_type, .get_caps = v4l_device_get_caps, .get_caps_count = v4l_device_get_caps_count, .get_prop_range = v4l_device_get_prop_range, diff --git a/dlls/qcap/vfwcapture.c b/dlls/qcap/vfwcapture.c index 910be107b0..4f1a21ddc8 100644 --- a/dlls/qcap/vfwcapture.c +++ b/dlls/qcap/vfwcapture.c @@ -524,18 +524,7 @@ static HRESULT source_get_media_type(struct strmbase_pin *pin, unsigned int index, AM_MEDIA_TYPE *pmt) { VfwCapture *filter = impl_from_strmbase_pin(pin); - AM_MEDIA_TYPE *vfw_pmt; - HRESULT hr; - - if (index >= filter->device->ops->get_caps_count(filter->device)) - return VFW_S_NO_MORE_ITEMS; - - if (SUCCEEDED(hr = filter->device->ops->get_caps(filter->device, index, &vfw_pmt, NULL))) - { - CopyMediaType(pmt, vfw_pmt); - DeleteMediaType(vfw_pmt); - } - return hr; + return filter->device->ops->get_media_type(filter->device, index, pmt); } static HRESULT source_query_interface(struct strmbase_pin *iface, REFIID iid, void **out) -- 2.23.0
participants (2)
-
Jeff Smith -
Zebediah Figura