Re: [PATCH v4 06/12] media: verisilicon: Check AV1 bitstreams bit depth
From: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Date: 2023-02-18 13:12:03
Also in:
linux-arm-kernel, linux-media, linux-rockchip, lkml
On Tue, Feb 14, 2023 at 11:06 AM Benjamin Gaignard [off-list ref] 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
quoted hunk ↗ jump to hunk
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> --- 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