[PATCH 2/4] qcap/tests: Test effect of setting stream format on media type enumeration.
Zebediah Figura
z.figura12 at gmail.com
Fri Aug 28 14:07:40 CDT 2020
On 8/28/20 1:33 PM, Jeff Smith wrote:
> On Fri, Aug 28, 2020 at 11:34 AM Zebediah Figura <z.figura12 at gmail.com> wrote:
>>
>> On 8/28/20 10:52 AM, Jeff Smith wrote:
>>> Signed-off-by: Jeff Smith <whydoubt at gmail.com>
>>> ---
>>> dlls/qcap/tests/videocapture.c | 137 ++++++++++++++++++++++++++++++---
>>> 1 file changed, 127 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/dlls/qcap/tests/videocapture.c b/dlls/qcap/tests/videocapture.c
>>> index 326601a398..c0fd30aee3 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)
>>> {
>>> @@ -78,11 +84,13 @@ static void test_media_types(IPin *pin)
>>> static void test_stream_config(IPin *pin)
>>> {
>>> VIDEOINFOHEADER *video_info, *video_info2;
>>> - AM_MEDIA_TYPE *format, *format2;
>>> + AM_MEDIA_TYPE *format, *format2, *formats[2];
>>> VIDEO_STREAM_CONFIG_CAPS vscc;
>>> IAMStreamConfig *stream_config;
>>> + IEnumMediaTypes *enum_media_types;
>>> LONG depth, compression;
>>> - LONG count, size;
>>> + LONG count, size, i;
>>> + ULONG fetched;
>>> HRESULT hr;
>>>
>>> hr = IPin_QueryInterface(pin, &IID_IAMStreamConfig, (void **)&stream_config);
>>> @@ -93,9 +101,27 @@ static void test_stream_config(IPin *pin)
>>> ok(IsEqualGUID(&format->majortype, &MEDIATYPE_Video), "Got wrong majortype: %s.\n",
>>> debugstr_guid(&format->majortype));
>>>
>>> + /* Before setting format, multiple media types are enumerated. */
>>> + IPin_EnumMediaTypes(pin, &enum_media_types);
>>> + hr = IEnumMediaTypes_Next(enum_media_types, 2, formats, &fetched);
>>> + ok(hr == S_OK, "Got hr %#x.\n", hr);
>>> + ok(fetched == 2, "Fetched %u.\n", fetched);
>>> + DeleteMediaType(formats[0]);
>>> + DeleteMediaType(formats[1]);
>>> + IEnumMediaTypes_Release(enum_media_types);
>>> +
>>
>> I'm not exactly sure we need to test this, and I'm a little nervous to
>
> Which 'this' do you mean?
> That >1 media types are enumerated before SetFormat?
> Or that exactly 1 media type is enumerated after SetFormat?
> Or that SetFormat is the point at which this change occurs?
That more than one media type is returned.
>
>> given how hardware-dependent behaviour can be.
>
> A video capture pin that only supported one media type would break
> this, but unless we find an exception to the rule, isn't it good to attempt
> testing the rule?
As far as I see, it doesn't prove anything, and doesn't follow any
documented guarantees, so there's no reason to test the rule.
>
>>> 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, 2, formats, &fetched);
>>> + todo_wine ok(hr == S_FALSE, "Got hr %#x.\n", hr);
>>> + todo_wine ok(fetched == 1, "Fetched %u.\n", fetched);
>>> + DeleteMediaType(formats[0]);
>>> + 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 +188,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);
>>> +
>>> + FreeMediaType(format);
>>> + }
>>>
>>> IAMStreamConfig_Release(stream_config);
>>> }
>>> @@ -260,6 +310,50 @@ static void test_misc_flags(IBaseFilter *filter)
>>> IAMFilterMiscFlags_Release(misc_flags);
>>> }
>>>
>>> +static int count_media_types(IPin *pin)
>>> +{
>>> + IEnumMediaTypes *enum_media_types;
>>> + AM_MEDIA_TYPE *pmt;
>>> + int count = 0;
>>> +
>>> + IPin_EnumMediaTypes(pin, &enum_media_types);
>>> + while (IEnumMediaTypes_Next(enum_media_types, 1, &pmt, NULL) == S_OK)
>>> + {
>>> + CoTaskMemFree(pmt);
>>> + count++;
>>> + }
>>> + IEnumMediaTypes_Release(enum_media_types);
>>> +
>>> + return count;
>>> +}
>>> +
>>> +static void count_pins_media_types(IBaseFilter *filter, int *pin_count, int *media_type_count)
>>> +{
>>> + IEnumPins *enum_pins;
>>> + IPin *pin;
>>> + HRESULT hr;
>>> +
>>> + *pin_count = 0;
>>> + *media_type_count = 0;
>>> +
>>> + hr = IBaseFilter_EnumPins(filter, &enum_pins);
>>> + ok(hr == S_OK, "Got hr %#x.\n", hr);
>>> +
>>> + while ((hr = IEnumPins_Next(enum_pins, 1, &pin, NULL)) == S_OK)
>>> + {
>>> + PIN_DIRECTION pin_direction;
>>> + IPin_QueryDirection(pin, &pin_direction);
>>> + if (pin_direction == PINDIR_OUTPUT)
>>> + {
>>> + (*pin_count)++;
>>> + (*media_type_count) += count_media_types(pin);
>>> + }
>>> + IPin_Release(pin);
>>> + }
>>> +
>>> + IEnumPins_Release(enum_pins);
>>> +}
>>> +
>>> START_TEST(videocapture)
>>> {
>>> ICreateDevEnum *dev_enum;
>>> @@ -269,6 +363,7 @@ START_TEST(videocapture)
>>> WCHAR *name;
>>> HRESULT hr;
>>> ULONG ref;
>>> + int pin_count, media_type_count;
>>>
>>> CoInitialize(NULL);
>>>
>>> @@ -297,7 +392,19 @@ START_TEST(videocapture)
>>> if (hr == S_OK)
>>> {
>>> test_filter_interfaces(filter);
>>> +
>>> + count_pins_media_types(filter, &pin_count, &media_type_count);
>>> + ok(media_type_count > pin_count,
>>> + "Expected media type count (%d) to be greater than pin count (%d)\n",
>>> + media_type_count, pin_count);
>>> +
>>> test_pins(filter);
>>> +
>>> + count_pins_media_types(filter, &pin_count, &media_type_count);
>>> + todo_wine ok(media_type_count == pin_count,
>>> + "Expected media type count (%d) to be equal to pin count (%d)\n",
>>> + media_type_count, pin_count);
>>> +
>>
>> This is a bit awkward (and, for that matter, doesn't exactly prove
>
> Yes, it is awkward. However, I'm not really sure what would be better
> for what I am trying to test here.
Well, as stated, I think the tests you've add above already do a good
enough job of proving it.
>
>> either of the propositions desired). Moreover, it seems that the tests
>> added to test_stream_config() already prove this.
>>
>>> test_misc_flags(filter);
>>> ref = IBaseFilter_Release(filter);
>>> ok(!ref, "Got outstanding refcount %d.\n", ref);
>>> @@ -305,6 +412,16 @@ START_TEST(videocapture)
>>> else
>>> skip("Failed to open capture device, hr=%#x.\n", hr);
>>>
>>> + hr = IMoniker_BindToObject(moniker, NULL, NULL, &IID_IBaseFilter, (void**)&filter);
>>> + if (hr == S_OK)
>>> + {
>>> + count_pins_media_types(filter, &pin_count, &media_type_count);
>>> + ok(media_type_count > pin_count,
>>> + "Expected media type count (%d) to be greater than pin count (%d)\n",
>>> + media_type_count, pin_count);
>>> + ref = IBaseFilter_Release(filter);
>>> + }
>>> +
>>
>> I guess, but I also don't really see a reason why the previous
>> instantiation of a filter would affect this one.
>
> The three tests with count_pins_media_types are to check that
> releasing pins DOES NOT
> clear the EnumMediaTypes state, but releasing the filter DOES. TBH, my
> first assumption
> had been that that state would be cleared when the pin was released.
> Perhaps that just
> proves my naïveté, but it was the test that enlightened me. Also, I
> have seen states in
> other areas that persist beyond a particular instantiation, so I
> didn't want to assume that
> either.
The lifetime of a filter and its pins are pretty well tied together.
I'm not necessarily opposed to such a test, but it doesn't seem necessary.
>
>>> IMoniker_Release(moniker);
>>> }
>>>
>>>
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20200828/29d19f57/attachment-0001.sig>
More information about the wine-devel
mailing list