linux-kernel - Re: [PATCH] media: amphion: Drop the sequence header after seek for VC1L

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: <ede4226a80e27c8b047b0eb25fe8f5ba90214d12.camel@ndufresne.ca>
Date: Fri, 29 Aug 2025 16:07:19 -0400
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: ming.qian@....nxp.com, mchehab@...nel.org, hverkuil-cisco@...all.nl
Cc: sebastian.fricke@...labora.com, shawnguo@...nel.org,
 s.hauer@...gutronix.de, 	kernel@...gutronix.de, festevam@...il.com,
 linux-imx@....com, xiahong.bao@....com, 	eagle.zhou@....com,
 imx@...ts.linux.dev, linux-media@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] media: amphion: Drop the sequence header after seek for
 VC1L

Hi,

Le vendredi 25 juillet 2025 à 16:07 +0800, ming.qian@....nxp.com a écrit :
> From: Ming Qian <ming.qian@....nxp.com>
> 
> For Simple and Main Profiles of VC-1 format stream, the amphion vpu
> requires driver to discard the sequence header, but insert a custom
> sequence start code at the beginning.
> The first buffer after a seek always contains only the sequence header.
> But vpu_vb_is_codecconfig() always return false as there is currently no
> flag indicating that the buffer contains only sequence header data and
> not frame data.
> So driver needs to drop the first buffer after seek, otherwise the driver
> will treat the sequence header as a frame, which will cause the image to
> be corrupted after the vpu decodes.
> 
> Signed-off-by: Ming Qian <ming.qian@....nxp.com>
> ---
>  drivers/media/platform/amphion/vpu_malone.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/amphion/vpu_malone.c b/drivers/media/platform/amphion/vpu_malone.c
> index ba688566dffd..a4c423600d70 100644
> --- a/drivers/media/platform/amphion/vpu_malone.c
> +++ b/drivers/media/platform/amphion/vpu_malone.c
> @@ -1373,11 +1373,9 @@ static int vpu_malone_insert_scode_vc1_l_seq(struct malone_scode_t *scode)
>  	int size = 0;
>  	u8 rcv_seqhdr[MALONE_VC1_RCV_SEQ_HEADER_LEN];
>  
> -	if (vpu_vb_is_codecconfig(to_vb2_v4l2_buffer(scode->vb)))

Please remove vpu_vb_is_codecconfig() entirely, it always returns false, so its
miss-leading.

> -		scode->need_data = 0;
> +	scode->need_data = 0;
>  	if (scode->inst->total_input_count)
>  		return 0;
> -	scode->need_data = 0;

I only remember testing this once quickly on Exynos 4 and I had no clue what
Annex G vs J was and most likley the MFC firmware was detecting it. Checking
quickly, I'm not sure GStreamer actually support both, despite the v4l2 wrapper
pretending. I would expect one to be used in ASF/ISOMP4/AVI, and the other used
in MPEG Transport Stream. GStreamer does not support VC1 in MPEG TS.

Have you tested this with FFMPEG ? It only maps annex G.

In general, I don't mind the the change if this is correct userspace behavior.
If ffmpeg and gstreamer don't agree though, we'll have to rethink. GStreamer
code back in the days was design in a way that it should not matter if the
header is split or not. This limitation came with lower latency decoder later,
but none had VC1.

Please test both userspace, and lets see if this solution is acceptable. ffmpeg
have ffplay and you can seek with your keyboard arrows.

Nicolas

>  
>  	ret = vpu_malone_insert_scode_seq(scode, MALONE_CODEC_ID_VC1_SIMPLE, sizeof(rcv_seqhdr));
>  	if (ret < 0)

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)