[PATCH 0/2] MR3789: Improve media session handling of unexpected errors.
From: Santino Mazza <smazza(a)codeweavers.com> --- dlls/mf/tests/mf.c | 146 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 115 insertions(+), 31 deletions(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index aefc1b92b4d..1e46897bac8 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -48,18 +48,12 @@ #define SET_EXPECT(func) \ expect_ ## func = TRUE -#define CHECK_EXPECT2(func) \ +#define CHECK_EXPECT(func) \ do { \ ok(expect_ ##func, "unexpected call " #func "\n"); \ called_ ## func = TRUE; \ }while(0) -#define CHECK_EXPECT(func) \ - do { \ - CHECK_EXPECT2(func); \ - expect_ ## func = FALSE; \ - }while(0) - #define CHECK_CALLED(func) \ do { \ ok(called_ ## func, "expected " #func "\n"); \ @@ -272,6 +266,7 @@ static void init_sink_node(IMFStreamSink *stream_sink, MF_CONNECT_METHOD method, } DEFINE_EXPECT(test_source_BeginGetEvent); +DEFINE_EXPECT(test_source_EndGetEvent); DEFINE_EXPECT(test_source_QueueEvent); DEFINE_EXPECT(test_source_Start); @@ -279,7 +274,9 @@ struct test_source { IMFMediaSource IMFMediaSource_iface; LONG refcount; - HRESULT begin_get_event_res; + IMFMediaEventQueue *queue; + HRESULT begin_get_event_hr, end_get_event_hr, + start_hr, queue_event_hr; IMFPresentationDescriptor *pd; }; @@ -319,6 +316,8 @@ static ULONG WINAPI test_source_Release(IMFMediaSource *iface) if (!refcount) { + IMFMediaEventQueue_Shutdown(source->queue); + IMFMediaEventQueue_Release(source->queue); IMFPresentationDescriptor_Release(source->pd); free(source); } @@ -336,20 +335,33 @@ static HRESULT WINAPI test_source_BeginGetEvent(IMFMediaSource *iface, IMFAsyncC { struct test_source *source = impl_from_IMFMediaSource(iface); CHECK_EXPECT(test_source_BeginGetEvent); - return source->begin_get_event_res; + if (source->begin_get_event_hr == S_OK) + return IMFMediaEventQueue_BeginGetEvent(source->queue, callback, state); + else return source->begin_get_event_hr; } static HRESULT WINAPI test_source_EndGetEvent(IMFMediaSource *iface, IMFAsyncResult *result, IMFMediaEvent **event) { - ok(0, "Unexpected call.\n"); - return E_NOTIMPL; + struct test_source *source = impl_from_IMFMediaSource(iface); + CHECK_EXPECT(test_source_EndGetEvent); + + if (source->end_get_event_hr == S_OK) + return IMFMediaEventQueue_EndGetEvent(source->queue, result, event); + else { + *event = NULL; + return source->end_get_event_hr; + } } static HRESULT WINAPI test_source_QueueEvent(IMFMediaSource *iface, MediaEventType event_type, REFGUID ext_type, HRESULT hr, const PROPVARIANT *value) { + struct test_source *source = impl_from_IMFMediaSource(iface); CHECK_EXPECT(test_source_QueueEvent); - return E_NOTIMPL; + + if (source->queue_event_hr == S_OK) + return IMFMediaEventQueue_QueueEventParamVar(source->queue, event_type, ext_type, hr, value); + else return source->queue_event_hr; } static HRESULT WINAPI test_source_GetCharacteristics(IMFMediaSource *iface, DWORD *flags) @@ -367,8 +379,14 @@ static HRESULT WINAPI test_source_CreatePresentationDescriptor(IMFMediaSource *i static HRESULT WINAPI test_source_Start(IMFMediaSource *iface, IMFPresentationDescriptor *pd, const GUID *time_format, const PROPVARIANT *start_position) { + struct test_source *source = impl_from_IMFMediaSource(iface); CHECK_EXPECT(test_source_Start); - return E_NOTIMPL; + if (source->start_hr == S_OK) + { + IMFMediaEventQueue_QueueEventParamUnk(source->queue, MESourceStarted, &GUID_NULL, S_OK, NULL); + return S_OK; + } + else return source->start_hr; } static HRESULT WINAPI test_source_Stop(IMFMediaSource *iface) @@ -413,12 +431,25 @@ static IMFMediaSource *create_test_source(IMFPresentationDescriptor *pd) source = calloc(1, sizeof(*source)); source->IMFMediaSource_iface.lpVtbl = &test_source_vtbl; source->refcount = 1; - source->begin_get_event_res = E_NOTIMPL; + source->begin_get_event_hr = E_NOTIMPL; + source->end_get_event_hr = E_NOTIMPL; + source->queue_event_hr = E_NOTIMPL; + source->start_hr = E_NOTIMPL; + MFCreateEventQueue(&source->queue); IMFPresentationDescriptor_AddRef((source->pd = pd)); return &source->IMFMediaSource_iface; } +static void test_media_source_reinitialize_queue(IMFMediaSource *iface) +{ + struct test_source *source = impl_from_IMFMediaSource(iface); + + IMFMediaEventQueue_Shutdown(source->queue); + IMFMediaEventQueue_Release(source->queue); + MFCreateEventQueue(&source->queue); +} + static HRESULT WINAPI test_unk_QueryInterface(IUnknown *iface, REFIID riid, void **obj) { if (IsEqualIID(riid, &IID_IUnknown)) @@ -2495,6 +2526,9 @@ static void test_media_session_events(void) /* test IMFMediaSession_Start with source returning an error in BeginGetEvent */ source_impl = impl_from_IMFMediaSource(source); + source_impl->begin_get_event_hr = 0x80001234; + source_impl->end_get_event_hr = MF_E_SHUTDOWN; + source_impl->start_hr = MF_E_SHUTDOWN; hr = MFCreateMediaSession(NULL, &session); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); @@ -2506,34 +2540,27 @@ static void test_media_session_events(void) ok(propvar.punkVal != (IUnknown *)topology, "got punkVal %p\n", propvar.punkVal); PropVariantClear(&propvar); - source_impl->begin_get_event_res = 0x80001234; - SET_EXPECT(test_source_BeginGetEvent); - SET_EXPECT(test_source_QueueEvent); - SET_EXPECT(test_source_Start); - propvar.vt = VT_EMPTY; hr = IMFMediaSession_Start(session, &GUID_NULL, &propvar); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = wait_media_event_until_blocking(session, callback, MESessionStarted, 1000, &propvar); ok(hr == 0x80001234, "Unexpected hr %#lx.\n", hr); ok(propvar.vt == VT_EMPTY, "got vt %u\n", propvar.vt); - ok(propvar.punkVal != (IUnknown *)topology, "got punkVal %p\n", propvar.punkVal); - PropVariantClear(&propvar); - CHECK_CALLED(test_source_BeginGetEvent); - CHECK_NOT_CALLED(test_source_Start); + SET_EXPECT(test_source_QueueEvent); hr = IMFMediaSession_ClearTopologies(session); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = wait_media_event(session, callback, MESessionTopologiesCleared, 1000, &propvar); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(propvar.vt == VT_EMPTY, "got vt %u\n", propvar.vt); hr = IMFMediaSession_Shutdown(session); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok(media_sink.shutdown, "media sink didn't shutdown.\n"); media_sink.shutdown = FALSE; - source_impl->begin_get_event_res = E_NOTIMPL; - CLEAR_CALLED(test_source_BeginGetEvent); CLEAR_CALLED(test_source_QueueEvent); CLEAR_CALLED(test_source_Start); @@ -2554,18 +2581,18 @@ static void test_media_session_events(void) ok(propvar.punkVal != (IUnknown *)topology, "got punkVal %p\n", propvar.punkVal); PropVariantClear(&propvar); - source_impl = impl_from_IMFMediaSource(source); - source_impl->begin_get_event_res = S_OK; + source_impl->begin_get_event_hr = S_OK; + source_impl->end_get_event_hr = MF_E_SHUTDOWN; + source_impl->start_hr = 0x80001234; SET_EXPECT(test_source_BeginGetEvent); - SET_EXPECT(test_source_QueueEvent); SET_EXPECT(test_source_Start); propvar.vt = VT_EMPTY; hr = IMFMediaSession_Start(session, &GUID_NULL, &propvar); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = wait_media_event_until_blocking(session, callback, MESessionStarted, 1000, &propvar); - ok(hr == E_NOTIMPL, "Unexpected hr %#lx.\n", hr); + ok(hr == 0x80001234, "Unexpected hr %#lx.\n", hr); ok(propvar.vt == VT_EMPTY, "got vt %u\n", propvar.vt); ok(propvar.punkVal != (IUnknown *)topology, "got punkVal %p\n", propvar.punkVal); PropVariantClear(&propvar); @@ -2576,17 +2603,74 @@ static void test_media_session_events(void) hr = IMFMediaSession_ClearTopologies(session); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + SET_EXPECT(test_source_QueueEvent); hr = IMFMediaSession_Shutdown(session); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok(media_sink.shutdown, "media sink didn't shutdown.\n"); media_sink.shutdown = FALSE; - source_impl->begin_get_event_res = E_NOTIMPL; - CLEAR_CALLED(test_source_BeginGetEvent); CLEAR_CALLED(test_source_QueueEvent); CLEAR_CALLED(test_source_Start); + test_media_source_reinitialize_queue(source); + + /* sometimes briefly leaking */ + IMFMediaSession_Release(session); + + + /* test IMFMediaSession_Start when event queue fails. */ + hr = MFCreateMediaSession(NULL, &session); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IMFMediaSession_SetTopology(session, 0, topology); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = wait_media_event(session, callback, MESessionTopologySet, 1000, &propvar); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(propvar.vt == VT_UNKNOWN, "got vt %u\n", propvar.vt); + ok(propvar.punkVal != (IUnknown *)topology, "got punkVal %p\n", propvar.punkVal); + PropVariantClear(&propvar); + + SET_EXPECT(test_source_BeginGetEvent); + SET_EXPECT(test_source_EndGetEvent); + SET_EXPECT(test_source_Start); + source_impl->begin_get_event_hr = S_OK; + source_impl->end_get_event_hr = 0x80001234; + source_impl->start_hr = S_OK; + propvar.vt = VT_EMPTY; + hr = IMFMediaSession_Start(session, &GUID_NULL, &propvar); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = wait_media_event_until_blocking(session, callback, MEError, 1000, &propvar); + todo_wine ok(hr == 0x80001234, "Unexpected hr %#lx.\n", hr); + ok(propvar.vt == VT_EMPTY, "got vt %u\n", propvar.vt); + CHECK_CALLED(test_source_BeginGetEvent); + CHECK_CALLED(test_source_EndGetEvent); + CHECK_CALLED(test_source_Start); + + source_impl->begin_get_event_hr = MF_E_SHUTDOWN; + propvar.vt = VT_EMPTY; + hr = IMFMediaSession_Start(session, &GUID_NULL, &propvar); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + todo_wine + hr = wait_media_event_until_blocking(session, callback, MESessionStarted, 1000, &propvar); + ok(hr == WAIT_TIMEOUT, "Unexpected hr %#lx.\n", hr); /* New commands are not handled */ + + hr = IMFMediaSession_ClearTopologies(session); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFMediaSession_BeginGetEvent(session, callback, NULL); + ok(hr == MF_S_MULTIPLE_BEGIN || hr == MF_E_MULTIPLE_BEGIN /* Win 7 */, "Unexpected hr %#lx\n", hr); + + SET_EXPECT(test_source_QueueEvent); + hr = IMFMediaSession_Shutdown(session); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(media_sink.shutdown, "media sink didn't shutdown.\n"); + media_sink.shutdown = FALSE; + + test_media_source_reinitialize_queue(source); + + hr = IMFMediaEventQueue_Shutdown(source_impl->queue); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + /* sometimes briefly leaking */ IMFMediaSession_Release(session); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3789
From: Santino Mazza <smazza(a)codeweavers.com> --- dlls/mf/session.c | 8 +++++++- dlls/mf/tests/mf.c | 3 +-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/dlls/mf/session.c b/dlls/mf/session.c index b5b631fb00a..1c72fa8bd38 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -3679,7 +3679,9 @@ static HRESULT WINAPI session_events_callback_Invoke(IMFAsyncCallback *iface, IM if (FAILED(hr = IMFMediaEventGenerator_EndGetEvent(event_source, result, &event))) { WARN("Failed to get event from %p, hr %#lx.\n", event_source, hr); - goto failed; + IMFMediaEventQueue_QueueEventParamVar(session->event_queue, MEError, &GUID_NULL, hr, &value); + IMFMediaEventGenerator_Release(event_source); + return hr; } if (FAILED(hr = IMFMediaEvent_GetType(event, &event_type))) @@ -3876,7 +3878,11 @@ failed: IMFMediaEvent_Release(event); if (FAILED(hr = IMFMediaEventGenerator_BeginGetEvent(event_source, iface, (IUnknown *)event_source))) + { WARN("Failed to re-subscribe, hr %#lx.\n", hr); + value.vt = VT_EMPTY; + IMFMediaEventQueue_QueueEventParamVar(session->event_queue, MEError, &GUID_NULL, hr, &value); + } IMFMediaEventGenerator_Release(event_source); diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 1e46897bac8..1d0b021da01 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -2641,7 +2641,7 @@ static void test_media_session_events(void) hr = IMFMediaSession_Start(session, &GUID_NULL, &propvar); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = wait_media_event_until_blocking(session, callback, MEError, 1000, &propvar); - todo_wine ok(hr == 0x80001234, "Unexpected hr %#lx.\n", hr); + ok(hr == 0x80001234, "Unexpected hr %#lx.\n", hr); ok(propvar.vt == VT_EMPTY, "got vt %u\n", propvar.vt); CHECK_CALLED(test_source_BeginGetEvent); CHECK_CALLED(test_source_EndGetEvent); @@ -2651,7 +2651,6 @@ static void test_media_session_events(void) propvar.vt = VT_EMPTY; hr = IMFMediaSession_Start(session, &GUID_NULL, &propvar); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine hr = wait_media_event_until_blocking(session, callback, MESessionStarted, 1000, &propvar); ok(hr == WAIT_TIMEOUT, "Unexpected hr %#lx.\n", hr); /* New commands are not handled */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3789
Nikolay Sivov (@nsivov) commented about dlls/mf/session.c:
if (FAILED(hr = IMFMediaEventGenerator_EndGetEvent(event_source, result, &event))) { WARN("Failed to get event from %p, hr %#lx.\n", event_source, hr); - goto failed; + IMFMediaEventQueue_QueueEventParamVar(session->event_queue, MEError, &GUID_NULL, hr, &value); + IMFMediaEventGenerator_Release(event_source); + return hr; }
MEError is supposed to indicate fatal non-recoverable error. Have you checked what happens if you get this condition in the middle of the playback for example? I suspect such error reporting should happen in many more places, and we can work on this later, but right now it's useful to figure out if session gets into error state when this happens. Could you check what happens if End/BeginGetEvent() fails suddenly after a number of samples was successfully delivered and playback is still active? Should we stop the clock when this happens? Should we send first fatal error event only, or following failures are always reported with MEError? Note that 'value' is passed here uninitialized, if it's unused I believe you can use NULL. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3789#note_44911
participants (3)
-
Nikolay Sivov (＠nsivov) -
Santino Mazza
-
Santino Mazza (＠tati1454)