<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 22, 2019 at 11:06 PM Zebediah Figura <<a href="mailto:z.figura12@gmail.com">z.figura12@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 4/22/19 2:40 AM, Damjan Jovanovic wrote:<br>
> +static void test_connect_null_renderer(IBaseFilter *captureDevice)<br>
> +{<br>
> +    IGraphBuilder *graph = NULL;<br>
> +    IBaseFilter *nullRenderer = NULL;<br>
> +    IPin *captureDeviceOut = NULL;<br>
> +    IPin *nullRendererIn = NULL;<br>
> +    HRESULT hr;<br>
> +<br>
> +    hr = CoCreateInstance(&CLSID_FilterGraph, NULL, CLSCTX_INPROC_SERVER, &IID_IGraphBuilder,<br>
> +            (LPVOID*)&graph);<br>
> +    ok(SUCCEEDED(hr), "couldn't create graph builder, hr=0x%08x\n", hr);<br>
> +    if (FAILED(hr))<br>
> +        goto end;<br>
> +<br>
> +    hr = CoCreateInstance(&CLSID_NullRenderer, NULL, CLSCTX_INPROC_SERVER,<br>
> +            &IID_IBaseFilter, (LPVOID*)&nullRenderer);<br>
> +    ok(SUCCEEDED(hr) ||<br>
> +            /* Windows 2008: <a href="http://stackoverflow.com/questions/29410348/initialize-nullrender-failed-with-error-regdb-e-classnotreg-on-win2008-r2" rel="noreferrer" target="_blank">http://stackoverflow.com/questions/29410348/initialize-nullrender-failed-with-error-regdb-e-classnotreg-on-win2008-r2</a> */<br>
> +            broken(hr == REGDB_E_CLASSNOTREG), "couldn't create NullRenderer, hr=0x%08x\n", hr);<br>
> +    if (FAILED(hr))<br>
> +        goto end;<br>
> +    hr = IGraphBuilder_AddFilter(graph, nullRenderer, NULL);<br>
> +    ok(SUCCEEDED(hr), "IGraphBuilder_AddFilter() failed, hr=0x%08x\n", hr);<br>
> +    if (FAILED(hr))<br>
> +        goto end;<br>
> +<br>
> +    hr = IGraphBuilder_AddFilter(graph, captureDevice, NULL);<br>
> +    ok(SUCCEEDED(hr), "IGraphBuilder_AddFilter() failed, hr=0x%08x\n", hr);<br>
> +    if (FAILED(hr))<br>
> +        goto end;<br>
> +<br>
> +    hr = find_pin(nullRenderer, PINDIR_INPUT, &nullRendererIn);<br>
> +    ok(hr == S_OK, "couldn't find NullRenderer input pin, hr=0x%08x\n", hr);<br>
> +    if (hr != S_OK)<br>
> +        goto end;<br>
> +<br>
> +    hr = find_pin(captureDevice, PINDIR_OUTPUT, &captureDeviceOut);<br>
> +    ok(hr == S_OK, "couldn't find capture device output pin, hr=0x%08x\n", hr);<br>
> +    if (hr != S_OK)<br>
> +        goto end;<br>
> +<br>
> +    hr = IGraphBuilder_Connect(graph, captureDeviceOut, nullRendererIn);<br>
> +    ok(SUCCEEDED(hr), "couldn't connect capture device to NullRenderer, hr=0x%08x\n", hr);<br>
> +<br>
> +end:<br>
> +    if (graph != NULL)<br>
> +        IGraphBuilder_Release(graph);<br>
> +    if (nullRenderer != NULL)<br>
> +        IBaseFilter_Release(nullRenderer);<br>
> +    if (captureDeviceOut != NULL)<br>
> +        IPin_Release(captureDeviceOut);<br>
> +    if (nullRendererIn != NULL)<br>
> +        IPin_Release(nullRendererIn);<br>
> +}<br>
> +<br>
<br>
This is not especially convincing. I know that we can't really test <br>
individual formats, given that devices themselves are going to vary, and <br>
I know also that you're using the same logic as is already present in <br>
IAMStreamConfig::SetFormat(), but it would be nice to test that <br>
IPin::QueryAccept() accepts formats returned by IEnumMediaTypes (if any) <br>
and by IAMStreamConfig::GetFormat() and IAMStreamConfig::GetStreamCaps() <br>
and rejects formats which are malformed, or use a major type other than <br>
MEDIATYPE_Video or GUID_NULL, or use a format type other than <br>
FORMAT_VideoInfo or FORMAT_NULL, etc.<br>
<br>
Stylistically I'd prefer if you'd use the same style as the quartz tests <br>
I've been recently adding. This means e.g. not using camel case, <br>
avoiding LPJUNK typedefs, braces on a new line, messages and comments <br>
formatted with consistent wording and period. Additionally I'd prefer <br>
that you test for S_OK where possible rather than using SUCCEEDED(), and <br>
omit superfluous failure paths where we expect success.<br>
<br></blockquote><div><br></div><div>Thank you. A new version with some of those changes is out.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> diff --git a/dlls/qcap/tests/Makefile.in b/dlls/qcap/tests/Makefile.in<br>
> index 2b988476ad..a5b9d0e474 100644<br>
> --- a/dlls/qcap/tests/Makefile.in<br>
> +++ b/dlls/qcap/tests/Makefile.in<br>
> @@ -5,4 +5,5 @@ C_SRCS = \<br>
>       audiorecord.c \<br>
>       avico.c \<br>
>       qcap.c \<br>
> -     smartteefilter.c<br>
> +     smartteefilter.c \<br>
> +     videocapture.c<br>
As long as these tests are aimed towards the WDM video capture filter <br>
rather than the VFW capture filter, I'd prefer a name like wdmcapture.c.<br>
<br></blockquote><div><br></div><div>I don't see how that's relevant. We test DirectShow, not the kernel-mode device drivers.</div><div><br></div><div>Also wdmcapture.c doesn't tell us what media is being captured.<br></div><div> </div></div></div>