Re: [PATCH v2 13/13] qedit: Implement MediaDet_get_StreamLength.

[Zebediah Figura]

On 4/16/20 10:25 AM, Gabriel Ivăncescu wrote:
> Signed-off-by: Gabriel Ivăncescu
>
> dlls/qedit/mediadet.c       | 39 +++++++++++++++++++++++++++++++++++--
>  dlls/qedit/tests/mediadet.c | 19 ++++++++++++++++++
>  2 files changed, 56 insertions(+), 2 deletions(-)

I think adding the IMediaPosition and IMediaSeeking interfaces to
testfilter should be part of this patch rather than 12/13.
> diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
> index 9903238..4990dbe 100644
> --- a/dlls/qedit/mediadet.c
> +++ b/dlls/qedit/mediadet.c
> @@ -569,8 +569,43 @@ static HRESULT WINAPI MediaDet_get_StreamTypeB(IMediaDet* iface, BSTR *pVal)
>  static HRESULT WINAPI MediaDet_get_StreamLength(IMediaDet* iface, double *pVal)
>  {
>      MediaDetImpl *This = impl_from_IMediaDet(iface);
> - FIXME("(%p): stub!\n", This);
> - return VFW_E_INVALIDMEDIATYPE;
> - IMediaSeeking *seeking;
> - IMediaPosition *pos;
> - HRESULT hr;
> -
> - TRACE("(%p)\n", This);
> -
> - if (!pVal)
> -
>    return E_POINTER;
> -
> - if (!This->cur_pin)
> -
>    return E_INVALIDARG;
> -
> - hr = IPin_QueryInterface(This->cur_pin, &IID_IMediaPosition, (void **) &pos);
> - if (SUCCEEDED(hr))
> - {
> -
>    hr = IMediaPosition_get_Duration(pos, pVal);
> -
>    IMediaPosition_Release(pos);
> -
>    return hr;
> - }
> -
> - hr = IPin_QueryInterface(This->cur_pin, &IID_IMediaSeeking, (void **) &seeking);
> - if (SUCCEEDED(hr))
> - {
> -
>    LONGLONG duration;
> -
> -
>    hr = IMediaSeeking_GetDuration(seeking, &duration);
> -
>    if (SUCCEEDED(hr))
> -
>    {
> -
>        hr = IMediaSeeking_ConvertTimeFormat(seeking, &duration, &TIME_FORMAT_MEDIA_TIME, duration, NULL);
> -
>        if (SUCCEEDED(hr))
> -
>            *pVal = (REFTIME)duration / 10000000;
> -
>    }
> -
>    IMediaSeeking_Release(seeking);
> -
>    return hr;
> - }

You're not really testing this, and for that matter, IMediaSeeking is
essentially unused in your tests.
> -
> - return hr;
>
> }
>
> static HRESULT WINAPI MediaDet_get_Filename(IMediaDet* iface, BSTR *pVal)
> diff --git a/dlls/qedit/tests/mediadet.c b/dlls/qedit/tests/mediadet.c
> index 10127cf..14c99e8 100644
> --- a/dlls/qedit/tests/mediadet.c
> +++ b/dlls/qedit/tests/mediadet.c
> @@ -1060,6 +1060,7 @@ static void test_mediadet(void)
>      GUID guid;
>      BSTR bstr;
>      AM_MEDIA_TYPE mt;
> - double duration;
> IUnknown *unk;
> double fps;
> int flags;
>
> @@ -1122,6 +1123,12 @@ static void test_mediadet(void)
>      hr = IMediaDet_get_StreamTypeB(pM, NULL);
>      ok(hr == E_INVALIDARG, "IMediaDet_get_StreamTypeB failed: %08x\n", hr);
> - hr = IMediaDet_get_StreamLength(pM, &duration);
> - ok(hr == E_INVALIDARG, "IMediaDet_get_StreamLength failed: %08x\n", hr);
> -
> - hr = IMediaDet_get_StreamLength(pM, NULL);
> - ok(hr == E_POINTER, "IMediaDet_get_StreamLength failed: %08x\n", hr);
> - hr = IMediaDet_get_Filter(pM, NULL);
>    ok(hr == E_POINTER, "IMediaDet_get_Filter failed: %08x\n", hr);
>
> @@ -1202,6 +1209,10 @@ static void test_mediadet(void)
>      ok(!wcscmp(bstr, L"{73646976-0000-0010-8000-00AA00389B71}"), "Wrong GUID %s\n", wine_dbgstr_w(bstr));
>      SysFreeString(bstr);
> - hr = IMediaDet_get_StreamLength(pM, &duration);
> - ok(hr == S_OK, "IMediaDet_get_StreamLength failed: %08x\n", hr);
> - ok(duration >= 0.1 && duration < 0.10000001, "Wrong duration %.17g\n", duration);

%.16e.

I'm also not sure that I particularly care about adding these tests; as
mentioned in 12/13, I'd rather see the AVI parts go away entirely.
> - /* Even before get_OutputStreams.  */
>    hr = IMediaDet_put_CurrentStream(pM, 1);
>    ok(hr == E_INVALIDARG, "IMediaDet_put_CurrentStream failed: %08x\n", hr);
>
> @@ -1264,6 +1275,10 @@ static void test_mediadet(void)
>      ok(!wcscmp(bstr, L"{73646976-0000-0010-8000-00AA00389B71}"), "Wrong GUID %s\n", wine_dbgstr_w(bstr));
>      SysFreeString(bstr);
> - hr = IMediaDet_get_StreamLength(pM, &duration);
> - ok(hr == S_OK, "IMediaDet_get_StreamLength failed: %08x\n", hr);
> - ok(duration >= 0.1 && duration < 0.10000001, "Wrong duration %.17g\n", duration);
> - hr = IMediaDet_get_FrameRate(pM, NULL);
>    ok(hr == E_POINTER, "IMediaDet_get_FrameRate failed: %08x\n", hr);
>
> @@ -1392,6 +1407,10 @@ static void test_mediadet(void)
>      ok(hr == S_OK, "IMediaDet_get_OutputStreams failed: %08x\n", hr);
>      ok(nstrms == 1, "IMediaDet_get_OutputStreams: nstrms is %i\n", nstrms);
> - hr = IMediaDet_get_StreamLength(pM, &duration);
> - ok(hr == S_OK, "IMediaDet_get_StreamLength failed: %08x\n", hr);
> - ok(duration >= 4.2 && duration < 4.20000001, "Wrong duration %.17g\n", duration);
> -

Surely we can at least compare this to 4.2 exactly? That's the double
that we return.
>  hr = IMediaDet_Release(pM);
>  ok(hr == 0, "IMediaDet_Release returned: %x\n", hr);