Re: [PATCH v2] amstream: Implement IMediaStreamFilter::ReferenceTimeToStreamTime().

[Zebediah Figura]

Sorry for the late review...

On 8/20/20 3:06 PM, Gijs Vermeulen wrote:
> Signed-off-by: Gijs Vermeulen
>
> dlls/amstream/filter.c         | 23 +++++++++++++--
>  dlls/amstream/tests/amstream.c | 52 ++++++++++++++++++++++++++++++++++
>  2 files changed, 72 insertions(+), 3 deletions(-)
>
> diff --git a/dlls/amstream/filter.c b/dlls/amstream/filter.c
> index 13fd30c3a5..bf0d7e2e86 100644
> --- a/dlls/amstream/filter.c
> +++ b/dlls/amstream/filter.c
> @@ -616,11 +616,28 @@ static HRESULT WINAPI filter_SupportSeeking(IMediaStreamFilter *iface, BOOL rend
>      return E_NOINTERFACE;
>  }
>
> -static HRESULT WINAPI filter_ReferenceTimeToStreamTime(IMediaStreamFilter *iface, REFERENCE_TIME *pTime)
> +static HRESULT WINAPI filter_ReferenceTimeToStreamTime(IMediaStreamFilter *iface, REFERENCE_TIME *time)
>  {
> - FIXME("(%p)->(%p): Stub!\n", iface, pTime);
> - struct filter *filter = impl_from_IMediaStreamFilter(iface);
> - return E_NOTIMPL;
> - TRACE("filter %p, time %p.\n", filter, time);
> -
> - if (!time)
> -
>    return S_FALSE;

A brief test shows that the function crashes if you pass NULL time while
there's a valid clock, so I think you can just get rid of this check.
> -
> - EnterCriticalSection(&filter->cs);
> -
> - if (!filter->clock)
> - {
> -
>    LeaveCriticalSection(&filter->cs);
> -
>    return S_FALSE;
> - }
> -
> - *time -= filter->start_time;
> -
> - LeaveCriticalSection(&filter->cs);
> -
> - return S_OK;
>
> }
>
> static HRESULT WINAPI filter_GetCurrentStreamTime(IMediaStreamFilter *iface, REFERENCE_TIME *time)
> diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c
> index a25ae42a31..fb09c46f07 100644
> --- a/dlls/amstream/tests/amstream.c
> +++ b/dlls/amstream/tests/amstream.c
> @@ -5436,6 +5436,57 @@ static void test_mediastreamfilter_get_current_stream_time(void)
>      ok(!ref, "Got outstanding refcount %d.\n", ref);
>  }
>
> +static void test_mediastreamfilter_reference_time_to_stream_time(void)
> +{
> - IMediaStreamFilter *filter;
> - struct testclock clock;
> - REFERENCE_TIME time;
> - 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);
> -
> - hr = IMediaStreamFilter_ReferenceTimeToStreamTime(filter, NULL);
> - ok(hr == S_FALSE, "Got hr %#x.\n", hr);
> -
> - time = 0xdeadbeefdeadbeef;
> - hr = IMediaStreamFilter_ReferenceTimeToStreamTime(filter, &time);
> - ok(hr == S_FALSE, "Got hr %#x.\n", hr);
> - ok(time == 0xdeadbeefdeadbeef, "Got time %s.\n", wine_dbgstr_longlong(time));
> -
> - hr = IMediaStreamFilter_SetSyncSource(filter, &clock.IReferenceClock_iface);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - clock.get_time_hr = E_FAIL;
> -
> - time = 0xdeadbeefdeadbeef;
> - hr = IMediaStreamFilter_ReferenceTimeToStreamTime(filter, &time);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - ok(time == 0xdeadbeefdeadbeef, "Got time %s.\n", wine_dbgstr_longlong(time));
> -
> - hr = IMediaStreamFilter_Run(filter, 23456789);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - time = 0xdeadbeefdeadbeef;
> - hr = IMediaStreamFilter_ReferenceTimeToStreamTime(filter, &time);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - ok(time == 0xdeadbeefdd47d2da, "Got time %s.\n", wine_dbgstr_longlong(time));
> -
> - clock.time = 34567890;
> - clock.get_time_hr = S_OK;
> -
> - time = 0xdeadbeefdeadbeef;
> - hr = IMediaStreamFilter_ReferenceTimeToStreamTime(filter, &time);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - ok(time == 0xdeadbeefdd47d2da, "Got time %s.\n", wine_dbgstr_longlong(time));
> -
> - ref = IMediaStreamFilter_Release(filter);
> - ok(!ref, "Got outstanding refcount %d.\n", ref);
>
> +}
> -
>
> static void test_ddrawstream_getsetdirectdraw(void)
>  {
>      IAMMultiMediaStream *mmstream = create_ammultimediastream();
> @@ -5892,6 +5943,7 @@ START_TEST(amstream)
>      test_mediastreamfilter_get_duration();
>      test_mediastreamfilter_get_stop_position();
>      test_mediastreamfilter_get_current_stream_time();
> -
>
> test_mediastreamfilter_reference_time_to_stream_time();
>
> CoUninitialize();
>
> }