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