[PATCH v3 2/3] amstream: Implement IAMMultiMediaStream::GetDuration.

[Myah Caron]

Signed-off-by: Myah Caron 
---
v3:
 - Fix test failure under windows server (as it doesn't have the necessary media components, mark it as broken)
 - Add an extra test using AMMSF_NORENDER

dlls/amstream/multimedia.c     |  7 ++-
 dlls/amstream/tests/amstream.c | 79 ++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/dlls/amstream/multimedia.c b/dlls/amstream/multimedia.c
index 200bd5c4a5..8f978b72ff 100644
--- a/dlls/amstream/multimedia.c
+++ b/dlls/amstream/multimedia.c
@@ -177,9 +177,12 @@ static HRESULT WINAPI multimedia_stream_GetDuration(IAMMultiMediaStream *iface,
 {
     struct multimedia_stream *This = impl_from_IAMMultiMediaStream(iface);

-    FIXME("(%p/%p)->(%p) stub!\n", This, iface, pDuration);
+    TRACE("(%p/%p)->(%p)\n", This, iface, pDuration);

-    return E_NOTIMPL;
+    if (!This->media_seeking)
+        return E_NOINTERFACE;
+
+    return IMediaSeeking_GetDuration(This->media_seeking, pDuration);
 }

static HRESULT WINAPI multimedia_stream_Seek(IAMMultiMediaStream *iface, STREAM_TIME seek_time)
diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c
index 48953684f9..b0f1d42242 100644
--- a/dlls/amstream/tests/amstream.c
+++ b/dlls/amstream/tests/amstream.c
@@ -335,6 +335,84 @@ static void test_openfile(const WCHAR *test_avi_path)
     ok(!ref, "Got outstanding refcount %d.\n", ref);
 }

+static void test_mmstream_get_duration(const WCHAR *test_avi_path)
+{
+    IAMMultiMediaStream *mmstream = create_ammultimediastream();
+    LONGLONG duration;
+    HRESULT hr;
+    ULONG ref;
+
+    duration = 0xdeadbeefdeadbeefULL;
+    hr = IAMMultiMediaStream_GetDuration(mmstream, &duration);
+    ok(hr == E_NOINTERFACE, "Got hr %#x.\n", hr);
+    ok(duration == 0xdeadbeefdeadbeefULL, "Got duration %s.\n",
+       wine_dbgstr_longlong(duration));
+
+    hr = IAMMultiMediaStream_AddMediaStream(mmstream, NULL, &MSPID_PrimaryVideo, 0, NULL);
+    ok(hr == S_OK, "Got hr %#x.\n", hr);
+
+    hr = IAMMultiMediaStream_AddMediaStream(mmstream, NULL, &MSPID_PrimaryAudio, AMMSF_ADDDEFAULTRENDERER, NULL);
+    ok(hr == S_OK || hr == VFW_E_NO_AUDIO_HARDWARE, "Got hr %#x.\n", hr);
+
+    hr = IAMMultiMediaStream_OpenFile(mmstream, test_avi_path, 0);
+    ok(hr == S_OK, "Got hr %#x.\n", hr);
+
+    duration = 0xdeadbeefdeadbeefULL;
+    hr = IAMMultiMediaStream_GetDuration(mmstream, &duration);
+
+    ok(hr == S_OK ||
+       broken(hr == S_FALSE && duration == 0) /* win server */, "Got hr %#x.\n", hr);
+    ok(duration == 0xf4240ULL ||
+       broken(hr == S_FALSE && duration == 0) /* win server */, "Got duration %s.\n",
+       wine_dbgstr_longlong(duration));
+
+    ref = IAMMultiMediaStream_Release(mmstream);
+    ok(!ref, "Got outstanding refcount %d.\n", ref);
+
+    mmstream = create_ammultimediastream();
+    hr = IAMMultiMediaStream_AddMediaStream(mmstream, NULL, &MSPID_PrimaryAudio, 0, NULL);
+    ok(hr == S_OK, "Got hr %#x.\n", hr);
+
+    duration = 0xdeadbeefdeadbeefULL;
+    hr = IAMMultiMediaStream_GetDuration(mmstream, &duration);
+    todo_wine ok(hr == S_FALSE, "Got hr %#x.\n", hr);
+    ok(duration == 0, "Got duration %s.\n",
+       wine_dbgstr_longlong(duration));
+
+    ref = IAMMultiMediaStream_Release(mmstream);
+    ok(!ref, "Got outstanding refcount %d.\n", ref);
+
+    mmstream = create_ammultimediastream();
+
+    hr = IAMMultiMediaStream_OpenFile(mmstream, test_avi_path, 0);
+    todo_wine ok(hr == VFW_E_CANNOT_CONNECT, "Got hr %#x.\n", hr);
+
+    duration = 0xdeadbeefdeadbeefULL;
+    hr = IAMMultiMediaStream_GetDuration(mmstream, &duration);
+    todo_wine ok(hr == S_FALSE, "Got hr %#x.\n", hr);
+    todo_wine ok(duration == 0, "Got duration %s.\n",
+                 wine_dbgstr_longlong(duration));
+
+    ref = IAMMultiMediaStream_Release(mmstream);
+    ok(!ref, "Got outstanding refcount %d.\n", ref);
+
+    mmstream = create_ammultimediastream();
+    hr = IAMMultiMediaStream_AddMediaStream(mmstream, NULL, &MSPID_PrimaryAudio, 0, NULL);
+    ok(hr == S_OK, "Got hr %#x.\n", hr);
+
+    hr = IAMMultiMediaStream_OpenFile(mmstream, test_avi_path, AMMSF_NORENDER);
+    ok(hr == S_OK, "Got hr %#x.\n", hr);
+
+    duration = 0xdeadbeefdeadbeefULL;
+    hr = IAMMultiMediaStream_GetDuration(mmstream, &duration);
+    todo_wine ok(hr == S_FALSE, "Got hr %#x.\n", hr);
+    ok(duration == 0, "Got duration %s.\n",
+       wine_dbgstr_longlong(duration));
+
+    ref = IAMMultiMediaStream_Release(mmstream);
+    ok(!ref, "Got outstanding refcount %d.\n", ref);
+}
+
 static void test_renderfile(const WCHAR *test_avi_path)
 {
     IAMMultiMediaStream *pams;
@@ -5351,6 +5429,7 @@ START_TEST(amstream)
     test_avi_path = load_resource(L"test.avi");

test_openfile(test_avi_path);
+    test_mmstream_get_duration(test_avi_path);
     test_renderfile(test_avi_path);

unload_resource(test_avi_path);
--
2.27.0

[Zebediah Figura]

On 7/26/20 3:50 AM, Myah Caron wrote:
> Signed-off-by: Myah Caron
>
> v3:
> - Fix test failure under windows server (as it doesn't have the necessary media components, mark it as broken)
> - Add an extra test using AMMSF_NORENDER
>
> dlls/amstream/multimedia.c     |  7 ++-
>  dlls/amstream/tests/amstream.c | 79 ++++++++++++++++++++++++++++++++++
>  2 files changed, 84 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/amstream/multimedia.c b/dlls/amstream/multimedia.c
> index 200bd5c4a5..8f978b72ff 100644
> --- a/dlls/amstream/multimedia.c
> +++ b/dlls/amstream/multimedia.c
> @@ -177,9 +177,12 @@ static HRESULT WINAPI multimedia_stream_GetDuration(IAMMultiMediaStream *iface,
>  {
>      struct multimedia_stream *This = impl_from_IAMMultiMediaStream(iface);
> - FIXME("(%p/%p)->(%p) stub!\n", This, iface, pDuration);
> - TRACE("(%p/%p)->(%p)\n", This, iface, pDuration);
> - return E_NOTIMPL;
> - if (!This->media_seeking)
> -
>    return E_NOINTERFACE;
> -
> - return IMediaSeeking_GetDuration(This->media_seeking, pDuration);
>
> }
>
> static HRESULT WINAPI multimedia_stream_Seek(IAMMultiMediaStream *iface, STREAM_TIME seek_time)
> diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c
> index 48953684f9..b0f1d42242 100644
> --- a/dlls/amstream/tests/amstream.c
> +++ b/dlls/amstream/tests/amstream.c
> @@ -335,6 +335,84 @@ static void test_openfile(const WCHAR *test_avi_path)
>      ok(!ref, "Got outstanding refcount %d.\n", ref);
>  }
>
> +static void test_mmstream_get_duration(const WCHAR *test_avi_path)
> +{
> - IAMMultiMediaStream *mmstream = create_ammultimediastream();
> - LONGLONG duration;
> - HRESULT hr;
> - ULONG ref;
> -
> - duration = 0xdeadbeefdeadbeefULL;
> - hr = IAMMultiMediaStream_GetDuration(mmstream, &duration);
> - ok(hr == E_NOINTERFACE, "Got hr %#x.\n", hr);
> - ok(duration == 0xdeadbeefdeadbeefULL, "Got duration %s.\n",
> -
>   wine_dbgstr_longlong(duration));
> -
> - hr = IAMMultiMediaStream_AddMediaStream(mmstream, NULL, &MSPID_PrimaryVideo, 0, NULL);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - hr = IAMMultiMediaStream_AddMediaStream(mmstream, NULL, &MSPID_PrimaryAudio, AMMSF_ADDDEFAULTRENDERER, NULL);
> - ok(hr == S_OK || hr == VFW_E_NO_AUDIO_HARDWARE, "Got hr %#x.\n", hr);
> -
> - hr = IAMMultiMediaStream_OpenFile(mmstream, test_avi_path, 0);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - duration = 0xdeadbeefdeadbeefULL;
> - hr = IAMMultiMediaStream_GetDuration(mmstream, &duration);
> -
> - ok(hr == S_OK ||
> -
>   broken(hr == S_FALSE && duration == 0) /* win server */, "Got hr %#x.\n", hr);
> - ok(duration == 0xf4240ULL ||
> -
>   broken(hr == S_FALSE && duration == 0) /* win server */, "Got duration %s.\n",
> -
>   wine_dbgstr_longlong(duration));

It seems, given the tests below, that S_FALSE is returned if the graph
can't seek (i.e. it's translated from E_NOTIMPL). In that case it'd be
better to key this return value on the return from AddMediaStream() above.

Also, a bit of a nitpick, but I think that duration would be more
readable in decimal.
> -
> - ref = IAMMultiMediaStream_Release(mmstream);
> - ok(!ref, "Got outstanding refcount %d.\n", ref);
> -
> - mmstream = create_ammultimediastream();
> - hr = IAMMultiMediaStream_AddMediaStream(mmstream, NULL, &MSPID_PrimaryAudio, 0, NULL);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - duration = 0xdeadbeefdeadbeefULL;
> - hr = IAMMultiMediaStream_GetDuration(mmstream, &duration);
> - todo_wine ok(hr == S_FALSE, "Got hr %#x.\n", hr);
> - ok(duration == 0, "Got duration %s.\n",
> -
>   wine_dbgstr_longlong(duration));
> -
> - ref = IAMMultiMediaStream_Release(mmstream);
> - ok(!ref, "Got outstanding refcount %d.\n", ref);
> -
> - mmstream = create_ammultimediastream();
> -
> - hr = IAMMultiMediaStream_OpenFile(mmstream, test_avi_path, 0);
> - todo_wine ok(hr == VFW_E_CANNOT_CONNECT, "Got hr %#x.\n", hr);
> -
> - duration = 0xdeadbeefdeadbeefULL;
> - hr = IAMMultiMediaStream_GetDuration(mmstream, &duration);
> - todo_wine ok(hr == S_FALSE, "Got hr %#x.\n", hr);
> - todo_wine ok(duration == 0, "Got duration %s.\n",
> -
>             wine_dbgstr_longlong(duration));
> -
> - ref = IAMMultiMediaStream_Release(mmstream);
> - ok(!ref, "Got outstanding refcount %d.\n", ref);
> -
> - mmstream = create_ammultimediastream();
> - hr = IAMMultiMediaStream_AddMediaStream(mmstream, NULL, &MSPID_PrimaryAudio, 0, NULL);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - hr = IAMMultiMediaStream_OpenFile(mmstream, test_avi_path, AMMSF_NORENDER);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - duration = 0xdeadbeefdeadbeefULL;
> - hr = IAMMultiMediaStream_GetDuration(mmstream, &duration);
> - todo_wine ok(hr == S_FALSE, "Got hr %#x.\n", hr);
> - ok(duration == 0, "Got duration %s.\n",
> -
>   wine_dbgstr_longlong(duration));
> -
> - ref = IAMMultiMediaStream_Release(mmstream);
> - ok(!ref, "Got outstanding refcount %d.\n", ref);
>
> +}
> -
>
> static void test_renderfile(const WCHAR *test_avi_path)
>  {
>      IAMMultiMediaStream *pams;
> @@ -5351,6 +5429,7 @@ START_TEST(amstream)
>      test_avi_path = load_resource(L"test.avi");
>  test_openfile(test_avi_path);
> -
>
> test_mmstream_get_duration(test_avi_path);
> test_renderfile(test_avi_path);
>
> unload_resource(test_avi_path);
>
> --
> 2.27.0

[Myah Caron]

> It seems, given the tests below, that S_FALSE is returned if the graph
> can't seek (i.e. it's translated from E_NOTIMPL). In that case it'd be
> better to key this return value on the return from AddMediaStream() above.

Thanks for the review.

Sorry if I'm misunderstanding you, but I'm pretty sure the earlier AddMediaStream calls succeed under windows server. I believe the issue is that decoding AVI files isn't supported under windows server (I read something to that effect a few days ago, though for some reason I'm unable to find the link now).

Would adding an extra test (like IAMMultiMediaStream_GetInformation) to see if AVI files can be seeked be alright?
> Also, a bit of a nitpick, but I think that duration would be more
> readable in decimal.

Good point, will fix.

[Zebediah Figura]

On 7/28/20 3:53 AM, Myah Caron wrote:
> > It seems, given the tests below, that S_FALSE is returned if the graph
> > can't seek (i.e. it's translated from E_NOTIMPL). In that case it'd be
> > better to key this return value on the return from AddMediaStream() above.
>
> Thanks for the review.
>
> Sorry if I'm misunderstanding you, but I'm pretty sure the earlier AddMediaStream calls succeed under windows server. I believe the issue is that decoding AVI files isn't supported under windows server (I read something to that effect a few days ago, though for some reason I'm unable to find the link now).

We render and seek an AVI file (which admittedly is video-only) in
quartz:filtergraph, apparently successfully:

http://test.winehq.org/data/a22b295cc30d89e446038c4e677f8ea6fef159e1/2008_ne...

I'd expect OpenFile() to fail if the file can't be rendered.

On the other hand, w2008s uniquely lacks audio hardware, so I would
indeed expect VFW_E_NO_AUDIO_HARDWARE when adding the default audio
renderer.
> Would adding an extra test (like IAMMultiMediaStream_GetInformation) to see if AVI files can be seeked be alright?
> > Also, a bit of a nitpick, but I think that duration would be more
> > readable in decimal.
>
> Good point, will fix.