Re: [PATCH 5/6] staging: media: rkvdec: Enable S_CTRL IOCTL

Ezequiel Garcia
2022-07-13T19:12:18Z

Hi Sebastian,

On Wed, Jul 13, 2022 at 1:33 PM Sebastian Fricke
<sebastian.fricke@collabora.com> wrote:
>
> Enable user-space to set the SPS of the current byte-stream on the
> decoder. This action will trigger the decoder to pick the optimal
> pixel-format for the capture queue.
>
> Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> drivers/staging/media/rkvdec/rkvdec.c | 60 ++++++++++++++++++++-------
> 1 file changed, 46 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
> index 2625e0a736f4..f84579f764ab 100644
> --- a/drivers/staging/media/rkvdec/rkvdec.c
> +++ b/drivers/staging/media/rkvdec/rkvdec.c
> @@ -27,6 +27,17 @@
> #include "rkvdec.h"
> #include "rkvdec-regs.h"
>
> +static void rkvdec_fill_decoded_pixfmt(struct rkvdec_ctx *ctx,
> + struct v4l2_pix_format_mplane *pix_mp)
> +{
> + v4l2_fill_pixfmt_mp(pix_mp, pix_mp->pixelformat,
> + pix_mp->width, pix_mp->height);
> + pix_mp->plane_fmt[0].sizeimage += 128 *
> + DIV_ROUND_UP(pix_mp->width, 16) *
> + DIV_ROUND_UP(pix_mp->height, 16);
> + pix_mp->field = V4L2_FIELD_NONE;
> +}
> +
> /*
> * Fetch the optimal pixel-format directly from the codec variation. If the
> * valid_fmt callback is not implemented, then the context variable valid_fmt
> @@ -44,6 +55,27 @@ static int rkvdec_get_valid_fmt(struct rkvdec_ctx *ctx, struct v4l2_ctrl *ctrl)
> return 0;
> }
>
> +static int rkvdec_set_valid_fmt(struct rkvdec_ctx *ctx, struct v4l2_ctrl *ctrl)
> +{
> + struct v4l2_pix_format_mplane *pix_mp;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_STATELESS_H264_SPS:
> + case V4L2_CID_STATELESS_VP9_FRAME:
> + case V4L2_CID_STATELESS_HEVC_SPS:
> + ctx->valid_fmt = rkvdec_get_valid_fmt(ctx, ctrl);
> +
> + pix_mp = &ctx->decoded_fmt.fmt.pix_mp;
> + pix_mp->pixelformat = ctx->valid_fmt;
> + rkvdec_fill_decoded_pixfmt(ctx, pix_mp);
> + break;
> + default:
> + dev_err(ctx->dev->dev, "Unsupported stateless control ID: %x\n", ctrl->id);
> + return -EINVAL;
> + };
> + return 0;
> +}
> +
> static int rkvdec_try_ctrl(struct v4l2_ctrl *ctrl)
> {
> struct rkvdec_ctx *ctx = container_of(ctrl->handler, struct rkvdec_ctx, ctrl_hdl);
> @@ -58,8 +90,18 @@ static int rkvdec_try_ctrl(struct v4l2_ctrl *ctrl)
> return 0;
> }
>
> +static int rkvdec_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct rkvdec_ctx *ctx = container_of(ctrl->handler, struct rkvdec_ctx, ctrl_hdl);
> +
> + if (!ctx->valid_fmt)
> + return rkvdec_set_valid_fmt(ctx, ctrl);

We've discussed this approach with Jonas.
See "[PATCH v2 10/12] media: rkvdec: Lock capture pixel format in
s_ctrl and s_fmt".

https://lore.kernel.org/all/f26407dbf3efc6cc046daaabdbe75c516743a187.camel@collabora.com/

The outcome of the discussion is:
* setting SPS resets the CAPTURE format (as per the spec).
* the application uses ENUM/TRY/S_FMT to negotiate a format, and uses
SPS to filter formats.

Thanks!
Ezequiel