[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <91ba5098-2528-1e63-3a1a-b908db8d6f2a@collabora.com>
Date: Wed, 20 Oct 2021 15:07:11 +0200
From: Andrzej Pietrasiewicz <andrzej.p@...labora.com>
To: Alex Bee <knaerzche@...il.com>, linux-media@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-rockchip@...ts.infradead.org, linux-staging@...ts.linux.dev
Cc: Benjamin Gaignard <benjamin.gaignard@...labora.com>,
Boris Brezillon <boris.brezillon@...labora.com>,
Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
Fabio Estevam <festevam@...il.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Hans Verkuil <hverkuil-cisco@...all.nl>,
Heiko Stuebner <heiko@...ech.de>,
Jernej Skrabec <jernej.skrabec@...il.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Nicolas Dufresne <nicolas.dufresne@...labora.com>,
NXP Linux Team <linux-imx@....com>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Philipp Zabel <p.zabel@...gutronix.de>,
Sascha Hauer <s.hauer@...gutronix.de>,
Shawn Guo <shawnguo@...nel.org>, kernel@...labora.com,
Ezequiel Garcia <ezequiel@...labora.com>,
Adrian Ratiu <adrian.ratiu@...labora.com>
Subject: Re: [PATCH v7 07/11] media: rkvdec: Add the VP9 backend
Hi Alex,
W dniu 20.10.2021 o 01:24, Alex Bee pisze:
> Hi Andrzej,
>
> Am 29.09.21 um 18:04 schrieb Andrzej Pietrasiewicz:
>> From: Boris Brezillon <boris.brezillon@...labora.com>
>>
>> The Rockchip VDEC supports VP9 profile 0 up to 4096x2304@...ps. Add
>> a backend for this new format.
>>
>> Signed-off-by: Boris Brezillon <boris.brezillon@...labora.com>
>> Signed-off-by: Ezequiel Garcia <ezequiel@...labora.com>
>> Signed-off-by: Adrian Ratiu <adrian.ratiu@...labora.com>
>> Co-developed-by: Andrzej Pietrasiewicz <andrzej.p@...labora.com>
>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@...labora.com>
>> ---
>> drivers/staging/media/rkvdec/Kconfig | 1 +
>> drivers/staging/media/rkvdec/Makefile | 2 +-
>> drivers/staging/media/rkvdec/rkvdec-vp9.c | 1078 +++++++++++++++++++++
>> drivers/staging/media/rkvdec/rkvdec.c | 52 +-
>> drivers/staging/media/rkvdec/rkvdec.h | 12 +-
>> 5 files changed, 1137 insertions(+), 8 deletions(-)
>> create mode 100644 drivers/staging/media/rkvdec/rkvdec-vp9.c
<snip>
>> diff --git a/drivers/staging/media/rkvdec/rkvdec.c
>> b/drivers/staging/media/rkvdec/rkvdec.c
>> index 7131156c1f2c..6aa8aca66547 100644
>> --- a/drivers/staging/media/rkvdec/rkvdec.c
>> +++ b/drivers/staging/media/rkvdec/rkvdec.c
>> @@ -99,10 +99,30 @@ static const struct rkvdec_ctrls rkvdec_h264_ctrls = {
>> .num_ctrls = ARRAY_SIZE(rkvdec_h264_ctrl_descs),
>> };
>> -static const u32 rkvdec_h264_decoded_fmts[] = {
>> +static const u32 rkvdec_h264_vp9_decoded_fmts[] = {
>> V4L2_PIX_FMT_NV12,
>
> For H.264 rkvdec HW supports additional formats: V4L2_PIX_FMT_NV15,
> V4L2_PIX_FMT_NV16 and V4L2_PIX_FMT_NV20. Not all of those are upstreamed yet and
> thus not supported by rkvdec driver - but I think we should introduce a seperate
> rkvdec_vp9_decoded_fmts already a this point. (To avoid unnecessary diff
> afterwards)
I will do it if I get to re-spinning the series for other reasons.
>
>> };
>> +static const struct rkvdec_ctrl_desc rkvdec_vp9_ctrl_descs[] = {
>> + {
>> + .cfg.id = V4L2_CID_STATELESS_VP9_FRAME,
>> + },
>> + {
>> + .cfg.id = V4L2_CID_STATELESS_VP9_COMPRESSED_HDR,
>> + },
>> + {
>> + .cfg.id = V4L2_CID_MPEG_VIDEO_VP9_PROFILE,
>> + .cfg.min = V4L2_MPEG_VIDEO_VP9_PROFILE_0,
>> + .cfg.max = V4L2_MPEG_VIDEO_VP9_PROFILE_0,
>> + .cfg.def = V4L2_MPEG_VIDEO_VP9_PROFILE_0,
>> + },
>> +};
>> +
>> +static const struct rkvdec_ctrls rkvdec_vp9_ctrls = {
>> + .ctrls = rkvdec_vp9_ctrl_descs,
>> + .num_ctrls = ARRAY_SIZE(rkvdec_vp9_ctrl_descs),
>> +};
>> +
>> static const struct rkvdec_coded_fmt_desc rkvdec_coded_fmts[] = {
>> {
>> .fourcc = V4L2_PIX_FMT_H264_SLICE,
>> @@ -116,8 +136,23 @@ static const struct rkvdec_coded_fmt_desc
>> rkvdec_coded_fmts[] = {
>> },
>> .ctrls = &rkvdec_h264_ctrls,
>> .ops = &rkvdec_h264_fmt_ops,
>> - .num_decoded_fmts = ARRAY_SIZE(rkvdec_h264_decoded_fmts),
>> - .decoded_fmts = rkvdec_h264_decoded_fmts,
>> + .num_decoded_fmts = ARRAY_SIZE(rkvdec_h264_vp9_decoded_fmts),
>> + .decoded_fmts = rkvdec_h264_vp9_decoded_fmts,
>> + },
>> + {
>> + .fourcc = V4L2_PIX_FMT_VP9_FRAME,
>> + .frmsize = {
>> + .min_width = 64,
>> + .max_width = 4096,
>> + .step_width = 64,
>> + .min_height = 64,
>> + .max_height = 2304,
>> + .step_height = 64,
>> + },
> I checked (available) documentation and couldn't find any hint to the
> .step_width and .step_height, but I'm not sure that's correct: taking
> this values here neither framesize of 3840x2160 nor 1280x720 would be possible -
> but the HW seems to have no problem with those, i.e. decoding works fine.
> Given the output format is the same as the (only) currently supported H.264
> output format (NV12) and those steps are usually for alignment purposes need by
> the HW , I strongly guess .step_height and .step_width are the same as
> V4L2_PIX_FMT_H264_SLICE has.
>
Aren't these used primarily by v4l2_apply_frmsize_constraints()? Doesn't
this merely mean that even though userspace requests, say, 48x48,
it will get 64x64 instead?
I tried decoding a 720p video with gstreamer and it worked fine
(I got a properly sized 1280x720 output).
Regards,
Andrzej
Powered by blists - more mailing lists