diff mbox series

[v4l-utils] v4l2-compliance: encoders should not support capture side G_PARM

Message ID 20191107114519.16624-1-p.zabel@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series [v4l-utils] v4l2-compliance: encoders should not support capture side G_PARM | expand

Commit Message

Philipp Zabel Nov. 7, 2019, 11:45 a.m. UTC
Encoders that implement output side G/S_PARM and ENUM_FRAMEINTERVALS
to allow setting the nominal bitstream frame rate cause compliance
failures:

		fail: v4l2-test-formats.cpp(1330): ret && node->has_frmintervals
	test VIDIOC_G/S_PARM: FAIL

because v4l2-compliance is missing capture side G/S_PARM. Skip the
frameinterval check and make sure that capture side G_PARM fails.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 utils/v4l2-compliance/v4l2-test-formats.cpp | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Hans Verkuil Nov. 8, 2019, 3:19 p.m. UTC | #1
On 11/7/19 12:45 PM, Philipp Zabel wrote:
> Encoders that implement output side G/S_PARM and ENUM_FRAMEINTERVALS
> to allow setting the nominal bitstream frame rate cause compliance
> failures:
> 
> 		fail: v4l2-test-formats.cpp(1330): ret && node->has_frmintervals
> 	test VIDIOC_G/S_PARM: FAIL
> 
> because v4l2-compliance is missing capture side G/S_PARM. Skip the
> frameinterval check and make sure that capture side G_PARM fails.

It is completely undocumented that G/S_PARM should be valid only for the OUTPUT
type.

My opinion is that for M2M devices G/S_PARM should be valid for both CAPTURE
and OUTPUT. The framerate is a property of the m2m device itself, and both
queues share this information.

vidioc_enum_frameintervals is also shared between both queues.

For the stateful encoder spec I was planning to make this explicit: i.e. G/S_PARM
can be called for both capture and output buf types. And it should be mentioned
in the G/S_PARM documentation as well.

I think it makes sense, and I also think that that means that this test doesn't
need to be changed (but the coda driver does).

Regards,

	Hans

> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  utils/v4l2-compliance/v4l2-test-formats.cpp | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp b/utils/v4l2-compliance/v4l2-test-formats.cpp
> index b87e59c8be69..7336a823a269 100644
> --- a/utils/v4l2-compliance/v4l2-test-formats.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
> @@ -1283,6 +1283,11 @@ static int testParmType(struct node *node, unsigned type)
>  	switch (type) {
>  	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
>  	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> +		if (IS_ENCODER(node)) {
> +			fail_on_test(ret == 0);
> +			break;
> +		}
> +		/* fallthrough */
>  	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
>  	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
>  		if (type && (node->g_caps() & buftype2cap[type]))
>
diff mbox series

Patch

diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp b/utils/v4l2-compliance/v4l2-test-formats.cpp
index b87e59c8be69..7336a823a269 100644
--- a/utils/v4l2-compliance/v4l2-test-formats.cpp
+++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
@@ -1283,6 +1283,11 @@  static int testParmType(struct node *node, unsigned type)
 	switch (type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
+		if (IS_ENCODER(node)) {
+			fail_on_test(ret == 0);
+			break;
+		}
+		/* fallthrough */
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
 		if (type && (node->g_caps() & buftype2cap[type]))