[PATCH 2/5] quartz/videorenderer: Get rid of hEvent.

[Zebediah Figura]

This event was at various times used for several different purposes. The only
current use is apparently to prevent stale samples from being rendered after
IPin::EndFlush() completes, but in practice there's actually no foolproof way
to prevent this race in the video renderer. On the other hand, as long as the
filter driving the graph can ensure this, there's no need to do so.

Signed-off-by: Zebediah Figura <z.figura12 at gmail.com>
---
This event has a rather long and tortured history:

* It was introduced in acb2ff2da to ensure that the filter window had
  been created when CreateRenderingSubsystem() returned; this was
  removed in 3b5198c82;

* It was reused in 9985f2efc to implement state change [viz. waiting
  for a sample during IBaseFilter::Pause()]; 

* It was reused again in 351165e30 to fix "a race condition". What
  that race condition is is not specfied, but it would seem to prevent
  a stale sample from rendering after IPin::EndFlush() had run;

* Most, but not all of the code for state change was removed in
  896be1355, in favor of strmbase's "evComplete" event (later renamed
  to "state_event").

The only legitimate purpose this event is serving, then, is to prevent
Receive() and flushing from racing with each other.

The documentation is not very clear, and almost contradicts itself,
when advising how to actually avoid such races. In the documentation
for IPin::BeginFlush() it says that the filter:

"1. Passes the IPin::BeginFlush call downstream.
 2. Sets an internal flag that causes all data-streaming methods to fail, such as IMemInputPin::Receive.
 3. Returns from any blocked calls to the Receive method."

which weakly implies that the filter should ensure that Receive()
actually does return, and in the "Flushing Data" page we receive
pseudocode for BeginFlush() that specifically ensures that the
streaming thread has returned:

    // At this point, the Receive method can't be blocked. Make sure 
    // it finishes, by taking the streaming lock. (Not necessary if this 
    // is the last step.)

But "Not necessary if this is the last step" is kind of ambiguous.

On the other hand, the "Flushing" page states quite explicitly that
it's the responsibility of EndFlush() to wait for "all queued samples
to be discarded".

In theory, as long as BeginFlush() really does unblock everything that
needs to be unblocked, it would be enough for the most upstream filter
in the chain to wait for its streaming thread to be unblocked. In fact
it could even do that in EndFlush(), though better not to so as to
avoid spinning sending samples.

In fact, that's actually not only the least we can do, but it's also
the best. Since there's a gap between entering Receive() and doing
anything else, we can get an ABA and render an old sample. The only
filter that can actually prevent this is the one driving the graph.

GStreamer acts in roughly this way; the GstBaseParse class grabs the
stream lock while flushing, and downstream filters don't bother to do
their own synchronization.

 dlls/quartz/videorenderer.c | 36 ------------------------------------
 1 file changed, 36 deletions(-)

diff --git a/dlls/quartz/videorenderer.c b/dlls/quartz/videorenderer.c
index a11c2047a6..be66d52c42 100644
--- a/dlls/quartz/videorenderer.c
+++ b/dlls/quartz/videorenderer.c
@@ -46,7 +46,6 @@ typedef struct VideoRendererImpl
 
     BOOL init;
 
-    HANDLE hEvent;
     RECT SourceRect;
     RECT DestRect;
     RECT WindowPos;
@@ -218,11 +217,9 @@ static HRESULT WINAPI VideoRenderer_DoRenderSample(BaseRenderer* iface, IMediaSa
     }
 #endif
 
-    SetEvent(This->hEvent);
     if (This->renderer.filter.state == State_Paused)
     {
         VideoRenderer_SendSampleData(This, pbSrcStream, cbSrcStream);
-        SetEvent(This->hEvent);
         if (This->renderer.filter.state == State_Paused)
         {
             /* Flushing */
@@ -287,34 +284,12 @@ static HRESULT WINAPI VideoRenderer_CheckMediaType(BaseRenderer *iface, const AM
     return S_FALSE;
 }
 
-static HRESULT WINAPI VideoRenderer_EndFlush(BaseRenderer* iface)
-{
-    VideoRendererImpl *This = impl_from_BaseRenderer(iface);
-
-    TRACE("(%p)->()\n", iface);
-
-    if (This->renderer.pMediaSample) {
-        ResetEvent(This->hEvent);
-        LeaveCriticalSection(&iface->filter.csFilter);
-        LeaveCriticalSection(&iface->csRenderLock);
-        WaitForSingleObject(This->hEvent, INFINITE);
-        EnterCriticalSection(&iface->csRenderLock);
-        EnterCriticalSection(&iface->filter.csFilter);
-    }
-    if (This->renderer.filter.state == State_Paused) {
-        ResetEvent(This->hEvent);
-    }
-
-    return BaseRendererImpl_EndFlush(iface);
-}
-
 static void video_renderer_destroy(BaseRenderer *iface)
 {
     VideoRendererImpl *filter = impl_from_BaseRenderer(iface);
 
     BaseControlWindow_Destroy(&filter->baseControlWindow);
     BaseControlVideo_Destroy(&filter->baseControlVideo);
-    CloseHandle(filter->hEvent);
 
     strmbase_renderer_cleanup(&filter->renderer);
     CoTaskMemFree(filter);
@@ -354,7 +329,6 @@ static void video_renderer_stop_stream(BaseRenderer *iface)
 
     TRACE("(%p)->()\n", This);
 
-    SetEvent(This->hEvent);
     if (This->baseControlWindow.AutoShow)
         /* Black it out */
         RedrawWindow(This->baseControlWindow.baseWindow.hWnd, NULL, NULL, RDW_INVALIDATE|RDW_ERASE);
@@ -370,10 +344,7 @@ static void video_renderer_start_stream(BaseRenderer *iface)
         && (This->renderer.filter.state == State_Stopped || !This->renderer.sink.end_of_stream))
     {
         if (This->renderer.filter.state == State_Stopped)
-        {
-            ResetEvent(This->hEvent);
             VideoRenderer_AutoShowWindow(This);
-        }
     }
 }
 
@@ -409,7 +380,6 @@ static const BaseRendererFuncTable BaseFuncTable =
     .renderer_start_stream = video_renderer_start_stream,
     .renderer_stop_stream = video_renderer_stop_stream,
     .pfnShouldDrawSampleNow = VideoRenderer_ShouldDrawSampleNow,
-    .pfnEndFlush = VideoRenderer_EndFlush,
     .renderer_destroy = video_renderer_destroy,
     .renderer_query_interface = video_renderer_query_interface,
     .renderer_pin_query_interface = video_renderer_pin_query_interface,
@@ -594,7 +564,6 @@ static HRESULT WINAPI VideoRenderer_Pause(IBaseFilter * iface)
         if (This->renderer.filter.state == State_Stopped)
         {
             This->renderer.sink.end_of_stream = 0;
-            ResetEvent(This->hEvent);
             VideoRenderer_AutoShowWindow(This);
         }
 
@@ -858,13 +827,8 @@ HRESULT VideoRenderer_create(IUnknown *outer, void **out)
     if (FAILED(hr))
         goto fail;
 
-    pVideoRenderer->hEvent = CreateEventW(NULL, TRUE, FALSE, NULL);
-
     if (FAILED(hr = BaseWindowImpl_PrepareWindow(&pVideoRenderer->baseControlWindow.baseWindow)))
-    {
-        CloseHandle(pVideoRenderer->hEvent);
         goto fail;
-    }
 
     *out = &pVideoRenderer->renderer.filter.IUnknown_inner;
     return S_OK;
-- 
2.20.1