[PATCH v8 0/4] MR3319: evr: Leave critical section before waiting for streaming thread.
I'm not sure how could I test this behavior, right now the test I wrote works but there are times where it doesn't enter into the deadlock. -- v8: evr: Remove process input handling from streaming thread. evr: Don't lock presenter allocator when calling NotifyRelease evr: Create critical section for sample queue. https://gitlab.winehq.org/wine/wine/-/merge_requests/3319
From: Santino Mazza <smazza(a)codeweavers.com> --- dlls/evr/presenter.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/dlls/evr/presenter.c b/dlls/evr/presenter.c index 06592f8766e..97080717288 100644 --- a/dlls/evr/presenter.c +++ b/dlls/evr/presenter.c @@ -467,6 +467,18 @@ static BOOL video_presenter_sample_queue_pop(struct video_presenter *presenter, return *sample != NULL; } + +static void video_presenter_sample_queue_free(struct video_presenter *presenter) +{ + struct sample_queue *queue = &presenter->thread.queue; + IMFSample *sample; + + while (video_presenter_sample_queue_pop(presenter, &sample)) + IMFSample_Release(sample); + + free(queue->samples); +} + static HRESULT video_presenter_get_sample_surface(IMFSample *sample, IDirect3DSurface9 **surface) { IMFMediaBuffer *buffer; @@ -754,6 +766,7 @@ static HRESULT video_presenter_end_streaming(struct video_presenter *presenter) if (presenter->thread.queue.last_presented) IMFSample_Release(presenter->thread.queue.last_presented); + video_presenter_sample_queue_free(presenter); memset(&presenter->thread, 0, sizeof(presenter->thread)); video_presenter_set_allocator_callback(presenter, NULL); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3319
From: Santino Mazza <smazza(a)codeweavers.com> --- dlls/evr/presenter.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/dlls/evr/presenter.c b/dlls/evr/presenter.c index 97080717288..645a2461e4c 100644 --- a/dlls/evr/presenter.c +++ b/dlls/evr/presenter.c @@ -66,6 +66,7 @@ struct sample_queue unsigned int front; unsigned int back; IMFSample *last_presented; + CRITICAL_SECTION cs; }; struct streaming_thread @@ -425,6 +426,7 @@ static HRESULT video_presenter_sample_queue_init(struct video_presenter *present queue->size = presenter->allocator_capacity; queue->back = queue->size - 1; + InitializeCriticalSection(&queue->cs); return S_OK; } @@ -435,7 +437,7 @@ static void video_presenter_sample_queue_push(struct video_presenter *presenter, struct sample_queue *queue = &presenter->thread.queue; unsigned int idx; - EnterCriticalSection(&presenter->cs); + EnterCriticalSection(&queue->cs); if (queue->used != queue->size) { if (at_front) @@ -446,14 +448,14 @@ static void video_presenter_sample_queue_push(struct video_presenter *presenter, queue->used++; IMFSample_AddRef(sample); } - LeaveCriticalSection(&presenter->cs); + LeaveCriticalSection(&queue->cs); } static BOOL video_presenter_sample_queue_pop(struct video_presenter *presenter, IMFSample **sample) { struct sample_queue *queue = &presenter->thread.queue; - EnterCriticalSection(&presenter->cs); + EnterCriticalSection(&queue->cs); if (queue->used) { *sample = queue->samples[queue->front]; @@ -462,7 +464,7 @@ static BOOL video_presenter_sample_queue_pop(struct video_presenter *presenter, } else *sample = NULL; - LeaveCriticalSection(&presenter->cs); + LeaveCriticalSection(&queue->cs); return *sample != NULL; } @@ -477,6 +479,7 @@ static void video_presenter_sample_queue_free(struct video_presenter *presenter) IMFSample_Release(sample); free(queue->samples); + DeleteCriticalSection(&queue->cs); } static HRESULT video_presenter_get_sample_surface(IMFSample *sample, IDirect3DSurface9 **surface) @@ -502,6 +505,7 @@ static void video_presenter_sample_present(struct video_presenter *presenter, IM { IDirect3DSurface9 *surface, *backbuffer; IDirect3DDevice9 *device; + struct sample_queue *queue = &presenter->thread.queue; HRESULT hr; if (FAILED(hr = video_presenter_get_sample_surface(sample, &surface))) @@ -527,12 +531,12 @@ static void video_presenter_sample_present(struct video_presenter *presenter, IM WARN("Failed to get a backbuffer, hr %#lx.\n", hr); } - EnterCriticalSection(&presenter->cs); - if (presenter->thread.queue.last_presented) - IMFSample_Release(presenter->thread.queue.last_presented); - presenter->thread.queue.last_presented = sample; - IMFSample_AddRef(presenter->thread.queue.last_presented); - LeaveCriticalSection(&presenter->cs); + EnterCriticalSection(&queue->cs); + if (queue->last_presented) + IMFSample_Release(queue->last_presented); + queue->last_presented = sample; + IMFSample_AddRef(queue->last_presented); + LeaveCriticalSection(&queue->cs); IDirect3DSurface9_Release(surface); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3319
From: Santino Mazza <smazza(a)codeweavers.com> --- dlls/evr/evr_private.h | 2 ++ dlls/evr/presenter.c | 2 +- dlls/evr/sample.c | 29 ++++++++++++++++++++++++----- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/dlls/evr/evr_private.h b/dlls/evr/evr_private.h index ef38b0f70cf..93047b50c94 100644 --- a/dlls/evr/evr_private.h +++ b/dlls/evr/evr_private.h @@ -55,4 +55,6 @@ HRESULT evr_filter_create(IUnknown *outer_unk, void **ppv) DECLSPEC_HIDDEN; HRESULT evr_mixer_create(IUnknown *outer_unk, void **ppv) DECLSPEC_HIDDEN; HRESULT evr_presenter_create(IUnknown *outer_unk, void **ppv) DECLSPEC_HIDDEN; +HRESULT create_video_sample_allocator(BOOL lock_notify_release, REFIID riid, void **obj); + #endif /* __EVR_PRIVATE_INCLUDED__ */ diff --git a/dlls/evr/presenter.c b/dlls/evr/presenter.c index 645a2461e4c..3ae9b24ad4a 100644 --- a/dlls/evr/presenter.c +++ b/dlls/evr/presenter.c @@ -2149,7 +2149,7 @@ static HRESULT video_presenter_init_d3d(struct video_presenter *presenter) if (FAILED(hr)) WARN("Failed to set new device for the manager, hr %#lx.\n", hr); - if (SUCCEEDED(hr = MFCreateVideoSampleAllocator(&IID_IMFVideoSampleAllocator, (void **)&presenter->allocator))) + if (SUCCEEDED(hr = create_video_sample_allocator(FALSE, &IID_IMFVideoSampleAllocator, (void **)&presenter->allocator))) { hr = IMFVideoSampleAllocator_SetDirectXManager(presenter->allocator, (IUnknown *)presenter->device_manager); } diff --git a/dlls/evr/sample.c b/dlls/evr/sample.c index 6a1bbf564f5..8c261bc5b47 100644 --- a/dlls/evr/sample.c +++ b/dlls/evr/sample.c @@ -395,6 +395,7 @@ struct sample_allocator unsigned int free_sample_count; struct list free_samples; struct list used_samples; + BOOL lock_notify_release; CRITICAL_SECTION cs; }; @@ -809,6 +810,8 @@ static HRESULT WINAPI sample_allocator_tracking_callback_Invoke(IMFAsyncCallback struct queued_sample *iter; IUnknown *object = NULL; IMFSample *sample = NULL; + IMFVideoSampleAllocatorNotify *callback = NULL; + BOOL lock_notify_release; HRESULT hr; if (FAILED(IMFAsyncResult_GetObject(result, &object))) @@ -835,11 +838,21 @@ static HRESULT WINAPI sample_allocator_tracking_callback_Invoke(IMFAsyncCallback IMFSample_Release(sample); - if (allocator->callback) - IMFVideoSampleAllocatorNotify_NotifyRelease(allocator->callback); + lock_notify_release = allocator->lock_notify_release; + callback = allocator->callback; + + IMFVideoSampleAllocatorNotify_AddRef(callback); + + if (callback && lock_notify_release) + IMFVideoSampleAllocatorNotify_NotifyRelease(callback); LeaveCriticalSection(&allocator->cs); + if (callback && !lock_notify_release) + IMFVideoSampleAllocatorNotify_NotifyRelease(callback); + + IMFVideoSampleAllocatorNotify_Release(callback); + return S_OK; } @@ -852,13 +865,11 @@ static const IMFAsyncCallbackVtbl sample_allocator_tracking_callback_vtbl = sample_allocator_tracking_callback_Invoke, }; -HRESULT WINAPI MFCreateVideoSampleAllocator(REFIID riid, void **obj) +HRESULT create_video_sample_allocator(BOOL lock_notify_release, REFIID riid, void **obj) { struct sample_allocator *object; HRESULT hr; - TRACE("%s, %p.\n", debugstr_guid(riid), obj); - if (!(object = calloc(1, sizeof(*object)))) return E_OUTOFMEMORY; @@ -868,6 +879,7 @@ HRESULT WINAPI MFCreateVideoSampleAllocator(REFIID riid, void **obj) object->refcount = 1; list_init(&object->used_samples); list_init(&object->free_samples); + object->lock_notify_release = lock_notify_release; InitializeCriticalSection(&object->cs); hr = IMFVideoSampleAllocator_QueryInterface(&object->IMFVideoSampleAllocator_iface, riid, obj); @@ -876,6 +888,13 @@ HRESULT WINAPI MFCreateVideoSampleAllocator(REFIID riid, void **obj) return hr; } +HRESULT WINAPI MFCreateVideoSampleAllocator(REFIID riid, void **obj) +{ + TRACE("%s, %p.\n", debugstr_guid(riid), obj); + + return create_video_sample_allocator(TRUE, riid, obj); +} + static HRESULT WINAPI video_sample_QueryInterface(IMFSample *iface, REFIID riid, void **out) { struct video_sample *sample = impl_from_IMFSample(iface); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3319
From: Santino Mazza <smazza(a)codeweavers.com> --- dlls/evr/presenter.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/dlls/evr/presenter.c b/dlls/evr/presenter.c index 3ae9b24ad4a..2dc01608caf 100644 --- a/dlls/evr/presenter.c +++ b/dlls/evr/presenter.c @@ -55,7 +55,6 @@ enum streaming_thread_message { EVRM_STOP = WM_USER, EVRM_PRESENT = WM_USER + 1, - EVRM_PROCESS_INPUT = WM_USER + 2, }; struct sample_queue @@ -706,11 +705,6 @@ static DWORD CALLBACK video_presenter_streaming_thread(void *arg) } break; - case EVRM_PROCESS_INPUT: - EnterCriticalSection(&presenter->cs); - video_presenter_process_input(presenter); - LeaveCriticalSection(&presenter->cs); - break; default: ; } @@ -1811,9 +1805,9 @@ static HRESULT WINAPI video_presenter_allocator_cb_NotifyRelease(IMFVideoSampleA { struct video_presenter *presenter = impl_from_IMFVideoSampleAllocatorNotify(iface); - /* Release notification is executed under allocator lock, instead of processing samples here - notify streaming thread. */ - PostThreadMessageW(presenter->thread.tid, EVRM_PROCESS_INPUT, 0, 0); + EnterCriticalSection(&presenter->cs); + video_presenter_process_input(presenter); + LeaveCriticalSection(&presenter->cs); return S_OK; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3319
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=135888 Your paranoid android. === debian11 (32 bit report) === evr: Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x70090c38). === debian11b (64 bit WoW report) === evr: Unhandled exception: page fault on read access to 0x0000000000000000 in 64-bit code (0x0000006b10f56b).
participants (3)
-
Marvin -
Santino Mazza -
Santino Mazza (＠tati1454)