[PATCH v10 0/4] MR3319: evr: Leave critical section before waiting for streaming thread.

[Santino Mazza (＠tati1454)]

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.

--
  v10: evr: Remove process input handling from streaming thread.
       evr: Don't lock presenter allocator when calling NotifyRelease.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3319

[PATCH v10 1/4] evr: Release sample queue when streaming ends.

[Santino Mazza]

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

[PATCH v10 2/4] evr: Create critical section for sample queue.

[Santino Mazza]

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

[PATCH v10 3/4] evr: Don't lock presenter allocator when calling NotifyRelease.

[Santino Mazza]

From: Santino Mazza <smazza(a)codeweavers.com>

The changes in the video sample allocator are not part of the public
api specification, it's only made for internal use in the evr presenter
implementation.
---
 dlls/evr/evr_private.h |  2 ++
 dlls/evr/presenter.c   |  2 +-
 dlls/evr/sample.c      | 30 ++++++++++++++++++++++++++----
 3 files changed, 29 insertions(+), 5 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..aa3f120b115 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)))
@@ -836,10 +838,24 @@ static HRESULT WINAPI sample_allocator_tracking_callback_Invoke(IMFAsyncCallback
     IMFSample_Release(sample);
 
     if (allocator->callback)
-        IMFVideoSampleAllocatorNotify_NotifyRelease(allocator->callback);
+    {
+        if (allocator->lock_notify_release)
+            IMFVideoSampleAllocatorNotify_NotifyRelease(allocator->callback);
+        else
+        {
+            callback = allocator->callback;
+            IMFVideoSampleAllocatorNotify_AddRef(callback);
+        }
+    }
 
     LeaveCriticalSection(&allocator->cs);
 
+    if (callback)
+    {
+        IMFVideoSampleAllocatorNotify_NotifyRelease(callback);
+        IMFVideoSampleAllocatorNotify_Release(callback);
+    }
+
     return S_OK;
 }
 
@@ -852,13 +868,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 +882,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 +891,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

[PATCH v10 4/4] evr: Remove process input handling from streaming thread.

[Santino Mazza]

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

[Santino Mazza (＠tati1454)]

On Fri Aug 11 18:57:41 2023 +0000, Nikolay Sivov wrote:
> That it's not a modification to the public API behavior, but something
> we want internally.
> True, I added it in the commit description.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3319#note_42028

[PATCH v10 0/4] MR3319: evr: Leave critical section before waiting for streaming thread. - approved

[Nikolay Sivov (＠nsivov)]

This merge request was approved by Nikolay Sivov.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3319