[Spice-devel] [client v5 2/4] streaming: Stop streaming if frames cannot be decoded

Christophe Fergeau cfergeau at redhat.com
Tue Nov 15 16:40:50 UTC 2016


Missing commit log.

On Mon, Oct 31, 2016 at 09:25:37PM +0100, Francois Gouget wrote:
> Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
> ---
>  src/channel-display-gst.c   | 41 ++++++++++++++++++++++++++++++++++++-----
>  src/channel-display-mjpeg.c |  8 +++++---
>  src/channel-display-priv.h  |  4 +++-
>  src/channel-display.c       |  7 ++++++-
>  4 files changed, 50 insertions(+), 10 deletions(-)
> 
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index f52299f..5fb0b77 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -280,6 +280,28 @@ static void free_pipeline(SpiceGstDecoder *decoder)
>      decoder->pipeline = NULL;
>  }
>  
> +static gboolean handle_pipeline_message(GstBus *bus, GstMessage *msg, gpointer video_decoder)
> +{
> +    SpiceGstDecoder *decoder = video_decoder;
> +
> +    if (GST_MESSAGE_TYPE(msg) == GST_MESSAGE_ERROR) {
> +        GError *err = NULL;
> +        gchar *debug_info = NULL;
> +        gst_message_parse_error(msg, &err, &debug_info);
> +        spice_warning("GStreamer error from element %s: %s",
> +                      GST_OBJECT_NAME(msg->src), err->message);
> +        if (debug_info) {
> +            SPICE_DEBUG("debug information: %s", debug_info);
> +            g_free(debug_info);
> +        }
> +        g_clear_error(&err);
> +
> +        /* We won't be able to process any more frame anyway */
> +        free_pipeline(decoder);
> +    }
> +    return TRUE;
> +}
> +
>  static gboolean create_pipeline(SpiceGstDecoder *decoder)
>  {
>      gchar *desc;
> @@ -324,6 +346,8 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
>  
>      appsink_cbs.new_sample = new_sample;
>      gst_app_sink_set_callbacks(decoder->appsink, &appsink_cbs, decoder, NULL);
> +    gst_bus_add_watch(gst_pipeline_get_bus(GST_PIPELINE(decoder->pipeline)),
> +                      handle_pipeline_message, decoder);
>  
>      decoder->clock = gst_pipeline_get_clock(GST_PIPELINE(decoder->pipeline));
>  
> @@ -390,9 +414,9 @@ static void release_buffer_data(gpointer data)
>      spice_msg_in_unref(frame_msg);
>  }
>  
> -static void spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
> -                                          SpiceMsgIn *frame_msg,
> -                                          int32_t latency)
> +static gboolean spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
> +                                              SpiceMsgIn *frame_msg,
> +                                              int32_t latency)
>  {
>      SpiceGstDecoder *decoder = (SpiceGstDecoder*)video_decoder;
>  
> @@ -400,7 +424,7 @@ static void spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
>      uint32_t size = spice_msg_in_frame_data(frame_msg, &data);
>      if (size == 0) {
>          SPICE_DEBUG("got an empty frame buffer!");
> -        return;
> +        return TRUE;
>      }
>  
>      SpiceStreamDataHeader *frame_op = spice_msg_in_parsed(frame_msg);
> @@ -419,7 +443,13 @@ static void spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
>           * saves CPU so do it.
>           */
>          SPICE_DEBUG("dropping a late MJPEG frame");
> -        return;
> +        return TRUE;
> +    }
> +
> +    if (decoder->pipeline == NULL) {
> +        /* An error occurred, causing the GStreamer pipeline to be freed */
> +        spice_warning("An error occurred, stopping the video stream");
> +        return FALSE;
>      }
>  
>      /* ref() the frame_msg for the buffer */
> @@ -440,6 +470,7 @@ static void spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
>          SPICE_DEBUG("GStreamer error: unable to push frame of size %u", size);
>          stream_dropped_frame_on_playback(decoder->base.stream);
>      }
> +    return TRUE;
>  }
>  
>  static gboolean gstvideo_init(void)
> diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c
> index 4976d53..67746c3 100644
> --- a/src/channel-display-mjpeg.c
> +++ b/src/channel-display-mjpeg.c
> @@ -235,8 +235,9 @@ static void mjpeg_decoder_drop_queue(MJpegDecoder *decoder)
>  
>  /* ---------- VideoDecoder's public API ---------- */
>  
> -static void mjpeg_decoder_queue_frame(VideoDecoder *video_decoder,
> -                                      SpiceMsgIn *frame_msg, int32_t latency)
> +static gboolean mjpeg_decoder_queue_frame(VideoDecoder *video_decoder,
> +                                          SpiceMsgIn *frame_msg,
> +                                          int32_t latency)
>  {
>      MJpegDecoder *decoder = (MJpegDecoder*)video_decoder;
>      SpiceMsgIn *last_msg;
> @@ -262,12 +263,13 @@ static void mjpeg_decoder_queue_frame(VideoDecoder *video_decoder,
>       * So drop late frames as early as possible to save on processing time.
>       */
>      if (latency < 0) {
> -        return;
> +        return TRUE;
>      }
>  
>      spice_msg_in_ref(frame_msg);
>      g_queue_push_tail(decoder->msgq, frame_msg);
>      mjpeg_decoder_schedule(decoder);
> +    return TRUE;
>  }
>  
>  static void mjpeg_decoder_reschedule(VideoDecoder *video_decoder)
> diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
> index aa57f95..b9c08a3 100644
> --- a/src/channel-display-priv.h
> +++ b/src/channel-display-priv.h
> @@ -50,8 +50,10 @@ struct VideoDecoder {
>       * @frame_msg: The Spice message containing the compressed frame.
>       * @latency:   How long in milliseconds until the frame should be
>       *             displayed. Negative values mean the frame is late.
> +     * @return:    False if the decoder can no longer decode frames,
> +     *             True otherwise.
>       */
> -    void (*queue_frame)(VideoDecoder *decoder, SpiceMsgIn *frame_msg, int32_t latency);
> +    gboolean (*queue_frame)(VideoDecoder *decoder, SpiceMsgIn *frame_msg, int32_t latency);
>  
>      /* The format of the encoded video. */
>      int codec_type;
> diff --git a/src/channel-display.c b/src/channel-display.c
> index 6cbbdd3..3d88cb0 100644
> --- a/src/channel-display.c
> +++ b/src/channel-display.c
> @@ -1423,7 +1423,12 @@ static void display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn *in)
>       * decoding and best decide if/when to drop them when they are late,
>       * taking into account the impact on later frames.
>       */
> -    st->video_decoder->queue_frame(st->video_decoder, in,  latency);
> +    if (!st->video_decoder->queue_frame(st->video_decoder, in, latency)) {
> +        destroy_stream(channel, op->id);
> +        /* Trigger the error reporting code */
> +        get_stream_by_id(channel, op->id);

I'd prefer if this called a helper with a more descriptive name, which
would make the comment not useful.

Seems fine otherwise.

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


More information about the Spice-devel mailing list