Messages in this thread | | | Date | Thu, 30 Jun 2022 07:02:32 +0200 | From | Sebastian Fricke <> | Subject | Re: [PATCH] hantro: Remove incorrect HEVC SPS validation |
| |
Hey Ezequiel,
On 29.06.2022 16:56, Ezequiel Garcia wrote: >Currently, the driver tries to validat the HEVC SPS
s/validat/validate/
>against the CAPTURE queue format (i.e. the decoded format). >This is not correct, because typically the SPS control is set >before the CAPTURE queue is negotiated. > >In addition to this, a format validation in hantro_hevc_dec_prepare_run() >is also suboptimal, because hantro_hevc_dec_prepare_run() runs in the context >of v4l2_m2m_ops.device_run, as part of a decoding job. > >Format and control validations should happen before decoding starts, >in the context of ioctls such as S_CTRL, S_FMT, or STREAMON. > >Remove the validation for now.
Couldn't we add a small wrapper around STREAMON to perform that validation? I feel like "remove the validation for now", seems like a vague statement.
Greetings, Sebastian
> >Fixes: 135ad96cb4d6b ("media: hantro: Be more accurate on pixel formats step_width constraints") >Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> >--- > drivers/staging/media/hantro/hantro_drv.c | 12 ++++----- > drivers/staging/media/hantro/hantro_hevc.c | 30 ---------------------- > drivers/staging/media/hantro/hantro_hw.h | 1 - > 3 files changed, 6 insertions(+), 37 deletions(-) > >diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c >index afddf7ac0731..2387ca85ab54 100644 >--- a/drivers/staging/media/hantro/hantro_drv.c >+++ b/drivers/staging/media/hantro/hantro_drv.c >@@ -253,11 +253,6 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq) > > static int hantro_try_ctrl(struct v4l2_ctrl *ctrl) > { >- struct hantro_ctx *ctx; >- >- ctx = container_of(ctrl->handler, >- struct hantro_ctx, ctrl_handler); >- > if (ctrl->id == V4L2_CID_STATELESS_H264_SPS) { > const struct v4l2_ctrl_h264_sps *sps = ctrl->p_new.p_h264_sps; > >@@ -273,7 +268,12 @@ static int hantro_try_ctrl(struct v4l2_ctrl *ctrl) > } else if (ctrl->id == V4L2_CID_MPEG_VIDEO_HEVC_SPS) { > const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps; > >- return hantro_hevc_validate_sps(ctx, sps); >+ if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8) >+ /* Luma and chroma bit depth mismatch */ >+ return -EINVAL; >+ if (sps->bit_depth_luma_minus8 != 0) >+ /* Only 8-bit is supported */ >+ return -EINVAL; > } else if (ctrl->id == V4L2_CID_STATELESS_VP9_FRAME) { > const struct v4l2_ctrl_vp9_frame *dec_params = ctrl->p_new.p_vp9_frame; > >diff --git a/drivers/staging/media/hantro/hantro_hevc.c b/drivers/staging/media/hantro/hantro_hevc.c >index bd924896e409..f86c98e19177 100644 >--- a/drivers/staging/media/hantro/hantro_hevc.c >+++ b/drivers/staging/media/hantro/hantro_hevc.c >@@ -154,32 +154,6 @@ static int tile_buffer_reallocate(struct hantro_ctx *ctx) > return -ENOMEM; > } > >-int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps) >-{ >- if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8) >- /* Luma and chroma bit depth mismatch */ >- return -EINVAL; >- if (sps->bit_depth_luma_minus8 != 0) >- /* Only 8-bit is supported */ >- return -EINVAL; >- >- /* >- * for tile pixel format check if the width and height match >- * hardware constraints >- */ >- if (ctx->vpu_dst_fmt->fourcc == V4L2_PIX_FMT_NV12_4L4) { >- if (ctx->dst_fmt.width != >- ALIGN(sps->pic_width_in_luma_samples, ctx->vpu_dst_fmt->frmsize.step_width)) >- return -EINVAL; >- >- if (ctx->dst_fmt.height != >- ALIGN(sps->pic_height_in_luma_samples, ctx->vpu_dst_fmt->frmsize.step_height)) >- return -EINVAL; >- } >- >- return 0; >-} >- > int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx) > { > struct hantro_hevc_dec_hw_ctx *hevc_ctx = &ctx->hevc_dec; >@@ -203,10 +177,6 @@ int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx) > if (WARN_ON(!ctrls->sps)) > return -EINVAL; > >- ret = hantro_hevc_validate_sps(ctx, ctrls->sps); >- if (ret) >- return ret; >- > ctrls->pps = > hantro_get_ctrl(ctx, V4L2_CID_MPEG_VIDEO_HEVC_PPS); > if (WARN_ON(!ctrls->pps)) >diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h >index a2e0f0836281..5edff0f0be20 100644 >--- a/drivers/staging/media/hantro/hantro_hw.h >+++ b/drivers/staging/media/hantro/hantro_hw.h >@@ -359,7 +359,6 @@ int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx); > void hantro_hevc_ref_init(struct hantro_ctx *ctx); > dma_addr_t hantro_hevc_get_ref_buf(struct hantro_ctx *ctx, int poc); > int hantro_hevc_add_ref_buf(struct hantro_ctx *ctx, int poc, dma_addr_t addr); >-int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps); > > > static inline unsigned short hantro_vp9_num_sbs(unsigned short dimension) >-- >2.31.1 >
| |