Hi Jernej,
On 02/11/2022 19:08, Jernej Skrabec wrote:
> In some cases decoding engine needs extra space in capture buffers. This
> is the case for decoding 10-bit HEVC frames into 8-bit capture format.
> This commit only adds infrastructure for such cases.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
> ---
> drivers/staging/media/sunxi/cedrus/cedrus.c | 14 ++++++++++++++
> drivers/staging/media/sunxi/cedrus/cedrus.h | 2 ++
> drivers/staging/media/sunxi/cedrus/cedrus_video.c | 4 ++++
> 3 files changed, 20 insertions(+)
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
> index 6a2c08928613..c36999e47591 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> @@ -68,8 +68,22 @@ static int cedrus_try_ctrl(struct v4l2_ctrl *ctrl)
> return 0;
> }
>
> +static int cedrus_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct cedrus_ctx *ctx = container_of(ctrl->handler,
> + struct cedrus_ctx, hdl);
> + struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
> + V4L2_BUF_TYPE_VIDEO_CAPTURE);
> +
> + if (!vb2_is_busy(vq) && !vb2_is_streaming(vq))
You can drop the !vb2_is_streaming part. If you are streaming, then
it will also be 'busy'.
> + cedrus_reset_cap_format(ctx);
This is weird. I would expect that this is only called for specific controls,
since presumably not all will change the sizeimage value. It looks like
it applies only to V4L2_CID_STATELESS_HEVC_SPS, so I see no reason why
you would call cedrus_reset_cap_format() for other controls as well.
And what happens if someone makes such a change *while streaming*?
Shouldn't cedrus_try_ctrl detect that and fail with -EBUSY in that case?
Regardless, this is unexpected behavior, so some explanatory comments
would be useful.
I'm also not sure whether it isn't better to add cedrus_s_ctrl in the next
patch, I think it would be more understandable when seen with an actual
control.
Regards,
Hans
> +
> + return 0;
> +}
> +
> static const struct v4l2_ctrl_ops cedrus_ctrl_ops = {
> .try_ctrl = cedrus_try_ctrl,
> + .s_ctrl = cedrus_s_ctrl,
> };
>
> static const struct cedrus_control cedrus_controls[] = {
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
> index 5904294f3108..774fe8048ce3 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> @@ -162,6 +162,8 @@ struct cedrus_dec_ops {
> int (*start)(struct cedrus_ctx *ctx);
> void (*stop)(struct cedrus_ctx *ctx);
> void (*trigger)(struct cedrus_ctx *ctx);
> + unsigned int (*extra_cap_size)(struct cedrus_ctx *ctx,
> + struct v4l2_pix_format *pix_fmt);
> };
>
> struct cedrus_variant {
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> index dec5d3ae4871..dc67940f1ade 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> @@ -250,6 +250,10 @@ static int cedrus_try_fmt_vid_cap_p(struct cedrus_ctx *ctx,
> pix_fmt->height = ctx->src_fmt.height;
> cedrus_prepare_format(pix_fmt);
>
> + if (ctx->current_codec->extra_cap_size)
> + pix_fmt->sizeimage +=
> + ctx->current_codec->extra_cap_size(ctx, pix_fmt);
> +
> return 0;
> }
>
