[PATCH v3 3/4] qcap: Delay setting V4L format until creating stream.

[Zebediah Figura]

On 9/7/20 3:17 PM, Jeff Smith wrote:
> Signed-off-by: Jeff Smith <whydoubt at gmail.com>
> ---
> Set the V4L format during stream initialization only, and only use the
> VIDIOC_TRY_FMT ioctl elsewhere.  This, along with the previous patch,
> allows for using current_caps exclusively to indicate the latest format
> passed to SetFormat (or NULL if it has not been called).
> 
>  dlls/qcap/v4l.c | 45 ++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 36 insertions(+), 9 deletions(-)
> 

My point is that if I didn't have access to CodeWeavers' internal bug
tracker, I wouldn't understand why this is necessary. E.g. "Some
applications opens two filters simultaneously, although they only read
from of them. Some v4l2 drivers forbid setting the format on more than
one open descriptor at a time."

> diff --git a/dlls/qcap/v4l.c b/dlls/qcap/v4l.c
> index dade5a51e0..f151cc66a3 100644
> --- a/dlls/qcap/v4l.c
> +++ b/dlls/qcap/v4l.c
> @@ -196,13 +196,36 @@ static BOOL set_caps(struct v4l_device *device, const struct caps *caps)
>          return FALSE;
>      }
>  
> -    device->current_caps = caps;
>      device->image_size = width * height * caps->video_info.bmiHeader.biBitCount / 8;
>      device->image_pitch = width * caps->video_info.bmiHeader.biBitCount / 8;
>  
>      return TRUE;
>  }
>  
> +static BOOL try_caps(struct v4l_device *device, const struct caps *caps)
> +{
> +    struct v4l2_format format = {0};
> +    LONG width = caps->video_info.bmiHeader.biWidth;
> +    LONG height = caps->video_info.bmiHeader.biHeight;
> +
> +    TRACE("device %p, width %d, height %d, pixelformat %#x.\n", device, width, height, caps->pixelformat);
> +
> +    format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +    format.fmt.pix.pixelformat = caps->pixelformat;
> +    format.fmt.pix.width = width;
> +    format.fmt.pix.height = height;
> +    if (xioctl(device->fd, VIDIOC_TRY_FMT, &format) == -1
> +            || format.fmt.pix.pixelformat != caps->pixelformat
> +            || format.fmt.pix.width != width
> +            || format.fmt.pix.height != height)
> +    {
> +        ERR("Failed trying pixel format: %s.\n", strerror(errno));
> +        return FALSE;
> +    }
> +
> +    return TRUE;
> +}
> +
>  static HRESULT v4l_device_set_format(struct video_capture_device *iface, const AM_MEDIA_TYPE *mt)
>  {
>      struct v4l_device *device = v4l_device(iface);
> @@ -215,9 +238,11 @@ static HRESULT v4l_device_set_format(struct video_capture_device *iface, const A
>      if (device->current_caps == caps)
>          return S_OK;
>  
> -    if (!set_caps(device, caps))
> +    if (!try_caps(device, caps))
>          return VFW_E_TYPE_NOT_ACCEPTED;
>  
> +    device->current_caps = caps;
> +
>      return S_OK;
>  }
>  
> @@ -225,7 +250,10 @@ static HRESULT v4l_device_get_format(struct video_capture_device *iface, AM_MEDI
>  {
>      struct v4l_device *device = v4l_device(iface);
>  
> -    return CopyMediaType(mt, &device->current_caps->media_type);
> +    if (device->current_caps)
> +        return CopyMediaType(mt, &device->current_caps->media_type);
> +
> +    return CopyMediaType(mt, &device->caps[0].media_type);
>  }
>  
>  static __u32 v4l2_cid_from_qcap_property(VideoProcAmpProperty property)
> @@ -396,6 +424,10 @@ static void v4l_device_init_stream(struct video_capture_device *iface)
>      ALLOCATOR_PROPERTIES req_props, ret_props;
>      HRESULT hr;
>  
> +    /* We must commit to a particular format before reading from device. */
> +    if (!set_caps(device, (device->current_caps) ? device->current_caps : &device->caps[0]))
> +        ERR("Failed to set format.\n");
> +
>      req_props.cBuffers = 3;
>      req_props.cbBuffer = device->image_size;
>      req_props.cbAlign = 1;
> @@ -656,9 +688,8 @@ struct video_capture_device *v4l_device_create(struct strmbase_source *pin, USHO
>      for (i = 0; i < device->caps_count; ++i)
>          device->caps[i].media_type.pbFormat = (BYTE *)&device->caps[i].video_info;
>  
> -    if (!set_caps(device, &device->caps[0]))
> +    if (!try_caps(device, &device->caps[0]))
>      {
> -        ERR("Failed to set pixel format: %s\n", strerror(errno));
>          if (!have_libv4l2)
>              ERR_(winediag)("You may need libv4l2 to use this device.\n");
>          goto error;
> @@ -671,10 +702,6 @@ struct video_capture_device *v4l_device_create(struct strmbase_source *pin, USHO
>      InitializeCriticalSection(&device->state_cs);
>      device->state_cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": v4l_device.state_cs");
>  
> -    TRACE("Format: %d bpp - %dx%d.\n", device->current_caps->video_info.bmiHeader.biBitCount,
> -            device->current_caps->video_info.bmiHeader.biWidth,
> -            device->current_caps->video_info.bmiHeader.biHeight);
> -
>      return &device->d;
>  
>  error:
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20200908/2b2706d7/attachment.sig>