lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list] 
Message-ID: <cf81e5f2-45a4-4c82-890c-c8a4d17b22df@foss.st.com>
Date: Thu, 21 Nov 2024 11:07:06 +0100
From: Hugues FRUCHET <hugues.fruchet@...s.st.com>
To: Nicolas Dufresne <nicolas.dufresne@...labora.com>,
        Mauro Carvalho Chehab
	<mchehab@...nel.org>,
        Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Hans Verkuil
	<hverkuil-cisco@...all.nl>,
        Fritz Koenig <frkoenig@...omium.org>,
        Sebastian
 Fricke <sebastian.fricke@...labora.com>,
        Daniel Almeida
	<daniel.almeida@...labora.com>,
        Andrzej Pietrasiewicz
	<andrzej.p@...labora.com>,
        Benjamin Gaignard
	<benjamin.gaignard@...labora.com>,
        <linux-media@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-rockchip@...ts.infradead.org>,
        <linux-stm32@...md-mailman.stormreply.com>
Subject: Re: [PATCH v2 2/3] media: verisilicon: add WebP decoding support

Hi Nicolas,

On 11/20/24 15:25, Nicolas Dufresne wrote:
> Le mercredi 20 novembre 2024 à 12:01 +0100, Hugues Fruchet a écrit :
>> Add WebP picture decoding support to VP8 stateless decoder.
>>
>> Signed-off-by: Hugues Fruchet <hugues.fruchet@...s.st.com>
>> ---
>>   .../media/platform/verisilicon/hantro_g1_regs.h |  1 +
>>   .../platform/verisilicon/hantro_g1_vp8_dec.c    | 14 ++++++++++++++
>>   .../media/platform/verisilicon/hantro_v4l2.c    |  2 ++
>>   .../platform/verisilicon/stm32mp25_vpu_hw.c     | 17 +++++++++++++++--
>>   4 files changed, 32 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/verisilicon/hantro_g1_regs.h b/drivers/media/platform/verisilicon/hantro_g1_regs.h
>> index c623b3b0be18..e7d4db788e57 100644
>> --- a/drivers/media/platform/verisilicon/hantro_g1_regs.h
>> +++ b/drivers/media/platform/verisilicon/hantro_g1_regs.h
>> @@ -232,6 +232,7 @@
>>   #define     G1_REG_DEC_CTRL7_DCT7_START_BIT(x)		(((x) & 0x3f) << 0)
>>   #define G1_REG_ADDR_STR					0x030
>>   #define G1_REG_ADDR_DST					0x034
>> +#define G1_REG_ADDR_DST_CHROMA				0x038
>>   #define G1_REG_ADDR_REF(i)				(0x038 + ((i) * 0x4))
>>   #define     G1_REG_ADDR_REF_FIELD_E			BIT(1)
>>   #define     G1_REG_ADDR_REF_TOPC_E			BIT(0)
>> diff --git a/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c b/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
>> index 851eb67f19f5..c83ee6f5edc8 100644
>> --- a/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
>> +++ b/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
>> @@ -307,6 +307,12 @@ static void cfg_parts(struct hantro_ctx *ctx,
>>   			   G1_REG_DEC_CTRL3_STREAM_LEN(dct_part_total_len),
>>   			   G1_REG_DEC_CTRL3);
>>   
>> +	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
>> +		vdpu_write_relaxed(vpu,
>> +				   G1_REG_DEC_CTRL3_STREAM_LEN_EXT
>> +					(dct_part_total_len >> 24),
>> +				   G1_REG_DEC_CTRL3);
>> +
>>   	/* DCT partitions base address */
>>   	for (i = 0; i < hdr->num_dct_parts; i++) {
>>   		u32 byte_offset = dct_part_offset + dct_size_part_size + count;
>> @@ -427,6 +433,12 @@ static void cfg_buffers(struct hantro_ctx *ctx,
>>   
>>   	dst_dma = hantro_get_dec_buf_addr(ctx, &vb2_dst->vb2_buf);
>>   	vdpu_write_relaxed(vpu, dst_dma, G1_REG_ADDR_DST);
>> +
>> +	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
>> +		vdpu_write_relaxed(vpu, dst_dma +
>> +				   ctx->dst_fmt.plane_fmt[0].bytesperline *
>> +				   ctx->dst_fmt.height,
>> +				   G1_REG_ADDR_DST_CHROMA);
>>   }
>>   
>>   int hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
>> @@ -471,6 +483,8 @@ int hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
>>   		reg |= G1_REG_DEC_CTRL0_SKIP_MODE;
>>   	if (hdr->lf.level == 0)
>>   		reg |= G1_REG_DEC_CTRL0_FILTERING_DIS;
>> +	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
>> +		reg |= G1_REG_DEC_CTRL0_WEBP_E;
>>   	vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL0);
>>   
>>   	/* Frame dimensions */
>> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
>> index 2513adfbd825..7075b2ba1ec2 100644
>> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
>> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
>> @@ -470,6 +470,7 @@ hantro_update_requires_request(struct hantro_ctx *ctx, u32 fourcc)
>>   		break;
>>   	case V4L2_PIX_FMT_MPEG2_SLICE:
>>   	case V4L2_PIX_FMT_VP8_FRAME:
>> +	case V4L2_PIX_FMT_WEBP_FRAME:
>>   	case V4L2_PIX_FMT_H264_SLICE:
>>   	case V4L2_PIX_FMT_HEVC_SLICE:
>>   	case V4L2_PIX_FMT_VP9_FRAME:
>> @@ -492,6 +493,7 @@ hantro_update_requires_hold_capture_buf(struct hantro_ctx *ctx, u32 fourcc)
>>   	case V4L2_PIX_FMT_JPEG:
>>   	case V4L2_PIX_FMT_MPEG2_SLICE:
>>   	case V4L2_PIX_FMT_VP8_FRAME:
>> +	case V4L2_PIX_FMT_WEBP_FRAME:
>>   	case V4L2_PIX_FMT_HEVC_SLICE:
>>   	case V4L2_PIX_FMT_VP9_FRAME:
>>   		vq->subsystem_flags &= ~(VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF);
>> diff --git a/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c b/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
>> index 833821120b20..48d6912c3bab 100644
>> --- a/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
>> +++ b/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
>> @@ -22,10 +22,10 @@ static const struct hantro_fmt stm32mp25_vdec_fmts[] = {
>>   		.codec_mode = HANTRO_MODE_NONE,
>>   		.frmsize = {
>>   			.min_width = FMT_MIN_WIDTH,
>> -			.max_width = FMT_FHD_WIDTH,
>> +			.max_width = FMT_4K_WIDTH,
>>   			.step_width = MB_DIM,
>>   			.min_height = FMT_MIN_HEIGHT,
>> -			.max_height = FMT_FHD_HEIGHT,
>> +			.max_height = FMT_4K_HEIGHT,
> 
> I'm a little surprised of this change, since this is modifying VP8_FRAME, while
> we should instead introduce WEBP_FRAME.

This is the resolution of the YUV output of decoder, not the WebP input, 
and because of lack of post-processor, the output is not scaled, so can 
go up to 4K with WebP.
Before WebP introduction, the maximum output resolution was FHD for all 
codecs. Now WebP allows up to 4K but FHD constraint remains for 
H264/VP8. I don't see real problems because VP8/H264 compressed inputs 
are well limited to FHD and only WebP allows 4K...

> 
>>   			.step_height = MB_DIM,
>>   		},
>>   	},
>> @@ -68,6 +68,19 @@ static const struct hantro_fmt stm32mp25_venc_fmts[] = {
>>   		.codec_mode = HANTRO_MODE_NONE,
>>   		.enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUV420SP,
>>   	},
>> +	{
>> +		.fourcc = V4L2_PIX_FMT_WEBP_FRAME,
>> +		.codec_mode = HANTRO_MODE_VP8_DEC,
>> +		.max_depth = 2,
>> +		.frmsize = {
>> +			.min_width = FMT_MIN_WIDTH,
>> +			.max_width = FMT_4K_WIDTH,
>> +			.step_width = MB_DIM,
>> +			.min_height = FMT_MIN_HEIGHT,
>> +			.max_height = FMT_4K_HEIGHT,
>> +			.step_height = MB_DIM,
>> +		},
>> +	},
> 
> This is venc_fmt (encoder), this shouldn't be there.

All apologizes for this rebase issue, it is of course part of 
stm32mp25_vdec_fmts.

> 
>>   	{
>>   		.fourcc = V4L2_PIX_FMT_YUYV,
>>   		.codec_mode = HANTRO_MODE_NONE,
> 

BR,
Hugues.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ