[PATCH v7 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. -- v7: evr: Remove process input handling from streaming thread. 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 | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/dlls/evr/presenter.c b/dlls/evr/presenter.c index 97080717288..00bc00fbf2b 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) @@ -527,12 +530,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); + EnterCriticalSection(&presenter->thread.queue.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); + LeaveCriticalSection(&presenter->thread.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 | 25 +++++++++++++++++++------ 3 files changed, 22 insertions(+), 7 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 00bc00fbf2b..f9e372d0ed1 100644 --- a/dlls/evr/presenter.c +++ b/dlls/evr/presenter.c @@ -2148,7 +2148,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..36f15a0c48f 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,7 @@ static HRESULT WINAPI sample_allocator_tracking_callback_Invoke(IMFAsyncCallback struct queued_sample *iter; IUnknown *object = NULL; IMFSample *sample = NULL; + IMFVideoSampleAllocatorNotify *callback = NULL; HRESULT hr; if (FAILED(IMFAsyncResult_GetObject(result, &object))) @@ -834,12 +836,17 @@ static HRESULT WINAPI sample_allocator_tracking_callback_Invoke(IMFAsyncCallback } IMFSample_Release(sample); - - if (allocator->callback) - IMFVideoSampleAllocatorNotify_NotifyRelease(allocator->callback); - + callback = allocator->callback; + if (callback && allocator->lock_notify_release) + { + IMFVideoSampleAllocatorNotify_NotifyRelease(callback); + callback = NULL; + } LeaveCriticalSection(&allocator->cs); + if (callback) + IMFVideoSampleAllocatorNotify_NotifyRelease(callback); + return S_OK; } @@ -852,12 +859,12 @@ 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); + TRACE("%d, %s, %p.\n", lock_notify_release, debugstr_guid(riid), obj); if (!(object = calloc(1, sizeof(*object)))) return E_OUTOFMEMORY; @@ -868,6 +875,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 +884,11 @@ HRESULT WINAPI MFCreateVideoSampleAllocator(REFIID riid, void **obj) return hr; } +HRESULT WINAPI MFCreateVideoSampleAllocator(REFIID riid, void **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 +++--------- dlls/evr/sample.c | 1 + 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/dlls/evr/presenter.c b/dlls/evr/presenter.c index f9e372d0ed1..ae7563231be 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 @@ -705,11 +704,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: ; } @@ -1810,9 +1804,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; } diff --git a/dlls/evr/sample.c b/dlls/evr/sample.c index 36f15a0c48f..0a7300429d0 100644 --- a/dlls/evr/sample.c +++ b/dlls/evr/sample.c @@ -837,6 +837,7 @@ static HRESULT WINAPI sample_allocator_tracking_callback_Invoke(IMFAsyncCallback IMFSample_Release(sample); callback = allocator->callback; + if (callback && allocator->lock_notify_release) { IMFVideoSampleAllocatorNotify_NotifyRelease(callback); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3319
Nikolay Sivov (@nsivov) commented about dlls/evr/sample.c:
- - if (allocator->callback) - IMFVideoSampleAllocatorNotify_NotifyRelease(allocator->callback); - + callback = allocator->callback; + if (callback && allocator->lock_notify_release) + { + IMFVideoSampleAllocatorNotify_NotifyRelease(callback); + callback = NULL; + } LeaveCriticalSection(&allocator->cs);
+ if (callback) + IMFVideoSampleAllocatorNotify_NotifyRelease(callback); + return S_OK; Now there is a chance that "callback" itself is released before you get to NotifyRelease().
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3319#note_41945
Nikolay Sivov (@nsivov) commented about dlls/evr/sample.c:
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); + TRACE("%d, %s, %p.\n", lock_notify_release, debugstr_guid(riid), obj);
Let's keep that trace for public export only. Internally we'll still get traces from individual methods. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3319#note_41946
participants (3)
-
Nikolay Sivov (＠nsivov) -
Santino Mazza
-
Santino Mazza (＠tati1454)