On Tue, Feb 14, 2023 at 11:06 AM Benjamin Gaignard
<benjamin.gaignard@xxxxxxxxxxxxx> wrote:
>
> The driver supports 8 and 10 bits bitstreams, make sure to discard
> other cases.
> It could happens that userland test if V4L2_CID_STATELESS_AV1_SEQUENCE
> exists without setting bit_depth field in this case use
> HANTRO_DEFAULT_BIT_DEPTH value.
>
This shouldn't happen.
If the bit_depth argument in hantro_check_depth_match()
can be set unchecked by userspace, we have done something wrong!!
Are you sure that userspace can do a S_CTRL with an invalid bit-depth?
The try_or_set_cluster() function seems to always call try_ctrl before s_ctrl.
Thanks,
Ezequiel
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx>
> ---
> version 4:
> - This patch is the result of squashing "Save bit depth for AV1 decoder"
> and "Check AV1 bitstreams bit depth" of version 3 + adapation to
> "[PATCH v8 0/6] media: verisilicon: HEVC: fix 10bits handling" series.
>
> .../media/platform/verisilicon/hantro_drv.c | 36 +++++++++++++++++++
> .../media/platform/verisilicon/hantro_v4l2.c | 4 +++
> 2 files changed, 40 insertions(+)
>
> diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
> index bc1a85456142..666cd46902da 100644
> --- a/drivers/media/platform/verisilicon/hantro_drv.c
> +++ b/drivers/media/platform/verisilicon/hantro_drv.c
> @@ -275,7 +275,13 @@ static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
> /* We only support profile 0 */
> if (dec_params->profile != 0)
> return -EINVAL;
> + } else if (ctrl->id == V4L2_CID_STATELESS_AV1_SEQUENCE) {
> + const struct v4l2_ctrl_av1_sequence *sequence = ctrl->p_new.p_av1_sequence;
> +
> + if (sequence->bit_depth != 8 && sequence->bit_depth != 10)
> + return -EINVAL;
> }
> +
> return 0;
> }
>
> @@ -348,6 +354,30 @@ static int hantro_hevc_s_ctrl(struct v4l2_ctrl *ctrl)
> return 0;
> }
>
> +static int hantro_av1_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct hantro_ctx *ctx;
> +
> + ctx = container_of(ctrl->handler,
> + struct hantro_ctx, ctrl_handler);
> +
> + switch (ctrl->id) {
> + case V4L2_CID_STATELESS_AV1_SEQUENCE:
> + {
> + int bit_depth = ctrl->p_new.p_av1_sequence->bit_depth;
> +
> + if (ctx->bit_depth == bit_depth)
> + return 0;
> +
> + return hantro_reset_raw_fmt(ctx, bit_depth);
> + }
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static const struct v4l2_ctrl_ops hantro_ctrl_ops = {
> .try_ctrl = hantro_try_ctrl,
> };
> @@ -365,6 +395,11 @@ static const struct v4l2_ctrl_ops hantro_hevc_ctrl_ops = {
> .s_ctrl = hantro_hevc_s_ctrl,
> };
>
> +static const struct v4l2_ctrl_ops hantro_av1_ctrl_ops = {
> + .try_ctrl = hantro_try_ctrl,
> + .s_ctrl = hantro_av1_s_ctrl,
> +};
> +
> #define HANTRO_JPEG_ACTIVE_MARKERS (V4L2_JPEG_ACTIVE_MARKER_APP0 | \
> V4L2_JPEG_ACTIVE_MARKER_COM | \
> V4L2_JPEG_ACTIVE_MARKER_DQT | \
> @@ -542,6 +577,7 @@ static const struct hantro_ctrl controls[] = {
> .codec = HANTRO_AV1_DECODER,
> .cfg = {
> .id = V4L2_CID_STATELESS_AV1_SEQUENCE,
> + .ops = &hantro_av1_ctrl_ops,
> },
> }, {
> .codec = HANTRO_AV1_DECODER,
> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
> index 992c5baa929f..7e74e47c9a89 100644
> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
> @@ -86,6 +86,10 @@ hantro_check_depth_match(const struct hantro_fmt *fmt, int bit_depth)
> if (!fmt->match_depth && !fmt->postprocessed)
> return true;
>
> + /* 0 means default depth, which is 8 */
> + if (!bit_depth)
> + bit_depth = HANTRO_DEFAULT_BIT_DEPTH;
> +
> fmt_depth = hantro_get_format_depth(fmt->fourcc);
>
> /*
> --
> 2.34.1
>
