On Wed, 1 Feb 2017 15:28:24 -0500 Micah Fedke <[email protected]> wrote:
> The v4l2 API can be queried to detect if the input video image is
> horizontally or vertically flipped. If the image is y-flipped, we can
> set the ZWP_LINUX_BUFFER_PARAMS_V1_FLAGS_Y_INVERT flag to notify the
> compositor. If the image is h-flipped, we can only print a warning
> since linux_buffer_params_v1 does not support horizontal flipping.
> ---
> clients/simple-dmabuf-v4l.c | 54
> ++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 46 insertions(+), 8 deletions(-)
>
> diff --git a/clients/simple-dmabuf-v4l.c b/clients/simple-dmabuf-v4l.c
> index af25d0ea..e4e2d7c7 100644
> --- a/clients/simple-dmabuf-v4l.c
> +++ b/clients/simple-dmabuf-v4l.c
> @@ -351,21 +351,59 @@ static const struct zwp_linux_buffer_params_v1_listener
> params_listener = {
> create_failed
> };
Hi Micah,
I only have some easy comments, it would be nice if someone could
double-check the V4L2 API usage. The functionality looks correct to me.
>
> +static bool
> +check_v4l2_control(const int fd,
Since this seems to be hardcoded to boolean controls, how about calling
it check_v4l2_control_bool() instead?
> + const struct v4l2_query_ext_ctrl *qectrl,
> + const char *control_name,
> + const int len,
I don't think 'len' is necessary, everything should be nul-terminated,
and we don't want to accidentally match a substring.
> + const int expected_value)
> +{
> + struct v4l2_control ctrl;
> +
> + memset(&ctrl, 0, sizeof(ctrl));
> + ctrl.id = qectrl->id;
> +
> + if (!(qectrl->flags & V4L2_CTRL_FLAG_DISABLED) &&
> + (qectrl->type == V4L2_CTRL_TYPE_BOOLEAN) &&
> + !strncmp(qectrl->name, control_name, len) &&
> + !xioctl(fd, VIDIOC_G_CTRL, &ctrl) &&
> + (ctrl.value == expected_value))
This pretty compactly written. Personally I'd prefer testing things a
bit more separately, to e.g. emphasize that there is a call to xioctl()
hidden in there. This is a small function where it is easy to just
return early when something doesn't match.
> + return true;
> + else
> + return false;
> +}
> +
> static void
> create_dmabuf_buffer(struct display *display, struct buffer *buffer)
> {
> struct zwp_linux_buffer_params_v1 *params;
> uint64_t modifier;
> - uint32_t flags;
> + uint32_t lbp_flags;
> unsigned i;
> + struct v4l2_query_ext_ctrl qectrl;
> + const unsigned int v4l2_qec_flags =
> + V4L2_CTRL_FLAG_NEXT_CTRL |
> V4L2_CTRL_FLAG_NEXT_COMPOUND;
> + const char *vflip_ctrl = "Vertical Flip";
> + const char *hflip_ctrl = "Horizontal Flip";
>
> modifier = 0;
> - flags = 0;
> -
> - /* XXX: apparently some webcams may actually provide y-inverted images,
> - * in which case we should set
> - * flags = ZWP_LINUX_BUFFER_PARAMS_V1_FLAGS_Y_INVERT
> - */
> + lbp_flags = 0;
> +
> + /* handle relevant v4l2 controls */
> + memset(&qectrl, 0, sizeof(qectrl));
> + qectrl.id |= v4l2_qec_flags;
> + while (!xioctl(display->v4l_fd, VIDIOC_QUERY_EXT_CTRL, &qectrl)) {
> + if (check_v4l2_control(display->v4l_fd, &qectrl, vflip_ctrl,
> + strlen(vflip_ctrl), 0x1)) {
> + lbp_flags = ZWP_LINUX_BUFFER_PARAMS_V1_FLAGS_Y_INVERT;
> + printf ("\"%s\" control is set, inverting
> Y\n",vflip_ctrl);
> + } else if (check_v4l2_control(display->v4l_fd,
> &qectrl,hflip_ctrl,
Many missing spaces after commas.
> + strlen(hflip_ctrl), 0x1)) {
> + printf ("\"%s\" control is set, but dmabuf output
> cannot"
> + "be flipped horizontally\n", hflip_ctrl);
> + }
> + qectrl.id |= v4l2_qec_flags;
> + }
>
> params = zwp_linux_dmabuf_v1_create_params(display->dmabuf);
> for (i = 0; i < display->format.num_planes; ++i)
> @@ -382,7 +420,7 @@ create_dmabuf_buffer(struct display *display, struct
> buffer *buffer)
> display->format.width,
> display->format.height,
> display->drm_format,
> - flags);
> + lbp_flags);
> }
>
> static int
Anyway, this patch does what I wanted it to do.
Thanks,
pq
pgpnVfQR8zB6j.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
