2023-02-20 10:49:07

by Benjamin Gaignard

Subject: [PATCH v9 2/6] media: verisilicon: Do not use ctx fields as format storage when resetting

Source and destination pixel formats fields of context structure should
not be used as storage when resetting the format.
Use local variables instead and let hantro_set_fmt_out() and
hantro_set_fmt_cap() set them correctly later.

Signed-off-by: Benjamin Gaignard <[email protected]>
---
.../media/platform/verisilicon/hantro_v4l2.c | 40 +++++++++----------
1 file changed, 18 insertions(+), 22 deletions(-)

diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
index d8aa42bd4cd4..d94c99f875c8 100644
--- a/drivers/media/platform/verisilicon/hantro_v4l2.c
+++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
@@ -378,47 +378,43 @@ static void
hantro_reset_encoded_fmt(struct hantro_ctx *ctx)
{
const struct hantro_fmt *vpu_fmt;
- struct v4l2_pix_format_mplane *fmt;
+ struct v4l2_pix_format_mplane fmt;

vpu_fmt = hantro_get_default_fmt(ctx, true);
+ if (!vpu_fmt)
+ return;

+ hantro_reset_fmt(&fmt, vpu_fmt);
+ fmt.width = vpu_fmt->frmsize.min_width;
+ fmt.height = vpu_fmt->frmsize.min_height;
if (ctx->is_encoder)
- fmt = &ctx->dst_fmt;
- else
- fmt = &ctx->src_fmt;
-
- hantro_reset_fmt(fmt, vpu_fmt);
- fmt->width = vpu_fmt->frmsize.min_width;
- fmt->height = vpu_fmt->frmsize.min_height;
- if (ctx->is_encoder)
- hantro_set_fmt_cap(ctx, fmt);
+ hantro_set_fmt_cap(ctx, &fmt);
else
- hantro_set_fmt_out(ctx, fmt);
+ hantro_set_fmt_out(ctx, &fmt);
}

static void
hantro_reset_raw_fmt(struct hantro_ctx *ctx)
{
const struct hantro_fmt *raw_vpu_fmt;
- struct v4l2_pix_format_mplane *raw_fmt, *encoded_fmt;
+ struct v4l2_pix_format_mplane raw_fmt, *encoded_fmt;

raw_vpu_fmt = hantro_get_default_fmt(ctx, false);
+ if (!raw_vpu_fmt)
+ return;

- if (ctx->is_encoder) {
- raw_fmt = &ctx->src_fmt;
+ if (ctx->is_encoder)
encoded_fmt = &ctx->dst_fmt;
- } else {
- raw_fmt = &ctx->dst_fmt;
+ else
encoded_fmt = &ctx->src_fmt;
- }

- hantro_reset_fmt(raw_fmt, raw_vpu_fmt);
- raw_fmt->width = encoded_fmt->width;
- raw_fmt->height = encoded_fmt->height;
+ hantro_reset_fmt(&raw_fmt, raw_vpu_fmt);
+ raw_fmt.width = encoded_fmt->width;
+ raw_fmt.height = encoded_fmt->height;
if (ctx->is_encoder)
- hantro_set_fmt_out(ctx, raw_fmt);
+ hantro_set_fmt_out(ctx, &raw_fmt);
else
- hantro_set_fmt_cap(ctx, raw_fmt);
+ hantro_set_fmt_cap(ctx, &raw_fmt);
}

void hantro_reset_fmts(struct hantro_ctx *ctx)
--
2.34.1

2023-02-26 12:59:04

by Ezequiel Garcia

Subject: Re: [PATCH v9 2/6] media: verisilicon: Do not use ctx fields as format storage when resetting

On Mon, Feb 20 2023 at 11:48:45 AM +0100, Benjamin Gaignard
<[email protected]> wrote:
> Source and destination pixel formats fields of context structure
> should
> not be used as storage when resetting the format.
> Use local variables instead and let hantro_set_fmt_out() and
> hantro_set_fmt_cap() set them correctly later.
>
> Signed-off-by: Benjamin Gaignard <[email protected]>

Reviewed-by: Ezequiel Garcia <[email protected]>

> ---
> .../media/platform/verisilicon/hantro_v4l2.c | 40
> +++++++++----------
> 1 file changed, 18 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c
> b/drivers/media/platform/verisilicon/hantro_v4l2.c
> index d8aa42bd4cd4..d94c99f875c8 100644
> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
> @@ -378,47 +378,43 @@ static void
> hantro_reset_encoded_fmt(struct hantro_ctx *ctx)
> {
> const struct hantro_fmt *vpu_fmt;
> - struct v4l2_pix_format_mplane *fmt;
> + struct v4l2_pix_format_mplane fmt;
>
> vpu_fmt = hantro_get_default_fmt(ctx, true);
> + if (!vpu_fmt)
> + return;
>
> + hantro_reset_fmt(&fmt, vpu_fmt);
> + fmt.width = vpu_fmt->frmsize.min_width;
> + fmt.height = vpu_fmt->frmsize.min_height;
> if (ctx->is_encoder)
> - fmt = &ctx->dst_fmt;
> - else
> - fmt = &ctx->src_fmt;
> -
> - hantro_reset_fmt(fmt, vpu_fmt);
> - fmt->width = vpu_fmt->frmsize.min_width;
> - fmt->height = vpu_fmt->frmsize.min_height;
> - if (ctx->is_encoder)
> - hantro_set_fmt_cap(ctx, fmt);
> + hantro_set_fmt_cap(ctx, &fmt);
> else
> - hantro_set_fmt_out(ctx, fmt);
> + hantro_set_fmt_out(ctx, &fmt);
> }
>
> static void
> hantro_reset_raw_fmt(struct hantro_ctx *ctx)
> {
> const struct hantro_fmt *raw_vpu_fmt;
> - struct v4l2_pix_format_mplane *raw_fmt, *encoded_fmt;
> + struct v4l2_pix_format_mplane raw_fmt, *encoded_fmt;
>
> raw_vpu_fmt = hantro_get_default_fmt(ctx, false);
> + if (!raw_vpu_fmt)
> + return;
>
> - if (ctx->is_encoder) {
> - raw_fmt = &ctx->src_fmt;
> + if (ctx->is_encoder)
> encoded_fmt = &ctx->dst_fmt;
> - } else {
> - raw_fmt = &ctx->dst_fmt;
> + else
> encoded_fmt = &ctx->src_fmt;
> - }
>
> - hantro_reset_fmt(raw_fmt, raw_vpu_fmt);
> - raw_fmt->width = encoded_fmt->width;
> - raw_fmt->height = encoded_fmt->height;
> + hantro_reset_fmt(&raw_fmt, raw_vpu_fmt);
> + raw_fmt.width = encoded_fmt->width;
> + raw_fmt.height = encoded_fmt->height;
> if (ctx->is_encoder)
> - hantro_set_fmt_out(ctx, raw_fmt);
> + hantro_set_fmt_out(ctx, &raw_fmt);
> else
> - hantro_set_fmt_cap(ctx, raw_fmt);
> + hantro_set_fmt_cap(ctx, &raw_fmt);
> }
>
> void hantro_reset_fmts(struct hantro_ctx *ctx)
> --
> 2.34.1
>