[PATCH v2 0/2] MR10885: quartz: Call GetCurrentPosition() to get the latest position before stopping the graph.
Unless I missed something, info about sample delivery is not available in the quartz module, so I left this as a todo. With a typical framerate, the position set should be pretty close anyway. -- v2: quartz: Update current and elapsed positions in Stop(). quartz/tests: Test current position after stopping a graph. https://gitlab.winehq.org/wine/wine/-/merge_requests/10885
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/quartz/tests/filtergraph.c | 93 +++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/dlls/quartz/tests/filtergraph.c b/dlls/quartz/tests/filtergraph.c index 087c37be538..0dbc4b41b84 100644 --- a/dlls/quartz/tests/filtergraph.c +++ b/dlls/quartz/tests/filtergraph.c @@ -6071,6 +6071,98 @@ static void test_event_dispatch(void) ok(!ref, "Got outstanding refcount %ld.\n", ref); } +static void test_stopped_current_position(void) +{ + IEnumFilters *enum_filters; + LONG_PTR lparam1, lparam2; + IMediaEvent *media_event; + IMediaSeeking *seeking; + REFERENCE_TIME current; + IMediaControl *control; + IFilterGraph2 *graph; + IBaseFilter *filter; + unsigned int i = 0; + WCHAR *filename; + ULONG fetched; + long ev_code; + HANDLE event; + HRESULT hr; + LONG code; + + filename = load_resource(L"test.avi"); + + graph = create_graph(); + + hr = IFilterGraph2_RenderFile(graph, filename, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IFilterGraph2_QueryInterface(graph, &IID_IMediaControl, (void **)&control); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IFilterGraph2_QueryInterface(graph, &IID_IMediaEvent, (void **)&media_event); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IMediaEvent_GetEventHandle(media_event, (OAEVENT *)&event); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + /* Flush existing events. */ + while ((hr = IMediaEvent_GetEvent(media_event, &code, &lparam1, &lparam2, 0)) == S_OK); + + hr = IMediaControl_Run(control); + ok(SUCCEEDED(hr), "Got hr %#lx.\n", hr); + + ok(WaitForSingleObject(event, 1000) == 0, "Event should be signaled.\n"); + + /* Wait 2.5 seconds of playback time. Source video is 1 fps. */ + IMediaEvent_WaitForCompletion(media_event, 2500, &ev_code); + + hr = IMediaControl_Stop(control); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IFilterGraph2_QueryInterface(graph, &IID_IMediaSeeking, (void **)&seeking); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IMediaSeeking_GetCurrentPosition(seeking, ¤t); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + todo_wine + ok(compare_time(current, 2500 * 10000, 100 * 10000), "Expected about 2500ms, got %I64d.\n", current); + + IMediaSeeking_Release(seeking); + + hr = IFilterGraph2_EnumFilters(graph, &enum_filters); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + while ((hr = IEnumFilters_Next(enum_filters, 1, &filter, &fetched)) == S_OK) + { + winetest_push_context("Filter %u:", i++); + + ok(fetched, "Filter not fetched.\n"); + + if (FAILED(hr = IBaseFilter_QueryInterface(filter, &IID_IMediaSeeking, (void **)&seeking))) + { + ok(hr == E_NOINTERFACE, "Got hr %#lx.\n", hr); + } + else + { + hr = IMediaSeeking_GetCurrentPosition(seeking, ¤t); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + todo_wine + ok(current == 20000000, "Got current %I64d.\n", current); + + IMediaSeeking_Release(seeking); + } + + IBaseFilter_Release(filter); + winetest_pop_context(); + } + ok(hr == S_FALSE, "Got hr %#lx.\n", hr); + + IEnumFilters_Release(enum_filters); + IMediaEvent_Release(media_event); + IMediaControl_Release(control); + IFilterGraph2_Release(graph); + DeleteFileW(filename); +} + START_TEST(filtergraph) { CoInitializeEx(NULL, COINIT_MULTITHREADED); @@ -6100,6 +6192,7 @@ START_TEST(filtergraph) test_set_notify_flags(); test_events(); test_event_dispatch(); + test_stopped_current_position(); CoUninitialize(); test_render_with_multithread(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10885
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/quartz/filtergraph.c | 24 ++++++++++++++++-------- dlls/quartz/tests/filtergraph.c | 1 - 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/dlls/quartz/filtergraph.c b/dlls/quartz/filtergraph.c index 79fcc425e53..25116cea789 100644 --- a/dlls/quartz/filtergraph.c +++ b/dlls/quartz/filtergraph.c @@ -5061,6 +5061,17 @@ static HRESULT WINAPI MediaFilter_GetClassID(IMediaFilter *iface, CLSID * pClass return E_NOTIMPL; } +static void graph_update_positions(struct filter_graph *graph) +{ + if (graph->state == State_Running && !graph->needs_async_run && graph->refClock) + { + REFERENCE_TIME time; + IReferenceClock_GetTime(graph->refClock, &time); + graph->stream_elapsed += time - graph->stream_start; + graph->current_pos += graph->stream_elapsed; + } +} + static HRESULT WINAPI MediaFilter_Stop(IMediaFilter *iface) { struct filter_graph *graph = impl_from_IMediaFilter(iface); @@ -5080,6 +5091,8 @@ static HRESULT WINAPI MediaFilter_Stop(IMediaFilter *iface) sort_filters(graph); + graph_update_positions(graph); + if (graph->state == State_Running) { LIST_FOR_EACH_ENTRY(filter, &graph->filters, struct filter, entry) @@ -5101,7 +5114,8 @@ static HRESULT WINAPI MediaFilter_Stop(IMediaFilter *iface) graph->needs_async_run = 0; work = graph->async_run_work; - /* Update the current position, probably to synchronize multiple streams. */ + /* Update the current position, probably to synchronize multiple streams. + * TODO: on native, filters seem to be set to the position of the latest sample. */ IMediaSeeking_SetPositions(&graph->IMediaSeeking_iface, &graph->current_pos, AM_SEEKING_AbsolutePositioning, NULL, AM_SEEKING_NoPositioning); @@ -5139,13 +5153,7 @@ static HRESULT WINAPI MediaFilter_Pause(IMediaFilter *iface) if (graph->defaultclock && !graph->refClock) IFilterGraph2_SetDefaultSyncSource(&graph->IFilterGraph2_iface); - if (graph->state == State_Running && !graph->needs_async_run && graph->refClock) - { - REFERENCE_TIME time; - IReferenceClock_GetTime(graph->refClock, &time); - graph->stream_elapsed += time - graph->stream_start; - graph->current_pos += graph->stream_elapsed; - } + graph_update_positions(graph); LIST_FOR_EACH_ENTRY(filter, &graph->filters, struct filter, entry) { diff --git a/dlls/quartz/tests/filtergraph.c b/dlls/quartz/tests/filtergraph.c index 0dbc4b41b84..c0555db9ae1 100644 --- a/dlls/quartz/tests/filtergraph.c +++ b/dlls/quartz/tests/filtergraph.c @@ -6123,7 +6123,6 @@ static void test_stopped_current_position(void) hr = IMediaSeeking_GetCurrentPosition(seeking, ¤t); ok(hr == S_OK, "Got hr %#lx.\n", hr); - todo_wine ok(compare_time(current, 2500 * 10000, 100 * 10000), "Expected about 2500ms, got %I64d.\n", current); IMediaSeeking_Release(seeking); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10885
On Fri May 15 15:30:14 2026 +0000, Elizabeth Figura wrote:
I'm not sure I understand what you mean. If you're saying that the custom filter also needs to update its position after every frame, and that that should be the responsibility of the filter graph, that's not how it works. Filters are largely self-sufficient. The AVI splitter might report the position of the last sample it sent, but that doesn't mean that any other filter does. SetPositions() is not a command to update the reported position; it is a command to seek, and calling it every frame would be a bad idea. Ok, it looks like the filters are set to the position of the latest sample when `Stop()` is called. We probably don't need to do anything about that unless a bug arises out of it.
Positions in `test_graph_seeking()` are not updated at all in native or Wine if we sleep on a running graph for a second and then stop. I haven't moved the tests there because I don't know how to test this issue there. I updated the implementation though. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10885#note_140299
Ok, it looks like the filters are set to the position of the latest sample when `Stop()` is called. We probably don't need to do anything about that unless a bug arises out of it.
I don't think that's true per se. Again, according to our current understanding, and I would be surprised if this is wrong, the only time SetPositions() is ever called by the graph is when SetPositions() is called *on* the graph. There's not really a reason to seek at any other time. Filters may *choose* to report different times from GetPositions(), but that's not under the graph's control. As we've seen, they may or may not update the reported position more frequently. The graph never calls GetPositions() on filters as far as we know so it doesn't really matter much. It would be unlikely for anything to ask individual filters for their positions instead of the graph.
Positions in `test_graph_seeking()` are not updated at all in native or Wine if we sleep on a running graph for a second and then stop.
I'm looking at lines 5063-5084 in upstream Wine. We seek to 1234 ms, sleep for 100 ms, and then GetPositions() returns roughly 1334 ms. Again, this is about GetPositions() on the graph, not the filters.
I haven't moved the tests there because I don't know how to test this issue there. I updated the implementation though.
See the bit right after that, lines 5086-5104, where we pause the graph, wait 100 ms, then run it again, and test that the positions haven't moved in the meantime. In this case we'd want to duplicate that part, but stop instead of pause. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10885#note_140425
On Mon May 18 16:20:43 2026 +0000, Elizabeth Figura wrote:
Ok, it looks like the filters are set to the position of the latest sample when `Stop()` is called. We probably don't need to do anything about that unless a bug arises out of it. I don't think that's true per se. Again, according to our current understanding, and I would be surprised if this is wrong, the only time SetPositions() is ever called by the graph is when SetPositions() is called *on* the graph. There's not really a reason to seek at any other time. Filters may *choose* to report different times from GetPositions(), but that's not under the graph's control. As we've seen, they may or may not update the reported position more frequently. The graph never calls GetPositions() on filters as far as we know so it doesn't really matter much. It would be unlikely for anything to ask individual filters for their positions instead of the graph. Positions in `test_graph_seeking()` are not updated at all in native or Wine if we sleep on a running graph for a second and then stop. I'm looking at lines 5063-5084 in upstream Wine. We seek to 1234 ms, sleep for 100 ms, and then GetPositions() returns roughly 1334 ms. Again, this is about GetPositions() on the graph, not the filters. I haven't moved the tests there because I don't know how to test this issue there. I updated the implementation though. See the bit right after that, lines 5086-5104, where we pause the graph, wait 100 ms, then run it again, and test that the positions haven't moved in the meantime. In this case we'd want to duplicate that part, but stop instead of pause. Yes, I added a test in there, but it doesn't test the issue I'm fixing. The existing tests show it: `Stop()` is called on line 5106, but `GetCurrentPosition()` then returns 1234, after the previous `GetPositions()` call while it was running returned 1334. Results from the .avi file are different.
To put it another way, if I delete the lines which update `graph->stream_elapsed` and `graph->current_pos` in `Pause()`, none of the current tests fail. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10885#note_140515
Yes, I added a test in there, but it doesn't test the issue I'm fixing. The existing tests show it: `Stop()` is called on line 5106, but `GetCurrentPosition()` then returns 1234, after the previous `GetPositions()` call while it was running returned 1334. Results from the .avi file are different.
That's GetCurrentPositions() while stopped, though. This is about GetCurrentPositions() while running, after stopping for a measurable amount of time.
To put it another way, if I delete the lines which update `graph->stream_elapsed` and `graph->current_pos` in `Pause()`, none of the current tests fail.
That's probably because you're not running the tests in interactive mode. Running the tests in interactive mode after deleting those lines, I see failures on lines 5096 and 5102, which is exactly the tests I'd expect to see fail. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10885#note_141615
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Elizabeth Figura (＠zfigura)