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: <10a2f9865e96655c99ee377d542b5a06bb0146b3.camel@ndufresne.ca>
Date: Tue, 02 Sep 2025 13:36:52 -0400
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: "Ming Qian(OSS)" <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,

just adding to my anwer,

Le mardi 02 septembre 2025 à 13:01 -0400, Nicolas Dufresne a écrit :
> Le lundi 01 septembre 2025 à 17:41 +0800, Ming Qian(OSS) a écrit :
> > 
> > 

[...}

> > > 
> > > Nicolas
> > 
> > I tested this with gstreaer, not FFMPEG,
> > And I checked the gstreamer code in our repository, Then I found the
> > following related code:
> > 
> >    } else if (g_str_equal (mimetype, "video/x-wmv")) {
> >      const gchar *format = gst_structure_get_string (structure, "format");
> >      if (format) {
> >        if (!g_ascii_strcasecmp (format, "WVC1"))
> >          fourcc = V4L2_PIX_FMT_VC1_ANNEX_G;
> >        else if (!g_ascii_strcasecmp (format, "WMV3"))
> >          fourcc = V4L2_PIX_FMT_VC1_ANNEX_L;
> >      }
> > 
> > Basically it processes WMV3 into VC1_ANNEX_L, and WVC1 to VC1_ANNEX_G.
> > I didn't found them in the upstream gstreamer repository.
> > Now I'm not sure if it is correct userspace behavior.
> 
> Its a little concerning, since we are in the largely untested territory.
> Without
> proper documentation and with all the downstream changes done to userspace, we
> can easily endup with NXP considering this is the right mapping and let's say
> Qualcomm or Samsung thinking differently. Since this is for upstream, we need
> to
> ensure this is concistant. Have you reached to other driver maintainers
> already
> to discuss and resolve the subject in a way it works for everyone ?

So I checked Samsung implementation and your interpretation seems to be the
same. They MAP VC1_ANNEX_G to VC1 Advanced Profile Decoding in their
driver. Venus drivers does not care and just map both to VC1.

If I quote here Wikipedia's Window Media Video page:
   The Simple and Main profile levels in WMV 9 are compliant with the same
   profile levels in the VC-1 specification.[13] The Advanced Profile in VC-1 is
   implemented in a new WMV format called Windows Media Video 9 Advanced
   Profile. It improves compression efficiency for interlaced content and is
   made transport-independent, making it able to be encapsulated in an MPEG
   transport stream or RTP packet format. The format is not compatible with
   previous WMV 9 formats, however.[14]


It matches well with the fact Annex G introduce start codes and inline sequence
headers, since you absolutely need that to stream over MPEG TS. GStreamer uses
video/x-wmv as a family, and format=WVC1 for the advanced profiles, and WMV3 for
everything else it supports.

I think you should go ahead and upstream this mapping fix into GStreamer. V4L2
documentation should perhaps mention "Advanced Profile" to help devs.

Though, this gives me the impression that codec_data can be inline for ANNEX G.

Nicolas

> 
> > 
> > And the reason of this issue is the below code in gstreamer, that the
> > v4l2decoder may only send codec data after seek.
> > 
> >      codec_data = self->input_state->codec_data;
> > 
> >      /* We are running in byte-stream mode, so we don't know the 
> > headers, but
> >       * we need to send something, otherwise the decoder will refuse to
> >       * initialize.
> >       */
> >      if (codec_data) {
> >        gst_buffer_ref (codec_data);
> >      } else {
> >        codec_data = gst_buffer_ref (frame->input_buffer);
> >        processed = TRUE;
> >      }
> 
> That is truncating a bit too much of the context. The "processed" boolean is
> set
> when the codec data and frame is combined. In the case the codec data is only
> to
> be found in caps, it will queue the codec data and immediately queue the frame
> data. This is perfectly valid with the way the stateful decoder specification
> is
> written.
> 
> In practice, GStreamer stateful decoder is multi-threaded, so it will fill the
> OUTPUT queue with following frames too. What you can do to make your driver
> more
> flexible is whenever you didn't find a frame in a buffer, merge this buffer
> with
> the next one, and do that until there is no more space in one buffer. This way
> you don't copy all the time like ring buffers do, but you can survive abitrary
> splits of byte-stream.
> 
> Nicolas
> 
> > 
> > Regards,
> > Ming
> > 
> > 
> > > 
> > > >   
> > > >   	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)