[PATCH v5 0/7] MR7466: mf: Fix some issues with seeking.
This fixes failure to play the prologue video in Planet of the Apes: Last Frontier, which unpauses by calling Start() with a location. The exact state leading to this issue does not occur in the Start() tests, and it's not clear how to reproduce it reliably. -- v5: mf: Reset transform nodes when seeking. mf: Initialise the grabber sample count when setting state after a seek. mf: Reset transform node outputs when seeking. mf: Flush nodes before restarting a paused source at a specific position. mf: Drop transform node input events when unpausing at a specific position. mf: Reset transforms before restarting a paused source instead of after. https://gitlab.winehq.org/wine/wine/-/merge_requests/7466
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/mf/session.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/dlls/mf/session.c b/dlls/mf/session.c index 483ea6f904f..e595cadd185 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -1061,13 +1061,29 @@ static void session_handle_start_error(struct media_session *session, HRESULT hr session_command_complete_with_event(session, MESessionStarted, hr, NULL); } +static void session_reset_transforms(struct media_session *session) +{ + struct topo_node *topo_node; + UINT i; + + LIST_FOR_EACH_ENTRY(topo_node, &session->presentation.nodes, struct topo_node, entry) + { + if (topo_node->type != MF_TOPOLOGY_TRANSFORM_NODE) + continue; + + for (i = 0; i < topo_node->u.transform.input_count; i++) + { + struct transform_stream *stream = &topo_node->u.transform.inputs[i]; + stream->draining = FALSE; + } + } +} + static void session_start(struct media_session *session, const GUID *time_format, const PROPVARIANT *start_position) { struct media_source *source; - struct topo_node *topo_node; MFTIME duration; HRESULT hr; - UINT i; switch (session->state) { @@ -1103,17 +1119,7 @@ static void session_start(struct media_session *session, const GUID *time_format } } - LIST_FOR_EACH_ENTRY(topo_node, &session->presentation.nodes, struct topo_node, entry) - { - if (topo_node->type == MF_TOPOLOGY_TRANSFORM_NODE) - { - for (i = 0; i < topo_node->u.transform.input_count; i++) - { - struct transform_stream *stream = &topo_node->u.transform.inputs[i]; - stream->draining = FALSE; - } - } - } + session_reset_transforms(session); session->state = SESSION_STATE_STARTING_SOURCES; break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7466
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/mf/session.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/mf/session.c b/dlls/mf/session.c index e595cadd185..d9c44cef934 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -1109,6 +1109,8 @@ static void session_start(struct media_session *session, const GUID *time_format return; } + session_reset_transforms(session); + LIST_FOR_EACH_ENTRY(source, &session->presentation.sources, struct media_source, entry) { if (FAILED(hr = IMFMediaSource_Start(source->source, source->pd, &GUID_NULL, start_position))) @@ -1119,8 +1121,6 @@ static void session_start(struct media_session *session, const GUID *time_format } } - session_reset_transforms(session); - session->state = SESSION_STATE_STARTING_SOURCES; break; case SESSION_STATE_STARTED: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7466
From: Conor McCarthy <cmccarthy(a)codeweavers.com> The start position is empty when starting at the current position from the paused state, but if it is not empty, this is a seek operation. --- dlls/mf/session.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dlls/mf/session.c b/dlls/mf/session.c index d9c44cef934..66551e146a7 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -1061,7 +1061,7 @@ static void session_handle_start_error(struct media_session *session, HRESULT hr session_command_complete_with_event(session, MESessionStarted, hr, NULL); } -static void session_reset_transforms(struct media_session *session) +static void session_reset_transforms(struct media_session *session, BOOL drop) { struct topo_node *topo_node; UINT i; @@ -1075,6 +1075,8 @@ static void session_reset_transforms(struct media_session *session) { struct transform_stream *stream = &topo_node->u.transform.inputs[i]; stream->draining = FALSE; + if (drop) + transform_stream_drop_events(stream); } } } @@ -1082,6 +1084,7 @@ static void session_reset_transforms(struct media_session *session) static void session_start(struct media_session *session, const GUID *time_format, const PROPVARIANT *start_position) { struct media_source *source; + BOOL unpause_seek; MFTIME duration; HRESULT hr; @@ -1109,7 +1112,8 @@ static void session_start(struct media_session *session, const GUID *time_format return; } - session_reset_transforms(session); + unpause_seek = start_position->vt == VT_I8; + session_reset_transforms(session, unpause_seek); LIST_FOR_EACH_ENTRY(source, &session->presentation.sources, struct media_source, entry) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7466
From: Conor McCarthy <cmccarthy(a)codeweavers.com> Behaviour in Windows is to send MFT_MESSAGE_COMMAND_FLUSH before calling Start() on the source. --- dlls/mf/session.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/mf/session.c b/dlls/mf/session.c index 66551e146a7..0103d7a65df 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -1113,6 +1113,8 @@ static void session_start(struct media_session *session, const GUID *time_format } unpause_seek = start_position->vt == VT_I8; + if (unpause_seek) + session_flush_nodes(session); session_reset_transforms(session, unpause_seek); LIST_FOR_EACH_ENTRY(source, &session->presentation.sources, struct media_source, entry) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7466
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/mf/session.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/dlls/mf/session.c b/dlls/mf/session.c index 0103d7a65df..dc6bbec2b21 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -1078,6 +1078,16 @@ static void session_reset_transforms(struct media_session *session, BOOL drop) if (drop) transform_stream_drop_events(stream); } + + if (!drop) + continue; + + for (i = 0; i < topo_node->u.transform.output_count; ++i) + { + struct transform_stream *stream = &topo_node->u.transform.outputs[i]; + transform_stream_drop_events(stream); + stream->requests = 0; + } } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7466
From: Conor McCarthy <cmccarthy(a)codeweavers.com> If sample_count is zero in this case, we end up with no samples being requested. --- dlls/mf/samplegrabber.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dlls/mf/samplegrabber.c b/dlls/mf/samplegrabber.c index de599139736..4d54afc1d31 100644 --- a/dlls/mf/samplegrabber.c +++ b/dlls/mf/samplegrabber.c @@ -1195,6 +1195,10 @@ static HRESULT sample_grabber_set_state(struct sample_grabber *grabber, enum sin if (state == SINK_STATE_RUNNING && grabber->state != SINK_STATE_RUNNING) { + /* Unpause at a position is a seek operation which drops everything pending. */ + if (grabber->state == SINK_STATE_PAUSED && offset != PRESENTATION_CURRENT_POSITION) + grabber->sample_count = MAX_SAMPLE_QUEUE_LENGTH; + /* Every transition to running state sends a bunch requests to build up initial queue. */ for (i = 0; i < grabber->sample_count; ++i) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7466
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/mf/session.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/mf/session.c b/dlls/mf/session.c index dc6bbec2b21..e11528369e2 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -1164,6 +1164,8 @@ static void session_start(struct media_session *session, const GUID *time_format } } + session_reset_transforms(session, TRUE); + session->presentation.time_format = *time_format; session->presentation.start_position.vt = VT_EMPTY; PropVariantCopy(&session->presentation.start_position, start_position); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7466
Brendan McGrath (@redmcg) commented about dlls/mf/session.c:
return; }
+ unpause_seek = start_position->vt == VT_I8; + if (unpause_seek) + session_flush_nodes(session);
FYI - I just came up with a very similar fix to this: https://gitlab.winehq.org/redmcg/wine/-/commit/f44e24148a10a124865cb11537cc5... It is required to fix stop/start when using AVPro in VRChat (under the hood it is doing a Pause/Start with a position of 0.0 seconds). I found your MR after trying to merge my change in to Proton (and running in to a conflict with your change there). At the moment, `f44e241` is part of a branch with experimental fixes for the different timing issues that I've ran in to. But I do plan to upstream each fix eventually. I have another change in that branch that resets `stream->requests` to zero in `session_flush_nodes`: https://gitlab.winehq.org/redmcg/wine/-/commit/425c4f08e593fd45794dfcd1d3b4a... I don't believe we have a way of knowing the status of these requests when the flush takes place. The request might still be undelivered (i.e. in the initial queue), or it could have already resulted in a sample being produced and processed all the way to the very last transform. So I think it makes sense to set `requests` to zero when we flush. I was otherwise getting a scenario where the sink was flushing and requesting a new sample, but the media session wasn't passing it on because it was just incrementing `requests`. The problem was, the pending request had been flushed when we flushed the transforms. As a result, playback was frozen indefinitely. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7466#note_100858
participants (3)
-
Brendan McGrath (＠redmcg) -
Conor McCarthy
-
Conor McCarthy (＠cmccarthy)