[PATCH 0/6] 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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10677
From: Brendan McGrath <bmcgrath@codeweavers.com> --- dlls/amstream/tests/amstream.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c index 7077c638c9f..53dbef6edcd 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -2903,6 +2903,17 @@ static void test_enum_media_types(void) hr = IEnumMediaTypes_Next(enum1, 1, mts, &count); ok(hr == S_OK, "Got hr %#lx.\n", hr); ok(count == 1, "Got count %lu.\n", count); + ok(IsEqualGUID(&mts[0]->majortype, &MEDIATYPE_Video), "Got major type %s\n", + wine_dbgstr_guid(&mts[0]->majortype)); + ok(IsEqualGUID(&mts[0]->subtype, &MEDIASUBTYPE_RGB8), "Got subtype %s\n", + wine_dbgstr_guid(&mts[0]->subtype)); + ok(mts[0]->bFixedSizeSamples == TRUE, "Got fixed size %d.\n", mts[0]->bFixedSizeSamples); + ok(!mts[0]->bTemporalCompression, "Got temporal compression %d.\n", mts[0]->bTemporalCompression); + ok(mts[0]->lSampleSize == 10000, "Got sample size %lu.\n", mts[0]->lSampleSize); + ok(IsEqualGUID(&mts[0]->formattype, &GUID_NULL), "Got format type %s.\n", + wine_dbgstr_guid(&mts[0]->formattype)); + ok(!mts[0]->pUnk, "Got pUnk %p.\n", mts[0]->pUnk); + CoTaskMemFree(mts[0]); hr = IEnumMediaTypes_Next(enum1, 1, mts, &count); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10677
From: Brendan McGrath <bmcgrath@codeweavers.com> --- dlls/amstream/tests/amstream.c | 40 ++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c index 53dbef6edcd..d38d1e7eff7 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -2881,6 +2881,7 @@ static void test_enum_media_types(void) IEnumMediaTypes *enum1, *enum2; AM_MEDIA_TYPE *mts[2]; IMediaStream *stream; + IDirectDraw *ddraw; ULONG ref, count; HRESULT hr; IPin *pin; @@ -3011,6 +3012,45 @@ static void test_enum_media_types(void) ref = IAMMultiMediaStream_Release(mmstream); ok(!ref, "Got outstanding refcount %ld.\n", ref); + + /* Test PrimaryVideo stream with ddraw */ + mmstream = create_ammultimediastream(); + + hr = DirectDrawCreate(NULL, &ddraw, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IAMMultiMediaStream_AddMediaStream(mmstream, (IUnknown *)ddraw, &MSPID_PrimaryVideo, 0, &stream); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IMediaStream_QueryInterface(stream, &IID_IPin, (void **)&pin); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IPin_EnumMediaTypes(pin, &enum1); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IEnumMediaTypes_Next(enum1, 1, mts, &count); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(count == 1, "Got count %lu.\n", count); + ok(IsEqualGUID(&mts[0]->majortype, &MEDIATYPE_Video), "Got major type %s\n", + wine_dbgstr_guid(&mts[0]->majortype)); + todo_wine + ok(IsEqualGUID(&mts[0]->subtype, &MEDIASUBTYPE_RGB32), "Got subtype %s\n", + wine_dbgstr_guid(&mts[0]->subtype)); + ok(mts[0]->bFixedSizeSamples == TRUE, "Got fixed size %d.\n", mts[0]->bFixedSizeSamples); + ok(!mts[0]->bTemporalCompression, "Got temporal compression %d.\n", mts[0]->bTemporalCompression); + todo_wine + ok(mts[0]->lSampleSize == 40000, "Got sample size %lu.\n", mts[0]->lSampleSize); + ok(IsEqualGUID(&mts[0]->formattype, &GUID_NULL), "Got format type %s.\n", + wine_dbgstr_guid(&mts[0]->formattype)); + ok(!mts[0]->pUnk, "Got pUnk %p.\n", mts[0]->pUnk); + + CoTaskMemFree(mts[0]); + IEnumMediaTypes_Release(enum1); + IPin_Release(pin); + IMediaStream_Release(stream); + IDirectDraw_Release(ddraw); + + ref = IAMMultiMediaStream_Release(mmstream); + ok(!ref, "Got outstanding refcount %ld.\n", ref); } static void test_media_types(void) -- 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 d38d1e7eff7..5b702333aa4 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -3104,26 +3104,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); @@ -3189,41 +3194,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 5b702333aa4..fd14161271a 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -3245,6 +3245,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 | 33 +++++++++++++++++++++++++++++++-- dlls/amstream/tests/amstream.c | 3 --- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/dlls/amstream/ddrawstream.c b/dlls/amstream/ddrawstream.c index 2f616dfc1e8..cf42f850d22 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,18 @@ 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); + DWORD bytes_per_pixel, subtype; + struct format *format; TRACE("iface %p, count %lu, mts %p, ret_count %p.\n", iface, count, mts, ret_count); @@ -976,12 +983,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.Data1; + if (subtype == MEDIASUBTYPE_RGB8.Data1) + bytes_per_pixel = 1; + if (mts[0]->subtype.Data1 == MEDIASUBTYPE_RGB555.Data1 + || mts[0]->subtype.Data1 == MEDIASUBTYPE_RGB565.Data1) + bytes_per_pixel = 2; + else if (mts[0]->subtype.Data1 == MEDIASUBTYPE_RGB24.Data1) + bytes_per_pixel = 3; + else if (mts[0]->subtype.Data1 == MEDIASUBTYPE_RGB32.Data1) + 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 +1049,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 +1354,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 +1368,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 fd14161271a..713b8f996d7 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -3032,12 +3032,10 @@ static void test_enum_media_types(void) ok(count == 1, "Got count %lu.\n", count); ok(IsEqualGUID(&mts[0]->majortype, &MEDIATYPE_Video), "Got major type %s\n", wine_dbgstr_guid(&mts[0]->majortype)); - todo_wine ok(IsEqualGUID(&mts[0]->subtype, &MEDIASUBTYPE_RGB32), "Got subtype %s\n", wine_dbgstr_guid(&mts[0]->subtype)); ok(mts[0]->bFixedSizeSamples == TRUE, "Got fixed size %d.\n", mts[0]->bFixedSizeSamples); ok(!mts[0]->bTemporalCompression, "Got temporal compression %d.\n", mts[0]->bTemporalCompression); - todo_wine ok(mts[0]->lSampleSize == 40000, "Got sample size %lu.\n", mts[0]->lSampleSize); ok(IsEqualGUID(&mts[0]->formattype, &GUID_NULL), "Got format type %s.\n", wine_dbgstr_guid(&mts[0]->formattype)); @@ -3254,7 +3252,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); } -- 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 cf42f850d22..7893c24e9a8 100644 --- a/dlls/amstream/ddrawstream.c +++ b/dlls/amstream/ddrawstream.c @@ -987,7 +987,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 (!IsEqualGUID(&enum_media_types->stream->mt.subtype, &GUID_NULL)) + 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 713b8f996d7..20bf26ff768 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -3234,10 +3234,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. 5/6: ``` + subtype = mts[0]->subtype.Data1; + if (subtype == MEDIASUBTYPE_RGB8.Data1) + bytes_per_pixel = 1; ``` That's... odd, I can only assume you're trying to do a more 'optimized' comparison, but I don't think optimization matters here; it would be more idiomatic to just use IsEqualGUID() on the whole thing. 6/6: ``` + if (!IsEqualGUID(&enum_media_types->stream->mt.subtype, &GUID_NULL)) ``` That's not how to check if the pin is connected. It works, but it's not declarative. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10677#note_136576
participants (3)
-
Brendan McGrath -
Brendan McGrath (＠redmcg)
-
Elizabeth Figura (＠zfigura)