Re: [RFC spice-gtk 1/2] streaming: make draw-area visible on MJPEG encoder creation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 8/7/19 6:49 PM, Kevin Pouget wrote:
This patch allows the MJPEG encoder to inform the spice-widget that
its video drawing area (draw-area) should be made visible on screen.


So the first one does not work?

BTW i think the consensus is to mention the patch version in all
of the subjects (not only in the cover letter)


This is required to switch from GST video decoding to native MJPEG
decoding, otherwise the gst-area remained on top and the MJPEG video
stream was never shown.


Actually IIRC the first version of the gst-overlay signal included
a boolean value which could have been useful here :P



Signed-off-by: Kevin Pouget <kpouget@xxxxxxxxxx>

---
  src/channel-display-mjpeg.c |  2 ++
  src/channel-display-priv.h  |  2 +-
  src/channel-display.c       | 38 ++++++++++++++++++++++++++++++++++---
  src/spice-marshal.txt       |  1 +
  src/spice-widget.c          | 16 ++++++++++++++--
  5 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c
index 647d31b..6f1a4d7 100644
--- a/src/channel-display-mjpeg.c
+++ b/src/channel-display-mjpeg.c
@@ -300,5 +300,7 @@ VideoDecoder* create_mjpeg_decoder(int codec_type, display_stream *stream)

      /* All the other fields are initialized to zero by g_new0(). */

+    show_mjpeg_widget(stream);
+
      return (VideoDecoder*)decoder;
  }
diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
index 16c12c6..3424a8e 100644
--- a/src/channel-display-priv.h
+++ b/src/channel-display-priv.h
@@ -199,7 +199,7 @@ void stream_dropped_frame_on_playback(display_stream *st);
  void stream_display_frame(display_stream *st, SpiceFrame *frame, uint32_t width, uint32_t height, int stride, uint8_t* data);
  guintptr get_window_handle(display_stream *st);
  gboolean hand_pipeline_to_widget(display_stream *st,  GstPipeline *pipeline);
-
+gboolean show_mjpeg_widget(display_stream *st);
  void spice_frame_free(SpiceFrame *frame);

  G_END_DECLS
diff --git a/src/channel-display.c b/src/channel-display.c
index 44555e3..18e95b9 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -90,7 +90,8 @@ enum {
      SPICE_DISPLAY_MARK,
      SPICE_DISPLAY_GL_DRAW,
      SPICE_DISPLAY_STREAMING_MODE,
-    SPICE_DISPLAY_OVERLAY,
+    SPICE_DISPLAY_GST_OVERLAY,
+    SPICE_DISPLAY_MJPEG_OVERLAY,

The term overlay is derived from GstVideoOverlay which is an gstreamer
interface that overlays gstreamer output on the gtk widget, in the case
of builtin mjpeg (also with gst is not always true) we pass pixels and
render into the surface I don't think the name should invlude "overlay".



      SPICE_DISPLAY_LAST_SIGNAL,
  };
@@ -491,7 +492,7 @@ static void spice_display_channel_class_init(SpiceDisplayChannelClass *klass)
       *
       * Since: 0.36
       **/
-    signals[SPICE_DISPLAY_OVERLAY] =
+    signals[SPICE_DISPLAY_GST_OVERLAY] =
          g_signal_new("gst-video-overlay",
                       G_OBJECT_CLASS_TYPE(gobject_class),
                       0, 0,
@@ -501,6 +502,25 @@ static void spice_display_channel_class_init(SpiceDisplayChannelClass *klass)
                       1,
                       GST_TYPE_PIPELINE);

+    /**
+     * SpiceDisplayChannel::mjpeg-video-overlay:
+     * @display: the #SpiceDisplayChannel that emitted the signal
+     *
+     * The #SpiceDisplayChannel::mjpeg-video-overlay signal is emitted
+     * when the MJPEG encoder is ready and its drawing area should be
+     * made visible on screen
+     *
+     * Since: 0.37
+     **/
+    signals[SPICE_DISPLAY_MJPEG_OVERLAY] =
+        g_signal_new("mjpeg-video-overlay",
+                     G_OBJECT_CLASS_TYPE(gobject_class),
+                     0, 0,
+                     NULL, NULL,
+                     g_cclosure_user_marshal_BOOLEAN__VOID,
+                     G_TYPE_BOOLEAN,
+                     0);
+
      channel_set_handlers(SPICE_CHANNEL_CLASS(klass));
  }

@@ -1472,12 +1492,24 @@ gboolean hand_pipeline_to_widget(display_stream *st, GstPipeline *pipeline)
      gboolean res = false;

      if (st->surface->streaming_mode) {
-        g_signal_emit(st->channel, signals[SPICE_DISPLAY_OVERLAY], 0,
+        g_signal_emit(st->channel, signals[SPICE_DISPLAY_GST_OVERLAY], 0,
                        pipeline, &res);
      }
      return res;
  }

+G_GNUC_INTERNAL
+gboolean show_mjpeg_widget(display_stream *st)
+{
+    gboolean res = false;
+    g_warning("SHOW!");

I guess this was for testing purposes.


+    if (st->surface->streaming_mode) {
+        g_signal_emit(st->channel, signals[SPICE_DISPLAY_MJPEG_OVERLAY], 0, &res);
+    }
+
+    return res;
+}
+
  /* after a sequence of 3 drops, push a report to the server, even
   * if the report window is bigger */
  #define STREAM_REPORT_DROP_SEQ_LEN_LIMIT 3
diff --git a/src/spice-marshal.txt b/src/spice-marshal.txt
index 46be405..e60d725 100644
--- a/src/spice-marshal.txt
+++ b/src/spice-marshal.txt
@@ -8,6 +8,7 @@ VOID:POINTER,INT
  VOID:POINTER,BOOLEAN
  BOOLEAN:POINTER,UINT
  BOOLEAN:UINT
+BOOLEAN:VOID
  VOID:UINT,POINTER,UINT
  VOID:UINT,UINT,POINTER,UINT
  BOOLEAN:UINT,POINTER,UINT
diff --git a/src/spice-widget.c b/src/spice-widget.c
index a9ba1f1..34e3c5e 100644
--- a/src/spice-widget.c
+++ b/src/spice-widget.c
@@ -2210,6 +2210,8 @@ static void update_image(SpiceDisplay *display)
      SpiceDisplayPrivate *d = display->priv;

      spice_cairo_image_create(display);
+    gtk_stack_set_visible_child_name(d->stack, "draw-area");
+
      if (d->canvas.convert)
          do_color_convert(display, &d->area);
  }
@@ -2782,7 +2784,7 @@ static void gst_size_allocate(GtkWidget *widget, GdkRectangle *a, gpointer data)
  /* This callback should pass to the widget a pointer of the pipeline
   * so that we can set pipeline and overlay related calls from here.
   */
-static gboolean set_overlay(SpiceChannel *channel, void* pipeline_ptr, SpiceDisplay *display)
+static gboolean gst_set_overlay(SpiceChannel *channel, void* pipeline_ptr, SpiceDisplay *display)
  {
  #if defined(GDK_WINDOWING_X11)
      SpiceDisplayPrivate *d = display->priv;
@@ -2808,6 +2810,14 @@ static gboolean set_overlay(SpiceChannel *channel, void* pipeline_ptr, SpiceDisp
      return false;
  }

+static gboolean mjpeg_set_overlay(SpiceChannel *channel, SpiceDisplay *display)
+{
+    SpiceDisplayPrivate *d = display->priv;
+    g_warning("SET!");

Same

+    gtk_stack_set_visible_child_name(d->stack, "draw-area");
+    return true;
+}
+
  static void invalidate(SpiceChannel *channel,
                         gint x, gint y, gint w, gint h, gpointer data)
  {
@@ -3192,7 +3202,9 @@ static void channel_new(SpiceSession *s, SpiceChannel *channel, SpiceDisplay *di
                                        G_CALLBACK(spice_display_widget_update_monitor_area),
                                        display, G_CONNECT_AFTER | G_CONNECT_SWAPPED);
          spice_g_signal_connect_object(channel, "gst-video-overlay",
-                                      G_CALLBACK(set_overlay), display, G_CONNECT_AFTER);
+                                      G_CALLBACK(gst_set_overlay), display, G_CONNECT_AFTER);
+        spice_g_signal_connect_object(channel, "mjpeg-video-overlay",
+                                      G_CALLBACK(mjpeg_set_overlay), display, G_CONNECT_AFTER);
          if (spice_display_channel_get_primary(channel, 0, &primary)) {
              primary_create(channel, primary.format, primary.width, primary.height,
                             primary.stride, primary.shmid, primary.data, display);


I'd also suggest another option which is to not allow switching into built-in
mjpeg in case you started with gstreamer (i.e. one you started with
gstreamer decoder stick to it).
I guess it will be simpler but needs to see what this would require.

Snir.


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




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]