[PATCH v4 0/7] MR8415: mf: Get rid of the IMFMediaShutdownNotify interface.
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`? -- v4: 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. https://gitlab.winehq.org/wine/wine/-/merge_requests/8415
From: Rémi Bernon <rbernon(a)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); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8415
From: Rémi Bernon <rbernon(a)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); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8415
From: Rémi Bernon <rbernon(a)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); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8415
From: Rémi Bernon <rbernon(a)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 42b09ced2aa..4d461a464ca 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -4461,6 +4461,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: ; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8415
From: Rémi Bernon <rbernon(a)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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8415
From: Rémi Bernon <rbernon(a)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 4d461a464ca..9b1ed870fae 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -144,7 +144,6 @@ struct media_source IMFMediaSource *source; IUnknown *object; }; - IMFMediaShutdownNotify *shutdown_notify; IMFPresentationDescriptor *pd; enum object_state state; unsigned int flags; @@ -246,7 +245,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; @@ -298,11 +296,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); @@ -1580,11 +1573,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) @@ -1606,21 +1594,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); @@ -2953,59 +2926,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) || @@ -4859,7 +4779,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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8415
From: Rémi Bernon <rbernon(a)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);") -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8415
Nikolay Sivov (@nsivov) commented about dlls/mf/session.c:
}
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:
Since this is basically a Wine extension, could you please add a note mentioning that so people won't try to reproduce it on Windows? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8415#note_109076
participants (2)
-
Nikolay Sivov (＠nsivov) -
Rémi Bernon