[PATCH 2/6] qcap/videocapture: Use formats from capabilities instead of hard coding.

[Zebediah Figura]

On 4/23/20 12:28 AM, Jactry Zeng wrote:
> Signed-off-by: Jactry Zeng <jzeng at codeweavers.com>
> ---
>  dlls/qcap/capture.h            |   2 +-
>  dlls/qcap/tests/videocapture.c |  42 ++++++++
>  dlls/qcap/v4l.c                | 175 +++++++++++++--------------------
>  dlls/qcap/vfwcapture.c         |   2 +-
>  4 files changed, 113 insertions(+), 108 deletions(-)
> 
> diff --git a/dlls/qcap/capture.h b/dlls/qcap/capture.h
> index fa48324dd6..d3cf193e7f 100644
> --- a/dlls/qcap/capture.h
> +++ b/dlls/qcap/capture.h
> @@ -25,7 +25,7 @@ typedef struct _Capture Capture;
>  
>  Capture *qcap_driver_init(struct strmbase_source *,USHORT) DECLSPEC_HIDDEN;
>  HRESULT qcap_driver_destroy(Capture*) DECLSPEC_HIDDEN;
> -HRESULT qcap_driver_check_format(Capture*,const AM_MEDIA_TYPE*) DECLSPEC_HIDDEN;
> +HRESULT qcap_driver_check_format(Capture* device,const AM_MEDIA_TYPE *mt,LONG *index) DECLSPEC_HIDDEN;
>  HRESULT qcap_driver_set_format(Capture*,AM_MEDIA_TYPE*) DECLSPEC_HIDDEN;
>  HRESULT qcap_driver_get_format(const Capture*,AM_MEDIA_TYPE**) DECLSPEC_HIDDEN;
>  HRESULT qcap_driver_get_prop_range(Capture*,VideoProcAmpProperty,LONG*,LONG*,LONG*,LONG*,LONG*) DECLSPEC_HIDDEN;
> diff --git a/dlls/qcap/tests/videocapture.c b/dlls/qcap/tests/videocapture.c
> index 5532654650..a43875101c 100644
> --- a/dlls/qcap/tests/videocapture.c
> +++ b/dlls/qcap/tests/videocapture.c
> @@ -21,6 +21,7 @@
>  #define COBJMACROS
>  #include "dshow.h"
>  #include "wine/test.h"
> +#include "wine/strmbase.h"
>  
>  static void test_media_types(IPin *pin)
>  {
> @@ -59,6 +60,44 @@ static void test_media_types(IPin *pin)
>      ok(hr != S_OK, "Got hr %#x.\n", hr);
>  }
>  
> +static void test_stream_config(IPin *pin)
> +{
> +    IAMStreamConfig *stream_config;
> +    VIDEOINFOHEADER *video_info;
> +    AM_MEDIA_TYPE *format;
> +    HRESULT hr;
> +
> +    hr = IPin_QueryInterface(pin, &IID_IAMStreamConfig, (void **)&stream_config);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +
> +    hr = IAMStreamConfig_GetFormat(stream_config, &format);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +    ok(IsEqualGUID(&format->majortype, &MEDIATYPE_Video), "Got wrong majortype: %s.\n",
> +            debugstr_guid(&format->majortype));
> +
> +    hr = IAMStreamConfig_SetFormat(stream_config, format);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +
> +    format->majortype = MEDIATYPE_Audio;
> +    hr = IAMStreamConfig_SetFormat(stream_config, format);
> +    ok(hr == E_FAIL, "Got hr %#x.\n", hr);
> +
> +    format->majortype = MEDIATYPE_Video;
> +    video_info = (VIDEOINFOHEADER *)format->pbFormat;
> +    video_info->bmiHeader.biWidth--;
> +    video_info->bmiHeader.biHeight--;
> +    hr = IAMStreamConfig_SetFormat(stream_config, format);
> +    ok(hr == E_FAIL, "Got hr %#x.\n", hr);
> +
> +    video_info->bmiHeader.biWidth = 10000000;
> +    video_info->bmiHeader.biHeight = 10000000;
> +    hr = IAMStreamConfig_SetFormat(stream_config, format);
> +    ok(hr == E_FAIL, "Got hr %#x.\n", hr);
> +    FreeMediaType(format);
> +
> +    IAMStreamConfig_Release(stream_config);
> +}
> +
>  static void test_capture(IBaseFilter *filter)
>  {
>      IEnumPins *enum_pins;
> @@ -73,7 +112,10 @@ static void test_capture(IBaseFilter *filter)
>          PIN_DIRECTION pin_direction;
>          IPin_QueryDirection(pin, &pin_direction);
>          if (pin_direction == PINDIR_OUTPUT)
> +        {
>              test_media_types(pin);
> +            test_stream_config(pin);
> +        }
>          IPin_Release(pin);
>      }
>  
> diff --git a/dlls/qcap/v4l.c b/dlls/qcap/v4l.c
> index 26eccf6ee3..db9117d593 100644
> --- a/dlls/qcap/v4l.c
> +++ b/dlls/qcap/v4l.c
> @@ -103,7 +103,7 @@ struct capabilitie
>  
>  struct _Capture
>  {
> -    UINT width, height, bitDepth, fps, outputwidth, outputheight;
> +    UINT outputwidth, outputheight;
>      struct capabilitie *current_cap;
>      struct capabilitie **caps;
>      LONG caps_count;
> @@ -146,126 +146,78 @@ HRESULT qcap_driver_destroy(Capture *capBox)
>      return S_OK;
>  }
>  
> -HRESULT qcap_driver_check_format(Capture *device, const AM_MEDIA_TYPE *mt)
> +HRESULT qcap_driver_check_format(Capture *device, const AM_MEDIA_TYPE *mt, LONG *index)

I'd mildly prefer to leave this function signature like it is, and
instead introduce a helper, something like "const struct capabilitie
*find_caps(Capture *device, const AM_MEDIA_TYPE *mt)".

Somewhat separately, I also think we could shrink the "qcap_driver" API
surface to qcap_driver_get_caps(), and put find_caps() and the entire
pin_query_accept()/pin_get_media_type() implementation on the
"vfwcapture.c" side.

>  {
> -    HRESULT hr;
> +    LONG i;
> +
>      TRACE("device %p, mt %p.\n", device, mt);
>  
>      if (!mt)
>          return E_POINTER;
>  
>      if (!IsEqualGUID(&mt->majortype, &MEDIATYPE_Video))
> -        return S_FALSE;
> +        return E_FAIL;
>  
> -    if (IsEqualGUID(&mt->formattype, &FORMAT_VideoInfo) && mt->pbFormat
> -            && mt->cbFormat >= sizeof(VIDEOINFOHEADER))
> +    for (i = device->caps_count - 1; i >= 0; i--)

Is there a motivation for iterating through the array in reverse order here?

>      {
> -        VIDEOINFOHEADER *vih = (VIDEOINFOHEADER *)mt->pbFormat;
> -        if (vih->bmiHeader.biBitCount == 24 && vih->bmiHeader.biCompression == BI_RGB)
> -            hr = S_OK;
> -        else
> +        VIDEOINFOHEADER *video_info = (VIDEOINFOHEADER *)mt->pbFormat;
> +        struct capabilitie *cap = device->caps[i];
> +
> +        if (IsEqualIID(&mt->formattype, &cap->media_type.formattype)
> +                && mt->cbFormat >= sizeof(VIDEOINFOHEADER)
> +                && video_info
> +                && video_info->bmiHeader.biWidth == cap->width
> +                && video_info->bmiHeader.biHeight == cap->height)
>          {
> -            FIXME("Unsupported compression %#x, bpp %u.\n", vih->bmiHeader.biCompression,
> -                    vih->bmiHeader.biBitCount);
> -            hr = S_FALSE;
> +            if (index)
> +                *index = i;
> +            TRACE("matched with: %d.\n", i);
> +            return S_OK;
>          }
>      }
> -    else
> -        hr = VFW_E_INVALIDMEDIATYPE;
>  
> -    return hr;
> +    return E_FAIL;
>  }
>