| Message ID | 1661759670-19902-1-git-send-email-quic_dikshita@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Allow S/G_PARM for stateful decoder | expand |
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
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
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)
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(-)