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

[Nicolas Dufresne <nicolas@ndufresne.ca>]

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

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)

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