[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <baa85477b241880e1cf96efd7037fc1b2423fab5.camel@ndufresne.ca>
Date: Wed, 07 Feb 2024 12:55:29 -0500
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: "jackson.lee" <jackson.lee@...psnmedia.com>, mchehab@...nel.org,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
nas.chung@...psnmedia.com
Cc: lafley.kim@...psnmedia.com, b-brnich@...com
Subject: Re: [RESEND PATCH v0 1/5] wave5 : Support yuv422 input format for
encoder.
Hi Jackson,
Le mercredi 31 janvier 2024 à 10:30 +0900, jackson.lee a écrit :
> Encoder supports the following formats.
> YUV422P, NV16, NV61, NV16M, NV61M
>
> Signed-off-by: Jackson Lee <jackson.lee@...psnmedia.com>
> Signed-off-by: Nas Chung <nas.chung@...psnmedia.com>
> ---
> .../chips-media/wave5/wave5-vpu-enc.c | 79 ++++++++++++++++++-
> 1 file changed, 76 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> index f29cfa3af94a..0cb5bfb67258 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> @@ -70,6 +70,41 @@ static const struct vpu_format enc_fmt_list[FMT_TYPES][MAX_FMTS] = {
> .max_height = W5_MAX_ENC_PIC_HEIGHT,
> .min_height = W5_MIN_ENC_PIC_HEIGHT,
> },
> + {
> + .v4l2_pix_fmt = V4L2_PIX_FMT_YUV422P,
> + .max_width = W5_MAX_ENC_PIC_WIDTH,
> + .min_width = W5_MIN_ENC_PIC_WIDTH,
> + .max_height = W5_MAX_ENC_PIC_HEIGHT,
> + .min_height = W5_MIN_ENC_PIC_HEIGHT,
> + },
During upstreaming, we discussed the lack of usage of v4l2-common in this driver
and agreed that future updates such as this one should first port the driver to
use the common helpers instead.
This implies dropping this custom made structure in favour of
v4l2_frmsize_stepwise structure. Unlike yours, you can encoded the needed
padding, allowing to encode this in one place instead of spreading it across
numerous formulas in the code.
With this information, you will be able to use:
v4l2_apply_frmsize_constraints()
v4l2_fill_pixfmt_mp()
To adjust your dimensions to padded dimensions and compute your bytesperline
(stride) and sizeimage. You can of course increase the size image after this
call. You can have a look at rkvdec driver as an example.
Please port existing set of pixel formats support, and then add the new pixel
formats. This should remove about 3/4 of this patch and remove that huge risk of
miss-computing a size.
> + {
> + .v4l2_pix_fmt = V4L2_PIX_FMT_NV16,
> + .max_width = W5_MAX_ENC_PIC_WIDTH,
> + .min_width = W5_MIN_ENC_PIC_WIDTH,
> + .max_height = W5_MAX_ENC_PIC_HEIGHT,
> + .min_height = W5_MIN_ENC_PIC_HEIGHT,
> + },
> + {
> + .v4l2_pix_fmt = V4L2_PIX_FMT_NV61,
> + .max_width = W5_MAX_ENC_PIC_WIDTH,
> + .min_width = W5_MIN_ENC_PIC_WIDTH,
> + .max_height = W5_MAX_ENC_PIC_HEIGHT,
> + .min_height = W5_MIN_ENC_PIC_HEIGHT,
> + },
> + {
> + .v4l2_pix_fmt = V4L2_PIX_FMT_NV16M,
> + .max_width = W5_MAX_ENC_PIC_WIDTH,
> + .min_width = W5_MIN_ENC_PIC_WIDTH,
> + .max_height = W5_MAX_ENC_PIC_HEIGHT,
> + .min_height = W5_MIN_ENC_PIC_HEIGHT,
> + },
> + {
> + .v4l2_pix_fmt = V4L2_PIX_FMT_NV61M,
> + .max_width = W5_MAX_ENC_PIC_WIDTH,
> + .min_width = W5_MIN_ENC_PIC_WIDTH,
> + .max_height = W5_MAX_ENC_PIC_HEIGHT,
> + .min_height = W5_MIN_ENC_PIC_HEIGHT,
> + },
> }
> };
>
> @@ -136,6 +171,23 @@ static void wave5_update_pix_fmt(struct v4l2_pix_format_mplane *pix_mp, unsigned
> pix_mp->plane_fmt[1].bytesperline = round_up(width, 32);
> pix_mp->plane_fmt[1].sizeimage = round_up(width, 32) * height / 2;
> break;
> + case V4L2_PIX_FMT_YUV422P:
> + case V4L2_PIX_FMT_NV16:
> + case V4L2_PIX_FMT_NV61:
> + pix_mp->width = width;
> + pix_mp->height = height;
> + pix_mp->plane_fmt[0].bytesperline = round_up(width, 32);
> + pix_mp->plane_fmt[0].sizeimage = round_up(width, 32) * height * 2;
> + break;
> + case V4L2_PIX_FMT_NV16M:
> + case V4L2_PIX_FMT_NV61M:
> + pix_mp->width = width;
> + pix_mp->height = height;
> + pix_mp->plane_fmt[0].bytesperline = round_up(width, 32);
> + pix_mp->plane_fmt[0].sizeimage = round_up(width, 32) * height;
> + pix_mp->plane_fmt[1].bytesperline = round_up(width, 32);
> + pix_mp->plane_fmt[1].sizeimage = round_up(width, 32) * height;
> + break;
> default:
> pix_mp->width = width;
> pix_mp->height = height;
> @@ -155,11 +207,19 @@ static int start_encode(struct vpu_instance *inst, u32 *fail_res)
> struct enc_param pic_param;
> u32 stride = ALIGN(inst->dst_fmt.width, 32);
> u32 luma_size = (stride * inst->dst_fmt.height);
> - u32 chroma_size = ((stride / 2) * (inst->dst_fmt.height / 2));
> + u32 chroma_size;
>
> memset(&pic_param, 0, sizeof(struct enc_param));
> memset(&frame_buf, 0, sizeof(struct frame_buffer));
>
> + if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV420 ||
> + inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV420M)
> + chroma_size = ((stride / 2) * (inst->dst_fmt.height / 2));
> + else if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV422P)
> + chroma_size = ((stride) * (inst->dst_fmt.height / 2));
> + else
> + chroma_size = 0;
> +
> dst_buf = v4l2_m2m_next_dst_buf(m2m_ctx);
> if (!dst_buf) {
> dev_dbg(inst->dev->dev, "%s: No destination buffer found\n", __func__);
> @@ -550,11 +610,15 @@ static int wave5_vpu_enc_s_fmt_out(struct file *file, void *fh, struct v4l2_form
> }
>
> if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV12 ||
> - inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV12M) {
> + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV12M ||
> + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV16 ||
> + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV16M) {
> inst->cbcr_interleave = true;
> inst->nv21 = false;
> } else if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV21 ||
> - inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV21M) {
> + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV21M ||
> + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV61 ||
> + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV61M) {
> inst->cbcr_interleave = true;
> inst->nv21 = true;
> } else {
> @@ -1132,6 +1196,15 @@ static void wave5_set_enc_openparam(struct enc_open_param *open_param,
> u32 num_ctu_row = ALIGN(inst->dst_fmt.height, 64) / 64;
> u32 num_mb_row = ALIGN(inst->dst_fmt.height, 16) / 16;
>
> + if (inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV16 ||
> + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV61 ||
> + inst->src_fmt.pixelformat == V4L2_PIX_FMT_YUV422P ||
> + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV16M ||
> + inst->src_fmt.pixelformat == V4L2_PIX_FMT_NV61M)
> + open_param->src_format = FORMAT_422;
> + else
> + open_param->src_format = FORMAT_420;
> +
> open_param->wave_param.gop_preset_idx = PRESET_IDX_IPP_SINGLE;
> open_param->wave_param.hvs_qp_scale = 2;
> open_param->wave_param.hvs_max_delta_qp = 10;
Powered by blists - more mailing lists