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

[Ezequiel Garcia]

Hi Sebastian,

On Wed, Jul 13, 2022 at 1:33 PM Sebastian Fricke
<sebastian.fricke@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> Signed-off-by: Jonas Karlman <jonas@xxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxx/

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