Hi,

On Tue, Nov 22, 2016 at 06:01:38PM +0100, Francois Gouget wrote:
> Report the stream as invalid if the frames cannot be decoded. This will
> force the server to send regular screen updates instead.
>
> Signed-off-by: Francois Gouget <fgou...@codeweavers.com>
> ---
> 
> Uses report_invalid_stream() now.
> 
>  src/channel-display-gst.c   | 41 ++++++++++++++++++++++++++++++++++++-----
>  src/channel-display-mjpeg.c |  8 +++++---
>  src/channel-display-priv.h  |  4 +++-
>  src/channel-display.c       |  6 +++++-
>  4 files changed, 49 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);

You are leaking GstBus here. I'll fix it before pushing.

>  
>      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 500ddfb..ca56cd1 100644
> --- a/src/channel-display.c
> +++ b/src/channel-display.c
> @@ -1424,7 +1424,11 @@ 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);
> +        report_invalid_stream(channel, op->id);
> +        return;
> +    }

Looks good :)

Acked-by: Victor Toso <victort...@redhat.com>

Really sorry for keep this waiting. I'll do some small tests and push
both of them. I see there where pending comments in the previous series,
are they solved?

Cheers,
  toso

>      if (c->enable_adaptive_streaming) {
>          display_update_stream_report(SPICE_DISPLAY_CHANNEL(channel), op->id,
>                                       op->multi_media_time, latency);
> -- 
> 2.10.1
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Reply via email to