[PATCH v2 01/13] qedit: Implement MediaDet_get_StreamType.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/qedit/mediadet.c       | 17 +++++++++++++++--
 dlls/qedit/tests/mediadet.c | 19 +++++++++++++++++++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
index ecfafc4..a6b52c0 100644
--- a/dlls/qedit/mediadet.c
+++ b/dlls/qedit/mediadet.c
@@ -283,8 +283,21 @@ static HRESULT WINAPI MediaDet_put_CurrentStream(IMediaDet* iface, LONG newVal)
 static HRESULT WINAPI MediaDet_get_StreamType(IMediaDet* iface, GUID *pVal)
 {
     MediaDetImpl *This = impl_from_IMediaDet(iface);
-    FIXME("(%p)->(%s): not implemented!\n", This, debugstr_guid(pVal));
-    return E_NOTIMPL;
+    AM_MEDIA_TYPE mt;
+    HRESULT hr;
+
+    TRACE("(%p)->(%p)\n", This, pVal);
+
+    if (!pVal)
+        return E_POINTER;
+
+    hr = IMediaDet_get_StreamMediaType(&This->IMediaDet_iface, &mt);
+    if (FAILED(hr))
+        return hr;
+    CoTaskMemFree(mt.pbFormat);
+
+    *pVal = mt.majortype;
+    return hr;
 }

static HRESULT WINAPI MediaDet_get_StreamTypeB(IMediaDet* iface, BSTR *pVal)
diff --git a/dlls/qedit/tests/mediadet.c b/dlls/qedit/tests/mediadet.c
index 65f5af7..c92c187 100644
--- a/dlls/qedit/tests/mediadet.c
+++ b/dlls/qedit/tests/mediadet.c
@@ -192,6 +192,7 @@ static void test_mediadet(void)
     BSTR filename = NULL;
     LONG nstrms = 0;
     LONG strm;
+    GUID guid;
     AM_MEDIA_TYPE mt;
     double fps;
     int flags;
@@ -242,6 +243,12 @@ static void test_mediadet(void)
     hr = IMediaDet_get_StreamMediaType(pM, NULL);
     ok(hr == E_POINTER, "IMediaDet_get_StreamMediaType failed: %08x\n", hr);

+    hr = IMediaDet_get_StreamType(pM, &guid);
+    ok(hr == E_INVALIDARG, "IMediaDet_get_StreamType failed: %08x\n", hr);
+
+    hr = IMediaDet_get_StreamType(pM, NULL);
+    ok(hr == E_POINTER, "IMediaDet_get_StreamType failed: %08x\n", hr);
+
     filename = SysAllocString(test_avi_filename);
     hr = IMediaDet_put_Filename(pM, filename);
     ok(hr == S_OK, "IMediaDet_put_Filename failed: %08x\n", hr);
@@ -258,6 +265,10 @@ static void test_mediadet(void)
     ok(hr == S_OK, "IMediaDet_get_StreamMediaType failed: %08x\n", hr);
     CoTaskMemFree(mt.pbFormat);

+    hr = IMediaDet_get_StreamType(pM, &guid);
+    ok(hr == S_OK, "IMediaDet_get_StreamType failed: %08x\n", hr);
+    ok(IsEqualGUID(&guid, &MEDIATYPE_Video), "Wrong GUID %s\n", wine_dbgstr_guid(&guid));
+
     /* Even before get_OutputStreams.  */
     hr = IMediaDet_put_CurrentStream(pM, 1);
     ok(hr == E_INVALIDARG, "IMediaDet_put_CurrentStream failed: %08x\n", hr);
@@ -311,6 +322,10 @@ static void test_mediadet(void)
                  "IMediaDet_get_StreamMediaType\n");
     CoTaskMemFree(mt.pbFormat);

+    hr = IMediaDet_get_StreamType(pM, &guid);
+    ok(hr == S_OK, "IMediaDet_get_StreamType failed: %08x\n", hr);
+    ok(IsEqualGUID(&guid, &MEDIATYPE_Video), "Wrong GUID %s\n", wine_dbgstr_guid(&guid));
+
     hr = IMediaDet_get_FrameRate(pM, NULL);
     ok(hr == E_POINTER, "IMediaDet_get_FrameRate failed: %08x\n", hr);

@@ -370,6 +385,10 @@ static void test_mediadet(void)

if (IsEqualGUID(&mt.majortype, &MEDIATYPE_Audio))
         {
+            hr = IMediaDet_get_StreamType(pM, &guid);
+            ok(hr == S_OK, "IMediaDet_get_StreamType failed: %08x\n", hr);
+            ok(IsEqualGUID(&guid, &MEDIATYPE_Audio), "Wrong GUID %s\n", wine_dbgstr_guid(&guid));
+
             hr = IMediaDet_get_FrameRate(pM, &fps);
             ok(hr == VFW_E_INVALIDMEDIATYPE, "IMediaDet_get_FrameRate failed: %08x\n", hr);
         }

[PATCH v2 02/13] qedit: Implement MediaDet_get_StreamTypeB.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/qedit/mediadet.c       | 17 +++++++++++++++--
 dlls/qedit/tests/mediadet.c | 22 ++++++++++++++++++++++
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
index a6b52c0..dae98fd 100644
--- a/dlls/qedit/mediadet.c
+++ b/dlls/qedit/mediadet.c
@@ -303,8 +303,21 @@ static HRESULT WINAPI MediaDet_get_StreamType(IMediaDet* iface, GUID *pVal)
 static HRESULT WINAPI MediaDet_get_StreamTypeB(IMediaDet* iface, BSTR *pVal)
 {
     MediaDetImpl *This = impl_from_IMediaDet(iface);
-    FIXME("(%p)->(%p): not implemented!\n", This, pVal);
-    return E_NOTIMPL;
+    AM_MEDIA_TYPE mt;
+    HRESULT hr;
+
+    TRACE("(%p)->(%p)\n", This, pVal);
+
+    hr = IMediaDet_get_StreamMediaType(&This->IMediaDet_iface, &mt);
+    if (FAILED(hr))
+        return hr;
+    CoTaskMemFree(mt.pbFormat);
+
+    if (!(*pVal = SysAllocStringLen(NULL, CHARS_IN_GUID - 1)))
+        return E_OUTOFMEMORY;
+
+    StringFromGUID2(&mt.majortype, *pVal, CHARS_IN_GUID);
+    return hr;
 }

static HRESULT WINAPI MediaDet_get_StreamLength(IMediaDet* iface, double *pVal)
diff --git a/dlls/qedit/tests/mediadet.c b/dlls/qedit/tests/mediadet.c
index c92c187..596171b 100644
--- a/dlls/qedit/tests/mediadet.c
+++ b/dlls/qedit/tests/mediadet.c
@@ -193,6 +193,7 @@ static void test_mediadet(void)
     LONG nstrms = 0;
     LONG strm;
     GUID guid;
+    BSTR bstr;
     AM_MEDIA_TYPE mt;
     double fps;
     int flags;
@@ -249,6 +250,12 @@ static void test_mediadet(void)
     hr = IMediaDet_get_StreamType(pM, NULL);
     ok(hr == E_POINTER, "IMediaDet_get_StreamType failed: %08x\n", hr);

+    hr = IMediaDet_get_StreamTypeB(pM, &bstr);
+    ok(hr == E_INVALIDARG, "IMediaDet_get_StreamTypeB failed: %08x\n", hr);
+
+    hr = IMediaDet_get_StreamTypeB(pM, NULL);
+    ok(hr == E_INVALIDARG, "IMediaDet_get_StreamTypeB failed: %08x\n", hr);
+
     filename = SysAllocString(test_avi_filename);
     hr = IMediaDet_put_Filename(pM, filename);
     ok(hr == S_OK, "IMediaDet_put_Filename failed: %08x\n", hr);
@@ -269,6 +276,11 @@ static void test_mediadet(void)
     ok(hr == S_OK, "IMediaDet_get_StreamType failed: %08x\n", hr);
     ok(IsEqualGUID(&guid, &MEDIATYPE_Video), "Wrong GUID %s\n", wine_dbgstr_guid(&guid));

+    hr = IMediaDet_get_StreamTypeB(pM, &bstr);
+    ok(hr == S_OK, "IMediaDet_get_StreamTypeB failed: %08x\n", hr);
+    ok(!wcscmp(bstr, L"{73646976-0000-0010-8000-00AA00389B71}"), "Wrong GUID %s\n", wine_dbgstr_w(bstr));
+    SysFreeString(bstr);
+
     /* Even before get_OutputStreams.  */
     hr = IMediaDet_put_CurrentStream(pM, 1);
     ok(hr == E_INVALIDARG, "IMediaDet_put_CurrentStream failed: %08x\n", hr);
@@ -326,6 +338,11 @@ static void test_mediadet(void)
     ok(hr == S_OK, "IMediaDet_get_StreamType failed: %08x\n", hr);
     ok(IsEqualGUID(&guid, &MEDIATYPE_Video), "Wrong GUID %s\n", wine_dbgstr_guid(&guid));

+    hr = IMediaDet_get_StreamTypeB(pM, &bstr);
+    ok(hr == S_OK, "IMediaDet_get_StreamTypeB failed: %08x\n", hr);
+    ok(!wcscmp(bstr, L"{73646976-0000-0010-8000-00AA00389B71}"), "Wrong GUID %s\n", wine_dbgstr_w(bstr));
+    SysFreeString(bstr);
+
     hr = IMediaDet_get_FrameRate(pM, NULL);
     ok(hr == E_POINTER, "IMediaDet_get_FrameRate failed: %08x\n", hr);

@@ -389,6 +406,11 @@ static void test_mediadet(void)
             ok(hr == S_OK, "IMediaDet_get_StreamType failed: %08x\n", hr);
             ok(IsEqualGUID(&guid, &MEDIATYPE_Audio), "Wrong GUID %s\n", wine_dbgstr_guid(&guid));

+            hr = IMediaDet_get_StreamTypeB(pM, &bstr);
+            ok(hr == S_OK, "IMediaDet_get_StreamTypeB failed: %08x\n", hr);
+            ok(!wcscmp(bstr, L"{73647561-0000-0010-8000-00AA00389B71}"), "Wrong GUID %s\n", wine_dbgstr_w(bstr));
+            SysFreeString(bstr);
+
             hr = IMediaDet_get_FrameRate(pM, &fps);
             ok(hr == VFW_E_INVALIDMEDIATYPE, "IMediaDet_get_FrameRate failed: %08x\n", hr);
         }

[PATCH v2 02/13] qedit: Implement MediaDet_get_StreamTypeB.

[Zebediah Figura]

On 4/16/20 10:24 AM, Gabriel Ivăncescu wrote:
> Signed-off-by: Gabriel Ivăncescu
>
> dlls/qedit/mediadet.c       | 17 +++++++++++++++--
>  dlls/qedit/tests/mediadet.c | 22 ++++++++++++++++++++++
>  2 files changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
> index a6b52c0..dae98fd 100644
> --- a/dlls/qedit/mediadet.c
> +++ b/dlls/qedit/mediadet.c
> @@ -303,8 +303,21 @@ static HRESULT WINAPI MediaDet_get_StreamType(IMediaDet* iface, GUID *pVal)
>  static HRESULT WINAPI MediaDet_get_StreamTypeB(IMediaDet* iface, BSTR *pVal)
>  {
>      MediaDetImpl *This = impl_from_IMediaDet(iface);
> - FIXME("(%p)->(%p): not implemented!\n", This, pVal);
> - return E_NOTIMPL;
> - AM_MEDIA_TYPE mt;
> - HRESULT hr;
> -
> - TRACE("(%p)->(%p)\n", This, pVal);
> -
> - hr = IMediaDet_get_StreamMediaType(&This->IMediaDet_iface, &mt);

Wouldn't it be simpler just to call get_StreamType() here?
> - if (FAILED(hr))
> -
>    return hr;
> - CoTaskMemFree(mt.pbFormat);
> -
> - if (!(*pVal = SysAllocStringLen(NULL, CHARS_IN_GUID - 1)))
> -
>    return E_OUTOFMEMORY;
> -
> - StringFromGUID2(&mt.majortype, *pVal, CHARS_IN_GUID);
> - return hr;
>
> }
>
> static HRESULT WINAPI MediaDet_get_StreamLength(IMediaDet* iface, double *pVal)
> diff --git a/dlls/qedit/tests/mediadet.c b/dlls/qedit/tests/mediadet.c
> index c92c187..596171b 100644
> --- a/dlls/qedit/tests/mediadet.c
> +++ b/dlls/qedit/tests/mediadet.c
> @@ -193,6 +193,7 @@ static void test_mediadet(void)
>      LONG nstrms = 0;
>      LONG strm;
>      GUID guid;
> - BSTR bstr;
> AM_MEDIA_TYPE mt;
> double fps;
> int flags;
>
> @@ -249,6 +250,12 @@ static void test_mediadet(void)
>      hr = IMediaDet_get_StreamType(pM, NULL);
>      ok(hr == E_POINTER, "IMediaDet_get_StreamType failed: %08x\n", hr);
> - hr = IMediaDet_get_StreamTypeB(pM, &bstr);
> - ok(hr == E_INVALIDARG, "IMediaDet_get_StreamTypeB failed: %08x\n", hr);
> -
> - hr = IMediaDet_get_StreamTypeB(pM, NULL);
> - ok(hr == E_INVALIDARG, "IMediaDet_get_StreamTypeB failed: %08x\n", hr);
> - filename = SysAllocString(test_avi_filename);
>    hr = IMediaDet_put_Filename(pM, filename);
>    ok(hr == S_OK, "IMediaDet_put_Filename failed: %08x\n", hr);
>
> @@ -269,6 +276,11 @@ static void test_mediadet(void)
>      ok(hr == S_OK, "IMediaDet_get_StreamType failed: %08x\n", hr);
>      ok(IsEqualGUID(&guid, &MEDIATYPE_Video), "Wrong GUID %s\n", wine_dbgstr_guid(&guid));
> - hr = IMediaDet_get_StreamTypeB(pM, &bstr);
> - ok(hr == S_OK, "IMediaDet_get_StreamTypeB failed: %08x\n", hr);
> - ok(!wcscmp(bstr, L"{73646976-0000-0010-8000-00AA00389B71}"), "Wrong GUID %s\n", wine_dbgstr_w(bstr));
> - SysFreeString(bstr);
> - /* Even before get_OutputStreams.  */
>    hr = IMediaDet_put_CurrentStream(pM, 1);
>    ok(hr == E_INVALIDARG, "IMediaDet_put_CurrentStream failed: %08x\n", hr);
>
> @@ -326,6 +338,11 @@ static void test_mediadet(void)
>      ok(hr == S_OK, "IMediaDet_get_StreamType failed: %08x\n", hr);
>      ok(IsEqualGUID(&guid, &MEDIATYPE_Video), "Wrong GUID %s\n", wine_dbgstr_guid(&guid));
> - hr = IMediaDet_get_StreamTypeB(pM, &bstr);
> - ok(hr == S_OK, "IMediaDet_get_StreamTypeB failed: %08x\n", hr);
> - ok(!wcscmp(bstr, L"{73646976-0000-0010-8000-00AA00389B71}"), "Wrong GUID %s\n", wine_dbgstr_w(bstr));
> - SysFreeString(bstr);
> - hr = IMediaDet_get_FrameRate(pM, NULL);
>    ok(hr == E_POINTER, "IMediaDet_get_FrameRate failed: %08x\n", hr);
>
> @@ -389,6 +406,11 @@ static void test_mediadet(void)
>              ok(hr == S_OK, "IMediaDet_get_StreamType failed: %08x\n", hr);
>              ok(IsEqualGUID(&guid, &MEDIATYPE_Audio), "Wrong GUID %s\n", wine_dbgstr_guid(&guid));
> -
>        hr = IMediaDet_get_StreamTypeB(pM, &bstr);
> -
>        ok(hr == S_OK, "IMediaDet_get_StreamTypeB failed: %08x\n", hr);
> -
>        ok(!wcscmp(bstr, L"{73647561-0000-0010-8000-00AA00389B71}"), "Wrong GUID %s\n", wine_dbgstr_w(bstr));
> -
>        SysFreeString(bstr);
> -
>        hr = IMediaDet_get_FrameRate(pM, &fps);
>        ok(hr == VFW_E_INVALIDMEDIATYPE, "IMediaDet_get_FrameRate failed: %08x\n", hr);
>    }

[PATCH v2 03/13] qedit: Implement MediaDet_get_Filter.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/qedit/mediadet.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
index dae98fd..0a06d52 100644
--- a/dlls/qedit/mediadet.c
+++ b/dlls/qedit/mediadet.c
@@ -146,8 +146,19 @@ static ULONG WINAPI MediaDet_Release(IMediaDet *iface)
 static HRESULT WINAPI MediaDet_get_Filter(IMediaDet* iface, IUnknown **pVal)
 {
     MediaDetImpl *This = impl_from_IMediaDet(iface);
-    FIXME("(%p)->(%p): not implemented!\n", This, pVal);
-    return E_NOTIMPL;
+
+    TRACE("(%p)->(%p)\n", This, pVal);
+
+    if (!pVal)
+        return E_POINTER;
+
+    *pVal = (IUnknown*)This->source;
+    if (*pVal)
+        IUnknown_AddRef(*pVal);
+    else
+        return S_FALSE;
+
+    return S_OK;
 }

static HRESULT WINAPI MediaDet_put_Filter(IMediaDet* iface, IUnknown *newVal)

[PATCH v2 04/13] qedit: Store the filename for retrieval instead of querying the filter.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---

This will be needed after put_Filter is implemented so that it passes the
tests properly.

dlls/qedit/mediadet.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
index 0a06d52..5152eea 100644
--- a/dlls/qedit/mediadet.c
+++ b/dlls/qedit/mediadet.c
@@ -28,6 +28,7 @@
 #include "ole2.h"

#include "qedit_private.h"
+#include "wine/heap.h"
 #include "wine/debug.h"

WINE_DEFAULT_DEBUG_CHANNEL(qedit);
@@ -40,6 +41,7 @@ typedef struct MediaDetImpl {
     IGraphBuilder *graph;
     IBaseFilter *source;
     IBaseFilter *splitter;
+    WCHAR *filename;
     LONG num_streams;
     LONG cur_stream;
     IPin *cur_pin;
@@ -65,6 +67,8 @@ static void MD_cleanup(MediaDetImpl *This)
     This->splitter = NULL;
     if (This->graph) IGraphBuilder_Release(This->graph);
     This->graph = NULL;
+    if (This->filename) heap_free(This->filename);
+    This->filename = NULL;
     This->num_streams = -1;
     This->cur_stream = 0;
 }
@@ -341,9 +345,6 @@ static HRESULT WINAPI MediaDet_get_StreamLength(IMediaDet* iface, double *pVal)
 static HRESULT WINAPI MediaDet_get_Filename(IMediaDet* iface, BSTR *pVal)
 {
     MediaDetImpl *This = impl_from_IMediaDet(iface);
-    IFileSourceFilter *file;
-    LPOLESTR name;
-    HRESULT hr;

TRACE("(%p)\n", This);

@@ -353,21 +354,10 @@ static HRESULT WINAPI MediaDet_get_Filename(IMediaDet* iface, BSTR *pVal)
     *pVal = NULL;
     /* MSDN says it should return E_FAIL if no file is open, but tests
        show otherwise.  */
-    if (!This->source)
+    if (!This->filename)
         return S_OK;

-    hr = IBaseFilter_QueryInterface(This->source, &IID_IFileSourceFilter,
-                                    (void **) &file);
-    if (FAILED(hr))
-        return hr;
-
-    hr = IFileSourceFilter_GetCurFile(file, &name, NULL);
-    IFileSourceFilter_Release(file);
-    if (FAILED(hr))
-        return hr;
-
-    *pVal = SysAllocString(name);
-    CoTaskMemFree(name);
+    *pVal = SysAllocString(This->filename);
     if (!*pVal)
         return E_OUTOFMEMORY;

@@ -522,6 +512,7 @@ static HRESULT WINAPI MediaDet_put_Filename(IMediaDet* iface, BSTR newVal)
     MediaDetImpl *This = impl_from_IMediaDet(iface);
     IGraphBuilder *gb;
     IBaseFilter *bf;
+    WCHAR *filename;
     HRESULT hr;

TRACE("(%p)->(%s)\n", This, debugstr_w(newVal));
@@ -537,14 +528,23 @@ static HRESULT WINAPI MediaDet_put_Filename(IMediaDet* iface, BSTR newVal)
     if (FAILED(hr))
         return hr;

+    if (!(filename = heap_alloc((wcslen(newVal) + 1) * sizeof(WCHAR))))
+    {
+        IGraphBuilder_Release(gb);
+        return E_OUTOFMEMORY;
+    }
+
     if (FAILED(hr = IGraphBuilder_AddSourceFilter(gb, newVal, L"Reader", &bf)))
     {
+        heap_free(filename);
         IGraphBuilder_Release(gb);
         return hr;
     }

This->graph = gb;
     This->source = bf;
+    This->filename = filename;
+    wcscpy(filename, newVal);
     hr = GetSplitter(This);
     if (FAILED(hr))
         return hr;

[PATCH v2 04/13] qedit: Store the filename for retrieval instead of querying the filter.

[Zebediah Figura]

On 4/16/20 10:24 AM, Gabriel Ivăncescu wrote:
> Signed-off-by: Gabriel Ivăncescu
>
> This will be needed after put_Filter is implemented so that it passes the
> tests properly.
>
> dlls/qedit/mediadet.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
> index 0a06d52..5152eea 100644
> --- a/dlls/qedit/mediadet.c
> +++ b/dlls/qedit/mediadet.c
> @@ -28,6 +28,7 @@
>  #include "ole2.h"
>
> #include "qedit_private.h"
> +#include "wine/heap.h"
>  #include "wine/debug.h"
>
> WINE_DEFAULT_DEBUG_CHANNEL(qedit);
> @@ -40,6 +41,7 @@ typedef struct MediaDetImpl {
>      IGraphBuilder *graph;
>      IBaseFilter *source;
>      IBaseFilter *splitter;
> - WCHAR *filename;
> LONG num_streams;
> LONG cur_stream;
> IPin *cur_pin;
>
> @@ -65,6 +67,8 @@ static void MD_cleanup(MediaDetImpl *This)
>      This->splitter = NULL;
>      if (This->graph) IGraphBuilder_Release(This->graph);
>      This->graph = NULL;
> - if (This->filename) heap_free(This->filename);
> - This->filename = NULL;
> This->num_streams = -1;
> This->cur_stream = 0;
>
> }
> @@ -341,9 +345,6 @@ static HRESULT WINAPI MediaDet_get_StreamLength(IMediaDet* iface, double *pVal)
>  static HRESULT WINAPI MediaDet_get_Filename(IMediaDet* iface, BSTR *pVal)
>  {
>      MediaDetImpl *This = impl_from_IMediaDet(iface);
> -
>
> IFileSourceFilter *file;
> -
>
> LPOLESTR name;
> -
>
> HRESULT hr;
>
> TRACE("(%p)\n", This);
>
> @@ -353,21 +354,10 @@ static HRESULT WINAPI MediaDet_get_Filename(IMediaDet* iface, BSTR *pVal)
>      *pVal = NULL;
>      /* MSDN says it should return E_FAIL if no file is open, but tests
>         show otherwise.  */
> - if (!This->source)
> - if (!This->filename)
>     return S_OK;
> - hr = IBaseFilter_QueryInterface(This->source, &IID_IFileSourceFilter,
> -
>                                (void **) &file);
> - if (FAILED(hr))
> -
>    return hr;
> -
> - hr = IFileSourceFilter_GetCurFile(file, &name, NULL);
> - IFileSourceFilter_Release(file);
> - if (FAILED(hr))
> -
>    return hr;
> -
> - *pVal = SysAllocString(name);
> - CoTaskMemFree(name);
> - *pVal = SysAllocString(This->filename);
> if (!*pVal)
>     return E_OUTOFMEMORY;
>
> @@ -522,6 +512,7 @@ static HRESULT WINAPI MediaDet_put_Filename(IMediaDet* iface, BSTR newVal)
>      MediaDetImpl *This = impl_from_IMediaDet(iface);
>      IGraphBuilder *gb;
>      IBaseFilter *bf;
> -
>
> WCHAR *filename;
> HRESULT hr;
>
> TRACE("(%p)->(%s)\n", This, debugstr_w(newVal));
>
> @@ -537,14 +528,23 @@ static HRESULT WINAPI MediaDet_put_Filename(IMediaDet* iface, BSTR newVal)
>      if (FAILED(hr))
>          return hr;
> - if (!(filename = heap_alloc((wcslen(newVal) + 1) * sizeof(WCHAR))))

qedit is compiled with msvcrt, so this can be wcsdup().
> -
>
> {
> -
>    IGraphBuilder_Release(gb);
> -
>    return E_OUTOFMEMORY;
> -
>
> }
> -
>
> if (FAILED(hr = IGraphBuilder_AddSourceFilter(gb, newVal, L"Reader", &bf)))
>    {
> -
>    heap_free(filename);
>    IGraphBuilder_Release(gb);
>    return hr;
>
> }
>
> This->graph = gb;
>    This->source = bf;
> -
>
> This->filename = filename;
> -
>
> wcscpy(filename, newVal);
> hr = GetSplitter(This);
> if (FAILED(hr))
>     return hr;

[PATCH v2 05/13] qedit: Move GetFilterInfo up and rename it.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/qedit/mediadet.c | 70 +++++++++++++++++++++----------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
index 5152eea..bc7ae3d 100644
--- a/dlls/qedit/mediadet.c
+++ b/dlls/qedit/mediadet.c
@@ -73,6 +73,40 @@ static void MD_cleanup(MediaDetImpl *This)
     This->cur_stream = 0;
 }

+/* From quartz, 2008/04/07 */
+static HRESULT get_filter_info(IMoniker *pMoniker, GUID *pclsid, VARIANT *pvar)
+{
+    IPropertyBag *pPropBagCat = NULL;
+    HRESULT hr;
+
+    VariantInit(pvar);
+    V_VT(pvar) = VT_BSTR;
+
+    hr = IMoniker_BindToStorage(pMoniker, NULL, NULL, &IID_IPropertyBag,
+                                (LPVOID *) &pPropBagCat);
+
+    if (SUCCEEDED(hr))
+        hr = IPropertyBag_Read(pPropBagCat, L"CLSID", pvar, NULL);
+
+    if (SUCCEEDED(hr))
+    {
+        hr = CLSIDFromString(V_BSTR(pvar), pclsid);
+        VariantClear(pvar);
+        V_VT(pvar) = VT_BSTR;
+    }
+
+    if (SUCCEEDED(hr))
+        hr = IPropertyBag_Read(pPropBagCat, L"FriendlyName", pvar, NULL);
+
+    if (SUCCEEDED(hr))
+        TRACE("Moniker = %s - %s\n", debugstr_guid(pclsid), debugstr_w(V_BSTR(pvar)));
+
+    if (pPropBagCat)
+        IPropertyBag_Release(pPropBagCat);
+
+    return hr;
+}
+
 /* MediaDet inner IUnknown */
 static HRESULT WINAPI MediaDet_inner_QueryInterface(IUnknown *iface, REFIID riid, void **ppv)
 {
@@ -364,40 +398,6 @@ static HRESULT WINAPI MediaDet_get_Filename(IMediaDet* iface, BSTR *pVal)
     return S_OK;
 }

-/* From quartz, 2008/04/07 */
-static HRESULT GetFilterInfo(IMoniker *pMoniker, GUID *pclsid, VARIANT *pvar)
-{
-    IPropertyBag *pPropBagCat = NULL;
-    HRESULT hr;
-
-    VariantInit(pvar);
-    V_VT(pvar) = VT_BSTR;
-
-    hr = IMoniker_BindToStorage(pMoniker, NULL, NULL, &IID_IPropertyBag,
-                                (LPVOID *) &pPropBagCat);
-
-    if (SUCCEEDED(hr))
-        hr = IPropertyBag_Read(pPropBagCat, L"CLSID", pvar, NULL);
-
-    if (SUCCEEDED(hr))
-    {
-        hr = CLSIDFromString(V_BSTR(pvar), pclsid);
-        VariantClear(pvar);
-        V_VT(pvar) = VT_BSTR;
-    }
-
-    if (SUCCEEDED(hr))
-        hr = IPropertyBag_Read(pPropBagCat, L"FriendlyName", pvar, NULL);
-
-    if (SUCCEEDED(hr))
-        TRACE("Moniker = %s - %s\n", debugstr_guid(pclsid), debugstr_w(V_BSTR(pvar)));
-
-    if (pPropBagCat)
-        IPropertyBag_Release(pPropBagCat);
-
-    return hr;
-}
-
 static HRESULT GetSplitter(MediaDetImpl *This)
 {
     IFileSourceFilter *file;
@@ -450,7 +450,7 @@ static HRESULT GetSplitter(MediaDetImpl *This)
     hr = E_NOINTERFACE;
     while (IEnumMoniker_Next(filters, 1, &mon, NULL) == S_OK)
     {
-        hr = GetFilterInfo(mon, &clsid, &var);
+        hr = get_filter_info(mon, &clsid, &var);
         IMoniker_Release(mon);
         if (FAILED(hr))
             continue;

[PATCH v2 06/13] qedit: Move GetSplitter up and rename it.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/qedit/mediadet.c | 220 +++++++++++++++++++++---------------------
 1 file changed, 110 insertions(+), 110 deletions(-)

diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
index bc7ae3d..556e59e 100644
--- a/dlls/qedit/mediadet.c
+++ b/dlls/qedit/mediadet.c
@@ -107,6 +107,115 @@ static HRESULT get_filter_info(IMoniker *pMoniker, GUID *pclsid, VARIANT *pvar)
     return hr;
 }

+static HRESULT get_splitter(MediaDetImpl *This)
+{
+    IFileSourceFilter *file;
+    LPOLESTR name;
+    AM_MEDIA_TYPE mt;
+    GUID type[2];
+    IFilterMapper2 *map;
+    IEnumMoniker *filters;
+    IMoniker *mon;
+    VARIANT var;
+    GUID clsid;
+    IBaseFilter *splitter;
+    IEnumPins *pins;
+    IPin *source_pin, *splitter_pin;
+    HRESULT hr;
+
+    hr = CoCreateInstance(&CLSID_FilterMapper2, NULL, CLSCTX_INPROC_SERVER,
+                          &IID_IFilterMapper2, (void **) &map);
+    if (FAILED(hr))
+        return hr;
+
+    hr = IBaseFilter_QueryInterface(This->source, &IID_IFileSourceFilter,
+                                    (void **) &file);
+    if (FAILED(hr))
+    {
+        IFilterMapper2_Release(map);
+        return hr;
+    }
+
+    hr = IFileSourceFilter_GetCurFile(file, &name, &mt);
+    IFileSourceFilter_Release(file);
+    CoTaskMemFree(name);
+    if (FAILED(hr))
+    {
+        IFilterMapper2_Release(map);
+        return hr;
+    }
+    type[0] = mt.majortype;
+    type[1] = mt.subtype;
+    CoTaskMemFree(mt.pbFormat);
+
+    hr = IFilterMapper2_EnumMatchingFilters(map, &filters, 0, TRUE,
+                                            MERIT_UNLIKELY, FALSE, 1, type,
+                                            NULL, NULL, FALSE, TRUE,
+                                            0, NULL, NULL, NULL);
+    IFilterMapper2_Release(map);
+    if (FAILED(hr))
+        return hr;
+
+    hr = E_NOINTERFACE;
+    while (IEnumMoniker_Next(filters, 1, &mon, NULL) == S_OK)
+    {
+        hr = get_filter_info(mon, &clsid, &var);
+        IMoniker_Release(mon);
+        if (FAILED(hr))
+            continue;
+
+        hr = CoCreateInstance(&clsid, NULL, CLSCTX_INPROC_SERVER,
+                              &IID_IBaseFilter, (void **) &splitter);
+        if (FAILED(hr))
+        {
+            VariantClear(&var);
+            continue;
+        }
+
+        hr = IGraphBuilder_AddFilter(This->graph, splitter, V_BSTR(&var));
+        VariantClear(&var);
+        This->splitter = splitter;
+        if (FAILED(hr))
+            goto retry;
+
+        hr = IBaseFilter_EnumPins(This->source, &pins);
+        if (FAILED(hr))
+            goto retry;
+        IEnumPins_Next(pins, 1, &source_pin, NULL);
+        IEnumPins_Release(pins);
+
+        hr = IBaseFilter_EnumPins(splitter, &pins);
+        if (FAILED(hr))
+        {
+            IPin_Release(source_pin);
+            goto retry;
+        }
+        if (IEnumPins_Next(pins, 1, &splitter_pin, NULL) != S_OK)
+        {
+            IEnumPins_Release(pins);
+            IPin_Release(source_pin);
+            goto retry;
+        }
+        IEnumPins_Release(pins);
+
+        hr = IPin_Connect(source_pin, splitter_pin, NULL);
+        IPin_Release(source_pin);
+        IPin_Release(splitter_pin);
+        if (SUCCEEDED(hr))
+            break;
+
+retry:
+        IBaseFilter_Release(splitter);
+        This->splitter = NULL;
+    }
+
+    IEnumMoniker_Release(filters);
+    if (FAILED(hr))
+        return hr;
+
+    return S_OK;
+}
+
 /* MediaDet inner IUnknown */
 static HRESULT WINAPI MediaDet_inner_QueryInterface(IUnknown *iface, REFIID riid, void **ppv)
 {
@@ -398,115 +507,6 @@ static HRESULT WINAPI MediaDet_get_Filename(IMediaDet* iface, BSTR *pVal)
     return S_OK;
 }

-static HRESULT GetSplitter(MediaDetImpl *This)
-{
-    IFileSourceFilter *file;
-    LPOLESTR name;
-    AM_MEDIA_TYPE mt;
-    GUID type[2];
-    IFilterMapper2 *map;
-    IEnumMoniker *filters;
-    IMoniker *mon;
-    VARIANT var;
-    GUID clsid;
-    IBaseFilter *splitter;
-    IEnumPins *pins;
-    IPin *source_pin, *splitter_pin;
-    HRESULT hr;
-
-    hr = CoCreateInstance(&CLSID_FilterMapper2, NULL, CLSCTX_INPROC_SERVER,
-                          &IID_IFilterMapper2, (void **) &map);
-    if (FAILED(hr))
-        return hr;
-
-    hr = IBaseFilter_QueryInterface(This->source, &IID_IFileSourceFilter,
-                                    (void **) &file);
-    if (FAILED(hr))
-    {
-        IFilterMapper2_Release(map);
-        return hr;
-    }
-
-    hr = IFileSourceFilter_GetCurFile(file, &name, &mt);
-    IFileSourceFilter_Release(file);
-    CoTaskMemFree(name);
-    if (FAILED(hr))
-    {
-        IFilterMapper2_Release(map);
-        return hr;
-    }
-    type[0] = mt.majortype;
-    type[1] = mt.subtype;
-    CoTaskMemFree(mt.pbFormat);
-
-    hr = IFilterMapper2_EnumMatchingFilters(map, &filters, 0, TRUE,
-                                            MERIT_UNLIKELY, FALSE, 1, type,
-                                            NULL, NULL, FALSE, TRUE,
-                                            0, NULL, NULL, NULL);
-    IFilterMapper2_Release(map);
-    if (FAILED(hr))
-        return hr;
-
-    hr = E_NOINTERFACE;
-    while (IEnumMoniker_Next(filters, 1, &mon, NULL) == S_OK)
-    {
-        hr = get_filter_info(mon, &clsid, &var);
-        IMoniker_Release(mon);
-        if (FAILED(hr))
-            continue;
-
-        hr = CoCreateInstance(&clsid, NULL, CLSCTX_INPROC_SERVER,
-                              &IID_IBaseFilter, (void **) &splitter);
-        if (FAILED(hr))
-        {
-            VariantClear(&var);
-            continue;
-        }
-
-        hr = IGraphBuilder_AddFilter(This->graph, splitter, V_BSTR(&var));
-        VariantClear(&var);
-        This->splitter = splitter;
-        if (FAILED(hr))
-            goto retry;
-
-        hr = IBaseFilter_EnumPins(This->source, &pins);
-        if (FAILED(hr))
-            goto retry;
-        IEnumPins_Next(pins, 1, &source_pin, NULL);
-        IEnumPins_Release(pins);
-
-        hr = IBaseFilter_EnumPins(splitter, &pins);
-        if (FAILED(hr))
-        {
-            IPin_Release(source_pin);
-            goto retry;
-        }
-        if (IEnumPins_Next(pins, 1, &splitter_pin, NULL) != S_OK)
-        {
-            IEnumPins_Release(pins);
-            IPin_Release(source_pin);
-            goto retry;
-        }
-        IEnumPins_Release(pins);
-
-        hr = IPin_Connect(source_pin, splitter_pin, NULL);
-        IPin_Release(source_pin);
-        IPin_Release(splitter_pin);
-        if (SUCCEEDED(hr))
-            break;
-
-retry:
-        IBaseFilter_Release(splitter);
-        This->splitter = NULL;
-    }
-
-    IEnumMoniker_Release(filters);
-    if (FAILED(hr))
-        return hr;
-
-    return S_OK;
-}
-
 static HRESULT WINAPI MediaDet_put_Filename(IMediaDet* iface, BSTR newVal)
 {
     MediaDetImpl *This = impl_from_IMediaDet(iface);
@@ -545,7 +545,7 @@ static HRESULT WINAPI MediaDet_put_Filename(IMediaDet* iface, BSTR newVal)
     This->source = bf;
     This->filename = filename;
     wcscpy(filename, newVal);
-    hr = GetSplitter(This);
+    hr = get_splitter(This);
     if (FAILED(hr))
         return hr;

[PATCH v2 07/13] qedit: Retrieve the Pin media type using a helper function.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/qedit/mediadet.c | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
index 556e59e..046994f 100644
--- a/dlls/qedit/mediadet.c
+++ b/dlls/qedit/mediadet.c
@@ -73,6 +73,28 @@ static void MD_cleanup(MediaDetImpl *This)
     This->cur_stream = 0;
 }

+static HRESULT get_pin_media_type(IPin *pin, AM_MEDIA_TYPE *out)
+{
+    IEnumMediaTypes *types;
+    AM_MEDIA_TYPE *pmt;
+    HRESULT hr;
+
+    hr = IPin_EnumMediaTypes(pin, &types);
+    if (SUCCEEDED(hr))
+    {
+        hr = IEnumMediaTypes_Next(types, 1, &pmt, NULL) == S_OK ? S_OK : E_NOINTERFACE;
+        IEnumMediaTypes_Release(types);
+    }
+
+    if (SUCCEEDED(hr))
+    {
+        *out = *pmt;
+        CoTaskMemFree(pmt);
+    }
+
+    return hr;
+}
+
 /* From quartz, 2008/04/07 */
 static HRESULT get_filter_info(IMoniker *pMoniker, GUID *pclsid, VARIANT *pvar)
 {
@@ -576,9 +598,6 @@ static HRESULT WINAPI MediaDet_get_StreamMediaType(IMediaDet* iface,
                                                    AM_MEDIA_TYPE *pVal)
 {
     MediaDetImpl *This = impl_from_IMediaDet(iface);
-    IEnumMediaTypes *types;
-    AM_MEDIA_TYPE *pmt;
-    HRESULT hr;

TRACE("(%p)\n", This);

@@ -588,22 +607,7 @@ static HRESULT WINAPI MediaDet_get_StreamMediaType(IMediaDet* iface,
     if (!This->cur_pin)
         return E_INVALIDARG;

-    hr = IPin_EnumMediaTypes(This->cur_pin, &types);
-    if (SUCCEEDED(hr))
-    {
-        hr = (IEnumMediaTypes_Next(types, 1, &pmt, NULL) == S_OK
-              ? S_OK
-              : E_NOINTERFACE);
-        IEnumMediaTypes_Release(types);
-    }
-
-    if (SUCCEEDED(hr))
-    {
-        *pVal = *pmt;
-        CoTaskMemFree(pmt);
-    }
-
-    return hr;
+    return get_pin_media_type(This->cur_pin, pVal);
 }

static HRESULT WINAPI MediaDet_GetSampleGrabber(IMediaDet* iface,

[PATCH v2 08/13] qedit: Implement MediaDet_put_Filter.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/qedit/mediadet.c | 44 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
index 046994f..3b4369d 100644
--- a/dlls/qedit/mediadet.c
+++ b/dlls/qedit/mediadet.c
@@ -333,8 +333,48 @@ static HRESULT WINAPI MediaDet_get_Filter(IMediaDet* iface, IUnknown **pVal)
 static HRESULT WINAPI MediaDet_put_Filter(IMediaDet* iface, IUnknown *newVal)
 {
     MediaDetImpl *This = impl_from_IMediaDet(iface);
-    FIXME("(%p)->(%p): not implemented!\n", This, newVal);
-    return E_NOTIMPL;
+    IGraphBuilder *gb;
+    IBaseFilter *bf;
+    HRESULT hr;
+
+    TRACE("(%p)->(%p)\n", This, newVal);
+
+    if (!newVal)
+        return E_POINTER;
+
+    hr = IUnknown_QueryInterface(newVal, &IID_IBaseFilter, (void **) &bf);
+    if (FAILED(hr))
+        return hr;
+
+    if (This->graph)
+        MD_cleanup(This);
+
+    hr = CoCreateInstance(&CLSID_FilterGraph, NULL, CLSCTX_INPROC_SERVER,
+                          &IID_IGraphBuilder, (void **) &gb);
+    if (FAILED(hr))
+    {
+        IBaseFilter_Release(bf);
+        return hr;
+    }
+
+    if (FAILED(hr = IGraphBuilder_AddFilter(gb, bf, L"Source")))
+    {
+        IGraphBuilder_Release(gb);
+        IBaseFilter_Release(bf);
+        return hr;
+    }
+
+    This->graph = gb;
+    This->source = bf;
+    hr = get_splitter(This);
+    if (FAILED(hr))
+    {
+        /* No splitter found, use the source directly */
+        This->splitter = This->source;
+        IBaseFilter_AddRef(This->splitter);
+    }
+
+    return IMediaDet_put_CurrentStream(&This->IMediaDet_iface, 0);
 }

static HRESULT WINAPI MediaDet_get_OutputStreams(IMediaDet* iface, LONG *pVal)

[PATCH v2 08/13] qedit: Implement MediaDet_put_Filter.

[Zebediah Figura]

On 4/16/20 10:25 AM, Gabriel Ivăncescu wrote:
> Signed-off-by: Gabriel Ivăncescu
>
> dlls/qedit/mediadet.c | 44 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
> index 046994f..3b4369d 100644
> --- a/dlls/qedit/mediadet.c
> +++ b/dlls/qedit/mediadet.c
> @@ -333,8 +333,48 @@ static HRESULT WINAPI MediaDet_get_Filter(IMediaDet* iface, IUnknown **pVal)
>  static HRESULT WINAPI MediaDet_put_Filter(IMediaDet* iface, IUnknown *newVal)
>  {
>      MediaDetImpl *This = impl_from_IMediaDet(iface);
> - FIXME("(%p)->(%p): not implemented!\n", This, newVal);
> - return E_NOTIMPL;
> - IGraphBuilder *gb;
> - IBaseFilter *bf;
> - HRESULT hr;
> -
> - TRACE("(%p)->(%p)\n", This, newVal);
> -
> - if (!newVal)
> -
>    return E_POINTER;
> -
> - hr = IUnknown_QueryInterface(newVal, &IID_IBaseFilter, (void **) &bf);
> - if (FAILED(hr))
> -
>    return hr;
> -
> - if (This->graph)
> -
>    MD_cleanup(This);
> -
> - hr = CoCreateInstance(&CLSID_FilterGraph, NULL, CLSCTX_INPROC_SERVER,
> -
>                      &IID_IGraphBuilder, (void **) &gb);
> - if (FAILED(hr))
> - {
> -
>    IBaseFilter_Release(bf);
> -
>    return hr;
> - }
> -
> - if (FAILED(hr = IGraphBuilder_AddFilter(gb, bf, L"Source")))
> - {
> -
>    IGraphBuilder_Release(gb);
> -
>    IBaseFilter_Release(bf);
> -
>    return hr;
> - }
> -
> - This->graph = gb;
> - This->source = bf;
> - hr = get_splitter(This);

Can we have tests for this part (i.e. that we try to find a downstream
source instead of using the given filter)?
> - if (FAILED(hr))
> - {
> -
>    /* No splitter found, use the source directly */
> -
>    This->splitter = This->source;
> -
>    IBaseFilter_AddRef(This->splitter);
> - }
> -
> - return IMediaDet_put_CurrentStream(&This->IMediaDet_iface, 0);
>
> }
>
> static HRESULT WINAPI MediaDet_get_OutputStreams(IMediaDet* iface, LONG *pVal)

[PATCH v2 09/13] qedit: Use the first pin's media type for filters without a filename.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---

The IFileSourceFilter is not queried for filters placed via put_Filter
(as shown by tests later), so obtain the media type from the first pin to
use for finding a matching splitter.

dlls/qedit/mediadet.c | 45 +++++++++++++++++++++++++++++++------------
 1 file changed, 33 insertions(+), 12 deletions(-)

diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
index 3b4369d..8def1bc 100644
--- a/dlls/qedit/mediadet.c
+++ b/dlls/qedit/mediadet.c
@@ -150,22 +150,43 @@ static HRESULT get_splitter(MediaDetImpl *This)
     if (FAILED(hr))
         return hr;

-    hr = IBaseFilter_QueryInterface(This->source, &IID_IFileSourceFilter,
-                                    (void **) &file);
-    if (FAILED(hr))
+    if (This->filename)
     {
-        IFilterMapper2_Release(map);
-        return hr;
-    }
+        hr = IBaseFilter_QueryInterface(This->source, &IID_IFileSourceFilter,
+                                        (void **) &file);
+        if (FAILED(hr))
+        {
+            IFilterMapper2_Release(map);
+            return hr;
+        }

-    hr = IFileSourceFilter_GetCurFile(file, &name, &mt);
-    IFileSourceFilter_Release(file);
-    CoTaskMemFree(name);
-    if (FAILED(hr))
+        hr = IFileSourceFilter_GetCurFile(file, &name, &mt);
+        IFileSourceFilter_Release(file);
+        CoTaskMemFree(name);
+        if (FAILED(hr))
+        {
+            IFilterMapper2_Release(map);
+            return hr;
+        }
+    }
+    else
     {
-        IFilterMapper2_Release(map);
-        return hr;
+        mt.majortype = GUID_NULL;
+        mt.subtype = GUID_NULL;
+        mt.pbFormat = NULL;
+
+        hr = IBaseFilter_EnumPins(This->source, &pins);
+        if (SUCCEEDED(hr))
+        {
+            if (IEnumPins_Next(pins, 1, &source_pin, NULL) == S_OK)
+            {
+                get_pin_media_type(source_pin, &mt);
+                IPin_Release(source_pin);
+            }
+            IEnumPins_Release(pins);
+        }
     }
+
     type[0] = mt.majortype;
     type[1] = mt.subtype;
     CoTaskMemFree(mt.pbFormat);

[PATCH v2 10/13] qedit: Remove the splitter from the graph if it failed to connect.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/qedit/mediadet.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
index 8def1bc..9a69d12 100644
--- a/dlls/qedit/mediadet.c
+++ b/dlls/qedit/mediadet.c
@@ -219,7 +219,11 @@ static HRESULT get_splitter(MediaDetImpl *This)
         VariantClear(&var);
         This->splitter = splitter;
         if (FAILED(hr))
-            goto retry;
+        {
+            IBaseFilter_Release(splitter);
+            This->splitter = NULL;
+            continue;
+        }

hr = IBaseFilter_EnumPins(This->source, &pins);
         if (FAILED(hr))
@@ -248,6 +252,7 @@ static HRESULT get_splitter(MediaDetImpl *This)
             break;

retry:
+        IGraphBuilder_RemoveFilter(This->graph, splitter);
         IBaseFilter_Release(splitter);
         This->splitter = NULL;
     }

[PATCH v2 11/13] qedit: Rename the Reader to Source.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---

Needed to pass the tests.

dlls/qedit/mediadet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
index 9a69d12..9903238 100644
--- a/dlls/qedit/mediadet.c
+++ b/dlls/qedit/mediadet.c
@@ -622,7 +622,7 @@ static HRESULT WINAPI MediaDet_put_Filename(IMediaDet* iface, BSTR newVal)
         return E_OUTOFMEMORY;
     }

-    if (FAILED(hr = IGraphBuilder_AddSourceFilter(gb, newVal, L"Reader", &bf)))
+    if (FAILED(hr = IGraphBuilder_AddSourceFilter(gb, newVal, L"Source", &bf)))
     {
         heap_free(filename);
         IGraphBuilder_Release(gb);

[PATCH v2 12/13] qedit/tests: Add tests for get_Filter and put_Filter with a custom filter.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/qedit/tests/mediadet.c | 965 ++++++++++++++++++++++++++++++++++++
 1 file changed, 965 insertions(+)

diff --git a/dlls/qedit/tests/mediadet.c b/dlls/qedit/tests/mediadet.c
index 596171b..10127cf 100644
--- a/dlls/qedit/tests/mediadet.c
+++ b/dlls/qedit/tests/mediadet.c
@@ -24,6 +24,7 @@
 #include "ole2.h"
 #include "vfwmsgs.h"
 #include "uuids.h"
+#include "dshow.h"
 #include "wine/test.h"
 #include "qedit.h"
 #include "control.h"
@@ -71,6 +72,867 @@ static const IUnknownVtbl outer_vtbl =

static IUnknown test_outer = {&outer_vtbl};

+struct testfilter
+{
+    IBaseFilter IBaseFilter_iface;
+    IMediaSeeking IMediaSeeking_iface;
+    IMediaPosition IMediaPosition_iface;
+    LONG ref;
+    WCHAR *name;
+    IFilterGraph *graph;
+    FILTER_STATE state;
+    double rate;
+
+    IEnumPins IEnumPins_iface;
+    UINT enum_idx;
+
+    IPin IPin_iface;
+    IPin *peer;
+
+    IEnumMediaTypes IEnumMediaTypes_iface;
+    UINT mt_enum_idx;
+};
+
+static inline struct testfilter *impl_from_IEnumMediaTypes(IEnumMediaTypes *iface)
+{
+    return CONTAINING_RECORD(iface, struct testfilter, IEnumMediaTypes_iface);
+}
+
+static HRESULT WINAPI testenummt_QueryInterface(IEnumMediaTypes *iface, REFIID iid, void **out)
+{
+    *out = NULL;
+    return E_NOINTERFACE;
+}
+
+static ULONG WINAPI testenummt_AddRef(IEnumMediaTypes *iface)
+{
+    struct testfilter *filter = impl_from_IEnumMediaTypes(iface);
+    return IBaseFilter_AddRef(&filter->IBaseFilter_iface);
+}
+
+static ULONG WINAPI testenummt_Release(IEnumMediaTypes *iface)
+{
+    struct testfilter *filter = impl_from_IEnumMediaTypes(iface);
+    return IBaseFilter_Release(&filter->IBaseFilter_iface);
+}
+
+static HRESULT WINAPI testenummt_Next(IEnumMediaTypes *iface, ULONG count, AM_MEDIA_TYPE **out, ULONG *fetched)
+{
+    struct testfilter *filter = impl_from_IEnumMediaTypes(iface);
+    ULONG i = 0;
+
+    if (count && !filter->mt_enum_idx)
+    {
+        static const VIDEOINFOHEADER source_format =
+        {
+            .bmiHeader.biSize = sizeof(BITMAPINFOHEADER),
+            .bmiHeader.biWidth = 640,
+            .bmiHeader.biHeight = 480,
+            .bmiHeader.biPlanes = 1,
+            .bmiHeader.biBitCount = 24,
+            .bmiHeader.biCompression = BI_RGB,
+            .bmiHeader.biSizeImage = 640 * 480 * 3
+        };
+        AM_MEDIA_TYPE source_type =
+        {
+            .majortype = MEDIATYPE_Video,
+            .subtype = MEDIASUBTYPE_RGB24,
+            .bFixedSizeSamples = TRUE,
+            .lSampleSize = source_format.bmiHeader.biSizeImage,
+            .formattype = FORMAT_VideoInfo,
+            .cbFormat = sizeof(source_format)
+        };
+
+        source_type.pbFormat = CoTaskMemAlloc(sizeof(source_format));
+        memcpy(source_type.pbFormat, &source_format, sizeof(source_format));
+
+        out[i] = CoTaskMemAlloc(sizeof(*out[i]));
+        *out[i++] = source_type;
+    }
+
+    if (fetched) *fetched = i;
+    filter->mt_enum_idx += i;
+
+    return (i == count) ? S_OK : S_FALSE;
+}
+
+static HRESULT WINAPI testenummt_Skip(IEnumMediaTypes *iface, ULONG count)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI testenummt_Reset(IEnumMediaTypes *iface)
+{
+    struct testfilter *filter = impl_from_IEnumMediaTypes(iface);
+
+    filter->mt_enum_idx = 0;
+    return S_OK;
+}
+
+static HRESULT WINAPI testenummt_Clone(IEnumMediaTypes *iface, IEnumMediaTypes **out)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static const IEnumMediaTypesVtbl testenummt_vtbl =
+{
+    testenummt_QueryInterface,
+    testenummt_AddRef,
+    testenummt_Release,
+    testenummt_Next,
+    testenummt_Skip,
+    testenummt_Reset,
+    testenummt_Clone
+};
+
+static inline struct testfilter *impl_from_IPin(IPin *iface)
+{
+    return CONTAINING_RECORD(iface, struct testfilter, IPin_iface);
+}
+
+static HRESULT WINAPI testpin_QueryInterface(IPin *iface, REFIID iid, void **out)
+{
+    struct testfilter *filter = impl_from_IPin(iface);
+
+    if (IsEqualGUID(iid, &IID_IUnknown) || IsEqualGUID(iid, &IID_IPin))
+        *out = &filter->IPin_iface;
+    else if (IsEqualGUID(iid, &IID_IMediaSeeking))
+        *out = &filter->IMediaSeeking_iface;
+    else if (IsEqualGUID(iid, &IID_IMediaPosition))
+        *out = &filter->IMediaPosition_iface;
+    else
+    {
+        *out = NULL;
+        return E_NOINTERFACE;
+    }
+
+    IUnknown_AddRef((IUnknown *)*out);
+    return S_OK;
+}
+
+static ULONG WINAPI testpin_AddRef(IPin *iface)
+{
+    struct testfilter *filter = impl_from_IPin(iface);
+    return IBaseFilter_AddRef(&filter->IBaseFilter_iface);
+}
+
+static ULONG WINAPI testpin_Release(IPin *iface)
+{
+    struct testfilter *filter = impl_from_IPin(iface);
+    return IBaseFilter_Release(&filter->IBaseFilter_iface);
+}
+
+static HRESULT WINAPI testpin_Connect(IPin *iface, IPin *peer, const AM_MEDIA_TYPE *mt)
+{
+    struct testfilter *filter = impl_from_IPin(iface);
+    IEnumMediaTypes *enummt;
+    AM_MEDIA_TYPE *pmt;
+    HRESULT hr;
+
+    ok(!mt, "Got media type %p\n", mt);
+
+    IPin_EnumMediaTypes(iface, &enummt);
+    IEnumMediaTypes_Next(enummt, 1, &pmt, NULL);
+    IEnumMediaTypes_Reset(enummt);
+    IEnumMediaTypes_Release(enummt);
+
+    hr = IPin_ReceiveConnection(peer, &filter->IPin_iface, pmt);
+    CoTaskMemFree(pmt->pbFormat);
+    CoTaskMemFree(pmt);
+    if (FAILED(hr))
+        return hr;
+
+    filter->peer = peer;
+    IPin_AddRef(peer);
+    return S_OK;
+}
+
+static HRESULT WINAPI testpin_ReceiveConnection(IPin *iface, IPin *peer, const AM_MEDIA_TYPE *mt)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI testpin_Disconnect(IPin *iface)
+{
+    struct testfilter *filter = impl_from_IPin(iface);
+
+    if (filter->peer)
+    {
+        IPin_Release(filter->peer);
+        filter->peer = NULL;
+    }
+    return S_OK;
+}
+
+static HRESULT WINAPI testpin_ConnectedTo(IPin *iface, IPin **peer)
+{
+    struct testfilter *filter = impl_from_IPin(iface);
+
+    if (!filter->peer)
+        return VFW_E_NOT_CONNECTED;
+
+    *peer = filter->peer;
+    IPin_AddRef(*peer);
+    return S_OK;
+}
+
+static HRESULT WINAPI testpin_ConnectionMediaType(IPin *iface, AM_MEDIA_TYPE *mt)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI testpin_QueryPinInfo(IPin *iface, PIN_INFO *info)
+{
+    struct testfilter *filter = impl_from_IPin(iface);
+
+    info->pFilter = &filter->IBaseFilter_iface;
+    IBaseFilter_AddRef(&filter->IBaseFilter_iface);
+    info->dir = PINDIR_OUTPUT;
+    wcscpy(info->achName, L"testpin");
+    return S_OK;
+}
+
+static HRESULT WINAPI testpin_QueryDirection(IPin *iface, PIN_DIRECTION *dir)
+{
+    *dir = PINDIR_OUTPUT;
+    return S_OK;
+}
+
+static HRESULT WINAPI testpin_QueryId(IPin *iface, WCHAR **id)
+{
+    if (!(*id = CoTaskMemAlloc(sizeof(L"deadbeef"))))
+        return E_OUTOFMEMORY;
+
+    wcscpy(*id, L"deadbeef");
+    return S_OK;
+}
+
+static HRESULT WINAPI testpin_QueryAccept(IPin *iface, const AM_MEDIA_TYPE *mt)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI testpin_EnumMediaTypes(IPin *iface, IEnumMediaTypes **out)
+{
+    struct testfilter *filter = impl_from_IPin(iface);
+
+    *out = &filter->IEnumMediaTypes_iface;
+    IEnumMediaTypes_AddRef(*out);
+    filter->mt_enum_idx = 0;
+    return S_OK;
+}
+
+static HRESULT WINAPI testpin_QueryInternalConnections(IPin *iface, IPin **out, ULONG *count)
+{
+    *count = 0;
+    return S_OK;
+}
+
+static HRESULT WINAPI testpin_EndOfStream(IPin *iface)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI testpin_BeginFlush(IPin *iface)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI testpin_EndFlush(IPin *iface)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI testpin_NewSegment(IPin *iface, REFERENCE_TIME start, REFERENCE_TIME stop, double rate)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static const IPinVtbl testpin_vtbl =
+{
+    testpin_QueryInterface,
+    testpin_AddRef,
+    testpin_Release,
+    testpin_Connect,
+    testpin_ReceiveConnection,
+    testpin_Disconnect,
+    testpin_ConnectedTo,
+    testpin_ConnectionMediaType,
+    testpin_QueryPinInfo,
+    testpin_QueryDirection,
+    testpin_QueryId,
+    testpin_QueryAccept,
+    testpin_EnumMediaTypes,
+    testpin_QueryInternalConnections,
+    testpin_EndOfStream,
+    testpin_BeginFlush,
+    testpin_EndFlush,
+    testpin_NewSegment
+};
+
+static inline struct testfilter *impl_from_IEnumPins(IEnumPins *iface)
+{
+    return CONTAINING_RECORD(iface, struct testfilter, IEnumPins_iface);
+}
+
+static HRESULT WINAPI testenumpins_QueryInterface(IEnumPins *iface, REFIID iid, void **out)
+{
+    ok(0, "Unexpected call with iid %s.\n", wine_dbgstr_guid(iid));
+    return E_NOINTERFACE;
+}
+
+static ULONG WINAPI testenumpins_AddRef(IEnumPins *iface)
+{
+    struct testfilter *filter = impl_from_IEnumPins(iface);
+    return IBaseFilter_AddRef(&filter->IBaseFilter_iface);
+}
+
+static ULONG WINAPI testenumpins_Release(IEnumPins *iface)
+{
+    struct testfilter *filter = impl_from_IEnumPins(iface);
+    return IBaseFilter_Release(&filter->IBaseFilter_iface);
+}
+
+static HRESULT WINAPI testenumpins_Next(IEnumPins *iface, ULONG count, IPin **out, ULONG *fetched)
+{
+    struct testfilter *filter = impl_from_IEnumPins(iface);
+    ULONG i = 0;
+
+    /* Only report one pin */
+    if (count && !filter->enum_idx)
+    {
+        out[i] = &filter->IPin_iface;
+        IPin_AddRef(out[i++]);
+    }
+
+    if (fetched) *fetched = i;
+    filter->enum_idx += i;
+
+    return (i == count) ? S_OK : S_FALSE;
+}
+
+static HRESULT WINAPI testenumpins_Skip(IEnumPins *iface, ULONG count)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI testenumpins_Reset(IEnumPins *iface)
+{
+    struct testfilter *filter = impl_from_IEnumPins(iface);
+
+    filter->enum_idx = 0;
+    return S_OK;
+}
+
+static HRESULT WINAPI testenumpins_Clone(IEnumPins *iface, IEnumPins **out)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static const IEnumPinsVtbl testenumpins_vtbl =
+{
+    testenumpins_QueryInterface,
+    testenumpins_AddRef,
+    testenumpins_Release,
+    testenumpins_Next,
+    testenumpins_Skip,
+    testenumpins_Reset,
+    testenumpins_Clone
+};
+
+static inline struct testfilter *impl_from_IMediaSeeking(IMediaSeeking *iface)
+{
+    return CONTAINING_RECORD(iface, struct testfilter, IMediaSeeking_iface);
+}
+
+static HRESULT WINAPI testseek_QueryInterface(IMediaSeeking *iface, REFIID iid, void **out)
+{
+    struct testfilter *filter = impl_from_IMediaSeeking(iface);
+    return IBaseFilter_QueryInterface(&filter->IBaseFilter_iface, iid, out);
+}
+
+static ULONG WINAPI testseek_AddRef(IMediaSeeking *iface)
+{
+    struct testfilter *filter = impl_from_IMediaSeeking(iface);
+    return IBaseFilter_AddRef(&filter->IBaseFilter_iface);
+}
+
+static ULONG WINAPI testseek_Release(IMediaSeeking *iface)
+{
+    struct testfilter *filter = impl_from_IMediaSeeking(iface);
+    return IBaseFilter_Release(&filter->IBaseFilter_iface);
+}
+
+static HRESULT WINAPI testseek_GetCapabilities(IMediaSeeking *iface, DWORD *caps)
+{
+    *caps = AM_SEEKING_CanGetDuration;
+    return S_OK;
+}
+
+static HRESULT WINAPI testseek_CheckCapabilities(IMediaSeeking *iface, DWORD *caps)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI testseek_IsFormatSupported(IMediaSeeking *iface, const GUID *format)
+{
+    return IsEqualGUID(format, &TIME_FORMAT_MEDIA_TIME) ? S_OK : S_FALSE;
+}
+
+static HRESULT WINAPI testseek_QueryPreferredFormat(IMediaSeeking *iface, GUID *format)
+{
+    *format = TIME_FORMAT_MEDIA_TIME;
+    return S_OK;
+}
+
+static HRESULT WINAPI testseek_GetTimeFormat(IMediaSeeking *iface, GUID *format)
+{
+    *format = TIME_FORMAT_MEDIA_TIME;
+    return S_OK;
+}
+
+static HRESULT WINAPI testseek_IsUsingTimeFormat(IMediaSeeking *iface, const GUID *format)
+{
+    return IsEqualGUID(format, &TIME_FORMAT_MEDIA_TIME) ? S_OK : S_FALSE;
+}
+
+static HRESULT WINAPI testseek_SetTimeFormat(IMediaSeeking *iface, const GUID *format)
+{
+    return IsEqualGUID(format, &TIME_FORMAT_MEDIA_TIME) ? S_OK : E_INVALIDARG;
+}
+
+static HRESULT WINAPI testseek_GetDuration(IMediaSeeking *iface, LONGLONG *duration)
+{
+    *duration = 42000000;
+    return S_OK;
+}
+
+static HRESULT WINAPI testseek_GetStopPosition(IMediaSeeking *iface, LONGLONG *stop)
+{
+    *stop = 42000000;
+    return S_OK;
+}
+
+static HRESULT WINAPI testseek_GetCurrentPosition(IMediaSeeking *iface, LONGLONG *current)
+{
+    *current = 0;
+    return S_OK;
+}
+
+static HRESULT WINAPI testseek_ConvertTimeFormat(IMediaSeeking *iface, LONGLONG *target,
+    const GUID *target_format, LONGLONG source, const GUID *source_format)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI testseek_SetPositions(IMediaSeeking *iface, LONGLONG *current,
+    DWORD current_flags, LONGLONG *stop, DWORD stop_flags)
+{
+    *current = 0;
+    *stop = 42000000;
+    return S_OK;
+}
+
+static HRESULT WINAPI testseek_GetPositions(IMediaSeeking *iface, LONGLONG *current, LONGLONG *stop)
+{
+    *current = 0;
+    *stop = 42000000;
+    return S_OK;
+}
+
+static HRESULT WINAPI testseek_GetAvailable(IMediaSeeking *iface, LONGLONG *earliest, LONGLONG *latest)
+{
+    *earliest = 0;
+    *latest = 42000000;
+    return S_OK;
+}
+
+static HRESULT WINAPI testseek_SetRate(IMediaSeeking *iface, double rate)
+{
+    struct testfilter *filter = impl_from_IMediaSeeking(iface);
+
+    filter->rate = rate;
+    return S_OK;
+}
+
+static HRESULT WINAPI testseek_GetRate(IMediaSeeking *iface, double *rate)
+{
+    struct testfilter *filter = impl_from_IMediaSeeking(iface);
+
+    *rate = filter->rate;
+    return S_OK;
+}
+
+static HRESULT WINAPI testseek_GetPreroll(IMediaSeeking *iface, LONGLONG *preroll)
+{
+    *preroll = 0;
+    return S_OK;
+}
+
+static const IMediaSeekingVtbl testseek_vtbl =
+{
+    testseek_QueryInterface,
+    testseek_AddRef,
+    testseek_Release,
+    testseek_GetCapabilities,
+    testseek_CheckCapabilities,
+    testseek_IsFormatSupported,
+    testseek_QueryPreferredFormat,
+    testseek_GetTimeFormat,
+    testseek_IsUsingTimeFormat,
+    testseek_SetTimeFormat,
+    testseek_GetDuration,
+    testseek_GetStopPosition,
+    testseek_GetCurrentPosition,
+    testseek_ConvertTimeFormat,
+    testseek_SetPositions,
+    testseek_GetPositions,
+    testseek_GetAvailable,
+    testseek_SetRate,
+    testseek_GetRate,
+    testseek_GetPreroll
+};
+
+static inline struct testfilter *impl_from_IMediaPosition(IMediaPosition *iface)
+{
+    return CONTAINING_RECORD(iface, struct testfilter, IMediaPosition_iface);
+}
+
+static HRESULT WINAPI testpos_QueryInterface(IMediaPosition *iface, REFIID iid, void **out)
+{
+    struct testfilter *filter = impl_from_IMediaPosition(iface);
+    return IBaseFilter_QueryInterface(&filter->IBaseFilter_iface, iid, out);
+}
+
+static ULONG WINAPI testpos_AddRef(IMediaPosition *iface)
+{
+    struct testfilter *filter = impl_from_IMediaPosition(iface);
+    return IBaseFilter_AddRef(&filter->IBaseFilter_iface);
+}
+
+static ULONG WINAPI testpos_Release(IMediaPosition *iface)
+{
+    struct testfilter *filter = impl_from_IMediaPosition(iface);
+    return IBaseFilter_Release(&filter->IBaseFilter_iface);
+}
+
+static HRESULT WINAPI testpos_GetTypeInfoCount(IMediaPosition *iface, UINT *pctinfo)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI testpos_GetTypeInfo(IMediaPosition *iface, UINT iTInfo, LCID lcid, ITypeInfo **ppTInfo)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI testpos_GetIDsOfNames(IMediaPosition *iface, REFIID riid, LPOLESTR *rgszNames,
+    UINT cNames, LCID lcid, DISPID *rgDispId)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI testpos_Invoke(IMediaPosition *iface, DISPID dispIdMember, REFIID riid, LCID lcid,
+    WORD wFlags, DISPPARAMS *pDispParams, VARIANT *pVarResult, EXCEPINFO *pExcepInfo, UINT *puArgErr)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI testpos_get_Duration(IMediaPosition *iface, REFTIME *duration)
+{
+    *duration = 4.2;
+    return S_OK;
+}
+
+static HRESULT WINAPI testpos_put_CurrentPosition(IMediaPosition *iface, REFTIME current)
+{
+    return S_OK;
+}
+
+static HRESULT WINAPI testpos_get_CurrentPosition(IMediaPosition *iface, REFTIME *current)
+{
+    *current = 0.0;
+    return S_OK;
+}
+
+static HRESULT WINAPI testpos_get_StopTime(IMediaPosition *iface, REFTIME *stop)
+{
+    *stop = 4.2;
+    return S_OK;
+}
+
+static HRESULT WINAPI testpos_put_StopTime(IMediaPosition *iface, REFTIME stop)
+{
+    return S_OK;
+}
+
+static HRESULT WINAPI testpos_get_PrerollTime(IMediaPosition *iface, REFTIME *preroll)
+{
+    *preroll = 0.0;
+    return S_OK;
+}
+
+static HRESULT WINAPI testpos_put_PrerollTime(IMediaPosition *iface, REFTIME preroll)
+{
+    return S_OK;
+}
+
+static HRESULT WINAPI testpos_put_Rate(IMediaPosition *iface, double rate)
+{
+    struct testfilter *filter = impl_from_IMediaPosition(iface);
+
+    filter->rate = rate;
+    return S_OK;
+}
+
+static HRESULT WINAPI testpos_get_Rate(IMediaPosition *iface, double *rate)
+{
+    struct testfilter *filter = impl_from_IMediaPosition(iface);
+
+    *rate = filter->rate;
+    return S_OK;
+}
+
+static HRESULT WINAPI testpos_CanSeekForward(IMediaPosition *iface, LONG *can_seek)
+{
+    *can_seek = OAFALSE;
+    return S_OK;
+}
+
+static HRESULT WINAPI testpos_CanSeekBackward(IMediaPosition *iface, LONG *can_seek)
+{
+    *can_seek = OAFALSE;
+    return S_OK;
+}
+
+static const IMediaPositionVtbl testpos_vtbl =
+{
+    testpos_QueryInterface,
+    testpos_AddRef,
+    testpos_Release,
+    testpos_GetTypeInfoCount,
+    testpos_GetTypeInfo,
+    testpos_GetIDsOfNames,
+    testpos_Invoke,
+    testpos_get_Duration,
+    testpos_put_CurrentPosition,
+    testpos_get_CurrentPosition,
+    testpos_get_StopTime,
+    testpos_put_StopTime,
+    testpos_get_PrerollTime,
+    testpos_put_PrerollTime,
+    testpos_put_Rate,
+    testpos_get_Rate,
+    testpos_CanSeekForward,
+    testpos_CanSeekBackward
+};
+
+static inline struct testfilter *impl_from_IBaseFilter(IBaseFilter *iface)
+{
+    return CONTAINING_RECORD(iface, struct testfilter, IBaseFilter_iface);
+}
+
+static HRESULT WINAPI testfilter_QueryInterface(IBaseFilter *iface, REFIID iid, void **out)
+{
+    struct testfilter *filter = impl_from_IBaseFilter(iface);
+
+    if (IsEqualGUID(iid, &IID_IUnknown)
+            || IsEqualGUID(iid, &IID_IPersist)
+            || IsEqualGUID(iid, &IID_IMediaFilter)
+            || IsEqualGUID(iid, &IID_IBaseFilter))
+    {
+        *out = &filter->IBaseFilter_iface;
+    }
+    else if (IsEqualGUID(iid, &IID_IMediaSeeking))
+        *out = &filter->IMediaSeeking_iface;
+    else if (IsEqualGUID(iid, &IID_IMediaPosition))
+        *out = &filter->IMediaPosition_iface;
+    else
+    {
+        if (IsEqualGUID(iid, &IID_IFileSourceFilter)) ok(0, "Filter queried for IFileSourceFilter\n");
+
+        *out = NULL;
+        return E_NOINTERFACE;
+    }
+
+    IUnknown_AddRef((IUnknown *)*out);
+    return S_OK;
+}
+
+static ULONG WINAPI testfilter_AddRef(IBaseFilter *iface)
+{
+    struct testfilter *filter = impl_from_IBaseFilter(iface);
+    return InterlockedIncrement(&filter->ref);
+}
+
+static ULONG WINAPI testfilter_Release(IBaseFilter *iface)
+{
+    struct testfilter *filter = impl_from_IBaseFilter(iface);
+    LONG ref = InterlockedDecrement(&filter->ref);
+
+    if (!ref)
+    {
+        HeapFree(GetProcessHeap(), 0, filter->name);
+        HeapFree(GetProcessHeap(), 0, filter);
+    }
+    return ref;
+}
+
+static HRESULT WINAPI testfilter_GetClassID(IBaseFilter *iface, CLSID *clsid)
+{
+    memset(clsid, 0xde, sizeof(*clsid));
+    return S_OK;
+}
+
+static HRESULT WINAPI testfilter_Stop(IBaseFilter *iface)
+{
+    struct testfilter *filter = impl_from_IBaseFilter(iface);
+
+    filter->state = State_Stopped;
+    return S_OK;
+}
+
+static HRESULT WINAPI testfilter_Pause(IBaseFilter *iface)
+{
+    struct testfilter *filter = impl_from_IBaseFilter(iface);
+
+    filter->state = State_Paused;
+    return S_OK;
+}
+
+static HRESULT WINAPI testfilter_Run(IBaseFilter *iface, REFERENCE_TIME start)
+{
+    struct testfilter *filter = impl_from_IBaseFilter(iface);
+
+    filter->state = State_Running;
+    return S_OK;
+}
+
+static HRESULT WINAPI testfilter_GetState(IBaseFilter *iface, DWORD timeout, FILTER_STATE *state)
+{
+    struct testfilter *filter = impl_from_IBaseFilter(iface);
+
+    *state = filter->state;
+    return S_OK;
+}
+
+static HRESULT WINAPI testfilter_SetSyncSource(IBaseFilter *iface, IReferenceClock *clock)
+{
+    return S_OK;
+}
+
+static HRESULT WINAPI testfilter_GetSyncSource(IBaseFilter *iface, IReferenceClock **clock)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI testfilter_EnumPins(IBaseFilter *iface, IEnumPins **out)
+{
+    struct testfilter *filter = impl_from_IBaseFilter(iface);
+
+    *out = &filter->IEnumPins_iface;
+    IEnumPins_AddRef(*out);
+    filter->enum_idx = 0;
+    return S_OK;
+}
+
+static HRESULT WINAPI testfilter_FindPin(IBaseFilter *iface, const WCHAR *id, IPin **pin)
+{
+    ok(0, "Unexpected call.\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI testfilter_QueryFilterInfo(IBaseFilter *iface, FILTER_INFO *info)
+{
+    struct testfilter *filter = impl_from_IBaseFilter(iface);
+
+    info->pGraph = filter->graph;
+    if (filter->graph)
+        IFilterGraph_AddRef(filter->graph);
+    if (filter->name)
+        wcscpy(info->achName, filter->name);
+    else
+        info->achName[0] = 0;
+    return S_OK;
+}
+
+static HRESULT WINAPI testfilter_JoinFilterGraph(IBaseFilter *iface, IFilterGraph *graph, const WCHAR *name)
+{
+    struct testfilter *filter = impl_from_IBaseFilter(iface);
+
+    filter->graph = graph;
+    HeapFree(GetProcessHeap(), 0, filter->name);
+    if (name)
+    {
+        filter->name = HeapAlloc(GetProcessHeap(), 0, (wcslen(name) + 1) * sizeof(WCHAR));
+        wcscpy(filter->name, name);
+    }
+    else
+        filter->name = NULL;
+    return S_OK;
+}
+
+static HRESULT WINAPI testfilter_QueryVendorInfo(IBaseFilter *iface, WCHAR **info)
+{
+    return E_NOTIMPL;
+}
+
+static const IBaseFilterVtbl testfilter_vtbl =
+{
+    testfilter_QueryInterface,
+    testfilter_AddRef,
+    testfilter_Release,
+    testfilter_GetClassID,
+    testfilter_Stop,
+    testfilter_Pause,
+    testfilter_Run,
+    testfilter_GetState,
+    testfilter_SetSyncSource,
+    testfilter_GetSyncSource,
+    testfilter_EnumPins,
+    testfilter_FindPin,
+    testfilter_QueryFilterInfo,
+    testfilter_JoinFilterGraph,
+    testfilter_QueryVendorInfo
+};
+
+static IBaseFilter *create_testfilter(void)
+{
+    struct testfilter *filter = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*filter));
+
+    if (!filter) return NULL;
+
+    filter->IBaseFilter_iface.lpVtbl = &testfilter_vtbl;
+    filter->IMediaSeeking_iface.lpVtbl = &testseek_vtbl;
+    filter->IMediaPosition_iface.lpVtbl = &testpos_vtbl;
+    filter->IEnumPins_iface.lpVtbl = &testenumpins_vtbl;
+    filter->IPin_iface.lpVtbl = &testpin_vtbl;
+    filter->IEnumMediaTypes_iface.lpVtbl = &testenummt_vtbl;
+    filter->ref = 1;
+    filter->state = State_Stopped;
+    filter->rate = 1.0;
+
+    return &filter->IBaseFilter_iface;
+}
+
 static void test_aggregation(void)
 {
     IMediaDet *detector, *detector2;
@@ -188,13 +1050,17 @@ static BOOL init_tests(void)
 static void test_mediadet(void)
 {
     HRESULT hr;
+    IBaseFilter *filter, *filter2;
+    FILTER_INFO filter_info;
     IMediaDet *pM = NULL;
     BSTR filename = NULL;
+    IFilterGraph *graph;
     LONG nstrms = 0;
     LONG strm;
     GUID guid;
     BSTR bstr;
     AM_MEDIA_TYPE mt;
+    IUnknown *unk;
     double fps;
     int flags;
     int i;
@@ -256,6 +1122,61 @@ 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_Filter(pM, NULL);
+    ok(hr == E_POINTER, "IMediaDet_get_Filter failed: %08x\n", hr);
+
+    unk = (IUnknown*)0xdeadbeef;
+    hr = IMediaDet_get_Filter(pM, &unk);
+    ok(hr == S_FALSE, "IMediaDet_get_Filter failed: %08x\n", hr);
+    ok(unk == NULL, "Wrong filter %p\n", unk);
+
+    hr = IMediaDet_put_Filter(pM, NULL);
+    ok(hr == E_POINTER, "IMediaDet_put_Filter failed: %08x\n", hr);
+
+    filter = create_testfilter();
+    hr = IMediaDet_put_Filter(pM, (IUnknown*)filter);
+    ok(hr == S_OK, "IMediaDet_put_Filter failed: %08x\n", hr);
+
+    hr = IMediaDet_get_Filter(pM, &unk);
+    ok(hr == S_OK, "IMediaDet_get_Filter failed: %08x\n", hr);
+    ok(unk != NULL, "NULL filter\n");
+    hr = IUnknown_QueryInterface(unk, &IID_IBaseFilter, (void**)&filter2);
+    IUnknown_Release(unk);
+    ok(hr == S_OK, "Could not get IBaseFilter interface: %08x\n", hr);
+    ok(filter == filter2, "Wrong filter\n");
+    IBaseFilter_Release(filter2);
+
+    hr = IBaseFilter_QueryFilterInfo(filter, &filter_info);
+    ok(hr == S_OK, "IBaseFilter_QueryFilterInfo failed: %08x\n", hr);
+    ok(!wcscmp(filter_info.achName, L"Source"), "Wrong filter name: %s\n", wine_dbgstr_w(filter_info.achName));
+    IBaseFilter_Release(filter);
+    graph = filter_info.pGraph;
+
+    filter = create_testfilter();
+    hr = IMediaDet_put_Filter(pM, (IUnknown*)filter);
+    ok(hr == S_OK, "IMediaDet_put_Filter failed: %08x\n", hr);
+    IBaseFilter_Release(filter);
+
+    hr = IMediaDet_get_Filter(pM, &unk);
+    ok(hr == S_OK, "IMediaDet_get_Filter failed: %08x\n", hr);
+    ok(unk != NULL, "NULL filter\n");
+    hr = IUnknown_QueryInterface(unk, &IID_IBaseFilter, (void**)&filter);
+    IUnknown_Release(unk);
+    ok(hr == S_OK, "Could not get IBaseFilter interface: %08x\n", hr);
+
+    hr = IBaseFilter_QueryFilterInfo(filter, &filter_info);
+    ok(hr == S_OK, "IBaseFilter_QueryFilterInfo failed: %08x\n", hr);
+    ok(!wcscmp(filter_info.achName, L"Source"), "Wrong filter name: %s\n", wine_dbgstr_w(filter_info.achName));
+    ok(graph != filter_info.pGraph, "Same filter graph was used\n");
+    IFilterGraph_Release(filter_info.pGraph);
+    IFilterGraph_Release(graph);
+    IBaseFilter_Release(filter);
+
+    strm = -1;
+    hr = IMediaDet_get_CurrentStream(pM, &strm);
+    ok(hr == S_OK, "IMediaDet_get_CurrentStream failed: %08x\n", hr);
+    ok(strm == 0, "IMediaDet_get_CurrentStream: strm is %i\n", strm);
+
     filename = SysAllocString(test_avi_filename);
     hr = IMediaDet_put_Filename(pM, filename);
     ok(hr == S_OK, "IMediaDet_put_Filename failed: %08x\n", hr);
@@ -427,6 +1348,50 @@ static void test_mediadet(void)
     ok(hr == S_OK, "IMediaDet_get_CurrentStream failed: %08x\n", hr);
     ok(strm == 1, "IMediaDet_get_CurrentStream: strm is %i\n", strm);

+    hr = IMediaDet_get_Filter(pM, &unk);
+    ok(hr == S_OK, "IMediaDet_get_Filter failed: %08x\n", hr);
+    ok(unk != NULL, "NULL filter\n");
+    hr = IUnknown_QueryInterface(unk, &IID_IBaseFilter, (void**)&filter2);
+    IUnknown_Release(unk);
+    ok(hr == S_OK, "Could not get IBaseFilter interface: %08x\n", hr);
+
+    hr = IBaseFilter_QueryFilterInfo(filter2, &filter_info);
+    ok(hr == S_OK, "IBaseFilter_QueryFilterInfo failed: %08x\n", hr);
+    ok(!wcscmp(filter_info.achName, L"Source"), "Wrong filter name: %s\n", wine_dbgstr_w(filter_info.achName));
+    graph = filter_info.pGraph;
+
+    filter = create_testfilter();
+    hr = IMediaDet_put_Filter(pM, (IUnknown*)filter);
+    ok(hr == S_OK, "IMediaDet_put_Filter failed: %08x\n", hr);
+    IBaseFilter_Release(filter);
+
+    hr = IMediaDet_get_Filter(pM, &unk);
+    ok(hr == S_OK, "IMediaDet_get_Filter failed: %08x\n", hr);
+    ok(unk != NULL, "NULL filter\n");
+    hr = IUnknown_QueryInterface(unk, &IID_IBaseFilter, (void**)&filter);
+    IUnknown_Release(unk);
+    ok(hr == S_OK, "Could not get IBaseFilter interface: %08x\n", hr);
+    ok(filter != filter2, "Same filter\n");
+    IBaseFilter_Release(filter2);
+
+    hr = IBaseFilter_QueryFilterInfo(filter, &filter_info);
+    ok(hr == S_OK, "IBaseFilter_QueryFilterInfo failed: %08x\n", hr);
+    ok(!wcscmp(filter_info.achName, L"Source"), "Wrong filter name: %s\n", wine_dbgstr_w(filter_info.achName));
+    ok(graph != filter_info.pGraph, "Same filter graph was used\n");
+    IFilterGraph_Release(filter_info.pGraph);
+    IFilterGraph_Release(graph);
+    IBaseFilter_Release(filter);
+
+    filename = NULL;
+    hr = IMediaDet_get_Filename(pM, &filename);
+    ok(hr == S_OK, "IMediaDet_get_Filename failed: %08x\n", hr);
+    ok(!filename, "Expected NULL filename, got %s.\n", debugstr_w(filename));
+    SysFreeString(filename);
+
+    hr = IMediaDet_get_OutputStreams(pM, &nstrms);
+    ok(hr == S_OK, "IMediaDet_get_OutputStreams failed: %08x\n", hr);
+    ok(nstrms == 1, "IMediaDet_get_OutputStreams: nstrms is %i\n", nstrms);
+
     hr = IMediaDet_Release(pM);
     ok(hr == 0, "IMediaDet_Release returned: %x\n", hr);

[PATCH v2 12/13] qedit/tests: Add tests for get_Filter and put_Filter with a custom filter.

[Zebediah Figura]

On 4/16/20 10:25 AM, Gabriel Ivăncescu wrote:
> Signed-off-by: Gabriel Ivăncescu
>
> dlls/qedit/tests/mediadet.c | 965 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 965 insertions(+)

It might also be nice to convert the rest of test_mediadet() to use a
custom filter, instead of relying on behaviour of the native AVI
demuxer. I won't call that necessary for this series, although I don't
particularly like that test_mediadet() is now a mixture of two different
source filters; it'd be preferable to me to at least add these tests in
a separate function.

Also, can this patch be moved earlier in the series? It's nice to have
tests before fixes, so we can see more clearly what's fixed by each
patch. That might require temporarily guarding some code by e.g. "if (hr
== S_OK)". If it can't be done reasonably, though, that's fine.
> diff --git a/dlls/qedit/tests/mediadet.c b/dlls/qedit/tests/mediadet.c
> index 596171b..10127cf 100644
> --- a/dlls/qedit/tests/mediadet.c
> +++ b/dlls/qedit/tests/mediadet.c
> @@ -24,6 +24,7 @@
>  #include "ole2.h"
>  #include "vfwmsgs.h"
>  #include "uuids.h"
> +#include "dshow.h"
>  #include "wine/test.h"
>  #include "qedit.h"
>  #include "control.h"
> @@ -71,6 +72,867 @@ static const IUnknownVtbl outer_vtbl =
>
> static IUnknown test_outer = {&outer_vtbl};
>
> +struct testfilter
> +{
> - IBaseFilter IBaseFilter_iface;
> - IMediaSeeking IMediaSeeking_iface;
> - IMediaPosition IMediaPosition_iface;
> - LONG ref;
> - WCHAR *name;
> - IFilterGraph *graph;
> - FILTER_STATE state;
> - double rate;
> -
> - IEnumPins IEnumPins_iface;
> - UINT enum_idx;
> -
> - IPin IPin_iface;
> - IPin *peer;
> -
> - IEnumMediaTypes IEnumMediaTypes_iface;
> - UINT mt_enum_idx;
>
> +};
> -

Could we use strmbase here instead?

...
> +static inline struct testfilter *impl_from_IMediaSeeking(IMediaSeeking *iface)
> +{
> - return CONTAINING_RECORD(iface, struct testfilter, IMediaSeeking_iface);
>
> +}
> -
>
> +static HRESULT WINAPI testseek_QueryInterface(IMediaSeeking *iface, REFIID iid, void **out)
> +{
> - struct testfilter *filter = impl_from_IMediaSeeking(iface);
> - return IBaseFilter_QueryInterface(&filter->IBaseFilter_iface, iid, out);
>
> +}
> -
>
> +static ULONG WINAPI testseek_AddRef(IMediaSeeking *iface)
> +{
> - struct testfilter *filter = impl_from_IMediaSeeking(iface);
> - return IBaseFilter_AddRef(&filter->IBaseFilter_iface);
>
> +}
> -
>
> +static ULONG WINAPI testseek_Release(IMediaSeeking *iface)
> +{
> - struct testfilter *filter = impl_from_IMediaSeeking(iface);
> - return IBaseFilter_Release(&filter->IBaseFilter_iface);
>
> +}
> -
>
> +static HRESULT WINAPI testseek_GetCapabilities(IMediaSeeking *iface, DWORD *caps)
> +{
> - *caps = AM_SEEKING_CanGetDuration;
> - return S_OK;
>
> +}

Can you please add trace messages to locally defined methods, along the
lines of "if (winetest_debug > 1) trace("GetCapabilities()\n");"
> -
>
> +static HRESULT WINAPI testseek_CheckCapabilities(IMediaSeeking *iface, DWORD *caps)
> +{
> - ok(0, "Unexpected call.\n");
> - return E_NOTIMPL;
>
> +}
> -
>
> +static HRESULT WINAPI testseek_IsFormatSupported(IMediaSeeking *iface, const GUID *format)
> +{
> - return IsEqualGUID(format, &TIME_FORMAT_MEDIA_TIME) ? S_OK : S_FALSE;
>
> +}
> -
>
> +static HRESULT WINAPI testseek_QueryPreferredFormat(IMediaSeeking *iface, GUID *format)
> +{
> - *format = TIME_FORMAT_MEDIA_TIME;
> - return S_OK;
>
> +}
> -
>
> +static HRESULT WINAPI testseek_GetTimeFormat(IMediaSeeking *iface, GUID *format)
> +{
> - *format = TIME_FORMAT_MEDIA_TIME;
> - return S_OK;
>
> +}
> -
>
> +static HRESULT WINAPI testseek_IsUsingTimeFormat(IMediaSeeking *iface, const GUID *format)
> +{
> - return IsEqualGUID(format, &TIME_FORMAT_MEDIA_TIME) ? S_OK : S_FALSE;
>
> +}
> -
>
> +static HRESULT WINAPI testseek_SetTimeFormat(IMediaSeeking *iface, const GUID *format)
> +{
> - return IsEqualGUID(format, &TIME_FORMAT_MEDIA_TIME) ? S_OK : E_INVALIDARG;
>
> +}
> -
>
> +static HRESULT WINAPI testseek_GetDuration(IMediaSeeking *iface, LONGLONG *duration)
> +{
> - *duration = 42000000;
> - return S_OK;
>
> +}
> -
>
> +static HRESULT WINAPI testseek_GetStopPosition(IMediaSeeking *iface, LONGLONG *stop)
> +{
> - *stop = 42000000;
> - return S_OK;
>
> +}
> -
>
> +static HRESULT WINAPI testseek_GetCurrentPosition(IMediaSeeking *iface, LONGLONG *current)
> +{
> - *current = 0;
> - return S_OK;
>
> +}
> -
>
> +static HRESULT WINAPI testseek_ConvertTimeFormat(IMediaSeeking *iface, LONGLONG *target,
> - const GUID *target_format, LONGLONG source, const GUID *source_format)
>
> +{
> - ok(0, "Unexpected call.\n");
> - return E_NOTIMPL;
>
> +}
> -
>
> +static HRESULT WINAPI testseek_SetPositions(IMediaSeeking *iface, LONGLONG *current,
> - DWORD current_flags, LONGLONG *stop, DWORD stop_flags)
>
> +{
> - *current = 0;
> - *stop = 42000000;
> - return S_OK;
>
> +}
> -
>
> +static HRESULT WINAPI testseek_GetPositions(IMediaSeeking *iface, LONGLONG *current, LONGLONG *stop)
> +{
> - *current = 0;
> - *stop = 42000000;
> - return S_OK;
>
> +}
> -
>
> +static HRESULT WINAPI testseek_GetAvailable(IMediaSeeking *iface, LONGLONG *earliest, LONGLONG *latest)
> +{
> - *earliest = 0;
> - *latest = 42000000;
> - return S_OK;
>
> +}
> -
>
> +static HRESULT WINAPI testseek_SetRate(IMediaSeeking *iface, double rate)
> +{
> - struct testfilter *filter = impl_from_IMediaSeeking(iface);
> -
> - filter->rate = rate;
> - return S_OK;
>
> +}
> -
>
> +static HRESULT WINAPI testseek_GetRate(IMediaSeeking *iface, double *rate)
> +{
> - struct testfilter *filter = impl_from_IMediaSeeking(iface);
> -
> - *rate = filter->rate;
> - return S_OK;
>
> +}
> -
>
> +static HRESULT WINAPI testseek_GetPreroll(IMediaSeeking *iface, LONGLONG *preroll)
> +{
> - *preroll = 0;
> - return S_OK;
>
> +}

Do you need to implement all of these methods? Does the implementation
need to return S_OK, or can it return E_NOTIMPL? Same for IMediaPosition
methods below.

...
> +static IBaseFilter *create_testfilter(void)
> +{
> - struct testfilter *filter = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*filter));
> -
> - if (!filter) return NULL;

I don't think we need to allocate this from heap. Note though that if
you do, we can use malloc() instead, and checking for failure in tests
is not really necessary.
> -
> - filter->IBaseFilter_iface.lpVtbl = &testfilter_vtbl;
> - filter->IMediaSeeking_iface.lpVtbl = &testseek_vtbl;
> - filter->IMediaPosition_iface.lpVtbl = &testpos_vtbl;
> - filter->IEnumPins_iface.lpVtbl = &testenumpins_vtbl;
> - filter->IPin_iface.lpVtbl = &testpin_vtbl;
> - filter->IEnumMediaTypes_iface.lpVtbl = &testenummt_vtbl;
> - filter->ref = 1;
> - filter->state = State_Stopped;
> - filter->rate = 1.0;
> -
> - return &filter->IBaseFilter_iface;
>
> +}
> -
>
> static void test_aggregation(void)
>  {
>      IMediaDet *detector, *detector2;
> @@ -188,13 +1050,17 @@ static BOOL init_tests(void)
>  static void test_mediadet(void)
>  {
>      HRESULT hr;
> - IBaseFilter *filter, *filter2;
> - FILTER_INFO filter_info;
> IMediaDet *pM = NULL;
> BSTR filename = NULL;
> - IFilterGraph *graph;
> LONG nstrms = 0;
> LONG strm;
> GUID guid;
> BSTR bstr;
> AM_MEDIA_TYPE mt;
> - IUnknown *unk;
> double fps;
> int flags;
> int i;
>
> @@ -256,6 +1122,61 @@ 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_Filter(pM, NULL);
> - ok(hr == E_POINTER, "IMediaDet_get_Filter failed: %08x\n", hr);
> -
> - unk = (IUnknown*)0xdeadbeef;
> - hr = IMediaDet_get_Filter(pM, &unk);
> - ok(hr == S_FALSE, "IMediaDet_get_Filter failed: %08x\n", hr);
> - ok(unk == NULL, "Wrong filter %p\n", unk);
> -
> - hr = IMediaDet_put_Filter(pM, NULL);
> - ok(hr == E_POINTER, "IMediaDet_put_Filter failed: %08x\n", hr);
> -
> - filter = create_testfilter();
> - hr = IMediaDet_put_Filter(pM, (IUnknown*)filter);
> - ok(hr == S_OK, "IMediaDet_put_Filter failed: %08x\n", hr);
> -
> - hr = IMediaDet_get_Filter(pM, &unk);
> - ok(hr == S_OK, "IMediaDet_get_Filter failed: %08x\n", hr);
> - ok(unk != NULL, "NULL filter\n");
> - hr = IUnknown_QueryInterface(unk, &IID_IBaseFilter, (void**)&filter2);
> - IUnknown_Release(unk);
> - ok(hr == S_OK, "Could not get IBaseFilter interface: %08x\n", hr);
> - ok(filter == filter2, "Wrong filter\n");
> - IBaseFilter_Release(filter2);

Couldn't you compare it directly to filter->IBaseFiler_iface instead?
> -
> - hr = IBaseFilter_QueryFilterInfo(filter, &filter_info);
> - ok(hr == S_OK, "IBaseFilter_QueryFilterInfo failed: %08x\n", hr);
> - ok(!wcscmp(filter_info.achName, L"Source"), "Wrong filter name: %s\n", wine_dbgstr_w(filter_info.achName));
> - IBaseFilter_Release(filter);
> - graph = filter_info.pGraph;

Similarly, you could just access filter->name directly instead of
calling IBaseFilter::QueryFilterInfo().
> -
> - filter = create_testfilter();
> - hr = IMediaDet_put_Filter(pM, (IUnknown*)filter);
> - ok(hr == S_OK, "IMediaDet_put_Filter failed: %08x\n", hr);
> - IBaseFilter_Release(filter);
> -
> - hr = IMediaDet_get_Filter(pM, &unk);
> - ok(hr == S_OK, "IMediaDet_get_Filter failed: %08x\n", hr);
> - ok(unk != NULL, "NULL filter\n");
> - hr = IUnknown_QueryInterface(unk, &IID_IBaseFilter, (void**)&filter);
> - IUnknown_Release(unk);
> - ok(hr == S_OK, "Could not get IBaseFilter interface: %08x\n", hr);
> -
> - hr = IBaseFilter_QueryFilterInfo(filter, &filter_info);
> - ok(hr == S_OK, "IBaseFilter_QueryFilterInfo failed: %08x\n", hr);
> - ok(!wcscmp(filter_info.achName, L"Source"), "Wrong filter name: %s\n", wine_dbgstr_w(filter_info.achName));
> - ok(graph != filter_info.pGraph, "Same filter graph was used\n");
> - IFilterGraph_Release(filter_info.pGraph);
> - IFilterGraph_Release(graph);
> - IBaseFilter_Release(filter);
> -
> - strm = -1;
> - hr = IMediaDet_get_CurrentStream(pM, &strm);
> - ok(hr == S_OK, "IMediaDet_get_CurrentStream failed: %08x\n", hr);
> - ok(strm == 0, "IMediaDet_get_CurrentStream: strm is %i\n", strm);
> - filename = SysAllocString(test_avi_filename);
>    hr = IMediaDet_put_Filename(pM, filename);
>    ok(hr == S_OK, "IMediaDet_put_Filename failed: %08x\n", hr);
>
> @@ -427,6 +1348,50 @@ static void test_mediadet(void)
>      ok(hr == S_OK, "IMediaDet_get_CurrentStream failed: %08x\n", hr);
>      ok(strm == 1, "IMediaDet_get_CurrentStream: strm is %i\n", strm);
> - hr = IMediaDet_get_Filter(pM, &unk);
> - ok(hr == S_OK, "IMediaDet_get_Filter failed: %08x\n", hr);
> - ok(unk != NULL, "NULL filter\n");
> - hr = IUnknown_QueryInterface(unk, &IID_IBaseFilter, (void**)&filter2);
> - IUnknown_Release(unk);
> - ok(hr == S_OK, "Could not get IBaseFilter interface: %08x\n", hr);
> -
> - hr = IBaseFilter_QueryFilterInfo(filter2, &filter_info);
> - ok(hr == S_OK, "IBaseFilter_QueryFilterInfo failed: %08x\n", hr);
> - ok(!wcscmp(filter_info.achName, L"Source"), "Wrong filter name: %s\n", wine_dbgstr_w(filter_info.achName));
> - graph = filter_info.pGraph;
> -
> - filter = create_testfilter();
> - hr = IMediaDet_put_Filter(pM, (IUnknown*)filter);
> - ok(hr == S_OK, "IMediaDet_put_Filter failed: %08x\n", hr);
> - IBaseFilter_Release(filter);
> -
> - hr = IMediaDet_get_Filter(pM, &unk);
> - ok(hr == S_OK, "IMediaDet_get_Filter failed: %08x\n", hr);
> - ok(unk != NULL, "NULL filter\n");
> - hr = IUnknown_QueryInterface(unk, &IID_IBaseFilter, (void**)&filter);
> - IUnknown_Release(unk);
> - ok(hr == S_OK, "Could not get IBaseFilter interface: %08x\n", hr);
> - ok(filter != filter2, "Same filter\n");
> - IBaseFilter_Release(filter2);
> -
> - hr = IBaseFilter_QueryFilterInfo(filter, &filter_info);
> - ok(hr == S_OK, "IBaseFilter_QueryFilterInfo failed: %08x\n", hr);
> - ok(!wcscmp(filter_info.achName, L"Source"), "Wrong filter name: %s\n", wine_dbgstr_w(filter_info.achName));
> - ok(graph != filter_info.pGraph, "Same filter graph was used\n");
> - IFilterGraph_Release(filter_info.pGraph);
> - IFilterGraph_Release(graph);
> - IBaseFilter_Release(filter);
> -
> - filename = NULL;
> - hr = IMediaDet_get_Filename(pM, &filename);
> - ok(hr == S_OK, "IMediaDet_get_Filename failed: %08x\n", hr);
> - ok(!filename, "Expected NULL filename, got %s.\n", debugstr_w(filename));
> - SysFreeString(filename);
> -
> - hr = IMediaDet_get_OutputStreams(pM, &nstrms);
> - ok(hr == S_OK, "IMediaDet_get_OutputStreams failed: %08x\n", hr);
> - ok(nstrms == 1, "IMediaDet_get_OutputStreams: nstrms is %i\n", nstrms);
> - hr = IMediaDet_Release(pM);
>    ok(hr == 0, "IMediaDet_Release returned: %x\n", hr);

[PATCH v2 12/13] qedit/tests: Add tests for get_Filter and put_Filter with a custom filter.

[Gabriel Ivăncescu]

On 16/04/2020 19:44, Zebediah Figura wrote:
> On 4/16/20 10:25 AM, Gabriel Ivăncescu wrote:
> > Signed-off-by: Gabriel Ivăncescu
> >
> > dlls/qedit/tests/mediadet.c | 965 ++++++++++++++++++++++++++++++++++++
> >   1 file changed, 965 insertions(+)
>
> It might also be nice to convert the rest of test_mediadet() to use a
> custom filter, instead of relying on behaviour of the native AVI
> demuxer. I won't call that necessary for this series, although I don't
> particularly like that test_mediadet() is now a mixture of two different
> source filters; it'd be preferable to me to at least add these tests in
> a separate function.
>
> Also, can this patch be moved earlier in the series? It's nice to have
> tests before fixes, so we can see more clearly what's fixed by each
> patch. That might require temporarily guarding some code by e.g. "if (hr
> == S_OK)". If it can't be done reasonably, though, that's fine.

Uhm, I think the current tests layout is fine. I'm not particularly 
thrilled with splitting it into two functions: half of the reason I put 
it here was to test the relationship between put_Filter, put_Filename 
and get_Filter (there's the reason I placed them at varying parts of the 
function). There's no much point splitting it since we'd end up still 
having to do those tests, anyway, which would just duplicate them.

Also, the put_Filename always uses the native AVI demuxer in this case, 
so obviously it will still have to remain to be able to test that 
properly. It shouldn't be removed.

I'll try to see if I can move it earlier, though.
> > diff --git a/dlls/qedit/tests/mediadet.c b/dlls/qedit/tests/mediadet.c
> > index 596171b..10127cf 100644
> > --- a/dlls/qedit/tests/mediadet.c
> > +++ b/dlls/qedit/tests/mediadet.c
> > @@ -24,6 +24,7 @@
> >   #include "ole2.h"
> >   #include "vfwmsgs.h"
> >   #include "uuids.h"
> > +#include "dshow.h"
> >   #include "wine/test.h"
> >   #include "qedit.h"
> >   #include "control.h"
> > @@ -71,6 +72,867 @@ static const IUnknownVtbl outer_vtbl =
> >
> > static IUnknown test_outer = {&outer_vtbl};
> >
> > +struct testfilter
> > +{
> > - IBaseFilter IBaseFilter_iface;
> > - IMediaSeeking IMediaSeeking_iface;
> > - IMediaPosition IMediaPosition_iface;
> > - LONG ref;
> > - WCHAR *name;
> > - IFilterGraph *graph;
> > - FILTER_STATE state;
> > - double rate;
> > -
> > - IEnumPins IEnumPins_iface;
> > - UINT enum_idx;
> > -
> > - IPin IPin_iface;
> > - IPin *peer;
> > -
> > - IEnumMediaTypes IEnumMediaTypes_iface;
> > - UINT mt_enum_idx;
> >
> > +};
> > -
>
> Could we use strmbase here instead?
>
> ...

I wasn't aware that we can use it in tests since they could potentially 
be compiled on Windows. I'm not familiar with strmbase much, though, so 
I'll have to look into it. (I mostly just used the layout of the test 
code from quartz's filtergraph tests)
> > +static inline struct testfilter *impl_from_IMediaSeeking(IMediaSeeking *iface)
> > +{
> > - return CONTAINING_RECORD(iface, struct testfilter, IMediaSeeking_iface);
> >
> > +}
> > -
> >
> > +static HRESULT WINAPI testseek_QueryInterface(IMediaSeeking *iface, REFIID iid, void **out)
> > +{
> > - struct testfilter *filter = impl_from_IMediaSeeking(iface);
> > - return IBaseFilter_QueryInterface(&filter->IBaseFilter_iface, iid, out);
> >
> > +}
> > -
> >
> > +static ULONG WINAPI testseek_AddRef(IMediaSeeking *iface)
> > +{
> > - struct testfilter *filter = impl_from_IMediaSeeking(iface);
> > - return IBaseFilter_AddRef(&filter->IBaseFilter_iface);
> >
> > +}
> > -
> >
> > +static ULONG WINAPI testseek_Release(IMediaSeeking *iface)
> > +{
> > - struct testfilter *filter = impl_from_IMediaSeeking(iface);
> > - return IBaseFilter_Release(&filter->IBaseFilter_iface);
> >
> > +}
> > -
> >
> > +static HRESULT WINAPI testseek_GetCapabilities(IMediaSeeking *iface, DWORD *caps)
> > +{
> > - *caps = AM_SEEKING_CanGetDuration;
> > - return S_OK;
> >
> > +}
>
> Can you please add trace messages to locally defined methods, along the
> lines of "if (winetest_debug > 1) trace("GetCapabilities()\n");"
> > -
> >
> > +static HRESULT WINAPI testseek_CheckCapabilities(IMediaSeeking *iface, DWORD *caps)
> > +{
> > - ok(0, "Unexpected call.\n");
> > - return E_NOTIMPL;
> >
> > +}
> > -
> >
> > +static HRESULT WINAPI testseek_IsFormatSupported(IMediaSeeking *iface, const GUID *format)
> > +{
> > - return IsEqualGUID(format, &TIME_FORMAT_MEDIA_TIME) ? S_OK : S_FALSE;
> >
> > +}
> > -
> >
> > +static HRESULT WINAPI testseek_QueryPreferredFormat(IMediaSeeking *iface, GUID *format)
> > +{
> > - *format = TIME_FORMAT_MEDIA_TIME;
> > - return S_OK;
> >
> > +}
> > -
> >
> > +static HRESULT WINAPI testseek_GetTimeFormat(IMediaSeeking *iface, GUID *format)
> > +{
> > - *format = TIME_FORMAT_MEDIA_TIME;
> > - return S_OK;
> >
> > +}
> > -
> >
> > +static HRESULT WINAPI testseek_IsUsingTimeFormat(IMediaSeeking *iface, const GUID *format)
> > +{
> > - return IsEqualGUID(format, &TIME_FORMAT_MEDIA_TIME) ? S_OK : S_FALSE;
> >
> > +}
> > -
> >
> > +static HRESULT WINAPI testseek_SetTimeFormat(IMediaSeeking *iface, const GUID *format)
> > +{
> > - return IsEqualGUID(format, &TIME_FORMAT_MEDIA_TIME) ? S_OK : E_INVALIDARG;
> >
> > +}
> > -
> >
> > +static HRESULT WINAPI testseek_GetDuration(IMediaSeeking *iface, LONGLONG *duration)
> > +{
> > - *duration = 42000000;
> > - return S_OK;
> >
> > +}
> > -
> >
> > +static HRESULT WINAPI testseek_GetStopPosition(IMediaSeeking *iface, LONGLONG *stop)
> > +{
> > - *stop = 42000000;
> > - return S_OK;
> >
> > +}
> > -
> >
> > +static HRESULT WINAPI testseek_GetCurrentPosition(IMediaSeeking *iface, LONGLONG *current)
> > +{
> > - *current = 0;
> > - return S_OK;
> >
> > +}
> > -
> >
> > +static HRESULT WINAPI testseek_ConvertTimeFormat(IMediaSeeking *iface, LONGLONG *target,
> > - const GUID *target_format, LONGLONG source, const GUID *source_format)
> >
> > +{
> > - ok(0, "Unexpected call.\n");
> > - return E_NOTIMPL;
> >
> > +}
> > -
> >
> > +static HRESULT WINAPI testseek_SetPositions(IMediaSeeking *iface, LONGLONG *current,
> > - DWORD current_flags, LONGLONG *stop, DWORD stop_flags)
> >
> > +{
> > - *current = 0;
> > - *stop = 42000000;
> > - return S_OK;
> >
> > +}
> > -
> >
> > +static HRESULT WINAPI testseek_GetPositions(IMediaSeeking *iface, LONGLONG *current, LONGLONG *stop)
> > +{
> > - *current = 0;
> > - *stop = 42000000;
> > - return S_OK;
> >
> > +}
> > -
> >
> > +static HRESULT WINAPI testseek_GetAvailable(IMediaSeeking *iface, LONGLONG *earliest, LONGLONG *latest)
> > +{
> > - *earliest = 0;
> > - *latest = 42000000;
> > - return S_OK;
> >
> > +}
> > -
> >
> > +static HRESULT WINAPI testseek_SetRate(IMediaSeeking *iface, double rate)
> > +{
> > - struct testfilter *filter = impl_from_IMediaSeeking(iface);
> > -
> > - filter->rate = rate;
> > - return S_OK;
> >
> > +}
> > -
> >
> > +static HRESULT WINAPI testseek_GetRate(IMediaSeeking *iface, double *rate)
> > +{
> > - struct testfilter *filter = impl_from_IMediaSeeking(iface);
> > -
> > - *rate = filter->rate;
> > - return S_OK;
> >
> > +}
> > -
> >
> > +static HRESULT WINAPI testseek_GetPreroll(IMediaSeeking *iface, LONGLONG *preroll)
> > +{
> > - *preroll = 0;
> > - return S_OK;
> >
> > +}
>
> Do you need to implement all of these methods? Does the implementation
> need to return S_OK, or can it return E_NOTIMPL? Same for IMediaPosition
> methods below.
>
> ...

I don't know, I considered it an implementation detail. I thought it 
trivial enough to implement fake values here. Should I really make it 
return E_NOTIMPL?

One example of such thing is: IMediaSeeking on the filter (not the pin) 
is *not* queried by Windows, but we do when we add the filter (because 
of the patch that caches it which doesn't match Windows 100%).
> > +static IBaseFilter *create_testfilter(void)
> > +{
> > - struct testfilter *filter = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*filter));
> > -
> > - if (!filter) return NULL;
>
> I don't think we need to allocate this from heap. Note though that if
> you do, we can use malloc() instead, and checking for failure in tests
> is not really necessary.
> > -
> > - filter->IBaseFilter_iface.lpVtbl = &testfilter_vtbl;
> > - filter->IMediaSeeking_iface.lpVtbl = &testseek_vtbl;
> > - filter->IMediaPosition_iface.lpVtbl = &testpos_vtbl;
> > - filter->IEnumPins_iface.lpVtbl = &testenumpins_vtbl;
> > - filter->IPin_iface.lpVtbl = &testpin_vtbl;
> > - filter->IEnumMediaTypes_iface.lpVtbl = &testenummt_vtbl;
> > - filter->ref = 1;
> > - filter->state = State_Stopped;
> > - filter->rate = 1.0;
> > -
> > - return &filter->IBaseFilter_iface;
> >
> > +}
> > - static void test_aggregation(void)
> > {
> >     IMediaDet *detector, *detector2;
> >
> > @@ -188,13 +1050,17 @@ static BOOL init_tests(void)
> >   static void test_mediadet(void)
> >   {
> >       HRESULT hr;
> > - IBaseFilter *filter, *filter2;
> > - FILTER_INFO filter_info;
> >  IMediaDet *pM = NULL;
> >  BSTR filename = NULL;
> > - IFilterGraph *graph;
> >  LONG nstrms = 0;
> >  LONG strm;
> >  GUID guid;
> >  BSTR bstr;
> >  AM_MEDIA_TYPE mt;
> > - IUnknown *unk;
> >  double fps;
> >  int flags;
> >  int i;
> >
> > @@ -256,6 +1122,61 @@ 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_Filter(pM, NULL);
> > - ok(hr == E_POINTER, "IMediaDet_get_Filter failed: %08x\n", hr);
> > -
> > - unk = (IUnknown*)0xdeadbeef;
> > - hr = IMediaDet_get_Filter(pM, &unk);
> > - ok(hr == S_FALSE, "IMediaDet_get_Filter failed: %08x\n", hr);
> > - ok(unk == NULL, "Wrong filter %p\n", unk);
> > -
> > - hr = IMediaDet_put_Filter(pM, NULL);
> > - ok(hr == E_POINTER, "IMediaDet_put_Filter failed: %08x\n", hr);
> > -
> > - filter = create_testfilter();
> > - hr = IMediaDet_put_Filter(pM, (IUnknown*)filter);
> > - ok(hr == S_OK, "IMediaDet_put_Filter failed: %08x\n", hr);
> > -
> > - hr = IMediaDet_get_Filter(pM, &unk);
> > - ok(hr == S_OK, "IMediaDet_get_Filter failed: %08x\n", hr);
> > - ok(unk != NULL, "NULL filter\n");
> > - hr = IUnknown_QueryInterface(unk, &IID_IBaseFilter, (void**)&filter2);
> > - IUnknown_Release(unk);
> > - ok(hr == S_OK, "Could not get IBaseFilter interface: %08x\n", hr);
> > - ok(filter == filter2, "Wrong filter\n");
> > - IBaseFilter_Release(filter2);
>
> Couldn't you compare it directly to filter->IBaseFiler_iface instead?

That would defeat the purpose of testing get_Filter, wouldn't it? Since 
we'd assume it is our filter in the first place (and if it's not, we 
would end up using random data from the object, which might succeed 
randomly and IMO is not a "correct" solution by any means).

What I'm trying to say is that there's no guarantee that get_Filter 
returns a "struct testfilter", hence the test. We can assume it is, but 
we shouldn't rely on the exact struct layout while *testing* that since 
it can access invalid data or memory otherwise. I'm really not 
comfortable with that.
> > -
> > - hr = IBaseFilter_QueryFilterInfo(filter, &filter_info);
> > - ok(hr == S_OK, "IBaseFilter_QueryFilterInfo failed: %08x\n", hr);
> > - ok(!wcscmp(filter_info.achName, L"Source"), "Wrong filter name: %s\n", wine_dbgstr_w(filter_info.achName));
> > - IBaseFilter_Release(filter);
> > - graph = filter_info.pGraph;
>
> Similarly, you could just access filter->name directly instead of
> calling IBaseFilter::QueryFilterInfo().

Same as above.
> > -
> > - filter = create_testfilter();
> > - hr = IMediaDet_put_Filter(pM, (IUnknown*)filter);
> > - ok(hr == S_OK, "IMediaDet_put_Filter failed: %08x\n", hr);
> > - IBaseFilter_Release(filter);
> > -
> > - hr = IMediaDet_get_Filter(pM, &unk);
> > - ok(hr == S_OK, "IMediaDet_get_Filter failed: %08x\n", hr);
> > - ok(unk != NULL, "NULL filter\n");
> > - hr = IUnknown_QueryInterface(unk, &IID_IBaseFilter, (void**)&filter);
> > - IUnknown_Release(unk);
> > - ok(hr == S_OK, "Could not get IBaseFilter interface: %08x\n", hr);
> > -
> > - hr = IBaseFilter_QueryFilterInfo(filter, &filter_info);
> > - ok(hr == S_OK, "IBaseFilter_QueryFilterInfo failed: %08x\n", hr);
> > - ok(!wcscmp(filter_info.achName, L"Source"), "Wrong filter name: %s\n", wine_dbgstr_w(filter_info.achName));
> > - ok(graph != filter_info.pGraph, "Same filter graph was used\n");
> > - IFilterGraph_Release(filter_info.pGraph);
> > - IFilterGraph_Release(graph);
> > - IBaseFilter_Release(filter);
> > -
> > - strm = -1;
> > - hr = IMediaDet_get_CurrentStream(pM, &strm);
> > - ok(hr == S_OK, "IMediaDet_get_CurrentStream failed: %08x\n", hr);
> > - ok(strm == 0, "IMediaDet_get_CurrentStream: strm is %i\n", strm);
> > -
> > filename = SysAllocString(test_avi_filename);
> > hr = IMediaDet_put_Filename(pM, filename);
> > ok(hr == S_OK, "IMediaDet_put_Filename failed: %08x\n", hr);
> >
> > @@ -427,6 +1348,50 @@ static void test_mediadet(void)
> >       ok(hr == S_OK, "IMediaDet_get_CurrentStream failed: %08x\n", hr);
> >       ok(strm == 1, "IMediaDet_get_CurrentStream: strm is %i\n", strm);
> > - hr = IMediaDet_get_Filter(pM, &unk);
> > - ok(hr == S_OK, "IMediaDet_get_Filter failed: %08x\n", hr);
> > - ok(unk != NULL, "NULL filter\n");
> > - hr = IUnknown_QueryInterface(unk, &IID_IBaseFilter, (void**)&filter2);
> > - IUnknown_Release(unk);
> > - ok(hr == S_OK, "Could not get IBaseFilter interface: %08x\n", hr);
> > -
> > - hr = IBaseFilter_QueryFilterInfo(filter2, &filter_info);
> > - ok(hr == S_OK, "IBaseFilter_QueryFilterInfo failed: %08x\n", hr);
> > - ok(!wcscmp(filter_info.achName, L"Source"), "Wrong filter name: %s\n", wine_dbgstr_w(filter_info.achName));
> > - graph = filter_info.pGraph;
> > -
> > - filter = create_testfilter();
> > - hr = IMediaDet_put_Filter(pM, (IUnknown*)filter);
> > - ok(hr == S_OK, "IMediaDet_put_Filter failed: %08x\n", hr);
> > - IBaseFilter_Release(filter);
> > -
> > - hr = IMediaDet_get_Filter(pM, &unk);
> > - ok(hr == S_OK, "IMediaDet_get_Filter failed: %08x\n", hr);
> > - ok(unk != NULL, "NULL filter\n");
> > - hr = IUnknown_QueryInterface(unk, &IID_IBaseFilter, (void**)&filter);
> > - IUnknown_Release(unk);
> > - ok(hr == S_OK, "Could not get IBaseFilter interface: %08x\n", hr);
> > - ok(filter != filter2, "Same filter\n");
> > - IBaseFilter_Release(filter2);
> > -
> > - hr = IBaseFilter_QueryFilterInfo(filter, &filter_info);
> > - ok(hr == S_OK, "IBaseFilter_QueryFilterInfo failed: %08x\n", hr);
> > - ok(!wcscmp(filter_info.achName, L"Source"), "Wrong filter name: %s\n", wine_dbgstr_w(filter_info.achName));
> > - ok(graph != filter_info.pGraph, "Same filter graph was used\n");
> > - IFilterGraph_Release(filter_info.pGraph);
> > - IFilterGraph_Release(graph);
> > - IBaseFilter_Release(filter);
> > -
> > - filename = NULL;
> > - hr = IMediaDet_get_Filename(pM, &filename);
> > - ok(hr == S_OK, "IMediaDet_get_Filename failed: %08x\n", hr);
> > - ok(!filename, "Expected NULL filename, got %s.\n", debugstr_w(filename));
> > - SysFreeString(filename);
> > -
> > - hr = IMediaDet_get_OutputStreams(pM, &nstrms);
> > - ok(hr == S_OK, "IMediaDet_get_OutputStreams failed: %08x\n", hr);
> > - ok(nstrms == 1, "IMediaDet_get_OutputStreams: nstrms is %i\n", nstrms);
> > -
> > hr = IMediaDet_Release(pM);
> > ok(hr == 0, "IMediaDet_Release returned: %x\n", hr);

[PATCH v2 12/13] qedit/tests: Add tests for get_Filter and put_Filter with a custom filter.

[Zebediah Figura]

On 4/17/20 7:26 AM, Gabriel Ivăncescu wrote:
> On 16/04/2020 19:44, Zebediah Figura wrote:
> > On 4/16/20 10:25 AM, Gabriel Ivăncescu wrote:
> > > Signed-off-by: Gabriel Ivăncescu
> > >
> > > dlls/qedit/tests/mediadet.c | 965 ++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 965 insertions(+)
> >
> > It might also be nice to convert the rest of test_mediadet() to use a
> > custom filter, instead of relying on behaviour of the native AVI
> > demuxer. I won't call that necessary for this series, although I don't
> > particularly like that test_mediadet() is now a mixture of two different
> > source filters; it'd be preferable to me to at least add these tests in
> > a separate function.
> >
> > Also, can this patch be moved earlier in the series? It's nice to have
> > tests before fixes, so we can see more clearly what's fixed by each
> > patch. That might require temporarily guarding some code by e.g. "if (hr
> > == S_OK)". If it can't be done reasonably, though, that's fine.
>
> Uhm, I think the current tests layout is fine. I'm not particularly 
> thrilled with splitting it into two functions: half of the reason I put 
> it here was to test the relationship between put_Filter, put_Filename 
> and get_Filter (there's the reason I placed them at varying parts of the 
> function). There's no much point splitting it since we'd end up still 
> having to do those tests, anyway, which would just duplicate them.
> Also, the put_Filename always uses the native AVI demuxer in this case, 
> so obviously it will still have to remain to be able to test that 
> properly. It shouldn't be removed.

Hmm, right, I forgot that put_Filename() does autoplugging.

I don't particularly like how big test_mediadet() is, but I guess I can 
deal with that.
> I'll try to see if I can move it earlier, though.
> > > diff --git a/dlls/qedit/tests/mediadet.c b/dlls/qedit/tests/mediadet.c
> > > index 596171b..10127cf 100644
> > > --- a/dlls/qedit/tests/mediadet.c
> > > +++ b/dlls/qedit/tests/mediadet.c
> > > @@ -24,6 +24,7 @@
> > >   #include "ole2.h"
> > >   #include "vfwmsgs.h"
> > >   #include "uuids.h"
> > > +#include "dshow.h"
> > >   #include "wine/test.h"
> > >   #include "qedit.h"
> > >   #include "control.h"
> > > @@ -71,6 +72,867 @@ static const IUnknownVtbl outer_vtbl =
> > >   static IUnknown test_outer = {&outer_vtbl};
> > > +struct testfilter
> > > +{
> > > +    IBaseFilter IBaseFilter_iface;
> > > +    IMediaSeeking IMediaSeeking_iface;
> > > +    IMediaPosition IMediaPosition_iface;
> > > +    LONG ref;
> > > +    WCHAR *name;
> > > +    IFilterGraph *graph;
> > > +    FILTER_STATE state;
> > > +    double rate;
> > > -
> > >
> > > +    IEnumPins IEnumPins_iface;
> > > +    UINT enum_idx;
> > > -
> > >
> > > +    IPin IPin_iface;
> > > +    IPin *peer;
> > > -
> > >
> > > +    IEnumMediaTypes IEnumMediaTypes_iface;
> > > +    UINT mt_enum_idx;
> > > +};
> > > -
> >
> > Could we use strmbase here instead?
> >
> > ...
>
> I wasn't aware that we can use it in tests since they could potentially 
> be compiled on Windows. I'm not familiar with strmbase much, though, so 
> I'll have to look into it. (I mostly just used the layout of the test 
> code from quartz's filtergraph tests)

Yes, we can; we do compile half of DirectShow as PE after all. 
quartz:filtergraph uses its own filters, partially because it preceded 
the ability to use strmbase in tests, but at this point also because it 
does some unusual things that aren't worth hacking into strmbase (in 
particular see e.g. test_connect_direct_vtbl).

If you want examples, there are plenty in other quartz tests, see e.g. 
qasf:dmowrapper, quartz:videorenderer, qcap:capturegraph...
> > > +static inline struct testfilter 
> > > *impl_from_IMediaSeeking(IMediaSeeking *iface)
> > > +{
> > > +    return CONTAINING_RECORD(iface, struct testfilter, 
> > > IMediaSeeking_iface);
> > > +}
> > > -
> > >
> > > +static HRESULT WINAPI testseek_QueryInterface(IMediaSeeking *iface, 
> > > REFIID iid, void **out)
> > > +{
> > > +    struct testfilter *filter = impl_from_IMediaSeeking(iface);
> > > +    return IBaseFilter_QueryInterface(&filter->IBaseFilter_iface, 
> > > iid, out);
> > > +}
> > > -
> > >
> > > +static ULONG WINAPI testseek_AddRef(IMediaSeeking *iface)
> > > +{
> > > +    struct testfilter *filter = impl_from_IMediaSeeking(iface);
> > > +    return IBaseFilter_AddRef(&filter->IBaseFilter_iface);
> > > +}
> > > -
> > >
> > > +static ULONG WINAPI testseek_Release(IMediaSeeking *iface)
> > > +{
> > > +    struct testfilter *filter = impl_from_IMediaSeeking(iface);
> > > +    return IBaseFilter_Release(&filter->IBaseFilter_iface);
> > > +}
> > > -
> > >
> > > +static HRESULT WINAPI testseek_GetCapabilities(IMediaSeeking *iface, 
> > > DWORD *caps)
> > > +{
> > > +    *caps = AM_SEEKING_CanGetDuration;
> > > +    return S_OK;
> > > +}
> >
> > Can you please add trace messages to locally defined methods, along the
> > lines of "if (winetest_debug > 1) trace("GetCapabilities()\n");"
> > > -
> > >
> > > +static HRESULT WINAPI testseek_CheckCapabilities(IMediaSeeking 
> > > *iface, DWORD *caps)
> > > +{
> > > +    ok(0, "Unexpected call.\n");
> > > +    return E_NOTIMPL;
> > > +}
> > > -
> > >
> > > +static HRESULT WINAPI testseek_IsFormatSupported(IMediaSeeking 
> > > *iface, const GUID *format)
> > > +{
> > > +    return IsEqualGUID(format, &TIME_FORMAT_MEDIA_TIME) ? S_OK : 
> > > S_FALSE;
> > > +}
> > > -
> > >
> > > +static HRESULT WINAPI testseek_QueryPreferredFormat(IMediaSeeking 
> > > *iface, GUID *format)
> > > +{
> > > +    *format = TIME_FORMAT_MEDIA_TIME;
> > > +    return S_OK;
> > > +}
> > > -
> > >
> > > +static HRESULT WINAPI testseek_GetTimeFormat(IMediaSeeking *iface, 
> > > GUID *format)
> > > +{
> > > +    *format = TIME_FORMAT_MEDIA_TIME;
> > > +    return S_OK;
> > > +}
> > > -
> > >
> > > +static HRESULT WINAPI testseek_IsUsingTimeFormat(IMediaSeeking 
> > > *iface, const GUID *format)
> > > +{
> > > +    return IsEqualGUID(format, &TIME_FORMAT_MEDIA_TIME) ? S_OK : 
> > > S_FALSE;
> > > +}
> > > -
> > >
> > > +static HRESULT WINAPI testseek_SetTimeFormat(IMediaSeeking *iface, 
> > > const GUID *format)
> > > +{
> > > +    return IsEqualGUID(format, &TIME_FORMAT_MEDIA_TIME) ? S_OK : 
> > > E_INVALIDARG;
> > > +}
> > > -
> > >
> > > +static HRESULT WINAPI testseek_GetDuration(IMediaSeeking *iface, 
> > > LONGLONG *duration)
> > > +{
> > > +    *duration = 42000000;
> > > +    return S_OK;
> > > +}
> > > -
> > >
> > > +static HRESULT WINAPI testseek_GetStopPosition(IMediaSeeking *iface, 
> > > LONGLONG *stop)
> > > +{
> > > +    *stop = 42000000;
> > > +    return S_OK;
> > > +}
> > > -
> > >
> > > +static HRESULT WINAPI testseek_GetCurrentPosition(IMediaSeeking 
> > > *iface, LONGLONG *current)
> > > +{
> > > +    *current = 0;
> > > +    return S_OK;
> > > +}
> > > -
> > >
> > > +static HRESULT WINAPI testseek_ConvertTimeFormat(IMediaSeeking 
> > > *iface, LONGLONG *target,
> > > +    const GUID *target_format, LONGLONG source, const GUID 
> > > *source_format)
> > > +{
> > > +    ok(0, "Unexpected call.\n");
> > > +    return E_NOTIMPL;
> > > +}
> > > -
> > >
> > > +static HRESULT WINAPI testseek_SetPositions(IMediaSeeking *iface, 
> > > LONGLONG *current,
> > > +    DWORD current_flags, LONGLONG *stop, DWORD stop_flags)
> > > +{
> > > +    *current = 0;
> > > +    *stop = 42000000;
> > > +    return S_OK;
> > > +}
> > > -
> > >
> > > +static HRESULT WINAPI testseek_GetPositions(IMediaSeeking *iface, 
> > > LONGLONG *current, LONGLONG *stop)
> > > +{
> > > +    *current = 0;
> > > +    *stop = 42000000;
> > > +    return S_OK;
> > > +}
> > > -
> > >
> > > +static HRESULT WINAPI testseek_GetAvailable(IMediaSeeking *iface, 
> > > LONGLONG *earliest, LONGLONG *latest)
> > > +{
> > > +    *earliest = 0;
> > > +    *latest = 42000000;
> > > +    return S_OK;
> > > +}
> > > -
> > >
> > > +static HRESULT WINAPI testseek_SetRate(IMediaSeeking *iface, double 
> > > rate)
> > > +{
> > > +    struct testfilter *filter = impl_from_IMediaSeeking(iface);
> > > -
> > >
> > > +    filter->rate = rate;
> > > +    return S_OK;
> > > +}
> > > -
> > >
> > > +static HRESULT WINAPI testseek_GetRate(IMediaSeeking *iface, double 
> > > *rate)
> > > +{
> > > +    struct testfilter *filter = impl_from_IMediaSeeking(iface);
> > > -
> > >
> > > +    *rate = filter->rate;
> > > +    return S_OK;
> > > +}
> > > -
> > >
> > > +static HRESULT WINAPI testseek_GetPreroll(IMediaSeeking *iface, 
> > > LONGLONG *preroll)
> > > +{
> > > +    *preroll = 0;
> > > +    return S_OK;
> > > +}
> >
> > Do you need to implement all of these methods? Does the implementation
> > need to return S_OK, or can it return E_NOTIMPL? Same for IMediaPosition
> > methods below.
> >
> > ...
>
> I don't know, I considered it an implementation detail. I thought it 
> trivial enough to implement fake values here. Should I really make it 
> return E_NOTIMPL?

It doesn't matter a lot, but returning E_NOTIMPL is generally "easier". 
For example, then you don't have to bother tracking playback rate.

Note also that SetPositions() isn't really correct, it should only 
modify its arguments when passed AM_SEEKING_ReturnTime. (Yes, I know 
that quartz:filtergraph doesn't check for that flag; it probably should.)
> One example of such thing is: IMediaSeeking on the filter (not the pin) 
> is *not* queried by Windows, but we do when we add the filter (because 
> of the patch that caches it which doesn't match Windows 100%).

In my opinion, deciding whether an implementation detail is worth 
replicating or testing depends largely on whether it's easier not to, 
and how likely it is that an application will rely on it later.

For example, my armchair analysis of 0320f165 is that applications 
aren't particularly likely to care when an interface is queried, and 
it's easier for us to just do it once.

By contrast, IMediaDet::get_StreamLength() could potentially work in 
several different ways, which are a roughly equal amount of work to 
implement: does it return the stream length from 
IMediaSeeking::GetDuration(), or GetAvailable(), or maybe even 
GetStopPosition()? Does it actually bother converting to 
TIME_FORMAT_MEDIA_TIME? In most cases these will end up being identical 
in practice, but we don't really gain anything by just assuming one is 
correct, and it's not hard to check.
> > > +static IBaseFilter *create_testfilter(void)
> > > +{
> > > +    struct testfilter *filter = HeapAlloc(GetProcessHeap(), 
> > > HEAP_ZERO_MEMORY, sizeof(*filter));
> > > -
> > >
> > > +    if (!filter) return NULL;
> >
> > I don't think we need to allocate this from heap. Note though that if
> > you do, we can use malloc() instead, and checking for failure in tests
> > is not really necessary.
> > > -
> > >
> > > +    filter->IBaseFilter_iface.lpVtbl = &testfilter_vtbl;
> > > +    filter->IMediaSeeking_iface.lpVtbl = &testseek_vtbl;
> > > +    filter->IMediaPosition_iface.lpVtbl = &testpos_vtbl;
> > > +    filter->IEnumPins_iface.lpVtbl = &testenumpins_vtbl;
> > > +    filter->IPin_iface.lpVtbl = &testpin_vtbl;
> > > +    filter->IEnumMediaTypes_iface.lpVtbl = &testenummt_vtbl;
> > > +    filter->ref = 1;
> > > +    filter->state = State_Stopped;
> > > +    filter->rate = 1.0;
> > > -
> > >
> > > +    return &filter->IBaseFilter_iface;
> > > +}
> > > -
> > >
> > > static void test_aggregation(void)
> > >   {
> > >       IMediaDet *detector, *detector2;
> > > @@ -188,13 +1050,17 @@ static BOOL init_tests(void)
> > >   static void test_mediadet(void)
> > >   {
> > >       HRESULT hr;
> > > +    IBaseFilter *filter, *filter2;
> > > +    FILTER_INFO filter_info;
> > >       IMediaDet *pM = NULL;
> > >       BSTR filename = NULL;
> > > +    IFilterGraph *graph;
> > >       LONG nstrms = 0;
> > >       LONG strm;
> > >       GUID guid;
> > >       BSTR bstr;
> > >       AM_MEDIA_TYPE mt;
> > > +    IUnknown *unk;
> > >       double fps;
> > >       int flags;
> > >       int i;
> > > @@ -256,6 +1122,61 @@ 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_Filter(pM, NULL);
> > > +    ok(hr == E_POINTER, "IMediaDet_get_Filter failed: %08x\n", hr);
> > > -
> > >
> > > +    unk = (IUnknown*)0xdeadbeef;
> > > +    hr = IMediaDet_get_Filter(pM, &unk);
> > > +    ok(hr == S_FALSE, "IMediaDet_get_Filter failed: %08x\n", hr);
> > > +    ok(unk == NULL, "Wrong filter %p\n", unk);
> > > -
> > >
> > > +    hr = IMediaDet_put_Filter(pM, NULL);
> > > +    ok(hr == E_POINTER, "IMediaDet_put_Filter failed: %08x\n", hr);
> > > -
> > >
> > > +    filter = create_testfilter();
> > > +    hr = IMediaDet_put_Filter(pM, (IUnknown*)filter);
> > > +    ok(hr == S_OK, "IMediaDet_put_Filter failed: %08x\n", hr);
> > > -
> > >
> > > +    hr = IMediaDet_get_Filter(pM, &unk);
> > > +    ok(hr == S_OK, "IMediaDet_get_Filter failed: %08x\n", hr);
> > > +    ok(unk != NULL, "NULL filter\n");
> > > +    hr = IUnknown_QueryInterface(unk, &IID_IBaseFilter, 
> > > (void**)&filter2);
> > > +    IUnknown_Release(unk);
> > > +    ok(hr == S_OK, "Could not get IBaseFilter interface: %08x\n", hr);
> > > +    ok(filter == filter2, "Wrong filter\n");
> > > +    IBaseFilter_Release(filter2);
> >
> > Couldn't you compare it directly to filter->IBaseFiler_iface instead?
>
> That would defeat the purpose of testing get_Filter, wouldn't it? Since 
> we'd assume it is our filter in the first place (and if it's not, we 
> would end up using random data from the object, which might succeed 
> randomly and IMO is not a "correct" solution by any means).
>
> What I'm trying to say is that there's no guarantee that get_Filter 
> returns a "struct testfilter", hence the test. We can assume it is, but 
> we shouldn't rely on the exact struct layout while *testing* that since 
> it can access invalid data or memory otherwise. I'm really not 
> comfortable with that.

Eh, sorry, I think I misread the query here.
> > > -
> > >
> > > +    hr = IBaseFilter_QueryFilterInfo(filter, &filter_info);
> > > +    ok(hr == S_OK, "IBaseFilter_QueryFilterInfo failed: %08x\n", hr);
> > > +    ok(!wcscmp(filter_info.achName, L"Source"), "Wrong filter name: 
> > > %s\n", wine_dbgstr_w(filter_info.achName));
> > > +    IBaseFilter_Release(filter);
> > > +    graph = filter_info.pGraph;
> >
> > Similarly, you could just access filter->name directly instead of
> > calling IBaseFilter::QueryFilterInfo().
>
> Same as above.

You're querying "filter", which is our own filter. Yes, "filter" is an 
IBaseFilter pointer rather than a "struct testfilter" pointer, but it 
doesn't really need to be.
> > > -
> > >
> > > +    filter = create_testfilter();
> > > +    hr = IMediaDet_put_Filter(pM, (IUnknown*)filter);
> > > +    ok(hr == S_OK, "IMediaDet_put_Filter failed: %08x\n", hr);
> > > +    IBaseFilter_Release(filter);
> > > -
> > >
> > > +    hr = IMediaDet_get_Filter(pM, &unk);
> > > +    ok(hr == S_OK, "IMediaDet_get_Filter failed: %08x\n", hr);
> > > +    ok(unk != NULL, "NULL filter\n");
> > > +    hr = IUnknown_QueryInterface(unk, &IID_IBaseFilter, 
> > > (void**)&filter);
> > > +    IUnknown_Release(unk);
> > > +    ok(hr == S_OK, "Could not get IBaseFilter interface: %08x\n", hr);
> > > -
> > >
> > > +    hr = IBaseFilter_QueryFilterInfo(filter, &filter_info);
> > > +    ok(hr == S_OK, "IBaseFilter_QueryFilterInfo failed: %08x\n", hr);
> > > +    ok(!wcscmp(filter_info.achName, L"Source"), "Wrong filter name: 
> > > %s\n", wine_dbgstr_w(filter_info.achName));
> > > +    ok(graph != filter_info.pGraph, "Same filter graph was used\n");
> > > +    IFilterGraph_Release(filter_info.pGraph);
> > > +    IFilterGraph_Release(graph);
> > > +    IBaseFilter_Release(filter);
> > > -
> > >
> > > +    strm = -1;
> > > +    hr = IMediaDet_get_CurrentStream(pM, &strm);
> > > +    ok(hr == S_OK, "IMediaDet_get_CurrentStream failed: %08x\n", hr);
> > > +    ok(strm == 0, "IMediaDet_get_CurrentStream: strm is %i\n", strm);
> > > -
> > >
> > > filename = SysAllocString(test_avi_filename);
> > >       hr = IMediaDet_put_Filename(pM, filename);
> > >       ok(hr == S_OK, "IMediaDet_put_Filename failed: %08x\n", hr);
> > > @@ -427,6 +1348,50 @@ static void test_mediadet(void)
> > >       ok(hr == S_OK, "IMediaDet_get_CurrentStream failed: %08x\n", hr);
> > >       ok(strm == 1, "IMediaDet_get_CurrentStream: strm is %i\n", strm);
> > > +    hr = IMediaDet_get_Filter(pM, &unk);
> > > +    ok(hr == S_OK, "IMediaDet_get_Filter failed: %08x\n", hr);
> > > +    ok(unk != NULL, "NULL filter\n");
> > > +    hr = IUnknown_QueryInterface(unk, &IID_IBaseFilter, 
> > > (void**)&filter2);
> > > +    IUnknown_Release(unk);
> > > +    ok(hr == S_OK, "Could not get IBaseFilter interface: %08x\n", hr);
> > > -
> > >
> > > +    hr = IBaseFilter_QueryFilterInfo(filter2, &filter_info);
> > > +    ok(hr == S_OK, "IBaseFilter_QueryFilterInfo failed: %08x\n", hr);
> > > +    ok(!wcscmp(filter_info.achName, L"Source"), "Wrong filter name: 
> > > %s\n", wine_dbgstr_w(filter_info.achName));
> > > +    graph = filter_info.pGraph;
> > > -
> > >
> > > +    filter = create_testfilter();
> > > +    hr = IMediaDet_put_Filter(pM, (IUnknown*)filter);
> > > +    ok(hr == S_OK, "IMediaDet_put_Filter failed: %08x\n", hr);
> > > +    IBaseFilter_Release(filter);
> > > -
> > >
> > > +    hr = IMediaDet_get_Filter(pM, &unk);
> > > +    ok(hr == S_OK, "IMediaDet_get_Filter failed: %08x\n", hr);
> > > +    ok(unk != NULL, "NULL filter\n");
> > > +    hr = IUnknown_QueryInterface(unk, &IID_IBaseFilter, 
> > > (void**)&filter);
> > > +    IUnknown_Release(unk);
> > > +    ok(hr == S_OK, "Could not get IBaseFilter interface: %08x\n", hr);
> > > +    ok(filter != filter2, "Same filter\n");
> > > +    IBaseFilter_Release(filter2);
> > > -
> > >
> > > +    hr = IBaseFilter_QueryFilterInfo(filter, &filter_info);
> > > +    ok(hr == S_OK, "IBaseFilter_QueryFilterInfo failed: %08x\n", hr);
> > > +    ok(!wcscmp(filter_info.achName, L"Source"), "Wrong filter name: 
> > > %s\n", wine_dbgstr_w(filter_info.achName));
> > > +    ok(graph != filter_info.pGraph, "Same filter graph was used\n");
> > > +    IFilterGraph_Release(filter_info.pGraph);
> > > +    IFilterGraph_Release(graph);
> > > +    IBaseFilter_Release(filter);
> > > -
> > >
> > > +    filename = NULL;
> > > +    hr = IMediaDet_get_Filename(pM, &filename);
> > > +    ok(hr == S_OK, "IMediaDet_get_Filename failed: %08x\n", hr);
> > > +    ok(!filename, "Expected NULL filename, got %s.\n", 
> > > debugstr_w(filename));
> > > +    SysFreeString(filename);
> > > -
> > >
> > > +    hr = IMediaDet_get_OutputStreams(pM, &nstrms);
> > > +    ok(hr == S_OK, "IMediaDet_get_OutputStreams failed: %08x\n", hr);
> > > +    ok(nstrms == 1, "IMediaDet_get_OutputStreams: nstrms is %i\n", 
> > > nstrms);
> > > -
> > >
> > > hr = IMediaDet_Release(pM);
> > >       ok(hr == 0, "IMediaDet_Release returned: %x\n", hr);

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

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/qedit/mediadet.c       | 39 +++++++++++++++++++++++++++++++++++--
 dlls/qedit/tests/mediadet.c | 19 ++++++++++++++++++
 2 files changed, 56 insertions(+), 2 deletions(-)

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;
+    }
+
+    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);
+
     /* 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);
+
     hr = IMediaDet_Release(pM);
     ok(hr == 0, "IMediaDet_Release returned: %x\n", hr);

[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);

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

[Gabriel Ivăncescu]

Hi Zeb,

On 16/04/2020 19:52, Zebediah Figura wrote:
> 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.

Right. Will do (same as some of the other emails I won't reply to, no 
point flooding the mailing list).
> > 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.

It's tested for the AVI source, since those don't expose IMediaPosition. 
It also seems to be tested on Windows XP because it doesn't query 
IMediaPosition.

Now, I can probably just drop the IMediaPosition entirely, if you want 
to be perfectly consistent with Windows, although I considered it more 
of an implementation detail. The reason I used it first, is because it 
has a method to retrieve the duration in the format we want, without 
having to resort to conversions.

I don't know about other Windows versions than XP; some (newer) ones 
don't even have qedit.

Should I drop IMediaPosition?
> > -
> > -
> >
> > 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.

I'd rather we keep the AVI parts since they test a different thing (more 
of that in the other patch reply).
> > -
> > /* 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.

Well, Windows XP reports something like 4.2000000000002 (not the exact 
amount of zeros), because it uses IMediaSeeking and does the conversion 
which probably loses some accuracy. So we have to keep it, I guess.
> >   hr = IMediaDet_Release(pM);
> >   ok(hr == 0, "IMediaDet_Release returned: %x\n", hr);

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

[Zebediah Figura]

On 4/17/20 7:24 AM, Gabriel Ivăncescu wrote:
> Hi Zeb,
>
> On 16/04/2020 19:52, Zebediah Figura wrote:
> > 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.
>
> Right. Will do (same as some of the other emails I won't reply to, no 
> point flooding the mailing list).
> > > 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.
>
> It's tested for the AVI source, since those don't expose IMediaPosition. 
> It also seems to be tested on Windows XP because it doesn't query 
> IMediaPosition.

The AVI splitter does expose IMediaPosition on Windows (see 
test_interfaces() in quartz:avisplit), so it's not really testing 
anything here unless you mix components.
> Now, I can probably just drop the IMediaPosition entirely, if you want 
> to be perfectly consistent with Windows, although I considered it more 
> of an implementation detail. The reason I used it first, is because it 
> has a method to retrieve the duration in the format we want, without 
> having to resort to conversions.

If Windows doesn't bother with IMediaPosition, then yes, it would be 
easier for us not to bother with it either.
> I don't know about other Windows versions than XP; some (newer) ones 
> don't even have qedit.

Just Windows Server 2003, as far as I'm aware.
> Should I drop IMediaPosition?
> > > -
> > >
> > > +    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.
>
> I'd rather we keep the AVI parts since they test a different thing (more 
> of that in the other patch reply).
> > > -
> > >
> > > /* 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.
>
> Well, Windows XP reports something like 4.2000000000002 (not the exact 
> amount of zeros), because it uses IMediaSeeking and does the conversion 
> which probably loses some accuracy. So we have to keep it, I guess.

In that case, can we instead borrow compare_double() from msvcrt:string?
> > > hr = IMediaDet_Release(pM);
> > >       ok(hr == 0, "IMediaDet_Release returned: %x\n", hr);

[Zebediah Figura]

On 4/16/20 10:24 AM, Gabriel Ivăncescu wrote:
> Signed-off-by: Gabriel Ivăncescu
>
> dlls/qedit/mediadet.c       | 17 +++++++++++++++--
>  dlls/qedit/tests/mediadet.c | 19 +++++++++++++++++++
>  2 files changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
> index ecfafc4..a6b52c0 100644
> --- a/dlls/qedit/mediadet.c
> +++ b/dlls/qedit/mediadet.c
> @@ -283,8 +283,21 @@ static HRESULT WINAPI MediaDet_put_CurrentStream(IMediaDet* iface, LONG newVal)
>  static HRESULT WINAPI MediaDet_get_StreamType(IMediaDet* iface, GUID *pVal)
>  {
>      MediaDetImpl *This = impl_from_IMediaDet(iface);
> - FIXME("(%p)->(%s): not implemented!\n", This, debugstr_guid(pVal));
> - return E_NOTIMPL;
> - AM_MEDIA_TYPE mt;
> - HRESULT hr;
> -
> - TRACE("(%p)->(%p)\n", This, pVal);
> -
> - if (!pVal)
> -
>    return E_POINTER;
> -
> - hr = IMediaDet_get_StreamMediaType(&This->IMediaDet_iface, &mt);
> - if (FAILED(hr))
> -
>    return hr;
> - CoTaskMemFree(mt.pbFormat);

This should be FreeMediaType().
> -
> - *pVal = mt.majortype;
> - return hr;
>
> }
>
> static HRESULT WINAPI MediaDet_get_StreamTypeB(IMediaDet* iface, BSTR *pVal)
> diff --git a/dlls/qedit/tests/mediadet.c b/dlls/qedit/tests/mediadet.c
> index 65f5af7..c92c187 100644
> --- a/dlls/qedit/tests/mediadet.c
> +++ b/dlls/qedit/tests/mediadet.c
> @@ -192,6 +192,7 @@ static void test_mediadet(void)
>      BSTR filename = NULL;
>      LONG nstrms = 0;
>      LONG strm;
> - GUID guid;
> AM_MEDIA_TYPE mt;
> double fps;
> int flags;
>
> @@ -242,6 +243,12 @@ static void test_mediadet(void)
>      hr = IMediaDet_get_StreamMediaType(pM, NULL);
>      ok(hr == E_POINTER, "IMediaDet_get_StreamMediaType failed: %08x\n", hr);
> - hr = IMediaDet_get_StreamType(pM, &guid);
> - ok(hr == E_INVALIDARG, "IMediaDet_get_StreamType failed: %08x\n", hr);
> -
> - hr = IMediaDet_get_StreamType(pM, NULL);
> - ok(hr == E_POINTER, "IMediaDet_get_StreamType failed: %08x\n", hr);
> - filename = SysAllocString(test_avi_filename);
>    hr = IMediaDet_put_Filename(pM, filename);
>    ok(hr == S_OK, "IMediaDet_put_Filename failed: %08x\n", hr);
>
> @@ -258,6 +265,10 @@ static void test_mediadet(void)
>      ok(hr == S_OK, "IMediaDet_get_StreamMediaType failed: %08x\n", hr);
>      CoTaskMemFree(mt.pbFormat);
> - hr = IMediaDet_get_StreamType(pM, &guid);
> - ok(hr == S_OK, "IMediaDet_get_StreamType failed: %08x\n", hr);
> - ok(IsEqualGUID(&guid, &MEDIATYPE_Video), "Wrong GUID %s\n", wine_dbgstr_guid(&guid));
> - /* Even before get_OutputStreams.  */
>    hr = IMediaDet_put_CurrentStream(pM, 1);
>    ok(hr == E_INVALIDARG, "IMediaDet_put_CurrentStream failed: %08x\n", hr);
>
> @@ -311,6 +322,10 @@ static void test_mediadet(void)
>                   "IMediaDet_get_StreamMediaType\n");
>      CoTaskMemFree(mt.pbFormat);
> - hr = IMediaDet_get_StreamType(pM, &guid);
> - ok(hr == S_OK, "IMediaDet_get_StreamType failed: %08x\n", hr);
> - ok(IsEqualGUID(&guid, &MEDIATYPE_Video), "Wrong GUID %s\n", wine_dbgstr_guid(&guid));
> - hr = IMediaDet_get_FrameRate(pM, NULL);
>    ok(hr == E_POINTER, "IMediaDet_get_FrameRate failed: %08x\n", hr);
>
> @@ -370,6 +385,10 @@ static void test_mediadet(void)
>      if (IsEqualGUID(&mt.majortype, &MEDIATYPE_Audio))
>      {
> -
>        hr = IMediaDet_get_StreamType(pM, &guid);
> -
>        ok(hr == S_OK, "IMediaDet_get_StreamType failed: %08x\n", hr);
> -
>        ok(IsEqualGUID(&guid, &MEDIATYPE_Audio), "Wrong GUID %s\n", wine_dbgstr_guid(&guid));
> -
>        hr = IMediaDet_get_FrameRate(pM, &fps);
>        ok(hr == VFW_E_INVALIDMEDIATYPE, "IMediaDet_get_FrameRate failed: %08x\n", hr);
>    }