On Mon, 2 May 2016, Christophe Fergeau wrote:
[...]
> > -/* A helper for spice_gst_encoder_encode_frame() */
> > +static const gchar* get_gst_codec_name(SpiceGstEncoder *encoder)
> > +{
> > +    switch (encoder->base.codec_type)
> > +    {
> > +    case SPICE_VIDEO_CODEC_TYPE_MJPEG:
> > +        return "avenc_mjpeg";
> > +    case SPICE_VIDEO_CODEC_TYPE_VP8:
> > +        return "vp8enc";
> > +    case SPICE_VIDEO_CODEC_TYPE_H264:
> > +        return "x264enc";
> > +    default:
> > +        /* gstreamer_encoder_new() should have rejected this codec type */
> > +        spice_warning("unsupported codec type %d", 
> > encoder->base.codec_type);
> > +        return NULL;
> > +    }
> > +}
> > +
> >  static gboolean create_pipeline(SpiceGstEncoder *encoder)
> >  {
> > -    gchar *gstenc;
> > +    const gchar* gstenc_name = get_gst_codec_name(encoder);
> > +    if (!gstenc_name) {
> > +        return FALSE;
> > +    }
> 
> Forgot if I commented about that earlier, but I don't think using
> get_gst_codec_name() here brings much, and that things are more readable
> without these changes to create_pipeline()
> I'm fine with keeping things as in this patch if you prefer it this way.

I think it's useful to be able to print the name of the GStreamer 
encoder, particularly for this warning:

        spice_warning("GStreamer error: could not find the %s bitrate 
parameter", gstenc_name);



-- 
Francois Gouget <fgou...@codeweavers.com>
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Reply via email to