| Message ID | 20230203091622.127279-3-benjamin.gaignard@collabora.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | media: verisilicon: HEVC: fix 10bits handling | expand |
Hi Benjamin, On Fri, Feb 3 2023 at 10:16:18 AM +0100, Benjamin Gaignard <benjamin.gaignard@collabora.com> 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. > > Fixes: dc39473d0340 ("media: hantro: imx8m: Enable 10bit decoding") > The above Fixes tag looks incorrect. I am unsure what would be the right Fixes, perhaps we can avoid putting any? Same for all the other patches, the Fixes tag look wrong. Thanks, Ezequiel > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> > --- > .../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 33cb865238de..e60151a8a401 100644 > --- a/drivers/media/platform/verisilicon/hantro_v4l2.c > +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c > @@ -377,47 +377,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 >
diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c index 33cb865238de..e60151a8a401 100644 --- a/drivers/media/platform/verisilicon/hantro_v4l2.c +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c @@ -377,47 +377,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)
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. Fixes: dc39473d0340 ("media: hantro: imx8m: Enable 10bit decoding") Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> --- .../media/platform/verisilicon/hantro_v4l2.c | 40 +++++++++---------- 1 file changed, 18 insertions(+), 22 deletions(-)