[PATCH v12 0/5] MR7569: mfplat/tests: Add tests for Byte Stream Timestamps.
This MR adds tests to `mfplat` to illustrate the timestamp values output on the samples for the respective Byte Stream handlers. I have also included a fix for a bug in `winegstreamer` (and copied to `mfsrcsnk`) where a `NULL` value for the `pguidTimeFormat` parameter in `IMFMediaSource::Start` causes a segmentation fault. On Windows this is allowed. It is included in this MR as the `mfplat` tests pass NULL (so without this fix, the tests crash). -- v12: mfsrcsnk: Allow NULL for time_format. winegstreamer: Allow NULL for time_format. mfplat/tests: Fix leak of media source. mfplat/tests: Fix leak of media events. mfplat/tests: Fix crash in MFShutdown on Windows. https://gitlab.winehq.org/wine/wine/-/merge_requests/7569
From: Brendan McGrath <bmcgrath(a)codeweavers.com> It seems Windows will crash on a subsequent call to MFShutdown if a local byte/scheme handler is registered outside corresponding calls to MFStartup/MFShutdown. --- dlls/mfplat/tests/mfplat.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index bdaffee9e83..f814de68427 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -6346,6 +6346,9 @@ static void test_local_handlers(void) return; } + hr = MFStartup(MF_VERSION, MFSTARTUP_FULL); + ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); + hr = pMFRegisterLocalSchemeHandler(NULL, NULL); ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); @@ -6375,6 +6378,9 @@ static void test_local_handlers(void) hr = pMFRegisterLocalByteStreamHandler(localW, localW, &local_activate); ok(hr == S_OK, "Failed to register stream handler, hr %#lx.\n", hr); + + hr = MFShutdown(); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); } static void test_create_property_store(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7569
From: Brendan McGrath <bmcgrath(a)codeweavers.com> --- dlls/mfplat/tests/mfplat.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index f814de68427..31f7ba006ce 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -1046,6 +1046,9 @@ static BOOL get_event(IMFMediaEventGenerator *generator, MediaEventType expected break; } + + IMFMediaEvent_Release(callback->media_event); + callback->media_event = NULL; } if (callback->media_event) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7569
From: Brendan McGrath <bmcgrath(a)codeweavers.com> If IMFMediaSource::Shutdown is not called, then the streams are not released and they continue to hold a reference to the IMFMediaSource, so the reference count of the source never reaches 0. --- dlls/mfplat/tests/mfplat.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 31f7ba006ce..1cb60d168e1 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -1233,6 +1233,7 @@ static void test_compressed_media_types(IMFSourceResolver *resolver) IMFStreamDescriptor_Release(sd); IMFPresentationDescriptor_Release(descriptor); + IMFMediaSource_Shutdown(source); IMFMediaSource_Release(source); IMFByteStream_Release(stream); @@ -1380,8 +1381,8 @@ static void test_source_resolver(void) ok(mediasource != NULL, "got %p\n", mediasource); ok(obj_type == MF_OBJECT_MEDIASOURCE, "got %d\n", obj_type); + IMFMediaSource_Shutdown(mediasource); refcount = IMFMediaSource_Release(mediasource); - todo_wine ok(!refcount, "Unexpected refcount %ld\n", refcount); IMFByteStream_Release(stream); @@ -1393,7 +1394,11 @@ static void test_source_resolver(void) hr = IMFSourceResolver_CreateObjectFromByteStream(resolver, stream, NULL, MF_RESOLUTION_MEDIASOURCE, NULL, &obj_type, (IUnknown **)&mediasource); ok(hr == S_OK || broken(hr == MF_E_UNSUPPORTED_BYTESTREAM_TYPE) /* w7 || w8 */, "Unexpected hr %#lx.\n", hr); - if (hr == S_OK) IMFMediaSource_Release(mediasource); + if (hr == S_OK) + { + IMFMediaSource_Shutdown(mediasource); + IMFMediaSource_Release(mediasource); + } IMFByteStream_Release(stream); hr = MFCreateFile(MF_ACCESSMODE_READ, MF_OPENMODE_FAIL_IF_NOT_EXIST, MF_FILEFLAGS_NONE, filename, &stream); @@ -1405,7 +1410,11 @@ static void test_source_resolver(void) hr = IMFSourceResolver_CreateObjectFromByteStream(resolver, stream, NULL, MF_RESOLUTION_MEDIASOURCE, NULL, &obj_type, (IUnknown **)&mediasource); ok(hr == S_OK || broken(hr == MF_E_UNSUPPORTED_BYTESTREAM_TYPE) /* w7 || w8 */, "Unexpected hr %#lx.\n", hr); - if (hr == S_OK) IMFMediaSource_Release(mediasource); + if (hr == S_OK) + { + IMFMediaSource_Shutdown(mediasource); + IMFMediaSource_Release(mediasource); + } IMFByteStream_Release(stream); /* stream must have a valid header, media cannot start in the middle of a stream */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7569
From: Brendan McGrath <bmcgrath(a)codeweavers.com> Allow the value of pguidTimeFormat to be NULL on a call to IMFMediaSource::Start. Currently, if NULL is used, Wine crashes with a SIGSEGV. MS documentation states: This parameter can be NULL. If the value is NULL, it is equivalent to GUID_NULL. https://learn.microsoft.com/en-us/windows/win32/api/mfidl/nf-mfidl-imfmedias... --- dlls/winegstreamer/media_source.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index 88056b27c5d..ab842b3e438 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -1552,6 +1552,9 @@ static HRESULT WINAPI media_source_Start(IMFMediaSource *iface, IMFPresentationD TRACE("%p, %p, %p, %p.\n", iface, descriptor, time_format, position); + if (!time_format) + time_format = &GUID_NULL; + EnterCriticalSection(&source->cs); if (source->state == SOURCE_SHUTDOWN) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7569
From: Brendan McGrath <bmcgrath(a)codeweavers.com> Allow the value of pguidTimeFormat to be NULL on a call to IMFMediaSource::Start. Currently, if NULL is used, Wine crashes with a SIGSEGV. MS documentation states: This parameter can be NULL. If the value is NULL, it is equivalent to GUID_NULL. https://learn.microsoft.com/en-us/windows/win32/api/mfidl/nf-mfidl-imfmedias... --- dlls/mfsrcsnk/media_source.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dlls/mfsrcsnk/media_source.c b/dlls/mfsrcsnk/media_source.c index 1e7918562fd..d7f20e511c8 100644 --- a/dlls/mfsrcsnk/media_source.c +++ b/dlls/mfsrcsnk/media_source.c @@ -1262,6 +1262,9 @@ static HRESULT WINAPI media_source_Start(IMFMediaSource *iface, IMFPresentationD TRACE("source %p, descriptor %p, format %s, position %s\n", source, descriptor, debugstr_guid(format), debugstr_propvar(position)); + if (!format) + format = &GUID_NULL; + EnterCriticalSection(&source->cs); if (source->state == SOURCE_SHUTDOWN) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7569
On Mon Mar 24 20:01:36 2025 +0000, Nikolay Sivov wrote:
This is fixing the leak of "unexpected" events that happened before the one we are checking for, right? Correct. We were leaking any event that occurred prior to the one we wanted (we were just looping and potentially grabbing another).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7569#note_98927
This merge request was approved by Nikolay Sivov. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7569
This is supposed to be MIME type string. Is there a reason for using a blob type here? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7569#note_99462
On Mon Mar 31 21:24:29 2025 +0000, Nikolay Sivov wrote:
This is supposed to be MIME type string. Is there a reason for using a blob type here? Ah, sorry. That's me commenting on older version.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7569#note_99464
This merge request was approved by Nikolay Sivov. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7569
On Tue Apr 1 04:38:17 2025 +0000, Nikolay Sivov wrote:
Ah, sorry. That's me commenting on older version. Thanks anyway. There was no good reason for using blob type; I was just using the wrong function. That code was moved over to MR !7672, so I have fixed it there now. Thanks again.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7569#note_99481
participants (3)
-
Brendan McGrath -
Brendan McGrath (＠redmcg)
-
Nikolay Sivov (＠nsivov)