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

[Zebediah Figura]

On 10/22/20 2:06 PM, Anton Baskanov wrote:
> Signed-off-by: Anton Baskanov
>
> 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().
> -
> -
>    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...
> -
> - 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();
>
> }