linux-kernel - Re: [PATCH 5/6] staging: media: rkvdec: Enable S

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 13 Jul 2022 16:11:57 -0300
From:   Ezequiel Garcia <ezequiel@...guardiasur.com.ar>
To:     Sebastian Fricke <sebastian.fricke@...labora.com>
Cc:     linux-media <linux-media@...r.kernel.org>,
        Jernej Škrabec <jernej.skrabec@...il.com>,
        Alex Bee <knaerzche@...il.com>,
        Collabora Kernel ML <kernel@...labora.com>,
        Robert Beckett <bob.beckett@...labora.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        "open list:STAGING SUBSYSTEM" <linux-staging@...ts.linux.dev>,
        Nicolas Dufresne <nicolas.dufresne@...labora.com>,
        Jonas Karlman <jonas@...boo.se>,
        Yury Norov <yury.norov@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>
Subject: Re: [PATCH 5/6] staging: media: rkvdec: Enable S_CTRL IOCTL

Hi Sebastian,

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