[Spice-devel] [PATCH 1/2] gstreamer-encoder: Use NV12 as the default vpp conversion format

[Frediano Ziglio]

Il giorno lun 18 set 2023 alle ore 06:49 Kasireddy, Vivek
<vivek.kasireddy at intel.com> ha scritto:
>
> Hi Frediano,
>
> > > From: Hazwan Arif Mazlan <hazwan.arif.mazlan at intel.com>
> > >
> > > Using NV12 as the output format for the videoconvert element would
> > > allow us to pair a s/w based encoder with a h/w based decoder for
> > > decoding the stream as most h/w based decoders only accept NV12 as
> > > the input format given its popularity.
> > >
> >
> > I don't fully understand the rationale.
> Yeah, I should have added more details.
>
> > Yes, the h/w codecs usually
> > would convert this to NV12 however should not this be done by
> > gstreamer instead?
> > Surely YUV conversion is useful but what if a software conversion
> > would like to use Y444 instead? With NV12 you would lose image
> > quality.
> > Isn't gstreamer supposed to come out with the best combination?
> > Maybe it would be easier to have a more complete pipeline string
> > instead specified for each codec?
> My understanding is that the goal with this patch is to ensure uniformity
> of the format and specifically address the case where we use x264enc
> on the encode side and msdkh264dec on the decode side. Since Y444
> is the default input format for x264enc, videoconvert converts the BGRx
> data into Y444. However, on the decode side, msdkh264dec can only work
> with NV12; so this patch is ensuring that we start with NV12 on the encode
> side as well. Jin Chung, feel free to add more details if I missed anything.
>
> Thanks,
> Vivek
>

The problem is compatibility. A client with h/w h264 support should be
able to talk to a server without any changes.
This patch is not fixing this, unless you use a time machine and you
apply it at the time h264 and x264enc were introduced.
So, basically the client should be able to decode y444 produced by x264enc.
Either keep using x264enc if the server could send it or be able to
detect from the first data frame (it should be possible,
maybe with a fail and try) and use the correct pipeline.

Regards,
  Frediano

> >
> > > Cc: Frediano Ziglio <freddy77 at gmail.com>
> > > Cc: Dongwon Kim <dongwon.kim at intel.com>
> > > Signed-off-by: Hazwan Arif Mazlan <hazwan.arif.mazlan at intel.com>
> > > Signed-off-by: Jin Chung Teng <jin.chung.teng at intel.com>
> > > Signed-off-by: Vivek Kasireddy <vivek.kasireddy at intel.com>
> > > ---
> > >  server/gstreamer-encoder.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
> > > index d8af91f1..057509b5 100644
> > > --- a/server/gstreamer-encoder.c
> > > +++ b/server/gstreamer-encoder.c
> > > @@ -918,7 +918,7 @@ static gboolean create_pipeline(SpiceGstEncoder
> > *encoder)
> > >  #ifdef HAVE_GSTREAMER_0_10
> > >      const gchar *converter = "ffmpegcolorspace";
> > >  #else
> > > -    const gchar *converter = "videoconvert";
> > > +    const gchar *converter = "videoconvert ! video/x-raw,format=NV12";
> > >  #endif
> > >      const gchar* gstenc_name = get_gst_codec_name(encoder);
> > >      if (!gstenc_name) {
> >
> >
> > Frediano