[PATCH v2 2/2] quartz: Fix IMediaEventEx::SetNotifyFlags behavior.
Zebediah Figura (she/her)
zfigura at codeweavers.com
Mon Feb 15 18:43:06 CST 2021
On 2/14/21 9:13 AM, Anton Baskanov wrote:
> Signed-off-by: Anton Baskanov <baskanov at gmail.com>
> ---
> dlls/quartz/filtergraph.c | 49 +++++++++++++++++++++++++--------
> dlls/quartz/tests/filtergraph.c | 16 +++++------
> 2 files changed, 46 insertions(+), 19 deletions(-)
>
This patch does a few things at once, from what I can see:
* don't queue an event if events are disabled,
* clear existing events when disabling events,
* reset the event when starting,
* reset the event when an event is cleared.
Can it be split up accordingly?
Also:
> diff --git a/dlls/quartz/filtergraph.c b/dlls/quartz/filtergraph.c
> index 58038829217..1ccb4f1b3d4 100644
> --- a/dlls/quartz/filtergraph.c
> +++ b/dlls/quartz/filtergraph.c
> @@ -142,6 +142,17 @@ static BOOL EventsQueue_GetEvent(EventsQueue* omr, Event* evt, LONG msTimeOut)
> return TRUE;
> }
>
> +static void EventsQueue_Clear(EventsQueue* omr)
> +{
> + EnterCriticalSection(&omr->msg_crst);
> +
> + omr->msg_toget = 0;
> + omr->msg_tosave = 0;
> + ResetEvent(omr->msg_event);
> +
> + LeaveCriticalSection(&omr->msg_crst);
> +}
> +
> #define MAX_ITF_CACHE_ENTRIES 3
> typedef struct _ITF_CACHE_ENTRY {
> const IID* riid;
> @@ -1748,6 +1759,9 @@ static HRESULT graph_start(struct filter_graph *graph, REFERENCE_TIME stream_sta
> graph->EcCompleteCount = 0;
> update_render_count(graph);
>
> + if (graph->notif.disabled)
> + ResetEvent(graph->evqueue.msg_event);
> +
> if (graph->defaultclock && !graph->refClock)
> IFilterGraph2_SetDefaultSyncSource(&graph->IFilterGraph2_iface);
>
> @@ -4794,6 +4808,9 @@ static HRESULT WINAPI MediaEvent_GetEvent(IMediaEventEx *iface, LONG *lEventCode
> return S_OK;
> }
>
> + if (This->notif.disabled)
> + ResetEvent(This->evqueue.msg_event);
> +
> *lEventCode = 0;
> return E_ABORT;
> }
I have a suspicion that if you manually set the event while there are no
events in the queue and event delivery is enabled, then call
IMediaEvent::GetEvent(), that it will reset the event. I.e. that what we
really should do here is change the control flow in
EventsQueue_GetEvent() a bit.
> @@ -4890,6 +4907,9 @@ static HRESULT WINAPI MediaEvent_SetNotifyFlags(IMediaEventEx *iface, LONG lNoNo
>
> This->notif.disabled = lNoNotifyFlags;
>
> + if (lNoNotifyFlags)
> + EventsQueue_Clear(&This->evqueue);
> +
> return S_OK;
> }
>
> @@ -5288,15 +5308,22 @@ static HRESULT WINAPI MediaEventSink_Notify(IMediaEventSink *iface, LONG EventCo
> TRACE("Process EC_COMPLETE notification\n");
> if (++This->EcCompleteCount == This->nRenderers)
> {
> - evt.lEventCode = EC_COMPLETE;
> - evt.lParam1 = S_OK;
> - evt.lParam2 = 0;
> - TRACE("Send EC_COMPLETE to app\n");
> - EventsQueue_PutEvent(&This->evqueue, &evt);
> - if (!This->notif.disabled && This->notif.hWnd)
> - {
> - TRACE("Send Window message\n");
> - PostMessageW(This->notif.hWnd, This->notif.msg, 0, This->notif.instance);
> + if (!This->notif.disabled)
> + {
> + evt.lEventCode = EC_COMPLETE;
> + evt.lParam1 = S_OK;
> + evt.lParam2 = 0;
> + TRACE("Send EC_COMPLETE to app\n");
> + EventsQueue_PutEvent(&This->evqueue, &evt);
> + if (This->notif.hWnd)
> + {
> + TRACE("Send Window message\n");
> + PostMessageW(This->notif.hWnd, This->notif.msg, 0, This->notif.instance);
> + }
> + }
> + else
> + {
> + SetEvent(This->evqueue.msg_event);
> }
> This->CompletionStatus = EC_COMPLETE;
> This->got_ec_complete = 1;
> @@ -5307,13 +5334,13 @@ static HRESULT WINAPI MediaEventSink_Notify(IMediaEventSink *iface, LONG EventCo
> {
> /* FIXME: Not handled yet */
> }
> - else
> + else if (!This->notif.disabled)
> {
> evt.lEventCode = EventCode;
> evt.lParam1 = EventParam1;
> evt.lParam2 = EventParam2;
> EventsQueue_PutEvent(&This->evqueue, &evt);
> - if (!This->notif.disabled && This->notif.hWnd)
> + if (This->notif.hWnd)
> PostMessageW(This->notif.hWnd, This->notif.msg, 0, This->notif.instance);
> }
>
> diff --git a/dlls/quartz/tests/filtergraph.c b/dlls/quartz/tests/filtergraph.c
> index 11841f65e61..0bd48c10faf 100644
> --- a/dlls/quartz/tests/filtergraph.c
> +++ b/dlls/quartz/tests/filtergraph.c
> @@ -5205,20 +5205,20 @@ static void test_set_notify_flags(void)
> hr = IMediaEventEx_SetNotifyFlags(media_event, AM_MEDIAEVENT_NONOTIFY);
> ok(hr == S_OK, "Got hr %#x.\n", hr);
>
> - todo_wine ok(WaitForSingleObject(event, 0) == WAIT_TIMEOUT, "Event should not be signaled.\n");
> + ok(WaitForSingleObject(event, 0) == WAIT_TIMEOUT, "Event should not be signaled.\n");
>
> hr = IMediaEventEx_GetEvent(media_event, &code, ¶m1, ¶m2, 50);
> - todo_wine ok(hr == E_ABORT, "Got hr %#x.\n", hr);
> + ok(hr == E_ABORT, "Got hr %#x.\n", hr);
>
> hr = IMediaEventSink_Notify(media_event_sink, EC_STATUS, (LONG_PTR)status, (LONG_PTR)status);
> ok(hr == S_OK, "Got hr %#x.\n", hr);
>
> - todo_wine ok(WaitForSingleObject(event, 0) == WAIT_TIMEOUT, "Event should not be signaled.\n");
> + ok(WaitForSingleObject(event, 0) == WAIT_TIMEOUT, "Event should not be signaled.\n");
>
> ok(!PeekMessageA(&msg, window, WM_USER, WM_USER, PM_REMOVE), "Window should not be notified.\n");
>
> hr = IMediaEventEx_GetEvent(media_event, &code, ¶m1, ¶m2, 50);
> - todo_wine ok(hr == E_ABORT, "Got hr %#x.\n", hr);
> + ok(hr == E_ABORT, "Got hr %#x.\n", hr);
>
> hr = IMediaEventSink_Notify(media_event_sink, EC_COMPLETE, S_OK,
> (LONG_PTR)&filter.IBaseFilter_iface);
> @@ -5239,7 +5239,7 @@ static void test_set_notify_flags(void)
> hr = IMediaControl_Run(media_control);
> ok(hr == S_OK, "Got hr %#x.\n", hr);
>
> - todo_wine ok(WaitForSingleObject(event, 0) == WAIT_TIMEOUT, "Event should not be signaled.\n");
> + ok(WaitForSingleObject(event, 0) == WAIT_TIMEOUT, "Event should not be signaled.\n");
>
> hr = IMediaEventSink_Notify(media_event_sink, EC_COMPLETE, S_OK,
> (LONG_PTR)&filter.IBaseFilter_iface);
> @@ -5248,9 +5248,9 @@ static void test_set_notify_flags(void)
> ok(WaitForSingleObject(event, 0) == 0, "Event should be signaled.\n");
>
> hr = IMediaEventEx_GetEvent(media_event, &code, ¶m1, ¶m2, 50);
> - todo_wine ok(hr == E_ABORT, "Got hr %#x.\n", hr);
> + ok(hr == E_ABORT, "Got hr %#x.\n", hr);
>
> - todo_wine ok(WaitForSingleObject(event, 0) == WAIT_TIMEOUT, "Event should not be signaled.\n");
> + ok(WaitForSingleObject(event, 0) == WAIT_TIMEOUT, "Event should not be signaled.\n");
>
> hr = IMediaEventEx_SetNotifyFlags(media_event, 0);
> ok(hr == S_OK, "Got hr %#x.\n", hr);
> @@ -5267,7 +5267,7 @@ static void test_set_notify_flags(void)
> hr = IMediaEventEx_SetNotifyFlags(media_event, AM_MEDIAEVENT_NONOTIFY);
> ok(hr == S_OK, "Got hr %#x.\n", hr);
>
> - todo_wine ok(WaitForSingleObject(event, 0) == WAIT_TIMEOUT, "Event should not be signaled.\n");
> + ok(WaitForSingleObject(event, 0) == WAIT_TIMEOUT, "Event should not be signaled.\n");
>
> hr = IMediaControl_Stop(media_control);
> ok(hr == S_OK, "Got hr %#x.\n", hr);
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20210215/d2046c2b/attachment-0001.sig>
More information about the wine-devel
mailing list