Signed-off-by: Jeff Smith whydoubt@gmail.com --- Supersedes 191583-191585
v2: - Removed some tests that weren't providing much value - Added calculation of image size when setting format - Made use of VIDIOC_TRY_FMT ioctl - Added get_media_type entry to video_capture_device_ops
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); }
Signed-off-by: Jeff Smith whydoubt@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 6d67dd19be..cb1cd5aee0 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;
@@ -196,6 +199,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; } @@ -306,15 +311,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) { @@ -333,14 +334,8 @@ static DWORD WINAPI ReadThread(void *arg) IMediaSample *pSample = NULL; ULONG framecount = 0; 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; @@ -366,15 +361,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) { @@ -407,8 +401,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;
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=78244
Your paranoid android.
=== debiant (build log) ===
error: patch failed: dlls/qcap/v4l.c:333 Task: Patch failed to apply
=== debiant (build log) ===
error: patch failed: dlls/qcap/v4l.c:333 Task: Patch failed to apply
Signed-off-by: Jeff Smith whydoubt@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 cb1cd5aee0..89b57def87 100644 --- a/dlls/qcap/v4l.c +++ b/dlls/qcap/v4l.c @@ -198,13 +198,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); @@ -217,9 +240,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; }
@@ -227,7 +252,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) @@ -400,6 +428,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; @@ -660,9 +692,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; @@ -675,10 +706,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:
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=78245
Your paranoid android.
=== debiant (build log) ===
error: patch failed: dlls/qcap/v4l.c:333 error: patch failed: dlls/qcap/v4l.c:198 Task: Patch failed to apply
=== debiant (build log) ===
error: patch failed: dlls/qcap/v4l.c:333 error: patch failed: dlls/qcap/v4l.c:198 Task: Patch failed to apply
Signed-off-by: Jeff Smith whydoubt@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 89b57def87..e64db880f3 100644 --- a/dlls/qcap/v4l.c +++ b/dlls/qcap/v4l.c @@ -258,6 +258,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) @@ -552,6 +567,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 17e56a7ee9..e5a1d6d1fc 100644 --- a/dlls/qcap/vfwcapture.c +++ b/dlls/qcap/vfwcapture.c @@ -526,18 +526,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)
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=78246
Your paranoid android.
=== build (build log) ===
error: patch failed: dlls/qcap/v4l.c:333 error: patch failed: dlls/qcap/v4l.c:198 error: patch failed: dlls/qcap/v4l.c:258 Task: Patch failed to apply
=== debiant (build log) ===
error: patch failed: dlls/qcap/v4l.c:333 error: patch failed: dlls/qcap/v4l.c:198 error: patch failed: dlls/qcap/v4l.c:258 Task: Patch failed to apply
=== debiant (build log) ===
error: patch failed: dlls/qcap/v4l.c:333 error: patch failed: dlls/qcap/v4l.c:198 error: patch failed: dlls/qcap/v4l.c:258 Task: Patch failed to apply