As discussed in !8402. @cmccarthy what do you think of this? I think queueing an event right before shutting down the queue is enough to get the media session notified? It won't get the actual event but it will be able to see the MF_E_SHUTDOWN error after calling `EndGetEvent`?
-- v2: include: Remove now unnecessary IMFMediaShutdownNotify interface. mf/session: Remove now unnecessary IMFMediaShutdownNotify. winegstreamer: Remove now unnecessary IMFMediaShutdownNotify. mf/session: Handle an optional MEError event from sources on shutdown. winegstreamer: Queue an event before shutting down the event queues. mfsrcsnk: Queue an event before shutting down the event queues. mfplat/tests: Add more tests for event queue shutdown.
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/mfplat/tests/mfplat.c | 41 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)
diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 1cdbb6f48ff..0a2471d3849 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -4654,12 +4654,26 @@ static void test_event_queue(void) hr = IMFMediaEventQueue_EndGetEvent(queue, result, &event); ok(hr == E_FAIL, "Unexpected hr %#lx.\n", hr);
+ /* Shutdown behavior. */ + hr = MFCreateMediaEvent(MEError, &GUID_NULL, E_FAIL, NULL, &event); + ok(hr == S_OK, "Failed to create event object, hr %#lx.\n", hr); + hr = IMFMediaEventQueue_QueueEvent(queue, event); + ok(hr == S_OK, "Failed to queue event, hr %#lx.\n", hr); + IMFMediaEvent_Release(event); + hr = IMFMediaEventQueue_Shutdown(queue); ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr);
+ event = (void *)0xdeadbeef; + hr = IMFMediaEventQueue_GetEvent(queue, 0, &event); + ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + ok(event == (void *)0xdeadbeef, "Unexpected event %p.\n", event); + event = (void *)0xdeadbeef; hr = IMFMediaEventQueue_GetEvent(queue, MF_EVENT_FLAG_NO_WAIT, &event); ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + ok(event == (void *)0xdeadbeef, "Unexpected event %p.\n", event); +
hr = MFCreateMediaEvent(MEError, &GUID_NULL, E_FAIL, NULL, &event); ok(hr == S_OK, "Failed to create event object, hr %#lx.\n", hr); @@ -4707,6 +4721,33 @@ static void test_event_queue(void) IMFAsyncCallback_Release(&callback->IMFAsyncCallback_iface); IMFAsyncCallback_Release(&callback2->IMFAsyncCallback_iface);
+ + callback = create_test_callback(&test_async_callback_result_vtbl); + + hr = MFCreateEventQueue(&queue); + ok(hr == S_OK, "Failed to create event queue, hr %#lx.\n", hr); + hr = IMFMediaEventQueue_BeginGetEvent(queue, &callback->IMFAsyncCallback_iface, NULL); + ok(hr == S_OK, "Failed to Begin*, hr %#lx.\n", hr); + hr = IMFMediaEventQueue_QueueEventParamVar(queue, MEError, &GUID_NULL, E_FAIL, NULL); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFMediaEventQueue_Shutdown(queue); + ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); + + ret = WaitForSingleObject(callback->event, 500); + ok(ret == WAIT_OBJECT_0, "Unexpected return value %#lx.\n", ret); + result = callback->result; + callback->result = NULL; + + event = (void *)0xdeadbeef; + hr = IMFMediaEventQueue_EndGetEvent(queue, result, &event); + ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + ok(event == (void *)0xdeadbeef, "Unexpected event %p.\n", event); + IMFAsyncResult_Release(result); + + IMFMediaEventQueue_Release(queue); + IMFAsyncCallback_Release(&callback->IMFAsyncCallback_iface); + + hr = MFShutdown(); ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); }
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/mfsrcsnk/media_source.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/dlls/mfsrcsnk/media_source.c b/dlls/mfsrcsnk/media_source.c index d7f20e511c8..068f64b9f03 100644 --- a/dlls/mfsrcsnk/media_source.c +++ b/dlls/mfsrcsnk/media_source.c @@ -1337,12 +1337,14 @@ static HRESULT WINAPI media_source_Shutdown(IMFMediaSource *iface) } source->state = SOURCE_SHUTDOWN;
+ IMFMediaEventQueue_QueueEventParamVar(source->queue, MEError, &GUID_NULL, MF_E_SHUTDOWN, NULL); IMFMediaEventQueue_Shutdown(source->queue); IMFByteStream_Close(source->stream);
while (source->stream_count--) { struct media_stream *stream = source->streams[source->stream_count]; + IMFMediaEventQueue_QueueEventParamVar(stream->queue, MEError, &GUID_NULL, MF_E_SHUTDOWN, NULL); IMFMediaEventQueue_Shutdown(stream->queue); IMFMediaStream_Release(&stream->IMFMediaStream_iface); }
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/winegstreamer/media_source.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index ab842b3e438..f6f7d6fd413 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -1649,6 +1649,7 @@ static HRESULT WINAPI media_source_Shutdown(IMFMediaSource *iface) WaitForSingleObject(source->read_thread, INFINITE); CloseHandle(source->read_thread);
+ IMFMediaEventQueue_QueueEventParamVar(source->event_queue, MEError, &GUID_NULL, MF_E_SHUTDOWN, NULL); IMFMediaEventQueue_Shutdown(source->event_queue); IMFByteStream_Close(source->byte_stream);
@@ -1656,6 +1657,7 @@ static HRESULT WINAPI media_source_Shutdown(IMFMediaSource *iface) { struct media_stream *stream = source->streams[source->stream_count]; IMFStreamDescriptor_Release(source->descriptors[source->stream_count]); + IMFMediaEventQueue_QueueEventParamVar(stream->event_queue, MEError, &GUID_NULL, MF_E_SHUTDOWN, NULL); IMFMediaEventQueue_Shutdown(stream->event_queue); IMFMediaStream_Release(&stream->IMFMediaStream_iface); }
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/mf/session.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/dlls/mf/session.c b/dlls/mf/session.c index 47760cc44a7..86db6233944 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -4436,6 +4436,15 @@ static HRESULT WINAPI session_events_callback_Invoke(IMFAsyncCallback *iface, IM }
break; + + case MEError: + EnterCriticalSection(&session->cs); + if (SUCCEEDED(IMFMediaEvent_GetStatus(event, &hr)) && hr == MF_E_SHUTDOWN && + session_get_media_source(session, (IMFMediaSource *)event_source)) + session_handle_source_shutdown(session); + LeaveCriticalSection(&session->cs); + break; + default: ; }
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/winegstreamer/media_source.c | 92 ------------------------------- 1 file changed, 92 deletions(-)
diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index f6f7d6fd413..f85a6ff1599 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -174,15 +174,12 @@ struct media_source IMFGetService IMFGetService_iface; IMFRateSupport IMFRateSupport_iface; IMFRateControl IMFRateControl_iface; - IMFMediaShutdownNotify IMFMediaShutdownNotify_iface; IMFAsyncCallback async_commands_callback; LONG ref; DWORD async_commands_queue; IMFMediaEventQueue *event_queue; IMFByteStream *byte_stream;
- IMFAsyncResult *shutdown_result; - CRITICAL_SECTION cs;
UINT64 file_size; @@ -232,11 +229,6 @@ static inline struct media_source *impl_from_IMFRateControl(IMFRateControl *ifac return CONTAINING_RECORD(iface, struct media_source, IMFRateControl_iface); }
-static inline struct media_source *impl_from_IMFMediaShutdownNotify(IMFMediaShutdownNotify *iface) -{ - return CONTAINING_RECORD(iface, struct media_source, IMFMediaShutdownNotify_iface); -} - static inline struct media_source *impl_from_async_commands_callback_IMFAsyncCallback(IMFAsyncCallback *iface) { return CONTAINING_RECORD(iface, struct media_source, async_commands_callback); @@ -1323,76 +1315,6 @@ static const IMFRateControlVtbl media_source_rate_control_vtbl = media_source_rate_control_GetRate, };
-static void media_source_release_shutdown_callback(struct media_source *source) -{ - if (source->shutdown_result) - IMFAsyncResult_Release(source->shutdown_result); - source->shutdown_result = NULL; -} - -static HRESULT WINAPI media_source_shutdown_notify_QueryInterface(IMFMediaShutdownNotify *iface, REFIID riid, void **obj) -{ - if (IsEqualIID(riid, &IID_IMFMediaShutdownNotify) || - IsEqualIID(riid, &IID_IUnknown)) - { - *obj = iface; - IUnknown_AddRef(iface); - return S_OK; - } - - WARN("Unsupported %s.\n", debugstr_guid(riid)); - *obj = NULL; - return E_NOINTERFACE; -} - -static ULONG WINAPI media_source_shutdown_notify_AddRef(IMFMediaShutdownNotify *iface) -{ - struct media_source *source = impl_from_IMFMediaShutdownNotify(iface); - return IMFMediaSource_AddRef(&source->IMFMediaSource_iface); -} - -static ULONG WINAPI media_source_shutdown_notify_Release(IMFMediaShutdownNotify *iface) -{ - struct media_source *source = impl_from_IMFMediaShutdownNotify(iface); - return IMFMediaSource_Release(&source->IMFMediaSource_iface); -} - -static HRESULT WINAPI media_source_shutdown_notify_set_notification_callback(IMFMediaShutdownNotify *iface, - IMFAsyncCallback *callback, IUnknown *state) -{ - struct media_source *source = impl_from_IMFMediaShutdownNotify(iface); - IMFAsyncResult *result = NULL; - HRESULT hr = S_OK; - - EnterCriticalSection(&source->cs); - - if (source->state == SOURCE_SHUTDOWN) - hr = MF_E_SHUTDOWN; - else - { - if (callback && FAILED(hr = MFCreateAsyncResult(NULL, callback, state, &result))) - { - LeaveCriticalSection(&source->cs); - return hr; - } - - media_source_release_shutdown_callback(source); - source->shutdown_result = result; - } - - LeaveCriticalSection(&source->cs); - - return hr; -} - -static const IMFMediaShutdownNotifyVtbl media_source_shutdown_notify_vtbl = -{ - media_source_shutdown_notify_QueryInterface, - media_source_shutdown_notify_AddRef, - media_source_shutdown_notify_Release, - media_source_shutdown_notify_set_notification_callback, -}; - static HRESULT WINAPI media_source_QueryInterface(IMFMediaSource *iface, REFIID riid, void **out) { struct media_source *source = impl_from_IMFMediaSource(iface); @@ -1409,10 +1331,6 @@ static HRESULT WINAPI media_source_QueryInterface(IMFMediaSource *iface, REFIID { *out = &source->IMFGetService_iface; } - else if (IsEqualIID(riid, &IID_IMFMediaShutdownNotify)) - { - *out = &source->IMFMediaShutdownNotify_iface; - } else { FIXME("%s, %p.\n", debugstr_guid(riid), out); @@ -1443,7 +1361,6 @@ static ULONG WINAPI media_source_Release(IMFMediaSource *iface)
if (!ref) { - media_source_release_shutdown_callback(source); IMFMediaSource_Shutdown(iface); IMFMediaEventQueue_Release(source->event_queue); IMFByteStream_Release(source->byte_stream); @@ -1629,7 +1546,6 @@ static HRESULT WINAPI media_source_Pause(IMFMediaSource *iface) static HRESULT WINAPI media_source_Shutdown(IMFMediaSource *iface) { struct media_source *source = impl_from_IMFMediaSource(iface); - HRESULT hr;
TRACE("%p.\n", iface);
@@ -1664,13 +1580,6 @@ static HRESULT WINAPI media_source_Shutdown(IMFMediaSource *iface) free(source->descriptors); free(source->streams);
- if (source->shutdown_result) - { - if (FAILED(hr = MFPutWorkItemEx(MFASYNC_CALLBACK_QUEUE_STANDARD, source->shutdown_result))) - WARN("Failed to put shutdown notification, hr %#lx.\n", hr); - media_source_release_shutdown_callback(source); - } - LeaveCriticalSection(&source->cs);
MFUnlockWorkQueue(source->async_commands_queue); @@ -1729,7 +1638,6 @@ static HRESULT media_source_create(struct object_context *context, IMFMediaSourc object->IMFGetService_iface.lpVtbl = &media_source_get_service_vtbl; object->IMFRateSupport_iface.lpVtbl = &media_source_rate_support_vtbl; object->IMFRateControl_iface.lpVtbl = &media_source_rate_control_vtbl; - object->IMFMediaShutdownNotify_iface.lpVtbl = &media_source_shutdown_notify_vtbl; object->async_commands_callback.lpVtbl = &source_async_commands_callback_vtbl; object->ref = 1; object->byte_stream = context->stream;
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/mf/session.c | 81 ----------------------------------------------- 1 file changed, 81 deletions(-)
diff --git a/dlls/mf/session.c b/dlls/mf/session.c index 86db6233944..1d519b6013a 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -129,7 +129,6 @@ struct media_source IMFMediaSource *source; IUnknown *object; }; - IMFMediaShutdownNotify *shutdown_notify; IMFPresentationDescriptor *pd; enum object_state state; unsigned int flags; @@ -233,7 +232,6 @@ struct media_session IMFTopologyNodeAttributeEditor IMFTopologyNodeAttributeEditor_iface; IMFAsyncCallback commands_callback; IMFAsyncCallback sa_ready_callback; - IMFAsyncCallback shutdown_callback; IMFAsyncCallback events_callback; IMFAsyncCallback sink_finalizer_callback; LONG refcount; @@ -283,11 +281,6 @@ static struct media_session *impl_from_sa_ready_callback_IMFAsyncCallback(IMFAsy return CONTAINING_RECORD(iface, struct media_session, sa_ready_callback); }
-static struct media_session *impl_from_shutdown_callback_IMFAsyncCallback(IMFAsyncCallback *iface) -{ - return CONTAINING_RECORD(iface, struct media_session, shutdown_callback); -} - static struct media_session *impl_from_events_callback_IMFAsyncCallback(IMFAsyncCallback *iface) { return CONTAINING_RECORD(iface, struct media_session, events_callback); @@ -1604,11 +1597,6 @@ static struct media_source *session_get_media_source(struct media_session *sessi
static void session_release_media_source(struct media_source *source) { - if (source->shutdown_notify) - { - IMFMediaShutdownNotify_set_notification_callback(source->shutdown_notify, NULL, NULL); - IMFMediaShutdownNotify_Release(source->shutdown_notify); - } if (source->source) IMFMediaSource_Release(source->source); if (source->pd) @@ -1630,21 +1618,6 @@ static HRESULT session_add_media_source(struct media_session *session, IMFTopolo media_source->source = source; IMFMediaSource_AddRef(media_source->source);
- if (SUCCEEDED(hr = IMFMediaSource_QueryInterface(media_source->source, - &IID_IMFMediaShutdownNotify, (void **)&media_source->shutdown_notify))) - { - hr = IMFMediaShutdownNotify_set_notification_callback(media_source->shutdown_notify, - &session->shutdown_callback, (IUnknown *)media_source->source); - if (hr == MF_E_SHUTDOWN) - { - session_release_media_source(media_source); - return hr; - } - } - - if (FAILED(hr)) - TRACE("Session is not robust to unexpected shutdown, hr %#lx.\n", hr); - hr = IMFTopologyNode_GetUnknown(node, &MF_TOPONODE_PRESENTATION_DESCRIPTOR, &IID_IMFPresentationDescriptor, (void **)&media_source->pd);
@@ -2966,59 +2939,6 @@ static void session_handle_source_shutdown(struct media_session *session) LeaveCriticalSection(&session->cs); }
-static HRESULT WINAPI session_shutdown_callback_QueryInterface(IMFAsyncCallback *iface, REFIID riid, void **obj) -{ - if (IsEqualIID(riid, &IID_IMFAsyncCallback) - || IsEqualIID(riid, &IID_IUnknown)) - { - *obj = iface; - IMFAsyncCallback_AddRef(iface); - return S_OK; - } - - WARN("Unsupported %s.\n", debugstr_guid(riid)); - *obj = NULL; - return E_NOINTERFACE; -} - -static ULONG WINAPI session_shutdown_callback_AddRef(IMFAsyncCallback *iface) -{ - struct media_session *session = impl_from_shutdown_callback_IMFAsyncCallback(iface); - return IMFMediaSession_AddRef(&session->IMFMediaSession_iface); -} - -static ULONG WINAPI session_shutdown_callback_Release(IMFAsyncCallback *iface) -{ - struct media_session *session = impl_from_shutdown_callback_IMFAsyncCallback(iface); - return IMFMediaSession_Release(&session->IMFMediaSession_iface); -} - -static HRESULT WINAPI session_shutdown_callback_GetParameters(IMFAsyncCallback *iface, DWORD *flags, DWORD *queue) -{ - return E_NOTIMPL; -} - -static HRESULT WINAPI session_shutdown_callback_Invoke(IMFAsyncCallback *iface, IMFAsyncResult *result) -{ - struct media_session *session = impl_from_shutdown_callback_IMFAsyncCallback(iface); - IMFMediaSource *source = (IMFMediaSource *)IMFAsyncResult_GetStateNoAddRef(result); - - TRACE("Source %p was shut down.\n", source); - - session_handle_source_shutdown(session); - - return S_OK; -} - -static const IMFAsyncCallbackVtbl session_shutdown_callback_vtbl = -{ - session_shutdown_callback_QueryInterface, - session_shutdown_callback_AddRef, - session_shutdown_callback_Release, - session_shutdown_callback_GetParameters, - session_shutdown_callback_Invoke, -}; - static HRESULT WINAPI session_events_callback_QueryInterface(IMFAsyncCallback *iface, REFIID riid, void **obj) { if (IsEqualIID(riid, &IID_IMFAsyncCallback) || @@ -4831,7 +4751,6 @@ HRESULT WINAPI MFCreateMediaSession(IMFAttributes *config, IMFMediaSession **ses object->IMFTopologyNodeAttributeEditor_iface.lpVtbl = &node_attribute_editor_vtbl; object->commands_callback.lpVtbl = &session_commands_callback_vtbl; object->sa_ready_callback.lpVtbl = &session_sa_ready_callback_vtbl; - object->shutdown_callback.lpVtbl = &session_shutdown_callback_vtbl; object->events_callback.lpVtbl = &session_events_callback_vtbl; object->sink_finalizer_callback.lpVtbl = &session_sink_finalizer_callback_vtbl; object->refcount = 1;
From: Rémi Bernon rbernon@codeweavers.com
--- include/wine/mfinternal.idl | 13 ------------- 1 file changed, 13 deletions(-)
diff --git a/include/wine/mfinternal.idl b/include/wine/mfinternal.idl index 0b77eaefab1..f1c1f8c9090 100644 --- a/include/wine/mfinternal.idl +++ b/include/wine/mfinternal.idl @@ -45,19 +45,6 @@ interface IMFSinkClassFactory : IUnknown ); }
-[ - uuid(0aab315c-33a4-43db-af6b-c9c86921034e), - object, - local -] -interface IMFMediaShutdownNotify : IUnknown -{ - HRESULT set_notification_callback( - [in] IMFAsyncCallback *callback, - [in] IUnknown *state - ); -} - cpp_quote("DEFINE_GUID(CLSID_MF3GPSinkClassFactory, 0xe54cdfaf, 0x2381, 0x4cad, 0xab, 0x99, 0xf3, 0x85, 0x17, 0x12, 0x7d, 0x5c);") cpp_quote("DEFINE_GUID(CLSID_MFAC3SinkClassFactory, 0x255a6fda, 0x6f93, 0x4e8a, 0x96, 0x11, 0xde, 0xd1, 0x16, 0x9e, 0xef, 0xb4);") cpp_quote("DEFINE_GUID(CLSID_MFADTSSinkClassFactory, 0xd7ca55ab, 0x5022, 0x4db3, 0xa5, 0x99, 0xab, 0xaf, 0xa3, 0x58, 0xe6, 0xf3);")
v2: Handle MEError/MF_E_SHUTDOWN events on the media sources, remove additional unnecessary session interface.
I'm not opposed to such notification interface in general, but we need to figure out this undocumented API first, if it exists at all.
Fwiw I have investigated this a bit, and the session and the media source use an undocumented interface to communicate. I don't have a definitive confirmation that it's used for shutdown, but the session requests an internal `0b5e1c7e-bd76-46bc-896c-b2edb40dd803` source interface when topology is loaded.
That interface seem to extend `IUnknown` with 3 methods but some other internal objects are then involved, some of which with a large number of methods. So it doesn't seem useful to try reverse engineering this for the only purpose of notifying of shutdown, if that's the actual purpose of this (I suspect it has a wider purpose and I could not find any other obvious notification mechanism).
No hangs in Killsquad from around 500 character switches. `MEError` is received in about 1 in every 100 cases. There is a chance of regression in games which use media sources directly if they don't handle the `MEError` gracefully, but that seems unlikely.
On Thu Jun 26 10:40:38 2025 +0000, Conor McCarthy wrote:
No hangs in Killsquad from around 500 character switches. `MEError` is received in about 1 in every 100 cases. There is a chance of regression in games which use media sources directly if they don't handle the `MEError` gracefully, but that seems unlikely.
If they use a source instance directly for some reason they are not going to handle IMFMediaShutdownNotify either.
On Thu Jun 26 10:40:38 2025 +0000, Nikolay Sivov wrote:
If they use a source instance directly for some reason they are not going to handle IMFMediaShutdownNotify either.
It doesn't matter in that case since the source is not used in a session, and in Windows the game has no access to the undocumented thing Windows uses anyway.
Anything here?
On Mon Jul 7 16:20:34 2025 +0000, Rémi Bernon wrote:
Anything here?
Let's do this one after !8402, to see make potential regressions easier to track.
-
Conor McCarthy (＠cmccarthy) -
Nikolay Sivov (＠nsivov)
-
Rémi Bernon
-
Rémi Bernon (＠rbernon)