[PATCH 0/3] MR7538: winegstreamer: Implement type handler functions for media sink.
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/mf/tests/mf.c | 5 ----- dlls/winegstreamer/media_sink.c | 9 +++++++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 73526802b63..53f22809dc4 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -6019,12 +6019,9 @@ static void test_mpeg4_media_sink(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFMediaTypeHandler_GetMajorType(type_handler, NULL); - todo_wine ok(hr == E_POINTER, "Unexpected hr %#lx.\n", hr); hr = IMFMediaTypeHandler_GetMajorType(type_handler, &guid); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine ok(IsEqualGUID(&guid, &MFMediaType_Audio), "Unexpected major type.\n"); hr = IMFMediaTypeHandler_GetMediaTypeCount(type_handler, &count); @@ -6062,10 +6059,8 @@ static void test_mpeg4_media_sink(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFMediaTypeHandler_GetMajorType(type_handler, NULL); - todo_wine ok(hr == E_POINTER, "Unexpected hr %#lx.\n", hr); hr = IMFMediaTypeHandler_GetMajorType(type_handler, &guid); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); IMFStreamSink_Release(stream_sink); diff --git a/dlls/winegstreamer/media_sink.c b/dlls/winegstreamer/media_sink.c index 5ee2c44dc70..3ee03d663d0 100644 --- a/dlls/winegstreamer/media_sink.c +++ b/dlls/winegstreamer/media_sink.c @@ -460,9 +460,14 @@ static HRESULT WINAPI stream_sink_type_handler_GetCurrentMediaType(IMFMediaTypeH static HRESULT WINAPI stream_sink_type_handler_GetMajorType(IMFMediaTypeHandler *iface, GUID *type) { - FIXME("iface %p, type %p.\n", iface, type); + struct stream_sink *stream_sink = impl_from_IMFMediaTypeHandler(iface); - return E_NOTIMPL; + TRACE("iface %p, type %p.\n", iface, type); + + if (!type) + return E_POINTER; + + return IMFMediaType_GetMajorType(stream_sink->type, type); } static const IMFMediaTypeHandlerVtbl stream_sink_type_handler_vtbl = -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7538
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/mf/tests/mf.c | 4 ++-- dlls/winegstreamer/media_sink.c | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 53f22809dc4..b9363163370 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -6024,10 +6024,10 @@ static void test_mpeg4_media_sink(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok(IsEqualGUID(&guid, &MFMediaType_Audio), "Unexpected major type.\n"); + hr = IMFMediaTypeHandler_GetMediaTypeCount(type_handler, NULL); + ok(hr == E_POINTER, "Unexpected hr %#lx.\n", hr); hr = IMFMediaTypeHandler_GetMediaTypeCount(type_handler, &count); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine ok(count == 1, "Unexpected count %lu.\n", count); hr = IMFMediaTypeHandler_GetCurrentMediaType(type_handler, &media_type); diff --git a/dlls/winegstreamer/media_sink.c b/dlls/winegstreamer/media_sink.c index 3ee03d663d0..e8ed90951bf 100644 --- a/dlls/winegstreamer/media_sink.c +++ b/dlls/winegstreamer/media_sink.c @@ -422,9 +422,13 @@ static HRESULT WINAPI stream_sink_type_handler_IsMediaTypeSupported(IMFMediaType static HRESULT WINAPI stream_sink_type_handler_GetMediaTypeCount(IMFMediaTypeHandler *iface, DWORD *count) { - FIXME("iface %p, count %p.\n", iface, count); + TRACE("iface %p, count %p.\n", iface, count); - return E_NOTIMPL; + if (!count) + return E_POINTER; + + *count = 1; + return S_OK; } static HRESULT WINAPI stream_sink_type_handler_GetMediaTypeByIndex(IMFMediaTypeHandler *iface, DWORD index, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7538
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/mf/tests/mf.c | 10 +++++++++- dlls/winegstreamer/media_sink.c | 13 +++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index b9363163370..37c1b0be456 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -5794,8 +5794,8 @@ static void test_mpeg4_media_sink(void) { IMFMediaSink *sink = NULL, *sink2 = NULL, *sink_audio = NULL, *sink_video = NULL, *sink_empty = NULL; IMFByteStream *bytestream, *bytestream_audio, *bytestream_video, *bytestream_empty; + IMFMediaType *audio_type, *video_type, *media_type, *media_type_out; DWORD id, count, flags, width = 96, height = 96; - IMFMediaType *audio_type, *video_type, *media_type; IMFMediaTypeHandler *type_handler = NULL; IMFPresentationClock *clock; IMFStreamSink *stream_sink; @@ -6033,6 +6033,14 @@ static void test_mpeg4_media_sink(void) hr = IMFMediaTypeHandler_GetCurrentMediaType(type_handler, &media_type); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFMediaTypeHandler_GetMediaTypeByIndex(type_handler, 1, &media_type_out); + ok(hr == MF_E_NO_MORE_TYPES, "Unexpected hr %#lx.\n", hr); + hr = IMFMediaTypeHandler_GetMediaTypeByIndex(type_handler, 0, NULL); + ok(hr == E_POINTER, "Unexpected hr %#lx.\n", hr); + hr = IMFMediaTypeHandler_GetMediaTypeByIndex(type_handler, 0, &media_type_out); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(media_type_out == media_type, "Got different media type pointer.\n"); + hr = IMFMediaType_SetUINT32(media_type, &MF_MT_AUDIO_NUM_CHANNELS, 1); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); diff --git a/dlls/winegstreamer/media_sink.c b/dlls/winegstreamer/media_sink.c index e8ed90951bf..391336993fe 100644 --- a/dlls/winegstreamer/media_sink.c +++ b/dlls/winegstreamer/media_sink.c @@ -434,9 +434,18 @@ static HRESULT WINAPI stream_sink_type_handler_GetMediaTypeCount(IMFMediaTypeHan static HRESULT WINAPI stream_sink_type_handler_GetMediaTypeByIndex(IMFMediaTypeHandler *iface, DWORD index, IMFMediaType **type) { - FIXME("iface %p, index %lu, type %p.\n", iface, index, type); + struct stream_sink *stream_sink = impl_from_IMFMediaTypeHandler(iface); - return E_NOTIMPL; + TRACE("iface %p, index %lu, type %p.\n", iface, index, type); + + if (!type) + return E_POINTER; + if (index > 0) + return MF_E_NO_MORE_TYPES; + + IMFMediaType_AddRef((*type = stream_sink->type)); + + return S_OK; } static HRESULT WINAPI stream_sink_type_handler_SetCurrentMediaType(IMFMediaTypeHandler *iface, IMFMediaType *type) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7538
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/media_sink.c:
static HRESULT WINAPI stream_sink_type_handler_GetMajorType(IMFMediaTypeHandler *iface, GUID *type) { - FIXME("iface %p, type %p.\n", iface, type); + struct stream_sink *stream_sink = impl_from_IMFMediaTypeHandler(iface);
- return E_NOTIMPL; + TRACE("iface %p, type %p.\n", iface, type); + + if (!type) + return E_POINTER; + + return IMFMediaType_GetMajorType(stream_sink->type, type);
If I'm reading the code correctly `stream_sink->type` may be NULL. There's a test for that case, and it looks like `IMFMediaSink_AddStreamSink` may also return `E_INVALIDARG` in that case. It's flagged as broken, but maybe we could decide to do that instead so that we can then assume media type is always set? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7538#note_97364
There's a test for that case, and it looks like `IMFMediaSink_AddStreamSink` may also return `E_INVALIDARG` in that case. It's flagged as broken
I can't find that tests, can you tell me where it is? I do find a IMFMediaSink_AddStreamSink test will NULL type which is flagged as broken, but it's for VideoRenderer in test_evr(), not for media sink(file sink). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7538#note_97370
On Tue Mar 11 09:16:16 2025 +0000, Ziqing Hui wrote:
There's a test for that case, and it looks like `IMFMediaSink_AddStreamSink` may also return `E_INVALIDARG` in that case. It's flagged as broken I can't find that tests, can you tell me where it is? I do find a IMFMediaSink_AddStreamSink test will NULL type which is flagged as broken, but it's for VideoRenderer in test_evr(), not for media sink(file sink). Yes you're right, it's an evr test. Well, then maybe a test for a media sink with NULL type would tell whether it can be NULL, which seems weird.
If it's indeed not allowed, the logic should probably be adjusted to reject it on stream creation so we can later assume it's not NULL. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7538#note_97378
On Tue Mar 11 09:24:56 2025 +0000, Rémi Bernon wrote:
Yes you're right, it's an evr test. Well, then maybe a test for a media sink with NULL type would tell whether it can be NULL, which seems weird. If it's indeed not allowed, the logic should probably be adjusted to reject it on stream creation so we can later assume it's not NULL. I just did a tests on win10 with these minutes ago:
``` diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 53f22809dc4..2514ffc5160 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -5959,6 +5959,8 @@ static void test_mpeg4_media_sink(void) /* Test adding and removing stream sink. */ if (!(flags & MEDIASINK_FIXED_STREAMS)) { + hr = IMFMediaSink_AddStreamSink(sink, 123, NULL, &stream_sink); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFMediaSink_AddStreamSink(sink, 123, video_type, &stream_sink); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); IMFStreamSink_Release(stream_sink); ``` It will crash the tests. So I think it is not allowed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7538#note_97379
On Tue Mar 11 09:30:05 2025 +0000, Ziqing Hui wrote:
I just did a tests on win10 with these minutes ago: ``` diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 53f22809dc4..2514ffc5160 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -5959,6 +5959,8 @@ static void test_mpeg4_media_sink(void) /* Test adding and removing stream sink. */ if (!(flags & MEDIASINK_FIXED_STREAMS)) { + hr = IMFMediaSink_AddStreamSink(sink, 123, NULL, &stream_sink); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFMediaSink_AddStreamSink(sink, 123, video_type, &stream_sink); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); IMFStreamSink_Release(stream_sink); ``` It will crash the tests. So I think it is not allowed. Sounds good, I think we can just assume it's not NULL in steam_sink_create, we don't even need to return an error but let's remove the `if (media_type)` (and similar change in Release and probably in stream_sink_type_handler_GetCurrentMediaType).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7538#note_97381
participants (3)
-
Rémi Bernon -
Ziqing Hui
-
Ziqing Hui (＠zhui)