[PATCH 0/2] MR7696: mf: Specify the timer queue for timer callbacks.
Calling MFScheduleWorkItemEx() schedules the operation in the timer queue, but unless GetParameters() returns the timer queue, callback invocation will occur in the standard queue. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7696
From: Conor McCarthy <cmccarthy(a)codeweavers.com> Calling MFScheduleWorkItemEx() schedules the operation in the timer queue, but unless GetParameters() returns the timer queue, callback invocation will occur in the standard queue. --- dlls/mf/clock.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/dlls/mf/clock.c b/dlls/mf/clock.c index e6be05d2794..f0e8dbdf0bd 100644 --- a/dlls/mf/clock.c +++ b/dlls/mf/clock.c @@ -1098,6 +1098,13 @@ static ULONG WINAPI present_clock_timer_callback_Release(IMFAsyncCallback *iface return IMFPresentationClock_Release(&clock->IMFPresentationClock_iface); } +static HRESULT WINAPI present_clock_timer_callback_GetParameters(IMFAsyncCallback *iface, DWORD *flags, DWORD *queue) +{ + *flags = 0; + *queue = MFASYNC_CALLBACK_QUEUE_TIMER; + return S_OK; +} + static HRESULT WINAPI present_clock_timer_callback_Invoke(IMFAsyncCallback *iface, IMFAsyncResult *result) { struct presentation_clock *clock = impl_from_timer_callback_IMFAsyncCallback(iface); @@ -1133,7 +1140,7 @@ static const IMFAsyncCallbackVtbl presentclocktimercallbackvtbl = present_clock_callback_QueryInterface, present_clock_timer_callback_AddRef, present_clock_timer_callback_Release, - present_clock_callback_GetParameters, + present_clock_timer_callback_GetParameters, present_clock_timer_callback_Invoke, }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7696
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/mf/samplegrabber.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dlls/mf/samplegrabber.c b/dlls/mf/samplegrabber.c index de599139736..c46cd8affab 100644 --- a/dlls/mf/samplegrabber.c +++ b/dlls/mf/samplegrabber.c @@ -744,7 +744,9 @@ static ULONG WINAPI sample_grabber_stream_timer_callback_Release(IMFAsyncCallbac static HRESULT WINAPI sample_grabber_stream_timer_callback_GetParameters(IMFAsyncCallback *iface, DWORD *flags, DWORD *queue) { - return E_NOTIMPL; + *flags = 0; + *queue = MFASYNC_CALLBACK_QUEUE_TIMER; + return S_OK; } static HRESULT WINAPI sample_grabber_stream_timer_callback_Invoke(IMFAsyncCallback *iface, IMFAsyncResult *result) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7696
Have you tested this manually somehow? I'm not sure why this is necessary, maybe only to unload some callbacks from standard queue? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7696#note_99397
On Tue Apr 1 01:18:18 2025 +0000, Nikolay Sivov wrote:
Have you tested this manually somehow? I'm not sure why this is necessary, maybe only to unload some callbacks from standard queue? I tested mp4 playback in Windows. All video and audio work is done in one thread, except for `OnProcessSample()`, which is called from another thread. We can replicate this by ensuring the timer queue is used. This doesn't cover the case where `MF_SAMPLEGRABBERSINK_IGNORE_CLOCK` is set, and I haven't tested it. Do games use that flag rarely?
Yes, the main reason is to unload the standard queue. If the game's `OnProcessSample()` takes a bit of time, this change makes it less likely that frames will take too long to decode, which can happen with 4K, but I don't have a example of that happening in a game which uses the session engine. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7696#note_99471
Actually I think this is an rtworkq issue. `invoke_async_callback()` should take as a second parameter the default queue, which should be TIMER if the callback is invoked as a result of `RtwqPutWaitingWorkItem()`, `RtwqScheduleWorkItem()` or `RtwqAddPeriodicCallback()`. Does that sound right? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7696#note_99626
On Tue Apr 8 16:31:42 2025 +0000, Conor McCarthy wrote:
Actually I think this is an rtworkq issue. `invoke_async_callback()` should take as a second parameter the default queue, which should be TIMER if the callback is invoked as a result of `RtwqPutWaitingWorkItem()`, `RtwqScheduleWorkItem()` or `RtwqAddPeriodicCallback()`. Does that sound right? Where do you see this? It's a two step process. First item should trigger and that's where QUEUE_TIMER is used, but where payload is executed is determined by GetParameters() essentially, with standard queue being a default one. Now in context of the media session, docs talk about explicit per topology branch queues. User queues do behave differently, using MFPutWorkItem() with a user queue makes Invoke happens consistently on another thread.
Current logic we have is almost like that except that MFPutWorkItem() always uses the queue it was told to use. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7696#note_100269
On Tue Apr 8 16:31:42 2025 +0000, Nikolay Sivov wrote:
Where do you see this? It's a two step process. First item should trigger and that's where QUEUE_TIMER is used, but where payload is executed is determined by GetParameters() essentially, with standard queue being a default one. Now in context of the media session, docs talk about explicit per topology branch queues. User queues do behave differently, using MFPutWorkItem() with a user queue makes Invoke happens consistently on another thread. Current logic we have is almost like that except that MFPutWorkItem() always uses the queue it was told to use. I tested this yesterday on Windows and the current rtworkq code is correct, yes. I also modified this MR to create a user queue for MF session sinks. I'll push that tomorrow. If you believe these changes are not a good idea, they're not needed to fix anything specific at the moment.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7696#note_100408
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Nikolay Sivov (＠nsivov)