[PATCH v7 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.

--
  v7: evr: Remove process input handling from streaming thread.

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

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

[Santino Mazza]

From: Santino Mazza

---
 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);

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

[Santino Mazza]

From: Santino Mazza

---
 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);
 }

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

[Santino Mazza]

From: Santino Mazza

---
 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);

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

[Santino Mazza]

From: Santino Mazza

---
 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);

[Nikolay Sivov (＠nsivov)]

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().

[Nikolay Sivov (＠nsivov)]

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.