[Spice-devel] [spice v13 16/29] server: Give up after a while if GStreamer cannot handle the video

[Christophe Fergeau]

On Tue, May 03, 2016 at 04:53:08PM +0200, Francois Gouget wrote:
> 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);

Ah, for sure, I was not suggesting totally removing the helper, just not
using it in create_pipeline() since there is already a switch
(codec_type) there anyway. I knew my comment was not clear :(

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160503/4a5f4859/attachment.sig>