Re: [PATCH v2] v4l2-compliance: add tests for VIDIOC_S_FBUF/OVERLAY and selection flags

[Hans Verkuil <hverkuil+cisco@kernel.org>]

Hi Rivka,

On 12/03/2026 09:18, Rivka Bukchin wrote:
> Add compliance tests for VIDIOC_S_FBUF and VIDIOC_OVERLAY to verify
> basic framebuffer and overlay handling.
> 
> Add tests for VIDIOC_S_SELECTION flag handling to ensure that valid
> flags (V4L2_SEL_FLAG_GE and V4L2_SEL_FLAG_LE) are accepted and that
> invalid flag combinations are rejected.
> 
> These tests extend the coverage of the format and selection ioctl
> compliance checks.
> 
> Signed-off-by: Rivka Bukchin <rivkab300@gmail.com>
> ---
> v2: Rebased on upstream v4l-utils tree
> 
>  utils/v4l2-compliance/v4l2-compliance.cpp   |  2 +
>  utils/v4l2-compliance/v4l2-compliance.h     |  2 +
>  utils/v4l2-compliance/v4l2-test-formats.cpp | 78 +++++++++++++++++++++
>  3 files changed, 82 insertions(+)
> 
> diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
> index 4e5c9d00deb5..20d5f329d335 100644
> --- a/utils/v4l2-compliance/v4l2-compliance.cpp
> +++ b/utils/v4l2-compliance/v4l2-compliance.cpp
> @@ -1464,6 +1464,8 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
>  		printf("\ttest Cropping: %s\n", ok(testCropping(&node)));
>  		printf("\ttest Composing: %s\n", ok(testComposing(&node)));
>  		printf("\ttest Scaling: %s\n", ok(testScaling(&node)));
> +		printf("\ttest Overlay: %s\n", ok(testOverlay(&node)));
> +        printf("\ttest Selection Flags: %s\n", ok(testSelectionFlags(&node)));

Bad indentation! Spaces instead of TABs.

>  		printf("\n");
>  
>  		/* Codec ioctls */
> diff --git a/utils/v4l2-compliance/v4l2-compliance.h b/utils/v4l2-compliance/v4l2-compliance.h
> index 4a7af5f5bce5..265cd08397fa 100644
> --- a/utils/v4l2-compliance/v4l2-compliance.h
> +++ b/utils/v4l2-compliance/v4l2-compliance.h
> @@ -375,6 +375,8 @@ int testSlicedVBICap(struct node *node);
>  int testCropping(struct node *node);
>  int testComposing(struct node *node);
>  int testScaling(struct node *node);
> +int testOverlay(struct node *node);
> +int testSelectionFlags(struct node *node);
>  
>  // Codec ioctl tests
>  int testEncoder(struct node *node);
> diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp b/utils/v4l2-compliance/v4l2-test-formats.cpp
> index 56b6614162cf..6c5743a3da47 100644
> --- a/utils/v4l2-compliance/v4l2-test-formats.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
> @@ -2048,3 +2048,81 @@ int testScaling(struct node *node)
>  	}
>  	return node->can_scale ? 0 : ENOTTY;
>  }
> +
> +int testOverlay(struct node *node)
> +{
> +    struct v4l2_framebuffer fbuf;
> +    int ret;
> +
> +    memset(&fbuf, 0xff, sizeof(fbuf));
> +    fbuf.fmt.priv = 0;
> +
> +    ret = doioctl(node, VIDIOC_G_FBUF, &fbuf);
> +    if (ret == ENOTTY)
> +        return ret;
> +    if (ret == EINVAL)
> +        return ENOTTY;
> +    fail_on_test(ret);
> +
> +    if (!(node->g_caps() & (V4L2_CAP_VIDEO_OVERLAY |
> +                V4L2_CAP_VIDEO_OUTPUT_OVERLAY)))
> +        return ENOTTY;
> +
> +    struct v4l2_framebuffer set_fbuf = fbuf;
> +
> +    ret = doioctl(node, VIDIOC_S_FBUF, &set_fbuf);
> +    fail_on_test(ret && ret != ENOTTY && ret != EINVAL);
> +
> +    int enable = 1;
> +
> +    ret = doioctl(node, VIDIOC_OVERLAY, &enable);
> +    fail_on_test(ret && ret != ENOTTY && ret != EINVAL);
> +
> +    enable = 0;
> +
> +    ret = doioctl(node, VIDIOC_OVERLAY, &enable);
> +    fail_on_test(ret && ret != ENOTTY && ret != EINVAL);
> +
> +    return 0;
> +}

Just drop this. It's not worth supporting this, overlays are almost never used
these days.

> +
> +int testSelectionFlags(struct node *node)
> +{
> +    struct v4l2_selection sel = {
> +        node->can_capture ?
> +            V4L2_BUF_TYPE_VIDEO_CAPTURE :
> +            V4L2_BUF_TYPE_VIDEO_OUTPUT,
> +        V4L2_SEL_TGT_CROP
> +    };
> +    int ret;
> +
> +    memset(sel.reserved, 0xff, sizeof(sel.reserved));
> +
> +    ret = doioctl(node, VIDIOC_G_SELECTION, &sel);
> +    if (ret == ENOTTY || ret == EINVAL || ret == ENODATA)
> +        return ENOTTY;
> +
> +    fail_on_test(ret);
> +    fail_on_test(check_0(sel.reserved, sizeof(sel.reserved)));
> +
> +    struct v4l2_selection s = sel;
> +
> +    s.flags = V4L2_SEL_FLAG_GE;
> +    doioctl(node, VIDIOC_S_SELECTION, &s);
> +
> +    s = sel;
> +    s.flags = V4L2_SEL_FLAG_LE;
> +    doioctl(node, VIDIOC_S_SELECTION, &s);
> +
> +    s = sel;
> +    s.flags = V4L2_SEL_FLAG_GE | V4L2_SEL_FLAG_LE;
> +    doioctl(node, VIDIOC_S_SELECTION, &s);
> +
> +    s = sel;
> +    s.flags = ~0;
> +
> +    ret = doioctl(node, VIDIOC_S_SELECTION, &s);
> +    fail_on_test(ret == 0);

You're not testing anything in this function.

It would be worthwhile improving testBasicSelection() by adding tests for flag
handling: e.g. get the current selection, increment the width by 1, then call
S_SELECTION with FLAG_LE: the result must either be the original selection or
the original selection but with width incremented by 1.

I think a lot of drivers probably do not implement this or implement it poorly,
so new tests would be welcome, but this code isn't useful.

Regards,

	Hans

> +
> +    return 0;
> +}