* [PATCH] Allow S/G_PARM for stateful decoder
@ 2022-08-29 7:54 Dikshita Agarwal
2022-09-09 12:16 ` Hans Verkuil
0 siblings, 1 reply; 3+ messages in thread
From: Dikshita Agarwal @ 2022-08-29 7:54 UTC (permalink / raw)
To: linux-media, linux-kernel, hverkuil-cisco
Cc: stanimir.varbanov, quic_vgarodia, quic_majja, quic_jdas,
Dikshita Agarwal
Some stateful decoder supports S/G_PARM similar to a
stateful encoder. S_PARM(OUTPUT) reserves hardware decoder
resources, and G_PARM(CAPTURE) returns the embedded
frame interval. Allow the same with this change.
Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
---
utils/v4l2-compliance/v4l2-test-formats.cpp | 4 ----
1 file changed, 4 deletions(-)
diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp b/utils/v4l2-compliance/v4l2-test-formats.cpp
index 269a383..e996558 100644
--- a/utils/v4l2-compliance/v4l2-test-formats.cpp
+++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
@@ -64,8 +64,6 @@ static int testEnumFrameIntervals(struct node *node, __u32 pixfmt,
ret = doioctl(node, VIDIOC_ENUM_FRAMEINTERVALS, &frmival);
if (ret == ENOTTY)
return ret;
- // M2M devices don't support this, except for stateful encoders
- fail_on_test(node->is_m2m && !(node->codec_mask & STATEFUL_ENCODER));
if (f == 0 && ret == EINVAL) {
if (type == V4L2_FRMSIZE_TYPE_DISCRETE)
warn("found framesize %dx%d, but no frame intervals\n", w, h);
@@ -1367,8 +1365,6 @@ static int testParmType(struct node *node, unsigned type)
}
if (ret == ENOTTY)
return ret;
- // M2M devices don't support this, except for stateful encoders
- fail_on_test(node->is_m2m && !is_stateful_enc);
if (ret == EINVAL)
return ENOTTY;
if (ret)
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Allow S/G_PARM for stateful decoder
2022-08-29 7:54 [PATCH] Allow S/G_PARM for stateful decoder Dikshita Agarwal
@ 2022-09-09 12:16 ` Hans Verkuil
2022-09-28 6:07 ` Dikshita Agarwal (QUIC)
0 siblings, 1 reply; 3+ messages in thread
From: Hans Verkuil @ 2022-09-09 12:16 UTC (permalink / raw)
To: Dikshita Agarwal, linux-media, linux-kernel
Cc: stanimir.varbanov, quic_vgarodia, quic_majja, quic_jdas
Hi Dikshita,
First I'll comment on this patch, then at the end I have more high-level
comments.
On 29/08/2022 09:54, Dikshita Agarwal wrote:
> Some stateful decoder supports S/G_PARM similar to a
decoder -> decoders
BTW, which decoder drivers support this in mainline? Or is this
for out-of-tree drivers?
> stateful encoder. S_PARM(OUTPUT) reserves hardware decoder
> resources, and G_PARM(CAPTURE) returns the embedded
> frame interval. Allow the same with this change.
>
> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
> ---
> utils/v4l2-compliance/v4l2-test-formats.cpp | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp b/utils/v4l2-compliance/v4l2-test-formats.cpp
> index 269a383..e996558 100644
> --- a/utils/v4l2-compliance/v4l2-test-formats.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
> @@ -64,8 +64,6 @@ static int testEnumFrameIntervals(struct node *node, __u32 pixfmt,
> ret = doioctl(node, VIDIOC_ENUM_FRAMEINTERVALS, &frmival);
> if (ret == ENOTTY)
> return ret;
> - // M2M devices don't support this, except for stateful encoders
> - fail_on_test(node->is_m2m && !(node->codec_mask & STATEFUL_ENCODER));
M2M devices still don't support this, except for stateful codecs. So this test
should still be there, just changed to:
!(node->codec_mask & (STATEFUL_ENCODER | STATEFUL_DECODER))
> if (f == 0 && ret == EINVAL) {
> if (type == V4L2_FRMSIZE_TYPE_DISCRETE)
> warn("found framesize %dx%d, but no frame intervals\n", w, h);
> @@ -1367,8 +1365,6 @@ static int testParmType(struct node *node, unsigned type)
> }
> if (ret == ENOTTY)
> return ret;
> - // M2M devices don't support this, except for stateful encoders
> - fail_on_test(node->is_m2m && !is_stateful_enc);
Similar.
> if (ret == EINVAL)
> return ENOTTY;
> if (ret)
But modifying the utility is something you only do if the spec has first been updated.
So the stateful decoder chapter needs to be modified so it explains how G/S_PARM
shall be used. And ideally have at least one driver that supports it.
Regards,
Hans
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH] Allow S/G_PARM for stateful decoder
2022-09-09 12:16 ` Hans Verkuil
@ 2022-09-28 6:07 ` Dikshita Agarwal (QUIC)
0 siblings, 0 replies; 3+ messages in thread
From: Dikshita Agarwal (QUIC) @ 2022-09-28 6:07 UTC (permalink / raw)
To: Hans Verkuil, Dikshita Agarwal (QUIC), linux-media, linux-kernel
Cc: stanimir.varbanov, Vikash Garodia (QUIC), Maheshwar Ajja (QUIC),
quic_jdas
Thanks, Hans, for your comments.
I will take care of all comments regarding this change in next patch.
Will also send a patch to update the documentation.
Venus driver already has support for S_PARM for decoder, will add support for G_PARM as well and send a patch.
Thanks,
Dikshita
-----Original Message-----
From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Sent: Friday, September 9, 2022 5:47 PM
To: Dikshita Agarwal (QUIC) <quic_dikshita@quicinc.com>; linux-media@vger.kernel.org; linux-kernel@vger.kernel.org
Cc: stanimir.varbanov@linaro.org; Vikash Garodia (QUIC) <quic_vgarodia@quicinc.com>; Maheshwar Ajja (QUIC) <quic_majja@quicinc.com>; quic_jdas <quic_jdas@quicinc.com>
Subject: Re: [PATCH] Allow S/G_PARM for stateful decoder
Hi Dikshita,
First I'll comment on this patch, then at the end I have more high-level comments.
On 29/08/2022 09:54, Dikshita Agarwal wrote:
> Some stateful decoder supports S/G_PARM similar to a
decoder -> decoders
BTW, which decoder drivers support this in mainline? Or is this for out-of-tree drivers?
> stateful encoder. S_PARM(OUTPUT) reserves hardware decoder resources,
> and G_PARM(CAPTURE) returns the embedded frame interval. Allow the
> same with this change.
>
> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
> ---
> utils/v4l2-compliance/v4l2-test-formats.cpp | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp
> b/utils/v4l2-compliance/v4l2-test-formats.cpp
> index 269a383..e996558 100644
> --- a/utils/v4l2-compliance/v4l2-test-formats.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
> @@ -64,8 +64,6 @@ static int testEnumFrameIntervals(struct node *node, __u32 pixfmt,
> ret = doioctl(node, VIDIOC_ENUM_FRAMEINTERVALS, &frmival);
> if (ret == ENOTTY)
> return ret;
> - // M2M devices don't support this, except for stateful encoders
> - fail_on_test(node->is_m2m && !(node->codec_mask & STATEFUL_ENCODER));
M2M devices still don't support this, except for stateful codecs. So this test should still be there, just changed to:
!(node->codec_mask & (STATEFUL_ENCODER | STATEFUL_DECODER))
> if (f == 0 && ret == EINVAL) {
> if (type == V4L2_FRMSIZE_TYPE_DISCRETE)
> warn("found framesize %dx%d, but no frame intervals\n", w, h); @@
> -1367,8 +1365,6 @@ static int testParmType(struct node *node, unsigned type)
> }
> if (ret == ENOTTY)
> return ret;
> - // M2M devices don't support this, except for stateful encoders
> - fail_on_test(node->is_m2m && !is_stateful_enc);
Similar.
> if (ret == EINVAL)
> return ENOTTY;
> if (ret)
But modifying the utility is something you only do if the spec has first been updated.
So the stateful decoder chapter needs to be modified so it explains how G/S_PARM shall be used. And ideally have at least one driver that supports it.
Regards,
Hans
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-28 6:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-29 7:54 [PATCH] Allow S/G_PARM for stateful decoder Dikshita Agarwal
2022-09-09 12:16 ` Hans Verkuil
2022-09-28 6:07 ` Dikshita Agarwal (QUIC)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).