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

[Kasireddy, Vivek]

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

> 
> > 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