[Spice-commits] 2 commits - src/channel-display.c src/channel-display-gst.c src/channel-display-mjpeg.c src/channel-display-priv.h

Victor Toso de Carvalho victortoso at kemper.freedesktop.org
Fri Dec 2 13:34:19 UTC 2016


 src/channel-display-gst.c   |   43 ++++++++++++++++++++++---
 src/channel-display-mjpeg.c |    8 ++--
 src/channel-display-priv.h  |    4 +-
 src/channel-display.c       |   75 +++++++++++++++++++++++++-------------------
 4 files changed, 90 insertions(+), 40 deletions(-)

New commits:
commit 4ed4def4273834971dc039ccb2b22fb16fa42a8c
Author: Francois Gouget <fgouget at codeweavers.com>
Date:   Tue Nov 22 18:01:38 2016 +0100

    streaming: Stop streaming if frames cannot be decoded
    
    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 <fgouget at codeweavers.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index f52299f..2b4ef7f 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;
@@ -287,6 +309,7 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
     guint opt;
     GstAppSinkCallbacks appsink_cbs = { NULL };
     GError *err = NULL;
+    GstBus *bus;
 
     auto_enabled = (g_getenv("SPICE_GSTVIDEO_AUTO") != NULL);
     if (auto_enabled || !VALID_VIDEO_CODEC_TYPE(decoder->base.codec_type)) {
@@ -324,6 +347,9 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
 
     appsink_cbs.new_sample = new_sample;
     gst_app_sink_set_callbacks(decoder->appsink, &appsink_cbs, decoder, NULL);
+    bus = gst_pipeline_get_bus(GST_PIPELINE(decoder->pipeline));
+    gst_bus_add_watch(bus, handle_pipeline_message, decoder);
+    gst_object_unref(bus);
 
     decoder->clock = gst_pipeline_get_clock(GST_PIPELINE(decoder->pipeline));
 
@@ -390,9 +416,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 +426,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 +445,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 +472,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;
+    }
     if (c->enable_adaptive_streaming) {
         display_update_stream_report(SPICE_DISPLAY_CHANNEL(channel), op->id,
                                      op->multi_media_time, latency);
commit fdd4dabf1127234c0c3d4a21b52385a0063e9b77
Author: Francois Gouget <fgouget at codeweavers.com>
Date:   Tue Nov 22 18:00:38 2016 +0100

    streaming: Report invalid streams to the server
    
    The error is sent using the existing client stream report message where
    the dropped frame count is maxed out while the received frame count is
    zero. Servers that recognize it can then switch to sending regular
    screen updates for that area so the client is not stuck with a frozen
    area on the screen.
    This can be useful in case the client is unable to decode the stream for
    some reason like a bug in the GStreamer plugins, the decoder not liking
    odd video dimensions, etc.
    
    Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/src/channel-display.c b/src/channel-display.c
index 709b3d2..500ddfb 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -1077,6 +1077,40 @@ static void display_update_stream_region(display_stream *st)
     }
 }
 
+static void report_invalid_stream(SpiceChannel *channel, uint32_t id)
+{
+    if (spice_channel_test_capability(channel, SPICE_DISPLAY_CAP_STREAM_REPORT)) {
+        SpiceMsgcDisplayStreamReport report;
+        SpiceMsgOut *msg;
+
+        /* Send a special stream report (UINT_MAX dropped frames out of zero)
+         * to indicate there is no such stream.
+         */
+        spice_printerr("notify the server that stream %u does not exist", id);
+        memset(&report, 0, sizeof(report));
+        report.stream_id = id;
+        report.num_frames = 0;
+        report.num_drops = UINT_MAX;
+
+        msg = spice_msg_out_new(SPICE_CHANNEL(channel), SPICE_MSGC_DISPLAY_STREAM_REPORT);
+        msg->marshallers->msgc_display_stream_report(msg->marshaller, &report);
+        spice_msg_out_send(msg);
+    }
+}
+
+static display_stream *get_stream_by_id(SpiceChannel *channel, uint32_t id)
+{
+    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
+
+    if (c != NULL && c->streams != NULL && id < c->nstreams &&
+        c->streams[id] != NULL) {
+        return c->streams[id];
+    }
+
+    report_invalid_stream(channel, id);
+    return NULL;
+}
+
 /* coroutine context */
 static void display_handle_stream_create(SpiceChannel *channel, SpiceMsgIn *in)
 {
@@ -1127,6 +1161,7 @@ static void display_handle_stream_create(SpiceChannel *channel, SpiceMsgIn *in)
     if (st->video_decoder == NULL) {
         spice_printerr("could not create a video decoder for codec %u", op->codec_type);
         destroy_stream(channel, op->id);
+        report_invalid_stream(channel, op->id);
     }
 }
 
@@ -1224,17 +1259,10 @@ void stream_display_frame(display_stream *st, SpiceMsgIn *frame_msg,
 static void display_update_stream_report(SpiceDisplayChannel *channel, uint32_t stream_id,
                                          uint32_t frame_time, int32_t latency)
 {
-    SpiceDisplayChannelPrivate *c = channel->priv;
-    display_stream *st;
+    display_stream *st = get_stream_by_id(SPICE_CHANNEL(channel), stream_id);
     guint64 now;
 
-    g_return_if_fail(c != NULL);
-    g_return_if_fail(c->streams != NULL);
-    g_return_if_fail(c->nstreams > stream_id);
-
-    st = channel->priv->streams[stream_id];
     g_return_if_fail(st != NULL);
-
     if (!st->report_is_active) {
         return;
     }
@@ -1347,15 +1375,10 @@ static void display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn *in)
 {
     SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
     SpiceStreamDataHeader *op = spice_msg_in_parsed(in);
-    display_stream *st;
+    display_stream *st = get_stream_by_id(channel, op->id);
     guint32 mmtime;
     int32_t latency;
 
-    g_return_if_fail(c != NULL);
-    g_return_if_fail(c->streams != NULL);
-    g_return_if_fail(c->nstreams > op->id);
-
-    st =  c->streams[op->id];
     g_return_if_fail(st != NULL);
     mmtime = stream_get_time(st);
 
@@ -1415,17 +1438,10 @@ static void display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn *in)
 /* coroutine context */
 static void display_handle_stream_clip(SpiceChannel *channel, SpiceMsgIn *in)
 {
-    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
     SpiceMsgDisplayStreamClip *op = spice_msg_in_parsed(in);
-    display_stream *st;
-
-    g_return_if_fail(c != NULL);
-    g_return_if_fail(c->streams != NULL);
-    g_return_if_fail(c->nstreams > op->id);
+    display_stream *st = get_stream_by_id(channel, op->id);
 
-    st = c->streams[op->id];
     g_return_if_fail(st != NULL);
-
     if (st->msg_clip) {
         spice_msg_in_unref(st->msg_clip);
     }
@@ -1524,17 +1540,10 @@ static void display_handle_stream_destroy_all(SpiceChannel *channel, SpiceMsgIn
 /* coroutine context */
 static void display_handle_stream_activate_report(SpiceChannel *channel, SpiceMsgIn *in)
 {
-    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
     SpiceMsgDisplayStreamActivateReport *op = spice_msg_in_parsed(in);
-    display_stream *st;
+    display_stream *st = get_stream_by_id(channel, op->stream_id);
 
-    g_return_if_fail(c != NULL);
-    g_return_if_fail(c->streams != NULL);
-    g_return_if_fail(c->nstreams > op->stream_id);
-
-    st = c->streams[op->stream_id];
     g_return_if_fail(st != NULL);
-
     st->report_is_active = TRUE;
     st->report_id = op->unique_id;
     st->report_max_window = op->max_window_size;


More information about the Spice-commits mailing list