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

[Nicolas Dufresne <nicolas@ndufresne.ca>]

[-- Attachment #1: Type: text/plain, Size: 2726 bytes --]

Hi,

Le vendredi 25 juillet 2025 à 16:07 +0800, ming.qian@oss.nxp.com a écrit :
> From: Ming Qian <ming.qian@oss.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@oss.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)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]