[Spice-devel] [RFC spice-gtk 1/2] streaming: make draw-area visible on MJPEG encoder creation
Snir Sheriber
ssheribe at redhat.com
Thu Aug 8 09:23:08 UTC 2019
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 at redhat.com>
>
> ---
> 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
More information about the Spice-devel
mailing list