[PATCH 0/3] MR607: mf: Fix session command handling and SESSION_CMD_END.
From: R��mi Bernon <rbernon(a)codeweavers.com> --- dlls/mf/session.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/dlls/mf/session.c b/dlls/mf/session.c index 87fe77a730f..8c6ab554f5d 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -258,7 +258,10 @@ struct media_session float rate; } presentation; struct list topologies; - struct list commands; + + struct session_op *pending_command; + struct list queued_commands; + enum session_state state; DWORD caps; CRITICAL_SECTION cs; @@ -459,7 +462,7 @@ static void session_push_back_command(struct media_session *session, enum sessio struct session_op *op; if (SUCCEEDED(create_session_op(command, &op))) - list_add_head(&session->commands, &op->entry); + list_add_head(&session->queued_commands, &op->entry); } static HRESULT session_submit_command(struct media_session *session, struct session_op *op) @@ -469,9 +472,9 @@ static HRESULT session_submit_command(struct media_session *session, struct sess EnterCriticalSection(&session->cs); if (SUCCEEDED(hr = session_is_shut_down(session))) { - if (list_empty(&session->commands)) + if (!session->pending_command && list_empty(&session->queued_commands)) hr = MFPutWorkItem(MFASYNC_CALLBACK_QUEUE_STANDARD, &session->commands_callback, &op->IUnknown_iface); - list_add_tail(&session->commands, &op->entry); + list_add_tail(&session->queued_commands, &op->entry); IUnknown_AddRef(&op->IUnknown_iface); } LeaveCriticalSection(&session->cs); @@ -735,7 +738,13 @@ static void session_clear_command_list(struct media_session *session) { struct session_op *op, *op2; - LIST_FOR_EACH_ENTRY_SAFE(op, op2, &session->commands, struct session_op, entry) + if ((op = session->pending_command)) + { + IUnknown_Release(&op->IUnknown_iface); + session->pending_command = NULL; + } + + LIST_FOR_EACH_ENTRY_SAFE(op, op2, &session->queued_commands, struct session_op, entry) { list_remove(&op->entry); IUnknown_Release(&op->IUnknown_iface); @@ -801,15 +810,15 @@ static void session_command_complete(struct media_session *session) struct session_op *op; struct list *e; - /* Pop current command, submit next. */ - if ((e = list_head(&session->commands))) + if (!(op = session->pending_command)) + WARN("No pending command to complete in session %p\n", session); + else { - op = LIST_ENTRY(e, struct session_op, entry); - list_remove(&op->entry); IUnknown_Release(&op->IUnknown_iface); + session->pending_command = NULL; } - if ((e = list_head(&session->commands))) + if ((e = list_head(&session->queued_commands))) { op = LIST_ENTRY(e, struct session_op, entry); MFPutWorkItem(MFASYNC_CALLBACK_QUEUE_STANDARD, &session->commands_callback, &op->IUnknown_iface); @@ -2384,6 +2393,12 @@ static HRESULT WINAPI session_commands_callback_Invoke(IMFAsyncCallback *iface, EnterCriticalSection(&session->cs); + if (session->pending_command) + WARN("Another command is already pending in session %p!\n", session); + + list_remove(&op->entry); + session->pending_command = op; + switch (op->command) { case SESSION_CMD_CLEAR_TOPOLOGIES: @@ -3930,7 +3945,7 @@ HRESULT WINAPI MFCreateMediaSession(IMFAttributes *config, IMFMediaSession **ses object->sink_finalizer_callback.lpVtbl = &session_sink_finalizer_callback_vtbl; object->refcount = 1; list_init(&object->topologies); - list_init(&object->commands); + list_init(&object->queued_commands); list_init(&object->presentation.sources); list_init(&object->presentation.sinks); list_init(&object->presentation.nodes); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/607
From: R��mi Bernon <rbernon(a)codeweavers.com> --- dlls/mf/session.c | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/dlls/mf/session.c b/dlls/mf/session.c index 8c6ab554f5d..991c70595d8 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -457,15 +457,7 @@ static HRESULT session_is_shut_down(struct media_session *session) return session->state == SESSION_STATE_SHUT_DOWN ? MF_E_SHUTDOWN : S_OK; } -static void session_push_back_command(struct media_session *session, enum session_command command) -{ - struct session_op *op; - - if (SUCCEEDED(create_session_op(command, &op))) - list_add_head(&session->queued_commands, &op->entry); -} - -static HRESULT session_submit_command(struct media_session *session, struct session_op *op) +static HRESULT session_submit_command(struct media_session *session, struct session_op *op, BOOL first) { HRESULT hr; @@ -474,7 +466,10 @@ static HRESULT session_submit_command(struct media_session *session, struct sess { if (!session->pending_command && list_empty(&session->queued_commands)) hr = MFPutWorkItem(MFASYNC_CALLBACK_QUEUE_STANDARD, &session->commands_callback, &op->IUnknown_iface); - list_add_tail(&session->queued_commands, &op->entry); + if (first) + list_add_head(&session->queued_commands, &op->entry); + else + list_add_tail(&session->queued_commands, &op->entry); IUnknown_AddRef(&op->IUnknown_iface); } LeaveCriticalSection(&session->cs); @@ -482,7 +477,7 @@ static HRESULT session_submit_command(struct media_session *session, struct sess return hr; } -static HRESULT session_submit_simple_command(struct media_session *session, enum session_command command) +static HRESULT session_submit_simple_command(struct media_session *session, enum session_command command, BOOL first) { struct session_op *op; HRESULT hr; @@ -490,7 +485,7 @@ static HRESULT session_submit_simple_command(struct media_session *session, enum if (FAILED(hr = create_session_op(command, &op))) return hr; - hr = session_submit_command(session, op); + hr = session_submit_command(session, op, first); IUnknown_Release(&op->IUnknown_iface); return hr; } @@ -1717,7 +1712,7 @@ static HRESULT session_set_current_topology(struct media_session *session, IMFTo { op->notify_topology.topology = topology; IMFTopology_AddRef(op->notify_topology.topology); - session_submit_command(session, op); + session_submit_command(session, op, FALSE); IUnknown_Release(&op->IUnknown_iface); } } @@ -2000,7 +1995,7 @@ static HRESULT WINAPI mfsession_SetTopology(IMFMediaSession *iface, DWORD flags, if (op->set_topology.topology) IMFTopology_AddRef(op->set_topology.topology); - hr = session_submit_command(session, op); + hr = session_submit_command(session, op, FALSE); IUnknown_Release(&op->IUnknown_iface); return hr; @@ -2012,7 +2007,7 @@ static HRESULT WINAPI mfsession_ClearTopologies(IMFMediaSession *iface) TRACE("%p.\n", iface); - return session_submit_simple_command(session, SESSION_CMD_CLEAR_TOPOLOGIES); + return session_submit_simple_command(session, SESSION_CMD_CLEAR_TOPOLOGIES, FALSE); } static HRESULT WINAPI mfsession_Start(IMFMediaSession *iface, const GUID *format, const PROPVARIANT *start_position) @@ -2033,7 +2028,7 @@ static HRESULT WINAPI mfsession_Start(IMFMediaSession *iface, const GUID *format hr = PropVariantCopy(&op->start.start_position, start_position); if (SUCCEEDED(hr)) - hr = session_submit_command(session, op); + hr = session_submit_command(session, op, FALSE); IUnknown_Release(&op->IUnknown_iface); return hr; @@ -2045,7 +2040,7 @@ static HRESULT WINAPI mfsession_Pause(IMFMediaSession *iface) TRACE("%p.\n", iface); - return session_submit_simple_command(session, SESSION_CMD_PAUSE); + return session_submit_simple_command(session, SESSION_CMD_PAUSE, FALSE); } static HRESULT WINAPI mfsession_Stop(IMFMediaSession *iface) @@ -2054,7 +2049,7 @@ static HRESULT WINAPI mfsession_Stop(IMFMediaSession *iface) TRACE("%p.\n", iface); - return session_submit_simple_command(session, SESSION_CMD_STOP); + return session_submit_simple_command(session, SESSION_CMD_STOP, FALSE); } static HRESULT WINAPI mfsession_Close(IMFMediaSession *iface) @@ -2063,7 +2058,7 @@ static HRESULT WINAPI mfsession_Close(IMFMediaSession *iface) TRACE("%p.\n", iface); - return session_submit_simple_command(session, SESSION_CMD_CLOSE); + return session_submit_simple_command(session, SESSION_CMD_CLOSE, FALSE); } static HRESULT WINAPI mfsession_Shutdown(IMFMediaSession *iface) @@ -3308,7 +3303,7 @@ static void session_raise_end_of_presentation(struct media_session *session) if (session_nodes_is_mask_set(session, MF_TOPOLOGY_MAX, SOURCE_FLAG_END_OF_PRESENTATION)) { session->presentation.flags |= SESSION_FLAG_END_OF_PRESENTATION; - session_push_back_command(session, SESSION_CMD_END); + session_submit_simple_command(session, SESSION_CMD_END, TRUE); IMFMediaEventQueue_QueueEventParamVar(session->event_queue, MEEndOfPresentation, &GUID_NULL, S_OK, NULL); } } @@ -3852,7 +3847,7 @@ static HRESULT WINAPI session_rate_control_SetRate(IMFRateControl *iface, BOOL t op->set_rate.thin = thin; op->set_rate.rate = rate; - hr = session_submit_command(session, op); + hr = session_submit_command(session, op, FALSE); IUnknown_Release(&op->IUnknown_iface); return hr; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/607
From: R��mi Bernon <rbernon(a)codeweavers.com> --- dlls/mf/session.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/mf/session.c b/dlls/mf/session.c index 991c70595d8..5f3f8851afd 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -2409,6 +2409,10 @@ static HRESULT WINAPI session_commands_callback_Invoke(IMFAsyncCallback *iface, case SESSION_CMD_PAUSE: session_pause(session); break; + case SESSION_CMD_END: + session_set_topo_status(session, S_OK, MF_TOPOSTATUS_ENDED); + session_set_caps(session, session->caps & ~MFSESSIONCAP_PAUSE); + /* fallthrough */ case SESSION_CMD_STOP: session_stop(session); break; @@ -2434,8 +2438,6 @@ static HRESULT WINAPI session_commands_callback_Invoke(IMFAsyncCallback *iface, case SESSION_CMD_SET_RATE: session_set_rate(session, op->set_rate.thin, op->set_rate.rate); break; - default: - ; } LeaveCriticalSection(&session->cs); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/607
Nikolay Sivov (@nsivov) commented about dlls/mf/session.c:
float rate; } presentation; struct list topologies; - struct list commands; + + struct session_op *pending_command; + struct list queued_commands; +
Renaming this field is what constitutes best part of the diff. If "commands" turns up to be a really bad fit, let's change it some time later, with a separate patch. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/607#note_5901
Nikolay Sivov (@nsivov) commented about dlls/mf/session.c:
struct session_op *op; struct list *e;
- /* Pop current command, submit next. */ - if ((e = list_head(&session->commands))) + if (!(op = session->pending_command)) + WARN("No pending command to complete in session %p\n", session); + else
I don't think it needs a warning here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/607#note_5902
Nikolay Sivov (@nsivov) commented about dlls/mf/session.c:
EnterCriticalSection(&session->cs);
+ if (session->pending_command) + WARN("Another command is already pending in session %p!\n", session); +
Same for this warning. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/607#note_5903
Nikolay Sivov (@nsivov) commented about dlls/mf/session.c:
{ if (!session->pending_command && list_empty(&session->queued_commands)) hr = MFPutWorkItem(MFASYNC_CALLBACK_QUEUE_STANDARD, &session->commands_callback, &op->IUnknown_iface); - list_add_tail(&session->queued_commands, &op->entry); + if (first) + list_add_head(&session->queued_commands, &op->entry); + else + list_add_tail(&session->queued_commands, &op->entry);
It would be less of a disturbance to special case SESSION_CMD_END here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/607#note_5904
I don't suppose there is a reliable way to test this, or to reproduce original issue? What was the original issue by the way? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/607#note_5905
On Tue Aug 9 12:18:05 2022 +0000, Nikolay Sivov wrote:
I don't suppose there is a reliable way to test this, or to reproduce original issue? What was the original issue by the way? It happens with Life Is Strange Remastered, after some video playback ends the game closes the session once, then again and then crashes. I believe it expects the session to end on its own, then, as SESSION_CMD_END has no effect, it tries to close it.
What happens is that 1) SESSION_CMD_END doesn't put command processing back to the work queue, and so later close command are ignored, and 2) I think the game still expects the playback end to have some side effect, and fixing the command processing isn't enough. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/607#note_5915
participants (2)
-
Nikolay Sivov (＠nsivov) -
Rémi Bernon