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.
-- v6: mf: Reset transform nodes when seeking. mf: Initialise the grabber sample count when setting state after a seek. mf: Set pending requests for transform outputs to zero on flush. 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. mf: Move transform node reset to a helper function.
From: Conor McCarthy cmccarthy@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;
From: Conor McCarthy cmccarthy@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:
From: Conor McCarthy cmccarthy@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) {
From: Conor McCarthy cmccarthy@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)
From: Conor McCarthy cmccarthy@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 0103d7a65df..423b7964d85 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -1078,6 +1078,15 @@ 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); + } } }
From: Brendan McGrath bmcgrath@codeweavers.com
--- dlls/mf/session.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/dlls/mf/session.c b/dlls/mf/session.c index 423b7964d85..2f53ed1838c 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -1009,13 +1009,18 @@ static HRESULT session_subscribe_sources(struct media_session *session) static void session_flush_nodes(struct media_session *session) { struct topo_node *node; + UINT i;
LIST_FOR_EACH_ENTRY(node, &session->presentation.nodes, struct topo_node, entry) { if (node->type == MF_TOPOLOGY_OUTPUT_NODE) IMFStreamSink_Flush(node->object.sink_stream); else if (node->type == MF_TOPOLOGY_TRANSFORM_NODE) + { IMFTransform_ProcessMessage(node->object.transform, MFT_MESSAGE_COMMAND_FLUSH, 0); + for (i = 0; i < node->u.transform.output_count; i++) + node->u.transform.outputs[i].requests = 0; /* these requests might have been flushed */ + } } }
From: Conor McCarthy cmccarthy@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) {
From: Conor McCarthy cmccarthy@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 2f53ed1838c..3e12f0642a4 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -1168,6 +1168,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);
I have another change in that branch that resets `stream->requests` to zero in `session_flush_nodes`
That makes sense. Updated this to include your commit instead of setting `stream->requests` in `session_reset_transforms()`.
Nikolay Sivov (@nsivov) commented about dlls/mf/samplegrabber.c:
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) {
This looks a bit messed up I think - first we reset this to max length when stopping, then when start-from-pause case only to execute this loop and reset it back to zero. But I don't see a cleaner way right away.
Nikolay Sivov (@nsivov) commented about dlls/mf/session.c:
} }
session_reset_transforms(session);session->state = SESSION_STATE_STARTING_SOURCES; break;
Why would moving this make any difference?
What bothers me about seeking changes is that it looks like patching pieces as they fail, to make transitions work. And there is no way to actually tell what is correct. The way I did it initially for other states was to create a full stub branch of source->transform->sink, let it run on empty samples, and then issue more commands to see what needs to happen. I don't see how this could be reviewed without doing something similar, especially if it's difficult to reproduce with a proper test.
How did you test individual changes, apart from testing the game with the whole set?
-
Brendan McGrath -
Conor McCarthy
-
Conor McCarthy (＠cmccarthy)
-
Nikolay Sivov (＠nsivov)