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

[Christophe Fergeau]

On Tue, Apr 19, 2016 at 09:50:12AM +0200, Francois Gouget wrote:
> This typically happens when sending very small frames (less than
> 16 pixels in one dimension) to the x264enc encoder.
> This avoids repeatedly wasting time rebuilding the pipeline.
> 
> Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
> ---
>  server/gstreamer-encoder.c | 53 ++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 46 insertions(+), 7 deletions(-)
> 
> diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
> index 9a16c18..7dc4f94 100644
> --- a/server/gstreamer-encoder.c
> +++ b/server/gstreamer-encoder.c
> @@ -82,6 +82,9 @@ typedef struct SpiceGstEncoder {
>      const SpiceFormatForGStreamer *format;
>      SpiceBitmapFmt spice_format;
>  
> +    /* Number of consecutive frame encoding errors. */
> +    uint32_t errors;
> +
>      /* ---------- GStreamer pipeline ---------- */
>  
>      /* Pointers to the GStreamer pipeline elements. If pipeline is NULL the
> @@ -829,15 +832,35 @@ static int get_physical_core_count(void)
>      return physical_core_count;
>  }
>  
> -/* 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.

Acked-by: Christophe Fergeau <cfergeau at redhat.com>

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/20160502/324a118b/attachment.sig>