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

Zebediah Figura zfigura at codeweavers.com
Thu Apr 16 11:44:37 CDT 2020


On 4/16/20 10:25 AM, Gabriel Ivăncescu wrote:
> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
> ---
>  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);
>  
> 



More information about the wine-devel mailing list