diff mbox series

[6/7] media: coda: enable capture G_PARM for stateful encoder

Message ID 20220404163533.707508-6-p.zabel@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series [1/7] media: coda: set output buffer bytesused to appease v4l2-compliance | expand

Commit Message

Philipp Zabel April 4, 2022, 4:35 p.m. UTC
Allow to call G_PARM with type == V4L2_BUF_TYPE_VIDEO_CAPTURE,
to fix the following v4l2-compliance test failure:

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

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/chips-media/coda-common.c | 3 ---
 1 file changed, 3 deletions(-)

Comments

Nicolas Dufresne April 5, 2022, 2:19 p.m. UTC | #1
Le lundi 04 avril 2022 à 18:35 +0200, Philipp Zabel a écrit :
> Allow to call G_PARM with type == V4L2_BUF_TYPE_VIDEO_CAPTURE,
> to fix the following v4l2-compliance test failure:
> 
> 		fail: v4l2-test-formats.cpp(1344): ret && node->has_frmintervals
> 	test VIDIOC_G/S_PARM: FAIL

So basically the rate written in the bitstream (if any) will be the same as the
target real-time rate, which matches my reading of the new spec as what default
behaviour we should have.

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/media/platform/chips-media/coda-common.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/media/platform/chips-media/coda-common.c b/drivers/media/platform/chips-media/coda-common.c
> index c068c16d1eb4..33fcd8c7d72b 100644
> --- a/drivers/media/platform/chips-media/coda-common.c
> +++ b/drivers/media/platform/chips-media/coda-common.c
> @@ -1341,9 +1341,6 @@ static int coda_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
>  	struct coda_ctx *ctx = fh_to_ctx(fh);
>  	struct v4l2_fract *tpf;
>  
> -	if (a->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> -		return -EINVAL;
> -
>  	a->parm.output.capability = V4L2_CAP_TIMEPERFRAME;
>  	tpf = &a->parm.output.timeperframe;
>  	tpf->denominator = ctx->params.framerate & CODA_FRATE_RES_MASK;
Hans Verkuil April 21, 2022, 10:30 a.m. UTC | #2
Hi Philipp,

On 04/04/2022 18:35, Philipp Zabel wrote:
> Allow to call G_PARM with type == V4L2_BUF_TYPE_VIDEO_CAPTURE,
> to fix the following v4l2-compliance test failure:
> 
> 		fail: v4l2-test-formats.cpp(1344): ret && node->has_frmintervals
> 	test VIDIOC_G/S_PARM: FAIL
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/media/platform/chips-media/coda-common.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/media/platform/chips-media/coda-common.c b/drivers/media/platform/chips-media/coda-common.c
> index c068c16d1eb4..33fcd8c7d72b 100644
> --- a/drivers/media/platform/chips-media/coda-common.c
> +++ b/drivers/media/platform/chips-media/coda-common.c
> @@ -1341,9 +1341,6 @@ static int coda_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
>  	struct coda_ctx *ctx = fh_to_ctx(fh);
>  	struct v4l2_fract *tpf;
>  
> -	if (a->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> -		return -EINVAL;
> -
>  	a->parm.output.capability = V4L2_CAP_TIMEPERFRAME;
>  	tpf = &a->parm.output.timeperframe;
>  	tpf->denominator = ctx->params.framerate & CODA_FRATE_RES_MASK;

I think this is actually a v4l2-compliance bug, not a driver bug.

G/S_PARM doesn't make sense for the capture queue of a stateful encoder, unless
V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL is set to reserve HW resources.

See https://hverkuil.home.xs4all.nl/spec/userspace-api/v4l/vidioc-enum-fmt.html#fmtdesc-flags

That flags isn't used, so v4l2-compliance shouldn't complain.

Try this v4l2-compliance patch to see if it resolved the fails for this patch
and the next patch (7/7).

v4l2-compliance patch:

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp b/utils/v4l2-compliance/v4l2-test-formats.cpp
index 3761b1fa..269a3832 100644
--- a/utils/v4l2-compliance/v4l2-test-formats.cpp
+++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
@@ -1341,8 +1341,16 @@ static int testParmType(struct node *node, unsigned type)
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
 		if (node->g_caps() & buftype2cap[type]) {
-			fail_on_test(ret && node->has_frmintervals);
-			fail_on_test(ret && node->has_enc_cap_frame_interval);
+			if (is_stateful_enc) {
+				if (V4L2_TYPE_IS_OUTPUT(type))
+					fail_on_test(ret && node->has_frmintervals);
+				else if (node->has_enc_cap_frame_interval)
+					fail_on_test(ret);
+				else
+					fail_on_test(!ret);
+			} else {
+				fail_on_test(ret && node->has_frmintervals);
+			}
 		}
 		break;
 	default:
Philipp Zabel April 21, 2022, 2:58 p.m. UTC | #3
On Do, 2022-04-21 at 12:30 +0200, Hans Verkuil wrote:
[...]
> I think this is actually a v4l2-compliance bug, not a driver bug.
> 
> G/S_PARM doesn't make sense for the capture queue of a stateful encoder, unless
> V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL is set to reserve HW resources.
> 
> See https://hverkuil.home.xs4all.nl/spec/userspace-api/v4l/vidioc-enum-fmt.html#fmtdesc-flags
> 
> That flags isn't used, so v4l2-compliance shouldn't complain.
> 
> Try this v4l2-compliance patch to see if it resolved the fails for this patch
> and the next patch (7/7).
> 
> v4l2-compliance patch:
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
> diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp b/utils/v4l2-compliance/v4l2-test-formats.cpp
> index 3761b1fa..269a3832 100644
> --- a/utils/v4l2-compliance/v4l2-test-formats.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
> @@ -1341,8 +1341,16 @@ static int testParmType(struct node *node, unsigned type)
>  	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
>  	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
>  		if (node->g_caps() & buftype2cap[type]) {
> -			fail_on_test(ret && node->has_frmintervals);
> -			fail_on_test(ret && node->has_enc_cap_frame_interval);
> +			if (is_stateful_enc) {
> +				if (V4L2_TYPE_IS_OUTPUT(type))
> +					fail_on_test(ret && node->has_frmintervals);
> +				else if (node->has_enc_cap_frame_interval)
> +					fail_on_test(ret);
> +				else
> +					fail_on_test(!ret);
> +			} else {
> +				fail_on_test(ret && node->has_frmintervals);
> +			}
>  		}
>  		break;
>  	default:

You are right, this patch resolves the compliance failures addressed by
patches 6 and 7.

regards
Philipp
Hans Verkuil April 25, 2022, 5:34 a.m. UTC | #4
On 21/04/2022 16:58, Philipp Zabel wrote:
> On Do, 2022-04-21 at 12:30 +0200, Hans Verkuil wrote:
> [...]
>> I think this is actually a v4l2-compliance bug, not a driver bug.
>>
>> G/S_PARM doesn't make sense for the capture queue of a stateful encoder, unless
>> V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL is set to reserve HW resources.
>>
>> See https://hverkuil.home.xs4all.nl/spec/userspace-api/v4l/vidioc-enum-fmt.html#fmtdesc-flags
>>
>> That flags isn't used, so v4l2-compliance shouldn't complain.
>>
>> Try this v4l2-compliance patch to see if it resolved the fails for this patch
>> and the next patch (7/7).
>>
>> v4l2-compliance patch:
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>> diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp b/utils/v4l2-compliance/v4l2-test-formats.cpp
>> index 3761b1fa..269a3832 100644
>> --- a/utils/v4l2-compliance/v4l2-test-formats.cpp
>> +++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
>> @@ -1341,8 +1341,16 @@ static int testParmType(struct node *node, unsigned type)
>>  	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
>>  	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
>>  		if (node->g_caps() & buftype2cap[type]) {
>> -			fail_on_test(ret && node->has_frmintervals);
>> -			fail_on_test(ret && node->has_enc_cap_frame_interval);
>> +			if (is_stateful_enc) {
>> +				if (V4L2_TYPE_IS_OUTPUT(type))
>> +					fail_on_test(ret && node->has_frmintervals);
>> +				else if (node->has_enc_cap_frame_interval)
>> +					fail_on_test(ret);
>> +				else
>> +					fail_on_test(!ret);
>> +			} else {
>> +				fail_on_test(ret && node->has_frmintervals);
>> +			}
>>  		}
>>  		break;
>>  	default:
> 
> You are right, this patch resolves the compliance failures addressed by
> patches 6 and 7.

Great! I've committed this change to v4l-utils.

Regards,

	Hans
diff mbox series

Patch

diff --git a/drivers/media/platform/chips-media/coda-common.c b/drivers/media/platform/chips-media/coda-common.c
index c068c16d1eb4..33fcd8c7d72b 100644
--- a/drivers/media/platform/chips-media/coda-common.c
+++ b/drivers/media/platform/chips-media/coda-common.c
@@ -1341,9 +1341,6 @@  static int coda_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
 	struct coda_ctx *ctx = fh_to_ctx(fh);
 	struct v4l2_fract *tpf;
 
-	if (a->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
-		return -EINVAL;
-
 	a->parm.output.capability = V4L2_CAP_TIMEPERFRAME;
 	tpf = &a->parm.output.timeperframe;
 	tpf->denominator = ctx->params.framerate & CODA_FRATE_RES_MASK;