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

Victor Toso de Carvalho victortoso at kemper.freedesktop.org
Tue Jul 11 12:49:14 UTC 2017


 src/channel-display-gst.c  |   54 ++++++++++++++++-----------------------------
 src/channel-display-priv.h |   41 ++++++++++++++++++++++++++++++++++
 src/channel-display.c      |   35 +++++++++++++++++------------
 3 files changed, 82 insertions(+), 48 deletions(-)

New commits:
commit 141c2d82f32c48d372c0ca13f090c58f87e82453
Author: Victor Toso <me at victortoso.com>
Date:   Wed Jun 21 12:14:26 2017 +0200

    display-gst: Debug video pipeline on stream-start message
    
    On GST_MESSAGE_STREAM_START our stream is about to start and it seems
    a good moment to debug GStreamer pipeline if requested.
    
    That we can do with GST_DEBUG_BIN_TO_DOT_FILE() which iterates
    recursively in the whole pipeline and generates a dot file with all
    the information, the filename is:
      spice-gtk-gst-pipeline-debug-$stream_id-$video_codec.dot
    
    One can generate png image out of this dot file with the command:
      dot -Tpng -oimage.png spice-gtk-gst-pipeline-debug.dot
    
    Note that GST_DEBUG_BIN_TO_DOT_FILE() will only work if Gstreamer was
    configured with --gst-enable-gst-debug and the environment variable
    GST_DEBUG_DUMP_DOT_DIR was properly set.
    
    Full example:
    
    1) GST_DEBUG_DUMP_DOT_DIR=/tmp remote-viewer ...
    2) dot -Tpng -oimage.png spice-gtk-gst-pipeline-debug.dot
    
    Signed-off-by: Victor Toso <victortoso at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 65f4422..3cf451b 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -268,7 +268,8 @@ static gboolean handle_pipeline_message(GstBus *bus, GstMessage *msg, gpointer v
 {
     SpiceGstDecoder *decoder = video_decoder;
 
-    if (GST_MESSAGE_TYPE(msg) == GST_MESSAGE_ERROR) {
+    switch(GST_MESSAGE_TYPE(msg)) {
+    case GST_MESSAGE_ERROR: {
         GError *err = NULL;
         gchar *debug_info = NULL;
         gst_message_parse_error(msg, &err, &debug_info);
@@ -282,6 +283,24 @@ static gboolean handle_pipeline_message(GstBus *bus, GstMessage *msg, gpointer v
 
         /* We won't be able to process any more frame anyway */
         free_pipeline(decoder);
+        break;
+    }
+    case GST_MESSAGE_STREAM_START: {
+        gchar *filename = g_strdup_printf("spice-gtk-gst-pipeline-debug-%ld-%s",
+                                          get_stream_id_by_stream(decoder->base.stream->channel,
+                                                                  decoder->base.stream),
+                                          gst_opts[decoder->base.codec_type].name);
+        GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(decoder->pipeline),
+                                  GST_DEBUG_GRAPH_SHOW_ALL
+                                    | GST_DEBUG_GRAPH_SHOW_FULL_PARAMS
+                                    | GST_DEBUG_GRAPH_SHOW_STATES,
+                                    filename);
+        g_free(filename);
+        break;
+    }
+    default:
+        /* not being handled */
+        break;
     }
     return TRUE;
 }
diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
index b6ccdcb..04cb4d1 100644
--- a/src/channel-display-priv.h
+++ b/src/channel-display-priv.h
@@ -193,6 +193,7 @@ G_STATIC_ASSERT(G_N_ELEMENTS(gst_opts) <= SPICE_VIDEO_CODEC_TYPE_ENUM_END);
 guint32 stream_get_time(display_stream *st);
 void stream_dropped_frame_on_playback(display_stream *st);
 void stream_display_frame(display_stream *st, SpiceFrame *frame, uint32_t width, uint32_t height, uint8_t* data);
+gint64 get_stream_id_by_stream(SpiceChannel *channel, display_stream *st);
 
 
 G_END_DECLS
diff --git a/src/channel-display.c b/src/channel-display.c
index e51064a..3c98d79 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -1164,6 +1164,23 @@ static display_stream *get_stream_by_id(SpiceChannel *channel, uint32_t id)
     return NULL;
 }
 
+G_GNUC_INTERNAL
+gint64 get_stream_id_by_stream(SpiceChannel *channel, display_stream *st)
+{
+    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
+    guint i;
+
+    g_return_val_if_fail(c->streams != NULL, -1);
+    g_return_val_if_fail(c->nstreams > 0, -1);
+
+    for (i = 0; i < c->nstreams; i++) {
+        if (c->streams[i] == st)
+            return i;
+    }
+
+    return -1;
+}
+
 /* coroutine context */
 static display_stream *display_stream_create(SpiceChannel *channel, uint32_t surface_id,
                                              uint32_t flags, uint32_t codec_type,
commit c96af50e2dd16b02b189e89b06761b1e308417a7
Author: Victor Toso <me at victortoso.com>
Date:   Fri Jun 30 14:05:53 2017 +0200

    channel-display: keep one struct array for video-codecs data
    
    On channel-display.c, for each SpiceVideoCodecType we need:
     - Its enum type;
     - The associated capability;
     - A name;
    
    On channel-display-gst.c, for each SpiceVideoCodecType we need:
     - Associated decoding elements for the pipeline;
     - Associated GstCaps, also for the video pipeline;
    
    Follow up patch will need the associated name for given
    SpiceVideoCodecType.
    
    Moving gst_opts[] array to channel-display-priv.h to be reused. This
    should also make slightly simpler when supporting a new video codec in
    the future.
    
    Signed-off-by: Victor Toso <victortoso at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 8669562..65f4422 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -50,39 +50,6 @@ typedef struct SpiceGstDecoder {
     guint timer_id;
 } SpiceGstDecoder;
 
-/* FIXME: With gstreamer version 1.9.0 and higher, we are using playbin to
- * create the pipeline for us and for that reason we don't need to keep track of
- * decoder's name anymore. */
-static struct {
-    const gchar *dec_name;
-    const gchar *dec_caps;
-} gst_opts[] = {
-    /* SpiceVideoCodecType starts at index 1 */
-    { NULL, NULL },
-
-    /* SPICE_VIDEO_CODEC_TYPE_MJPEG */
-    { "jpegdec", "image/jpeg" },
-
-    /* SPICE_VIDEO_CODEC_TYPE_VP8
-     *
-     * typefind is unable to identify VP8 streams by design.
-     * See: https://bugzilla.gnome.org/show_bug.cgi?id=756457
-     */
-    { "vp8dec", "video/x-vp8" },
-
-    /* SPICE_VIDEO_CODEC_TYPE_H264
-     * When setting video/x-h264, h264parse will complain if we don't have the
-     * stream-format or codec_data information. As stream-format is byte-stream
-     * (hardcoded in spice-server), let's add it here to avoid the warning.
-     */
-    { "h264parse ! avdec_h264", "video/x-h264,stream-format=byte-stream" },
-
-    /* SPICE_VIDEO_CODEC_TYPE_VP9 */
-    { "vp9dec", "video/x-vp9" },
-};
-
-G_STATIC_ASSERT(G_N_ELEMENTS(gst_opts) <= SPICE_VIDEO_CODEC_TYPE_ENUM_END);
-
 #define VALID_VIDEO_CODEC_TYPE(codec) \
     (codec > 0 && codec < G_N_ELEMENTS(gst_opts))
 
diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
index 3c9d119..b6ccdcb 100644
--- a/src/channel-display-priv.h
+++ b/src/channel-display-priv.h
@@ -150,6 +150,46 @@ struct display_stream {
     uint32_t report_drops_seq_len;
 };
 
+static const struct {
+    int cap;
+    const gchar name[8];
+
+    /* FIXME: With gstreamer version 1.9.0 and higher, we are using playbin to
+     * create the pipeline for us and for that reason we don't need to keep track of
+     * decoder's name anymore. */
+    const gchar *dec_name;
+    const gchar *dec_caps;
+} gst_opts[] = {
+    /* SpiceVideoCodecType starts at index 1 */
+    { 0 },
+
+    /* SPICE_VIDEO_CODEC_TYPE_MJPEG */
+    { SPICE_DISPLAY_CAP_CODEC_MJPEG, "mjpeg",
+      "jpegdec", "image/jpeg" },
+
+    /* SPICE_VIDEO_CODEC_TYPE_VP8
+     *
+     * typefind is unable to identify VP8 streams by design.
+     * See: https://bugzilla.gnome.org/show_bug.cgi?id=756457
+     */
+    { SPICE_DISPLAY_CAP_CODEC_VP8, "vp8",
+      "vp8dec", "video/x-vp8" },
+
+    /* SPICE_VIDEO_CODEC_TYPE_H264
+     * When setting video/x-h264, h264parse will complain if we don't have the
+     * stream-format or codec_data information. As stream-format is byte-stream
+     * (hardcoded in spice-server), let's add it here to avoid the warning.
+     */
+    { SPICE_DISPLAY_CAP_CODEC_H264, "h264",
+      "h264parse ! avdec_h264", "video/x-h264,stream-format=byte-stream" },
+
+    /* SPICE_VIDEO_CODEC_TYPE_VP9 */
+    { SPICE_DISPLAY_CAP_CODEC_VP9, "vp9",
+      "vp9dec", "video/x-vp9" },
+};
+
+G_STATIC_ASSERT(G_N_ELEMENTS(gst_opts) <= SPICE_VIDEO_CODEC_TYPE_ENUM_END);
+
 guint32 stream_get_time(display_stream *st);
 void stream_dropped_frame_on_playback(display_stream *st);
 void stream_display_frame(display_stream *st, SpiceFrame *frame, uint32_t width, uint32_t height, uint8_t* data);
diff --git a/src/channel-display.c b/src/channel-display.c
index 06c503c..e51064a 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -764,16 +764,6 @@ static HDC create_compatible_dc(void)
 static void spice_display_channel_reset_capabilities(SpiceChannel *channel)
 {
     guint i;
-    static const struct {
-        SpiceVideoCodecType type;
-        int cap;
-        const gchar name[8];
-    } gst_codecs[] = {
-        {SPICE_VIDEO_CODEC_TYPE_MJPEG, SPICE_DISPLAY_CAP_CODEC_MJPEG, "mjpeg"},
-        {SPICE_VIDEO_CODEC_TYPE_VP8, SPICE_DISPLAY_CAP_CODEC_VP8, "vp8"},
-        {SPICE_VIDEO_CODEC_TYPE_H264, SPICE_DISPLAY_CAP_CODEC_H264, "h264"},
-        {SPICE_VIDEO_CODEC_TYPE_VP9, SPICE_DISPLAY_CAP_CODEC_VP9, "vp9"},
-    };
 
     spice_channel_set_capability(SPICE_CHANNEL(channel), SPICE_DISPLAY_CAP_SIZED_STREAM);
     spice_channel_set_capability(SPICE_CHANNEL(channel), SPICE_DISPLAY_CAP_MONITORS_CONFIG);
@@ -792,11 +782,11 @@ static void spice_display_channel_reset_capabilities(SpiceChannel *channel)
 #ifdef HAVE_BUILTIN_MJPEG
     spice_channel_set_capability(SPICE_CHANNEL(channel), SPICE_DISPLAY_CAP_CODEC_MJPEG);
 #endif
-    for (i = 0; i < G_N_ELEMENTS(gst_codecs); i++) {
-        if (gstvideo_has_codec(gst_codecs[i].type)) {
-            spice_channel_set_capability(SPICE_CHANNEL(channel), gst_codecs[i].cap);
+    for (i = 1; i < G_N_ELEMENTS(gst_opts); i++) {
+        if (gstvideo_has_codec(i)) {
+            spice_channel_set_capability(SPICE_CHANNEL(channel), gst_opts[i].cap);
         } else {
-            SPICE_DEBUG("GStreamer does not support the %s codec", gst_codecs[i].name);
+            SPICE_DEBUG("GStreamer does not support the %s codec", gst_opts[i].name);
         }
     }
 }


More information about the Spice-commits mailing list