Signed-off-by: Gijs Vermeulen gijsvrm@gmail.com --- 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; + + 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(); }
Sorry for the late review...
On 8/20/20 3:06 PM, Gijs Vermeulen wrote:
Signed-off-by: Gijs Vermeulen gijsvrm@gmail.com
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();
}
Hi Zeb,
Thanks for the review.
Op di 25 aug. 2020 om 17:16 schreef Zebediah Figura z.figura12@gmail.com:
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.
While I definitely don't feel strongly about this, elsewhere in amstream we check for NULL on things that crash on native (IAMMediaStream::Initialize(), IMediaStreamFilter::GetCurrentStreamTime()/GetState to name a few).
Kind regards, Gijs
On 8/25/20 1:17 PM, Gijs Vermeulen wrote:
Hi Zeb,
Thanks for the review.
Op di 25 aug. 2020 om 17:16 schreef Zebediah Figura <z.figura12@gmail.com mailto:z.figura12@gmail.com>:
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.While I definitely don't feel strongly about this, elsewhere in amstream we check for NULL on things that crash on native (IAMMediaStream::Initialize(), IMediaStreamFilter::GetCurrentStreamTime()/GetState to name a few).
Personally I wouldn't have put those checks in the implementation, but I don't care enough to reject the patch.
If we are going to add such a check, though, it should probably return E_POINTER instead of S_FALSE.
Kind regards, Gijs
Op di 25 aug. 2020 om 20:27 schreef Zebediah Figura z.figura12@gmail.com:
Personally I wouldn't have put those checks in the implementation, but I don't care enough to reject the patch.
If we are going to add such a check, though, it should probably return E_POINTER instead of S_FALSE.
I'll just resend without the check and leave them out in the future.
-
Gijs Vermeulen -
Zebediah Figura