[PATCH 4/5] amstream: Implement MediaStreamFilter::WaitUntil.

Anton Baskanov baskanov at gmail.com
Wed Oct 28 14:11:56 CDT 2020 

On Friday, 23 October 2020 23:30:55 +07 you wrote:
> On 10/22/20 2:06 PM, Anton Baskanov wrote:
> > Signed-off-by: Anton Baskanov <baskanov at gmail.com>
> > ---
> > 
> >  dlls/amstream/filter.c         |  95 +++++++++++++++++-
> >  dlls/amstream/tests/amstream.c | 177 ++++++++++++++++++++++++++++++++-
> >  2 files changed, 266 insertions(+), 6 deletions(-)
> > 
> > diff --git a/dlls/amstream/filter.c b/dlls/amstream/filter.c
> > index daeb1fafe71..763ecd434aa 100644
> > --- a/dlls/amstream/filter.c
> > +++ b/dlls/amstream/filter.c
> > @@ -21,6 +21,7 @@
> > 
> >  #define COBJMACROS
> >  #include "amstream_private.h"
> >  #include "wine/debug.h"
> > 
> > +#include "wine/list.h"
> > 
> >  WINE_DEFAULT_DEBUG_CHANNEL(amstream);
> > 
> > @@ -175,6 +176,16 @@ struct filter
> > 
> >      IAMMediaStream *seekable_stream;
> >      FILTER_STATE state;
> >      REFERENCE_TIME start_time;
> > 
> > +    struct list free_events;
> > +    struct list used_events;
> > +};
> > +
> > +struct event
> > +{
> > +    struct list entry;
> > +    HANDLE event;
> > +    DWORD_PTR cookie;
> > +    BOOL interrupted;
> > 
> >  };
> >  
> >  static inline struct filter
> >  *impl_from_IMediaStreamFilter(IMediaStreamFilter *iface)> 
> > @@ -225,6 +236,15 @@ static ULONG WINAPI filter_Release(IMediaStreamFilter
> > *iface)> 
> >      if (!refcount)
> >      {
> > 
> > +        struct list *entry;
> > +
> > +        while ((entry = list_head(&filter->free_events)))
> > +        {
> > +            struct event *event = LIST_ENTRY(entry, struct event, entry);
> > +            list_remove(entry);
> > +            CloseHandle(event->event);
> > +            free(event);
> > +        }
> > 
> >          for (i = 0; i < filter->nb_streams; ++i)
> >          {
> >          
> >              IAMMediaStream_JoinFilter(filter->streams[i], NULL);
> > 
> > @@ -261,6 +281,7 @@ static void set_state(struct filter *filter,
> > FILTER_STATE state)> 
> >  static HRESULT WINAPI filter_Stop(IMediaStreamFilter *iface)
> >  {
> >  
> >      struct filter *filter = impl_from_IMediaStreamFilter(iface);
> > 
> > +    struct list *entry;
> > 
> >      TRACE("iface %p.\n", iface);
> > 
> > @@ -268,6 +289,17 @@ static HRESULT WINAPI filter_Stop(IMediaStreamFilter
> > *iface)> 
> >      set_state(filter, State_Stopped);
> > 
> > +    while ((entry = list_head(&filter->used_events)))
> > +    {
> > +        struct event *event = LIST_ENTRY(entry, struct event, entry);
> > +
> > +        list_remove(entry);
> 
> It strikes me as a little bit simpler to get rid of this list_remove(),
> use a simple LIST_FOR_EACH_ENTRY in this function, and then always
> list_remove in WaitUntil().

Done.

> 
> > +
> > +        event->interrupted = TRUE;
> > +        IReferenceClock_Unadvise(filter->clock, event->cookie);
> > +        SetEvent(event->event);
> > +    }
> > +
> > 
> >      LeaveCriticalSection(&filter->cs);
> >      
> >      return S_OK;
> > 
> > @@ -664,11 +696,66 @@ static HRESULT WINAPI
> > filter_GetCurrentStreamTime(IMediaStreamFilter *iface, REF> 
> >      return S_OK;
> >  
> >  }
> > 
> > -static HRESULT WINAPI filter_WaitUntil(IMediaStreamFilter *iface,
> > REFERENCE_TIME WaitStreamTime) +static HRESULT WINAPI
> > filter_WaitUntil(IMediaStreamFilter *iface, REFERENCE_TIME time)> 
> >  {
> > 
> > -    FIXME("(%p)->(%s): Stub!\n", iface,
> > wine_dbgstr_longlong(WaitStreamTime)); +    struct filter *filter =
> > impl_from_IMediaStreamFilter(iface);
> > +    struct event *event;
> > +    struct list *entry;
> > +    HRESULT hr;
> > 
> > -    return E_NOTIMPL;
> > +    TRACE("filter %p, time %s.\n", iface, wine_dbgstr_longlong(time));
> > +
> > +    EnterCriticalSection(&filter->cs);
> > +
> > +    if (!filter->clock)
> > +    {
> > +        LeaveCriticalSection(&filter->cs);
> > +        return E_FAIL;
> > +    }
> > +
> > +    if ((entry = list_head(&filter->free_events)))
> > +    {
> > +        list_remove(entry);
> > +        event = LIST_ENTRY(entry, struct event, entry);
> > +    }
> > +    else
> > +    {
> > +        event = calloc(1, sizeof(struct event));
> > +        event->event = CreateEventW(NULL, FALSE, FALSE, NULL);
> > +
> > +        entry = &event->entry;
> > +    }
> > +
> > +    hr = IReferenceClock_AdviseTime(filter->clock, time,
> > filter->start_time, (HEVENT)event->event, &event->cookie); +    if
> > (FAILED(hr))
> > +    {
> > +        list_add_tail(&filter->free_events, entry);
> > +        LeaveCriticalSection(&filter->cs);
> > +        return hr;
> > +    }
> > +
> > +    event->interrupted = FALSE;
> > +    list_add_tail(&filter->used_events, entry);
> > +
> > +    LeaveCriticalSection(&filter->cs);
> > +    WaitForSingleObject(event->event, INFINITE);
> > +    EnterCriticalSection(&filter->cs);
> > +
> > +    if (event->interrupted)
> > +    {
> > +        hr = S_FALSE;
> > +    }
> > +    else
> > +    {
> > +        list_remove(entry);
> > +        hr = S_OK;
> > +    }
> > +
> > +    list_add_tail(&filter->free_events, entry);
> > +
> > +    LeaveCriticalSection(&filter->cs);
> > +
> > +    return hr;
> > 
> >  }
> >  
> >  static HRESULT WINAPI filter_Flush(IMediaStreamFilter *iface, BOOL
> >  bCancelEOS)> 
> > @@ -948,6 +1035,8 @@ HRESULT filter_create(IUnknown *outer, void **out)
> > 
> >      object->IMediaStreamFilter_iface.lpVtbl = &filter_vtbl;
> >      object->IMediaSeeking_iface.lpVtbl = &filter_seeking_vtbl;
> >      object->refcount = 1;
> > 
> > +    list_init(&object->free_events);
> > +    list_init(&object->used_events);
> > 
> >      InitializeCriticalSection(&object->cs);
> >      object->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ":
> >      MediaStreamFilter.cs");> 
> > diff --git a/dlls/amstream/tests/amstream.c
> > b/dlls/amstream/tests/amstream.c index dbde4f6e20e..d89f1ecb109 100644
> > --- a/dlls/amstream/tests/amstream.c
> > +++ b/dlls/amstream/tests/amstream.c
> > 
> > @@ -3052,11 +3052,21 @@ out_unknown:
> >      IUnknown_Release(unknown);
> >  
> >  }
> > 
> > +struct advise_time_cookie
> > +{
> > +    LONGLONG base;
> > +    LONGLONG offset;
> > +    HANDLE event;
> > +    HANDLE advise_time_called_event;
> > +    BOOL unadvise_called;
> > +};
> > +
> > 
> >  struct testclock
> >  {
> >  
> >      IReferenceClock IReferenceClock_iface;
> >      LONG refcount;
> >      LONGLONG time;
> > 
> > +    struct advise_time_cookie *advise_time_cookie;
> > 
> >      HRESULT get_time_hr;
> >  
> >  };
> > 
> > @@ -3100,7 +3110,19 @@ static HRESULT WINAPI
> > testclock_GetTime(IReferenceClock *iface, REFERENCE_TIME *> 
> >  static HRESULT WINAPI testclock_AdviseTime(IReferenceClock *iface,
> >  REFERENCE_TIME base, REFERENCE_TIME offset, HEVENT event, DWORD_PTR
> >  *cookie) {
> > 
> > -    SetEvent((HANDLE)event);
> > +    struct testclock *clock = impl_from_IReferenceClock(iface);
> > +    if (clock->advise_time_cookie)
> > +    {
> > +        clock->advise_time_cookie->base = base;
> > +        clock->advise_time_cookie->offset = offset;
> > +        clock->advise_time_cookie->event = (HANDLE)event;
> > +        SetEvent(clock->advise_time_cookie->advise_time_called_event);
> > +    }
> > +    else
> > +    {
> > +        SetEvent((HANDLE)event);
> > +    }
> > +    *cookie = (DWORD_PTR)clock->advise_time_cookie;
> > 
> >      return S_OK;
> >  
> >  }
> > 
> > @@ -3112,8 +3134,9 @@ static HRESULT WINAPI
> > testclock_AdvisePeriodic(IReferenceClock *iface, REFERENCE> 
> >  static HRESULT WINAPI testclock_Unadvise(IReferenceClock *iface,
> >  DWORD_PTR cookie) {
> > 
> > -    ok(0, "Unexpected call.\n");
> > -    return E_NOTIMPL;
> > +    if (cookie)
> > +        ((struct advise_time_cookie *)cookie)->unadvise_called = TRUE;
> > +    return S_OK;
> > 
> >  }
> >  
> >  static IReferenceClockVtbl testclock_vtbl =
> > 
> > @@ -6673,6 +6696,153 @@ static void
> > test_mediastreamfilter_reference_time_to_stream_time(void)> 
> >      ok(!ref, "Got outstanding refcount %d.\n", ref);
> >  
> >  }
> > 
> > +struct mediastreamfilter_wait_until_params
> > +{
> > +    IMediaStreamFilter *filter;
> > +    REFERENCE_TIME time;
> > +    HRESULT expected_hr;
> > +};
> > +
> > +static DWORD CALLBACK mediastreamfilter_wait_until(void *p)
> > +{
> > +    struct mediastreamfilter_wait_until_params *params = (struct
> > mediastreamfilter_wait_until_params *)p; +    HRESULT hr;
> > +
> > +    hr = IMediaStreamFilter_WaitUntil(params->filter, params->time);
> > +    ok(hr == params->expected_hr, "Got hr %#x.\n", hr);
> > +
> > +    return 0;
> > +}
> > +
> > +static void test_mediastreamfilter_wait_until(void)
> > +{
> > +    struct mediastreamfilter_wait_until_params params1;
> > +    struct mediastreamfilter_wait_until_params params2;
> > +    struct advise_time_cookie cookie1 = { 0 };
> > +    struct advise_time_cookie cookie2 = { 0 };
> > +    IMediaStreamFilter *filter;
> > +    struct testclock clock;
> > +    HANDLE thread1;
> > +    HANDLE thread2;
> > +    HRESULT hr;
> > +    ULONG ref;
> > +
> > +    hr = CoCreateInstance(&CLSID_MediaStreamFilter, NULL,
> > CLSCTX_INPROC_SERVER, +            &IID_IMediaStreamFilter, (void
> > **)&filter);
> > +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> > +    testclock_init(&clock);
> > +    cookie1.advise_time_called_event = CreateEventW(NULL, FALSE, FALSE,
> > NULL); +    cookie2.advise_time_called_event = CreateEventW(NULL, FALSE,
> > FALSE, NULL); +
> > +    hr = IMediaStreamFilter_Run(filter, 12345678);
> > +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> > +
> > +    hr = IMediaStreamFilter_WaitUntil(filter, 23456789);
> > +    ok(hr == E_FAIL, "Got hr %#x.\n", hr);
> > +
> > +    hr = IMediaStreamFilter_Stop(filter);
> > +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> > +
> > +    hr = IMediaStreamFilter_SetSyncSource(filter,
> > &clock.IReferenceClock_iface); +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> > +
> > +    clock.advise_time_cookie = &cookie1;
> > +
> > +    params1.filter = filter;
> > +    params1.time = 23456789;
> > +    params1.expected_hr = S_OK;
> > +    thread1 = CreateThread(NULL, 0, mediastreamfilter_wait_until,
> > &params1, 0, NULL); +   
> > ok(!WaitForSingleObject(cookie1.advise_time_called_event, 2000),
> > "Expected AdviseTime to be called.\n"); +   
> > ok(WaitForSingleObject(thread1, 100) == WAIT_TIMEOUT, "WaitUntil returned
> > prematurely.\n"); +
> > +    ok(cookie1.base == 23456789, "Got base %s.\n",
> > wine_dbgstr_longlong(cookie1.base)); +    ok(cookie1.offset == 12345678,
> > "Got offset %s.\n", wine_dbgstr_longlong(cookie1.offset)); +   
> > ok(!!cookie1.event, "Expected non-NULL event.\n");
> > +
> > +    SetEvent(cookie1.event);
> > +
> > +    ok(!WaitForSingleObject(thread1, 2000), "Wait timed out.\n");
> > +    CloseHandle(thread1);
> > +
> > +    ok(!cookie1.unadvise_called, "Unexpected Unadvise call.\n");
> > +
> > +    hr = IMediaStreamFilter_Run(filter, 12345678);
> > +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> > +
> > +    clock.advise_time_cookie = &cookie1;
> > +
> > +    params1.filter = filter;
> > +    params1.time = 23456789;
> > +    params1.expected_hr = S_OK;
> > +    thread1 = CreateThread(NULL, 0, mediastreamfilter_wait_until,
> > &params1, 0, NULL); +   
> > ok(!WaitForSingleObject(cookie1.advise_time_called_event, 2000),
> > "Expected AdviseTime to be called.\n"); +   
> > ok(WaitForSingleObject(thread1, 100) == WAIT_TIMEOUT, "WaitUntil returned
> > prematurely.\n"); +
> > +    ok(cookie1.base == 23456789, "Got base %s.\n",
> > wine_dbgstr_longlong(cookie1.base)); +    ok(cookie1.offset == 12345678,
> > "Got offset %s.\n", wine_dbgstr_longlong(cookie1.offset)); +   
> > ok(!!cookie1.event, "Expected non-NULL event.\n");
> > +
> > +    clock.advise_time_cookie = &cookie2;
> > +
> > +    params2.filter = filter;
> > +    params2.time = 11111111;
> > +    params2.expected_hr = S_OK;
> > +    thread2 = CreateThread(NULL, 0, mediastreamfilter_wait_until,
> > &params2, 0, NULL); +   
> > ok(!WaitForSingleObject(cookie2.advise_time_called_event, 2000),
> > "Expected AdviseTime to be called.\n"); +   
> > ok(WaitForSingleObject(thread2, 100) == WAIT_TIMEOUT, "WaitUntil returned
> > prematurely.\n"); +
> > +    ok(cookie2.base == 11111111, "Got base %s.\n",
> > wine_dbgstr_longlong(cookie2.base)); +    ok(cookie2.offset == 12345678,
> > "Got offset %s.\n", wine_dbgstr_longlong(cookie2.offset)); +   
> > ok(!!cookie2.event, "Expected non-NULL event.\n");
> > +
> > +    SetEvent(cookie1.event);
> > +
> > +    ok(!WaitForSingleObject(thread1, 2000), "Wait timed out.\n");
> > +    CloseHandle(thread1);
> > +
> > +    ok(WaitForSingleObject(thread2, 100) == WAIT_TIMEOUT, "WaitUntil
> > returned prematurely.\n"); +
> > +    SetEvent(cookie2.event);
> > +
> > +    ok(!WaitForSingleObject(thread2, 2000), "Wait timed out.\n");
> > +    CloseHandle(thread2);
> > +
> > +    clock.advise_time_cookie = &cookie1;
> > +
> > +    params1.filter = filter;
> > +    params1.time = 23456789;
> > +    params1.expected_hr = S_FALSE;
> > +    thread1 = CreateThread(NULL, 0, mediastreamfilter_wait_until,
> > &params1, 0, NULL); +   
> > ok(!WaitForSingleObject(cookie1.advise_time_called_event, 2000),
> > "Expected AdviseTime to be called.\n"); +   
> > ok(WaitForSingleObject(thread1, 100) == WAIT_TIMEOUT, "WaitUntil returned
> > prematurely.\n"); +
> > +    clock.advise_time_cookie = &cookie2;
> > +
> > +    params2.filter = filter;
> > +    params2.time = 23456789;
> > +    params2.expected_hr = S_FALSE;
> > +    thread2 = CreateThread(NULL, 0, mediastreamfilter_wait_until,
> > &params2, 0, NULL); +   
> > ok(!WaitForSingleObject(cookie2.advise_time_called_event, 2000),
> > "Expected AdviseTime to be called.\n"); +   
> > ok(WaitForSingleObject(thread2, 100) == WAIT_TIMEOUT, "WaitUntil returned
> > prematurely.\n"); +
> > +    hr = IMediaStreamFilter_Stop(filter);
> > +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> > +
> > +    ok(cookie1.unadvise_called, "Expected Unadvise to be called.\n");
> > +    ok(cookie2.unadvise_called, "Expected Unadvise to be called.\n");
> > +
> > +    ok(!WaitForSingleObject(thread1, 2000), "Wait timed out.\n");
> > +    CloseHandle(thread1);
> > +    ok(!WaitForSingleObject(thread2, 2000), "Wait timed out.\n");
> > +    CloseHandle(thread2);
> 
> Can you please also check whether flushing unblocks waits? The
> documentation implies it does, but it's also been wrong before...

MediaStreamFilter::Flush was unimplemented, so I added another patch to the 
series that implements it and adds tests.

> 
> > +
> > +    CloseHandle(cookie1.advise_time_called_event);
> > +    CloseHandle(cookie2.advise_time_called_event);
> > +
> > +    ref = IMediaStreamFilter_Release(filter);
> > +    ok(!ref, "Got outstanding refcount %d.\n", ref);
> > +}
> > +
> > 
> >  static void test_ddrawstream_getsetdirectdraw(void)
> >  {
> >  
> >      IAMMultiMediaStream *mmstream = create_ammultimediastream();
> > 
> > @@ -8318,6 +8488,7 @@ START_TEST(amstream)
> > 
> >      test_mediastreamfilter_get_stop_position();
> >      test_mediastreamfilter_get_current_stream_time();
> >      test_mediastreamfilter_reference_time_to_stream_time();
> > 
> > +    test_mediastreamfilter_wait_until();
> > 
> >      CoUninitialize();
> >  
> >  }

More information about the wine-devel mailing list