[PATCH v2 0/5] MR10677: amstream: Test and fix media type enum on ddraw stream.
This MR adds new tests and subsequent fixes for the type returned by `IEnumMediaTypes::Next` on the `IEnumMediaTypes` retrieved from the `IPin::EnumMediaTypes` function on the Direct Draw media stream. -- v2: amstream: Return connected media subtype in enum if connected. amstream: Return current format in media type enum. amstream/tests: Test enum media type on disconnect. amstream/tests: Test enum media type whilst connected. amstream/tests: Test enum media type with ddraw. https://gitlab.winehq.org/wine/wine/-/merge_requests/10677
From: Brendan McGrath <bmcgrath@codeweavers.com> --- dlls/amstream/tests/amstream.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c index 7077c638c9f..d70149d2572 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -3248,6 +3248,28 @@ static void test_media_types(void) hr = IDirectDrawMediaStream_GetFormat(ddraw_stream, ¤t, &palette, &desired, &flags); ok(hr == MS_E_NOSTREAM, "Got hr %#lx.\n", hr); + hr = IPin_EnumMediaTypes(pin, &enummt); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IEnumMediaTypes_Next(enummt, 1, &pmt, &count); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(count == 1, "Got count %lu.\n", count); + ok(IsEqualGUID(&pmt->majortype, &MEDIATYPE_Video), "Got major type %s\n", + wine_dbgstr_guid(&pmt->majortype)); + todo_wine + ok(IsEqualGUID(&pmt->subtype, &MEDIASUBTYPE_RGB32), "Got subtype %s\n", + wine_dbgstr_guid(&pmt->subtype)); + ok(pmt->bFixedSizeSamples == TRUE, "Got fixed size %d.\n", pmt->bFixedSizeSamples); + ok(!pmt->bTemporalCompression, "Got temporal compression %d.\n", pmt->bTemporalCompression); + todo_wine + ok(pmt->lSampleSize == 40000, "Got sample size %lu.\n", pmt->lSampleSize); + ok(IsEqualGUID(&pmt->formattype, &GUID_NULL), "Got format type %s.\n", + wine_dbgstr_guid(&pmt->formattype)); + ok(!pmt->pUnk, "Got pUnk %p.\n", pmt->pUnk); + + DeleteMediaType(pmt); + IEnumMediaTypes_Release(enummt); + memset(&mt, 0, sizeof(mt)); mt.majortype = MEDIATYPE_Video; mt.subtype = MEDIASUBTYPE_RGB32; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10677
From: Brendan McGrath <bmcgrath@codeweavers.com> --- dlls/amstream/tests/amstream.c | 77 +++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 29 deletions(-) diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c index d70149d2572..3d578877ab2 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -3053,26 +3053,31 @@ static void test_media_types(void) {&FORMAT_VideoInfo, (BYTE *)&req_vih, sizeof(VIDEOINFOHEADER)}, }; - static const GUID *rejected_subtypes[] = + static const struct { - &MEDIASUBTYPE_RGB565, - &MEDIASUBTYPE_RGB555, - &MEDIASUBTYPE_RGB24, - &MEDIASUBTYPE_RGB32, - &MEDIASUBTYPE_RGB1, - &MEDIASUBTYPE_RGB4, - &MEDIASUBTYPE_ARGB32, - &MEDIASUBTYPE_ARGB1555, - &MEDIASUBTYPE_ARGB4444, - &MEDIASUBTYPE_Avi, - &MEDIASUBTYPE_I420, - &MEDIASUBTYPE_AYUV, - &MEDIASUBTYPE_YV12, - &MEDIASUBTYPE_YUY2, - &MEDIASUBTYPE_UYVY, - &MEDIASUBTYPE_YVYU, - &MEDIASUBTYPE_NV12, - &GUID_NULL, + const GUID *guid; + BYTE bytes_per_pixel; + } + rejected_subtypes[] = + { + {&MEDIASUBTYPE_RGB565, 2 }, + {&MEDIASUBTYPE_RGB555, 2 }, + {&MEDIASUBTYPE_RGB24, 3 }, + {&MEDIASUBTYPE_RGB32, 4 }, + {&MEDIASUBTYPE_RGB1}, + {&MEDIASUBTYPE_RGB4}, + {&MEDIASUBTYPE_ARGB32}, + {&MEDIASUBTYPE_ARGB1555}, + {&MEDIASUBTYPE_ARGB4444}, + {&MEDIASUBTYPE_Avi}, + {&MEDIASUBTYPE_I420}, + {&MEDIASUBTYPE_AYUV}, + {&MEDIASUBTYPE_YV12}, + {&MEDIASUBTYPE_YUY2}, + {&MEDIASUBTYPE_UYVY}, + {&MEDIASUBTYPE_YVYU}, + {&MEDIASUBTYPE_NV12}, + {&GUID_NULL}, }; hr = IAMMultiMediaStream_AddMediaStream(mmstream, NULL, &MSPID_PrimaryVideo, 0, &stream); @@ -3138,41 +3143,55 @@ static void test_media_types(void) ok(hr == S_OK, "Got hr %#lx.\n", hr); /* A negative height is never accepted */ - vih->bmiHeader.biHeight = -1; + vih->bmiHeader.biHeight = -200; hr = IPin_QueryAccept(pin, &mt); ok(hr == VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#lx.\n", hr); for (i = 0; i < ARRAY_SIZE(rejected_subtypes); ++i) { - mt.subtype = *rejected_subtypes[i]; - vih->bmiHeader.biHeight = 1; + mt.subtype = *rejected_subtypes[i].guid; + vih->bmiHeader.biHeight = 200; + vih->bmiHeader.biWidth = 200; hr = IPin_QueryAccept(pin, &mt); ok(hr == VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#lx for subtype %s.\n", - hr, wine_dbgstr_guid(rejected_subtypes[i])); + hr, wine_dbgstr_guid(rejected_subtypes[i].guid)); hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &mt); ok(hr == (i < 4) ? S_OK : VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#lx on ReceiveConnection for subtype %s.\n", hr, - wine_dbgstr_guid(rejected_subtypes[i])); + wine_dbgstr_guid(rejected_subtypes[i].guid)); if (hr == S_OK) { for (j = 0; j < ARRAY_SIZE(rejected_subtypes); ++j) { - mt.subtype = *rejected_subtypes[j]; + mt.subtype = *rejected_subtypes[j].guid; hr = IPin_QueryAccept(pin, &mt); ok(hr == (j < 4 ? S_OK : VFW_E_TYPE_NOT_ACCEPTED), "Got hr %#lx for subtype %s whilst connected.\n", - hr, wine_dbgstr_guid(rejected_subtypes[j])); + hr, wine_dbgstr_guid(rejected_subtypes[j].guid)); } /* A negative height is never accepted */ - vih->bmiHeader.biHeight = -1; + vih->bmiHeader.biHeight = -200; for (j = 0; j < ARRAY_SIZE(rejected_subtypes); ++j) { - mt.subtype = *rejected_subtypes[j]; + mt.subtype = *rejected_subtypes[j].guid; hr = IPin_QueryAccept(pin, &mt); ok(hr == VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#lx for subtype %s using negative height.\n", - hr, wine_dbgstr_guid(rejected_subtypes[j])); + hr, wine_dbgstr_guid(rejected_subtypes[j].guid)); } + hr = IEnumMediaTypes_Reset(enummt); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IEnumMediaTypes_Next(enummt, 1, &pmt, &count); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(IsEqualGUID(&pmt->majortype, &MEDIATYPE_Video), "Unexpected media type %s.\n", wine_dbgstr_guid(&pmt->majortype)); + todo_wine + ok(IsEqualGUID(&pmt->subtype, rejected_subtypes[i].guid), "Unexpected media subtype %s.\n", wine_dbgstr_guid(&pmt->subtype)); + ok(IsEqualGUID(&pmt->formattype, &GUID_NULL), "Unexpected media formattype %s.\n", wine_dbgstr_guid(&pmt->formattype)); + todo_wine + ok(pmt->lSampleSize == 40000 * rejected_subtypes[i].bytes_per_pixel, "Unexpected sample size %lu.\n", pmt->lSampleSize); + DeleteMediaType(pmt); + hr = IPin_Disconnect(pin); ok(hr == S_OK, "Got hr %#lx.\n", hr); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10677
From: Brendan McGrath <bmcgrath@codeweavers.com> --- dlls/amstream/tests/amstream.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c index 3d578877ab2..cc5d5432411 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -3194,6 +3194,18 @@ static void test_media_types(void) hr = IPin_Disconnect(pin); ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IEnumMediaTypes_Reset(enummt); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IEnumMediaTypes_Next(enummt, 1, &pmt, &count); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(IsEqualGUID(&pmt->majortype, &MEDIATYPE_Video), "Unexpected media type %s.\n", wine_dbgstr_guid(&pmt->majortype)); + ok(IsEqualGUID(&pmt->subtype, &MEDIASUBTYPE_RGB8), "Unexpected media subtype %s.\n", wine_dbgstr_guid(&pmt->subtype)); + ok(IsEqualGUID(&pmt->formattype, &GUID_NULL), "Unexpected media formattype %s.\n", wine_dbgstr_guid(&pmt->formattype)); + todo_wine + ok(pmt->lSampleSize == 40000, "Unexpected sample size %lu.\n", pmt->lSampleSize); + DeleteMediaType(pmt); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10677
From: Brendan McGrath <bmcgrath@codeweavers.com> --- dlls/amstream/ddrawstream.c | 34 ++++++++++++++++++++++++++++++++-- dlls/amstream/tests/amstream.c | 3 --- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/dlls/amstream/ddrawstream.c b/dlls/amstream/ddrawstream.c index 2f616dfc1e8..cdb1cfd3178 100644 --- a/dlls/amstream/ddrawstream.c +++ b/dlls/amstream/ddrawstream.c @@ -922,6 +922,8 @@ struct enum_media_types IEnumMediaTypes IEnumMediaTypes_iface; LONG refcount; unsigned int index; + + struct ddraw_stream *stream; }; static const IEnumMediaTypesVtbl enum_media_types_vtbl; @@ -961,13 +963,19 @@ static ULONG WINAPI enum_media_types_Release(IEnumMediaTypes *iface) ULONG refcount = InterlockedDecrement(&enum_media_types->refcount); TRACE("%p decreasing refcount to %lu.\n", enum_media_types, refcount); if (!refcount) + { + IAMMediaStream_Release(&enum_media_types->stream->IAMMediaStream_iface); free(enum_media_types); + } return refcount; } static HRESULT WINAPI enum_media_types_Next(IEnumMediaTypes *iface, ULONG count, AM_MEDIA_TYPE **mts, ULONG *ret_count) { struct enum_media_types *enum_media_types = impl_from_IEnumMediaTypes(iface); + struct format *format; + DWORD bytes_per_pixel; + GUID *subtype; TRACE("iface %p, count %lu, mts %p, ret_count %p.\n", iface, count, mts, ret_count); @@ -976,12 +984,29 @@ static HRESULT WINAPI enum_media_types_Next(IEnumMediaTypes *iface, ULONG count, if (count && !enum_media_types->index) { + format = &enum_media_types->stream->format; mts[0] = CoTaskMemAlloc(sizeof(AM_MEDIA_TYPE)); memset(mts[0], 0, sizeof(AM_MEDIA_TYPE)); mts[0]->majortype = MEDIATYPE_Video; - mts[0]->subtype = MEDIASUBTYPE_RGB8; + if (enum_media_types->stream->format.flags & DDSD_PIXELFORMAT) + subtype_from_pf(&mts[0]->subtype, &format->pf); + else + mts[0]->subtype = MEDIASUBTYPE_RGB8; + mts[0]->bFixedSizeSamples = TRUE; - mts[0]->lSampleSize = 10000; + + subtype = &mts[0]->subtype; + if (IsEqualGUID(subtype, &MEDIASUBTYPE_RGB8)) + bytes_per_pixel = 1; + if (IsEqualGUID(subtype, &MEDIASUBTYPE_RGB555) + || IsEqualGUID(subtype, &MEDIASUBTYPE_RGB565)) + bytes_per_pixel = 2; + else if (IsEqualGUID(subtype, &MEDIASUBTYPE_RGB24)) + bytes_per_pixel = 3; + else if (IsEqualGUID(subtype, &MEDIASUBTYPE_RGB32)) + bytes_per_pixel = 4; + + mts[0]->lSampleSize = format->width * format->height * bytes_per_pixel; ++enum_media_types->index; *ret_count = 1; return count == 1 ? S_OK : S_FALSE; @@ -1025,6 +1050,8 @@ static HRESULT WINAPI enum_media_types_Clone(IEnumMediaTypes *iface, IEnumMediaT object->IEnumMediaTypes_iface.lpVtbl = &enum_media_types_vtbl; object->refcount = 1; object->index = enum_media_types->index; + object->stream = enum_media_types->stream; + IAMMediaStream_AddRef(&object->stream->IAMMediaStream_iface); *out = &object->IEnumMediaTypes_iface; return S_OK; @@ -1328,6 +1355,7 @@ static HRESULT WINAPI ddraw_sink_QueryAccept(IPin *iface, const AM_MEDIA_TYPE *m static HRESULT WINAPI ddraw_sink_EnumMediaTypes(IPin *iface, IEnumMediaTypes **enum_media_types) { + struct ddraw_stream *stream = impl_from_IPin(iface); struct enum_media_types *object; TRACE("iface %p, enum_media_types %p.\n", iface, enum_media_types); @@ -1341,6 +1369,8 @@ static HRESULT WINAPI ddraw_sink_EnumMediaTypes(IPin *iface, IEnumMediaTypes **e object->IEnumMediaTypes_iface.lpVtbl = &enum_media_types_vtbl; object->refcount = 1; object->index = 0; + object->stream = stream; + IAMMediaStream_AddRef(&stream->IAMMediaStream_iface); *enum_media_types = &object->IEnumMediaTypes_iface; return S_OK; diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c index cc5d5432411..cba1f2f7443 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -3203,7 +3203,6 @@ static void test_media_types(void) ok(IsEqualGUID(&pmt->majortype, &MEDIATYPE_Video), "Unexpected media type %s.\n", wine_dbgstr_guid(&pmt->majortype)); ok(IsEqualGUID(&pmt->subtype, &MEDIASUBTYPE_RGB8), "Unexpected media subtype %s.\n", wine_dbgstr_guid(&pmt->subtype)); ok(IsEqualGUID(&pmt->formattype, &GUID_NULL), "Unexpected media formattype %s.\n", wine_dbgstr_guid(&pmt->formattype)); - todo_wine ok(pmt->lSampleSize == 40000, "Unexpected sample size %lu.\n", pmt->lSampleSize); DeleteMediaType(pmt); } @@ -3287,12 +3286,10 @@ static void test_media_types(void) ok(count == 1, "Got count %lu.\n", count); ok(IsEqualGUID(&pmt->majortype, &MEDIATYPE_Video), "Got major type %s\n", wine_dbgstr_guid(&pmt->majortype)); - todo_wine ok(IsEqualGUID(&pmt->subtype, &MEDIASUBTYPE_RGB32), "Got subtype %s\n", wine_dbgstr_guid(&pmt->subtype)); ok(pmt->bFixedSizeSamples == TRUE, "Got fixed size %d.\n", pmt->bFixedSizeSamples); ok(!pmt->bTemporalCompression, "Got temporal compression %d.\n", pmt->bTemporalCompression); - todo_wine ok(pmt->lSampleSize == 40000, "Got sample size %lu.\n", pmt->lSampleSize); ok(IsEqualGUID(&pmt->formattype, &GUID_NULL), "Got format type %s.\n", wine_dbgstr_guid(&pmt->formattype)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10677
From: Brendan McGrath <bmcgrath@codeweavers.com> --- dlls/amstream/ddrawstream.c | 4 +++- dlls/amstream/tests/amstream.c | 2 -- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dlls/amstream/ddrawstream.c b/dlls/amstream/ddrawstream.c index cdb1cfd3178..55ab7ab25d4 100644 --- a/dlls/amstream/ddrawstream.c +++ b/dlls/amstream/ddrawstream.c @@ -988,7 +988,9 @@ static HRESULT WINAPI enum_media_types_Next(IEnumMediaTypes *iface, ULONG count, mts[0] = CoTaskMemAlloc(sizeof(AM_MEDIA_TYPE)); memset(mts[0], 0, sizeof(AM_MEDIA_TYPE)); mts[0]->majortype = MEDIATYPE_Video; - if (enum_media_types->stream->format.flags & DDSD_PIXELFORMAT) + if (enum_media_types->stream->peer) + mts[0]->subtype = enum_media_types->stream->mt.subtype; + else if (enum_media_types->stream->format.flags & DDSD_PIXELFORMAT) subtype_from_pf(&mts[0]->subtype, &format->pf); else mts[0]->subtype = MEDIASUBTYPE_RGB8; diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c index cba1f2f7443..b3f555fa735 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -3185,10 +3185,8 @@ static void test_media_types(void) hr = IEnumMediaTypes_Next(enummt, 1, &pmt, &count); ok(hr == S_OK, "Got hr %#lx.\n", hr); ok(IsEqualGUID(&pmt->majortype, &MEDIATYPE_Video), "Unexpected media type %s.\n", wine_dbgstr_guid(&pmt->majortype)); - todo_wine ok(IsEqualGUID(&pmt->subtype, rejected_subtypes[i].guid), "Unexpected media subtype %s.\n", wine_dbgstr_guid(&pmt->subtype)); ok(IsEqualGUID(&pmt->formattype, &GUID_NULL), "Unexpected media formattype %s.\n", wine_dbgstr_guid(&pmt->formattype)); - todo_wine ok(pmt->lSampleSize == 40000 * rejected_subtypes[i].bytes_per_pixel, "Unexpected sample size %lu.\n", pmt->lSampleSize); DeleteMediaType(pmt); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10677
It's a bit confusing but test_enum_media_types() is for testing some behaviour related to the enumerator; the media type itself is already tested in test_media_types() and the new tests should go there instead.
OK, thanks for that. I've removed the first commit (as that was a duplicate test) and moved the second from `test_enum_media_types` to `test_media_types`.
it would be more idiomatic to just use IsEqualGUID() on the whole thing.
I agree. I have changed that.
That's not how to check if the pin is connected.
I've changed it to check for `enum_media_types->stream->peer` instead. Hopefully that's what you meant? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10677#note_136641
v2: - Remove a duplicated test - Move the ddraw test to `test_media_types` - Use `IsEqualGUID` rather than compare just `Data1` - Use `enum_media_types->stream->peer` to check if the pin is connected -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10677#note_136642
This merge request was approved by Elizabeth Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10677
participants (3)
-
Brendan McGrath -
Brendan McGrath (＠redmcg)
-
Elizabeth Figura (＠zfigura)