Re: [Spice-devel] Spice agent and LXC
Using the "Spice for Newbies" (which doesn't really seem to fit its title IMO) documentation I was able to answer some of my questions regarding data flow. However, I'm not closer to solving my problem. After building from the git repositories I get the same results in a Fedora LXC while an Ubuntu LXC segfaults during startup. (II) spiceqxl(0): 249: 1080x7680, 16 bits, stride 2160, 317mm x 2255mm, orientation 1 (II) spiceqxl(0): 250: 7680x1080, 16 bits, stride 15360, 2255mm x 317mm, orientation 2 (II) spiceqxl(0): 251: 1080x7680, 16 bits, stride 2160, 317mm x 2255mm, orientation 3 (II) spiceqxl(0): PreInit complete (--) Depth 24 pixmap format is 32 bpp (II) spiceqxl(0): framebuffer at 0x7f708425d010 (16384 KB) (II) spiceqxl(0): command ram at 0x7f708525d010 (114680 KB) (II) spiceqxl(0): vram at 0x7f707c25c010 (131072 KB) (II) spiceqxl(0): rom at 0x7f70945498a0 resizing surface0 to 16777216 memory space from 0x7f708525d010 to 0x7f708c25a010 memory space from 0x7f707c25c010 to 0x7f708425c010 (EE) (EE) Backtrace: (EE) 0: X (xorg_backtrace+0x48) [0x7f7093a48d28] (EE) 1: X (0x7f70938a+0x1aca19) [0x7f7093a4ca19] (EE) 2: /lib/x86_64-linux-gnu/libpthread.so.0 (0x7f709299d000+0x10340) [0x7f70929ad340] (EE) 3: /usr/lib/x86_64-linux-gnu/libspice-server.so.1 (0x7f708d21a000+0x20d3c) [0x7f708d23ad3c] (EE) 4: /usr/lib/x86_64-linux-gnu/libspice-server.so.1 (0x7f708d21a000+0x210dc) [0x7f708d23b0dc] (EE) 5: /usr/lib/xorg/modules/drivers/spiceqxl_drv.so (0x7f708d537000+0x97b5) [0x7f708d5407b5] (EE) 6: /usr/lib/x86_64-linux-gnu/libspice-server.so.1 (0x7f708d21a000+0x22dde) [0x7f708d23cdde] (EE) 7: /usr/lib/x86_64-linux-gnu/libspice-server.so.1 (spice_server_add_interface+0x3a6) [0x7f708d263206] (EE) 8: /usr/lib/xorg/modules/drivers/spiceqxl_drv.so (0x7f708d537000+0xc193) [0x7f708d543193] (EE) 9: X (AddScreen+0x71) [0x7f70938f5ca1] (EE) 10: X (InitOutput+0x3c8) [0x7f7093936ce8] (EE) 11: X (0x7f70938a+0x596bb) [0x7f70938f96bb] (EE) 12: /lib/x86_64-linux-gnu/libc.so.6 (__libc_start_main+0xf5) [0x7f70913dcec5] (EE) 13: X (0x7f70938a+0x44dde) [0x7f70938e4dde] (EE) (EE) Segmentation fault at address 0xd8 (EE) Fatal server error: (EE) Caught signal 11 (Segmentation fault). Server aborting ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 1/2][spice-gtk] audio: new-sample callback must return GST_FLOW_OK
On Mon, Nov 24, 2014 at 6:57 PM, Victor Toso wrote: > This fix is necessary when using windows-client with gstreamer 1.0 to > record-audio in the target machine. > > --- > gtk/spice-gstaudio.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/gtk/spice-gstaudio.c b/gtk/spice-gstaudio.c > index 6173783..df2fc8e 100644 > --- a/gtk/spice-gstaudio.c > +++ b/gtk/spice-gstaudio.c > @@ -120,7 +120,7 @@ static void spice_gstaudio_class_init(SpiceGstaudioClass > *klass) > g_type_class_add_private(klass, sizeof(SpiceGstaudioPrivate)); > } > > -static void record_new_buffer(GstAppSink *appsink, gpointer data) > +static GstFlowReturn record_new_buffer(GstAppSink *appsink, gpointer data) > { > SpiceGstaudio *gstaudio = data; > SpiceGstaudioPrivate *p = gstaudio->priv; > @@ -135,6 +135,7 @@ static void record_new_buffer(GstAppSink *appsink, > gpointer data) > msg = gst_message_new_application(GST_OBJECT(p->record.pipe), NULL); > #endif > gst_element_post_message(p->record.pipe, msg); > +return GST_FLOW_OK; > } > > static void record_stop(SpiceGstaudio *gstaudio) > -- > 2.1.0 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/spice-devel ACK! -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 2/2] audio: Avoid bad pipelines from gst_parse_launch
On Mon, Nov 24, 2014 at 6:57 PM, Victor Toso wrote: > gst_parse_launch may return not NULL even when error is set. > This can lead to data loss when playing or recording audio. Is there any case where gst_parse_launch returns NULL but does not set the error? If no, I'd go for a simple: if (error != NULL) { } Best Regards, -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [xf86-video-qxl v2] Enable smartcard support for XSpice.
Sorry; this was wrong. v3 should hopefully be correct. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [xf86-video-qxl v3] Enable smartcard support for XSpice.
This is done by creating a Unix domain socket to which smartcard messages are transferred, using the vscard protocol. A further system library, spiceccid, is used to provide an interface into pcsc-lite, specifically the pcsc-lite daemon, so that regular Unix applications can access the passed through smartcard information. Signed-off-by: Jeremy White --- configure.ac | 21 ++ examples/spiceqxl.xorg.conf.example|3 + src/Makefile.am|3 +- src/qxl.h |2 + src/qxl_driver.c | 14 +- src/spiceccid/Makefile.am | 29 ++ src/spiceccid/spice.pcsc.conf.template |7 + src/spiceccid/spiceccid.c | 455 src/spiceqxl_smartcard.c | 193 ++ src/spiceqxl_smartcard.h | 31 +++ 10 files changed, 756 insertions(+), 2 deletions(-) create mode 100644 src/spiceccid/Makefile.am create mode 100644 src/spiceccid/spice.pcsc.conf.template create mode 100644 src/spiceccid/spiceccid.c create mode 100644 src/spiceqxl_smartcard.c create mode 100644 src/spiceqxl_smartcard.h diff --git a/configure.ac b/configure.ac index 14e0597..d9da852 100644 --- a/configure.ac +++ b/configure.ac @@ -137,8 +137,27 @@ if test "x$enable_xspice" = "xyes"; then else enable_xspice=no fi + +AC_ARG_ENABLE([ccid], +[AS_HELP_STRING([--enable-ccid], +[Build the spiceccid SmartCard driver (default is no)])], +[enable_ccid=$enableval], +[enable_ccid=no]) +AC_ARG_WITH(ccid-module-dir, +[AS_HELP_STRING([--with-ccid-module-dir=DIR ], +[Specify the install path for spiceccid driver (default is $libdir/pcsc/drivers/serial)])], +[ cciddir="$withval" ], +[ cciddir="$libdir/pcsc/drivers/serial" ]) +AC_SUBST(cciddir) +if test "x$enable_ccid" != "xno"; then +PKG_CHECK_MODULES(LIBPCSCLITE, [libpcsclite]) +PKG_CHECK_MODULES(LIBCACARD, [libcacard]) +fi + + AM_CONDITIONAL(BUILD_XSPICE, test "x$enable_xspice" = "xyes") AM_CONDITIONAL(BUILD_QXL, test "x$enable_qxl" = "xyes") +AM_CONDITIONAL(BUILD_SPICECCID, test "x$enable_ccid" = "xyes") AC_ARG_ENABLE([udev], AS_HELP_STRING([--disable-udev], [Disable libudev support [default=auto]]), @@ -168,6 +187,7 @@ fi AC_CONFIG_FILES([ Makefile src/Makefile +src/spiceccid/Makefile src/uxa/Makefile scripts/Makefile examples/Makefile @@ -187,4 +207,5 @@ echo " KMS: ${DRM_MODE} Build qxl:${enable_qxl} Build xspice: ${enable_xspice} +Build spiceccid: ${enable_ccid} " diff --git a/examples/spiceqxl.xorg.conf.example b/examples/spiceqxl.xorg.conf.example index 597a5bd..d15f7f2 100644 --- a/examples/spiceqxl.xorg.conf.example +++ b/examples/spiceqxl.xorg.conf.example @@ -143,6 +143,9 @@ Section "Device" # to the client. Default is no mixing. #Option "SpicePlaybackFIFODir" "/tmp/" +# A unix domain name for a unix domain socket +# to communicate with a spiceccid smartcard driver +#Option "SpiceSmartCardFile" "/tmp/spice.pcsc.comm" EndSection Section "InputDevice" diff --git a/src/Makefile.am b/src/Makefile.am index bf50ae1..6c72bbd 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -25,7 +25,7 @@ # _ladir passes a dummy rpath to libtool so the thing will actually link # TODO: -nostdlib/-Bstatic/-lgcc platform magic, not installing the .a, etc. -SUBDIRS=uxa +SUBDIRS=uxa spiceccid AM_CFLAGS = $(SPICE_PROTOCOL_CFLAGS) $(XORG_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(DRM_CFLAGS) @LIBUDEV_CFLAGS@ @@ -96,6 +96,7 @@ spiceqxl_drv_la_SOURCES = \ spiceqxl_uinput.c \ spiceqxl_uinput.h \ spiceqxl_audio.c\ + spiceqxl_smartcard.c\ spiceqxl_audio.h\ spiceqxl_inputs.c \ spiceqxl_inputs.h \ diff --git a/src/qxl.h b/src/qxl.h index 603faca..54995cf 100644 --- a/src/qxl.h +++ b/src/qxl.h @@ -157,6 +157,7 @@ enum { OPTION_FRAME_BUFFER_SIZE, OPTION_SURFACE_BUFFER_SIZE, OPTION_COMMAND_BUFFER_SIZE, +OPTION_SPICE_SMARTCARD_FILE, #endif OPTION_COUNT, }; @@ -352,6 +353,7 @@ struct _qxl_screen_t char playback_fifo_dir[PATH_MAX]; void *playback_opaque; +char smartcard_file[PATH_MAX]; #endif /* XSPICE */ uint32_t deferred_fps; diff --git a/src/qxl_driver.c b/src/qxl_driver.c index 165f468..9ad8921 100644 --- a/src/qxl_driver.c +++ b/src/qxl_driver.c @@ -55,6 +55,7 @@ #include "spiceqxl_io_port.h" #include "spiceqxl_spice_server.h" #include "spiceqxl_audio.h" +#include "spiceqxl_smartcard.
[Spice-devel] [PATCH 2/2] audio: Avoid bad pipelines from gst_parse_launch
gst_parse_launch may return not NULL even when error is set. This can lead to data loss when playing or recording audio. --- gtk/spice-gstaudio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gtk/spice-gstaudio.c b/gtk/spice-gstaudio.c index df2fc8e..2b7f683 100644 --- a/gtk/spice-gstaudio.c +++ b/gtk/spice-gstaudio.c @@ -245,7 +245,7 @@ static void record_start(SpiceRecordChannel *channel, gint format, gint channels "appsink caps=\"%s\" name=appsink", audio_caps); p->record.pipe = gst_parse_launch(pipeline, &error); -if (p->record.pipe == NULL) { +if (error != NULL || p->record.pipe == NULL) { g_warning("Failed to create pipeline: %s", error->message); goto lerr; } @@ -345,7 +345,7 @@ static void playback_start(SpicePlaybackChannel *channel, gint format, gint chan "audioconvert ! audioresample ! autoaudiosink name=\"audiosink\"", audio_caps); SPICE_DEBUG("audio pipeline: %s", pipeline); p->playback.pipe = gst_parse_launch(pipeline, &error); -if (p->playback.pipe == NULL) { +if (error != NULL || p->playback.pipe == NULL) { g_warning("Failed to create pipeline: %s", error->message); goto lerr; } -- 2.1.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 1/2][spice-gtk] audio: new-sample callback must return GST_FLOW_OK
This fix is necessary when using windows-client with gstreamer 1.0 to record-audio in the target machine. --- gtk/spice-gstaudio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gtk/spice-gstaudio.c b/gtk/spice-gstaudio.c index 6173783..df2fc8e 100644 --- a/gtk/spice-gstaudio.c +++ b/gtk/spice-gstaudio.c @@ -120,7 +120,7 @@ static void spice_gstaudio_class_init(SpiceGstaudioClass *klass) g_type_class_add_private(klass, sizeof(SpiceGstaudioPrivate)); } -static void record_new_buffer(GstAppSink *appsink, gpointer data) +static GstFlowReturn record_new_buffer(GstAppSink *appsink, gpointer data) { SpiceGstaudio *gstaudio = data; SpiceGstaudioPrivate *p = gstaudio->priv; @@ -135,6 +135,7 @@ static void record_new_buffer(GstAppSink *appsink, gpointer data) msg = gst_message_new_application(GST_OBJECT(p->record.pipe), NULL); #endif gst_element_post_message(p->record.pipe, msg); +return GST_FLOW_OK; } static void record_stop(SpiceGstaudio *gstaudio) -- 2.1.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice] chardev: remove write polling
Hi - Original Message - > Hey, > > On Mon, Nov 24, 2014 at 04:56:26PM +0100, Marc-André Lureau wrote: > > In an effort to reduce the wakeups per second, get rid of the > > "write_to_dev" timer when the implementation supports > > SPICE_CHAR_DEVICE_NOTIFY_WRITABLE. > > Do you know what is the status of getting support for this in QEMU? Did > the char device changes this feature depends on get committed to master? It is in Gerd spice-next branch, I assume it will be merge soon, so it would be better to make sure it is indeed in 0.12.6. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice] chardev: remove write polling
Hey, On Mon, Nov 24, 2014 at 04:56:26PM +0100, Marc-André Lureau wrote: > In an effort to reduce the wakeups per second, get rid of the > "write_to_dev" timer when the implementation supports > SPICE_CHAR_DEVICE_NOTIFY_WRITABLE. Do you know what is the status of getting support for this in QEMU? Did the char device changes this feature depends on get committed to master? Christophe pgpnj_W1ZrDAY.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice] Remove guest side video time-stamping
On Mon, Nov 24, 2014 at 4:46 PM, Christophe Fergeau wrote: > This could be renamed to reds_enable_mm_time in a followup patch as > there is no longer a timer It took me some time to understand, all mm_timer could be renamed to mm_time after the patch, I agree. -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk][PATCH] spice-widget: Don't crash when Display is NULL
On Mon, Nov 24, 2014 at 4:28 PM, Christophe Fergeau wrote: > On Mon, Nov 24, 2014 at 01:25:51AM +0100, Fabiano Fidêncio wrote: >> Return early or warn when SpiceDisplay is NULL instead of crash, avoiding >> segfaults when running on windows using GTK3 >> --- >> gtk/spice-widget.c | 16 +--- >> 1 file changed, 13 insertions(+), 3 deletions(-) >> >> diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c >> index ae11073..67429f0 100644 >> --- a/gtk/spice-widget.c >> +++ b/gtk/spice-widget.c >> @@ -701,9 +701,12 @@ SpiceGrabSequence >> *spice_display_get_grab_keys(SpiceDisplay *display) >> static void try_keyboard_grab(SpiceDisplay *display) >> { >> GtkWidget *widget = GTK_WIDGET(display); >> +GdkWindow *window = gtk_widget_get_window(widget); >> SpiceDisplayPrivate *d = display->priv; >> GdkGrabStatus status; >> >> +g_return_if_fail(window != NULL); >> + >> if (g_getenv("SPICE_NOGRAB")) >> return; >> if (d->disable_inputs) >> @@ -731,8 +734,8 @@ static void try_keyboard_grab(SpiceDisplay *display) >> GetModuleHandle(NULL), 0); >> g_warn_if_fail(d->keyboard_hook != NULL); >> #endif >> -status = gdk_keyboard_grab(gtk_widget_get_window(widget), FALSE, >> - GDK_CURRENT_TIME); >> + >> +status = gdk_keyboard_grab(window, FALSE, GDK_CURRENT_TIME); >> if (status != GDK_GRAB_SUCCESS) { >> g_warning("keyboard grab failed %d", status); >> d->keyboard_grab_active = false; >> @@ -1294,7 +1297,14 @@ static gboolean check_for_grab_key(SpiceDisplay >> *display, int type, int keyval) >> static void update_display(SpiceDisplay *display) >> { >> #ifdef G_OS_WIN32 >> -win32_window = display ? >> GDK_WINDOW_HWND(gtk_widget_get_window(GTK_WIDGET(display))) : NULL; >> +GdkWindow *window = NULL; >> + >> +if (display != NULL) { >> +window = gtk_widget_get_window(GTK_WIDGET(display)); >> +g_warn_if_fail (window != NULL); >> +} >> + >> +win32_window = window ? GDK_WINDOW_HWND(window) : NULL; >> #endif > > For what it's worth, this hunk seems to be more about protecting against > a NULL window (which could come from > gtk_widget_get_window(GTK_WIDGET(NULL))) than protecting against a NULL > display. > I see your point. Would be better a different patch for this part. Best Regards, -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice] chardev: remove write polling
In an effort to reduce the wakeups per second, get rid of the "write_to_dev" timer when the implementation supports SPICE_CHAR_DEVICE_NOTIFY_WRITABLE. When this flag is set, the frontend instance is responsible for calling spice_char_device_wakeup() when the device is ready to perform IO. Related to: https://bugzilla.redhat.com/show_bug.cgi?id=912763 --- v3: fix interface version check, spotted by Uri server/char_device.c | 36 +++- server/spice.h | 9 +++-- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/server/char_device.c b/server/char_device.c index 6d2339e..5f1b32d 100644 --- a/server/char_device.c +++ b/server/char_device.c @@ -438,7 +438,10 @@ static int spice_char_device_write_to_device(SpiceCharDeviceState *dev) } spice_char_device_state_ref(dev); -core->timer_cancel(dev->write_to_dev_timer); + +if (dev->write_to_dev_timer) { +core->timer_cancel(dev->write_to_dev_timer); +} sif = SPICE_CONTAINEROF(dev->sin->base.sif, SpiceCharDeviceInterface, base); while (dev->running) { @@ -473,8 +476,10 @@ static int spice_char_device_write_to_device(SpiceCharDeviceState *dev) /* retry writing as long as the write queue is not empty */ if (dev->running) { if (dev->cur_write_buf) { -core->timer_start(dev->write_to_dev_timer, - CHAR_DEVICE_WRITE_TO_TIMEOUT); +if (dev->write_to_dev_timer) { +core->timer_start(dev->write_to_dev_timer, + CHAR_DEVICE_WRITE_TO_TIMEOUT); +} } else { spice_assert(ring_is_empty(&dev->write_queue)); } @@ -488,7 +493,9 @@ static void spice_char_dev_write_retry(void *opaque) { SpiceCharDeviceState *dev = opaque; -core->timer_cancel(dev->write_to_dev_timer); +if (dev->write_to_dev_timer) { +core->timer_cancel(dev->write_to_dev_timer); +} spice_char_device_write_to_device(dev); } @@ -635,6 +642,7 @@ SpiceCharDeviceState *spice_char_device_state_create(SpiceCharDeviceInstance *si void *opaque) { SpiceCharDeviceState *char_dev; +SpiceCharDeviceInterface *sif; spice_assert(sin); spice_assert(cbs->read_one_msg_from_device && cbs->ref_msg_to_client && @@ -652,10 +660,15 @@ SpiceCharDeviceState *spice_char_device_state_create(SpiceCharDeviceInstance *si ring_init(&char_dev->write_bufs_pool); ring_init(&char_dev->clients); -char_dev->write_to_dev_timer = core->timer_add(spice_char_dev_write_retry, char_dev); -if (!char_dev->write_to_dev_timer) { -spice_error("failed creating char dev write timer"); +sif = SPICE_CONTAINEROF(char_dev->sin->base.sif, SpiceCharDeviceInterface, base); +if ((sif->base.minor_version <= 1 && sif->base.minor_version <= 2) || +!(sif->flags & SPICE_CHAR_DEVICE_NOTIFY_WRITABLE)) { +char_dev->write_to_dev_timer = core->timer_add(spice_char_dev_write_retry, char_dev); +if (!char_dev->write_to_dev_timer) { +spice_error("failed creating char dev write timer"); +} } + char_dev->refs = 1; sin->st = char_dev; spice_debug("sin %p dev_state %p", sin, char_dev); @@ -697,7 +710,9 @@ static void spice_char_device_state_unref(SpiceCharDeviceState *char_dev) void spice_char_device_state_destroy(SpiceCharDeviceState *char_dev) { reds_on_char_device_state_destroy(char_dev); -core->timer_remove(char_dev->write_to_dev_timer); +if (char_dev->write_to_dev_timer) { +core->timer_remove(char_dev->write_to_dev_timer); +} write_buffers_queue_free(&char_dev->write_queue); write_buffers_queue_free(&char_dev->write_bufs_pool); if (char_dev->cur_write_buf) { @@ -805,7 +820,9 @@ void spice_char_device_stop(SpiceCharDeviceState *dev) spice_debug("dev_state %p", dev); dev->running = FALSE; dev->active = FALSE; -core->timer_cancel(dev->write_to_dev_timer); +if (dev->write_to_dev_timer) { +core->timer_cancel(dev->write_to_dev_timer); +} } void spice_char_device_reset(SpiceCharDeviceState *dev) @@ -842,6 +859,7 @@ void spice_char_device_reset(SpiceCharDeviceState *dev) void spice_char_device_wakeup(SpiceCharDeviceState *dev) { +spice_char_device_write_to_device(dev); spice_char_device_read_from_device(dev); } diff --git a/server/spice.h b/server/spice.h index 58700d1..bd5bba8 100644 --- a/server/spice.h +++ b/server/spice.h @@ -24,7 +24,7 @@ #include #include -#define SPICE_SERVER_VERSION 0x000c05 /* release 0.12.5 */ +#define SPICE_SERVER_VERSION 0x000c06 /* release 0.12.6 */ #ifdef SPICE_SERVER_INTERNAL #undef SPICE_GNUC_DEPRECATED @@ -402,11 +402,15 @@ void spice_server_set_record_rate(SpiceRecordInstance *sin, uint32_t frequen #define SPICE_INTERFACE_CHAR_DEVICE "char_device" #define SPICE_INTERFACE_CHAR_DEVIC
Re: [Spice-devel] [PATCH spice] Remove guest side video time-stamping
On Fri, Nov 14, 2014 at 01:55:27PM +0100, Marc-André Lureau wrote: > The multimedia time is defined by the server side monotonic time [1], > but the drawing time-stamp is done in guest side, so it requires > synchronization between host and guest. This is expensive, when no audio > is playing, there is a ~30x/sec wakeup to update the qxl device mmtime, > and it requires marking dirty the rom region. > > Instead, the video timestamping can be done more efficiently on server > side, without visible drawbacks. > > [1] a better timestamp could be the audio time, since audio players are > usually sync with audio time) > > Related to: > https://bugzilla.redhat.com/show_bug.cgi?id=912763 > --- > > This is a resend, as a non-RFC version this time. > > server/red_dispatcher.c | 9 - > server/red_worker.c | 1 + > server/reds-private.h | 2 -- > server/reds.c | 13 - > server/snd_worker.c | 1 - > server/spice-qxl.h | 2 +- This file does not exist upstream > 6 files changed, 2 insertions(+), 26 deletions(-) > > diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c > index a6ffe7b..d8389bc 100644 > --- a/server/red_dispatcher.c > +++ b/server/red_dispatcher.c > @@ -751,15 +751,6 @@ static void qxl_worker_loadvm_commands(QXLWorker > *qxl_worker, > red_dispatcher_loadvm_commands((RedDispatcher*)qxl_worker, ext, count); > } > > -void red_dispatcher_set_mm_time(uint32_t mm_time) > -{ > -RedDispatcher *now = dispatchers; > -while (now) { > -now->qxl->st->qif->set_mm_time(now->qxl, mm_time); > -now = now->next; > -} > -} > - > static inline int calc_compression_level(void) > { > spice_assert(streaming_video != STREAM_VIDEO_INVALID); > diff --git a/server/red_worker.c b/server/red_worker.c > index 9f18495..cbb78a2 100644 > --- a/server/red_worker.c > +++ b/server/red_worker.c > @@ -4195,6 +4195,7 @@ static inline void red_process_drawable(RedWorker > *worker, RedDrawable *red_draw > return; > } > > +red_drawable->mm_time = reds_get_mm_time(); > surface_id = drawable->surface_id; > > worker->surfaces[surface_id].refs++; > diff --git a/server/reds-private.h b/server/reds-private.h > index ee09e7c..0bfbf3b 100644 > --- a/server/reds-private.h > +++ b/server/reds-private.h > @@ -6,7 +6,6 @@ > #include > > #define MIGRATE_TIMEOUT (1000 * 10) /* 10sec */ > -#define MM_TIMER_GRANULARITY_MS (1000 / 30) > #define MM_TIME_DELTA 400 /*ms*/ > > typedef struct TicketAuthentication { > @@ -159,7 +158,6 @@ typedef struct RedsState { > int dispatcher_allows_client_mouse; > MonitorMode monitor_mode; > SpiceTimer *mig_timer; > -SpiceTimer *mm_timer; > > int vm_running; > Ring char_devs_states; /* list of SpiceCharDeviceStateItem */ > diff --git a/server/reds.c b/server/reds.c > index 505bacd..c2d71f6 100644 > --- a/server/reds.c > +++ b/server/reds.c > @@ -2799,7 +2799,6 @@ uint32_t reds_get_mm_time(void) > > void reds_enable_mm_timer(void) This could be renamed to reds_enable_mm_time in a followup patch as there is no longer a timer > { > -core->timer_start(reds->mm_timer, MM_TIMER_GRANULARITY_MS); > reds->mm_timer_enabled = TRUE; > reds->mm_time_latency = MM_TIME_DELTA; > reds_send_mm_time(); > @@ -2807,16 +2806,9 @@ void reds_enable_mm_timer(void) > > void reds_disable_mm_timer(void) Same here. ACK otherwise. Christophe pgp10pmHGziMq.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk][PATCH] spice-widget: Don't crash when Display is NULL
On Mon, Nov 24, 2014 at 01:25:51AM +0100, Fabiano Fidêncio wrote: > Return early or warn when SpiceDisplay is NULL instead of crash, avoiding > segfaults when running on windows using GTK3 > --- > gtk/spice-widget.c | 16 +--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c > index ae11073..67429f0 100644 > --- a/gtk/spice-widget.c > +++ b/gtk/spice-widget.c > @@ -701,9 +701,12 @@ SpiceGrabSequence > *spice_display_get_grab_keys(SpiceDisplay *display) > static void try_keyboard_grab(SpiceDisplay *display) > { > GtkWidget *widget = GTK_WIDGET(display); > +GdkWindow *window = gtk_widget_get_window(widget); > SpiceDisplayPrivate *d = display->priv; > GdkGrabStatus status; > > +g_return_if_fail(window != NULL); > + > if (g_getenv("SPICE_NOGRAB")) > return; > if (d->disable_inputs) > @@ -731,8 +734,8 @@ static void try_keyboard_grab(SpiceDisplay *display) > GetModuleHandle(NULL), 0); > g_warn_if_fail(d->keyboard_hook != NULL); > #endif > -status = gdk_keyboard_grab(gtk_widget_get_window(widget), FALSE, > - GDK_CURRENT_TIME); > + > +status = gdk_keyboard_grab(window, FALSE, GDK_CURRENT_TIME); > if (status != GDK_GRAB_SUCCESS) { > g_warning("keyboard grab failed %d", status); > d->keyboard_grab_active = false; > @@ -1294,7 +1297,14 @@ static gboolean check_for_grab_key(SpiceDisplay > *display, int type, int keyval) > static void update_display(SpiceDisplay *display) > { > #ifdef G_OS_WIN32 > -win32_window = display ? > GDK_WINDOW_HWND(gtk_widget_get_window(GTK_WIDGET(display))) : NULL; > +GdkWindow *window = NULL; > + > +if (display != NULL) { > +window = gtk_widget_get_window(GTK_WIDGET(display)); > +g_warn_if_fail (window != NULL); > +} > + > +win32_window = window ? GDK_WINDOW_HWND(window) : NULL; > #endif For what it's worth, this hunk seems to be more about protecting against a NULL window (which could come from gtk_widget_get_window(GTK_WIDGET(NULL))) than protecting against a NULL display. Christophe pgpI1jqQcLdaW.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk][PATCH] spice-widget: Don't crash when Display is NULL
On Mon, Nov 24, 2014 at 3:35 PM, Marc-André Lureau wrote: > On Mon, Nov 24, 2014 at 3:32 PM, Fabiano Fidêncio > wrote: >> When opening the very first window and when a any other display was being >> added. >> I know this is probably a virt-viewer fault that we have to fix there, >> I just think that if we can avoid the crash here and warn the user is >> better than just crash. > > > I mean, could you provide a backtrace where the crash happens? Not so easily (thanks to the windows builds). I'll generate the packages in a way I can have the debug info on not stripped out. I will do it soon, but probably not today :-( Best Regards, -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk][PATCH] spice-widget: Don't crash when Display is NULL
On Mon, Nov 24, 2014 at 3:32 PM, Fabiano Fidêncio wrote: > When opening the very first window and when a any other display was being > added. > I know this is probably a virt-viewer fault that we have to fix there, > I just think that if we can avoid the crash here and warn the user is > better than just crash. I mean, could you provide a backtrace where the crash happens? -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk][PATCH] spice-widget: Don't crash when Display is NULL
On Mon, Nov 24, 2014 at 3:27 PM, Marc-André Lureau wrote: > On Mon, Nov 24, 2014 at 1:25 AM, Fabiano Fidêncio wrote: >> Return early or warn when SpiceDisplay is NULL instead of crash, avoiding >> segfaults when running on windows using GTK3 > > > Where did the crash happen? When opening the very first window and when a any other display was being added. I know this is probably a virt-viewer fault that we have to fix there, I just think that if we can avoid the crash here and warn the user is better than just crash. Best Regards, -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk][PATCH] spice-widget: Don't crash when Display is NULL
On Mon, Nov 24, 2014 at 1:25 AM, Fabiano Fidêncio wrote: > Return early or warn when SpiceDisplay is NULL instead of crash, avoiding > segfaults when running on windows using GTK3 Where did the crash happen? -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk v4] Release keyboard grab using keyboard shortcut
It works now, ack On Mon, Nov 24, 2014 at 8:24 AM, Pavel Grunt wrote: > This commit adds the ability to release the keyboard grab when > the release keys (ctrl+alt) are pressed and released. It allows > to use keyboard shortcuts (eg alt+tab, alt+f4) on the client. > > The keyboard is grabbed again when the release keys are pressed > and released or when the mouse moves. > > https://bugs.freedesktop.org/show_bug.cgi?id=85331 > --- > v4: > - when 'focus in' clear activeseq, because it can wrongly contain keys that > were pressed >but missed the event they were released > v3: > - don't grab when the keyboard doesn't have the focus > v2: > - add missing 'release_keys(display)' calls > --- > gtk/spice-widget-priv.h | 2 ++ > gtk/spice-widget.c | 52 > ++--- > 2 files changed, 47 insertions(+), 7 deletions(-) > > diff --git a/gtk/spice-widget-priv.h b/gtk/spice-widget-priv.h > index 9c38e2e..0e1f661 100644 > --- a/gtk/spice-widget-priv.h > +++ b/gtk/spice-widget-priv.h > @@ -109,6 +109,8 @@ struct _SpiceDisplayPrivate { > guint key_delayed_id; > SpiceGrabSequence *grabseq; /* the configured key sequence */ > gboolean*activeseq; /* the currently pressed keys */ > +gbooleanseq_pressed; > +gbooleankeyboard_grab_released; > gintmark; > #ifdef WIN32 > HHOOK keyboard_hook; > diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c > index ae11073..87d2bb0 100644 > --- a/gtk/spice-widget.c > +++ b/gtk/spice-widget.c > @@ -719,6 +719,8 @@ static void try_keyboard_grab(SpiceDisplay *display) > return; > if (!d->mouse_have_pointer) > return; > +if (d->keyboard_grab_released) > +return; > > g_return_if_fail(gtk_widget_is_focus(widget)); > > @@ -1198,6 +1200,9 @@ static void send_key(SpiceDisplay *display, int > scancode, SendKeyType type, gboo > if (d->disable_inputs) > return; > > +if (d->keyboard_grab_released) > +return; > + > i = scancode / 32; > b = scancode % 32; > m = (1 << b); > @@ -1259,7 +1264,8 @@ static void release_keys(SpiceDisplay *display) > } > } > > -static gboolean check_for_grab_key(SpiceDisplay *display, int type, int > keyval) > +static gboolean check_for_grab_key(SpiceDisplay *display, int type, int > keyval, > + int check_type, int reset_type) > { > SpiceDisplayPrivate *d = display->priv; > int i; > @@ -1267,13 +1273,13 @@ static gboolean check_for_grab_key(SpiceDisplay > *display, int type, int keyval) > if (!d->grabseq->nkeysyms) > return FALSE; > > -if (type == GDK_KEY_PRESS) { > -/* Record the new key press */ > +if (type == check_type) { > +/* Record the new key */ > for (i = 0 ; i < d->grabseq->nkeysyms ; i++) > if (d->grabseq->keysyms[i] == keyval) > d->activeseq[i] = TRUE; > > -/* Return if any key is not pressed */ > +/* Return if any key is missing */ > for (i = 0 ; i < d->grabseq->nkeysyms ; i++) > if (d->activeseq[i] == FALSE) > return FALSE; > @@ -1281,9 +1287,10 @@ static gboolean check_for_grab_key(SpiceDisplay > *display, int type, int keyval) > /* resets the whole grab sequence on success */ > memset(d->activeseq, 0, sizeof(gboolean) * d->grabseq->nkeysyms); > return TRUE; > -} else if (type == GDK_KEY_RELEASE) { > -/* Any key release resets the whole grab sequence */ > +} else if (type == reset_type) { > +/* reset key event type resets the whole grab sequence */ > memset(d->activeseq, 0, sizeof(gboolean) * d->grabseq->nkeysyms); > +d->seq_pressed = FALSE; > return FALSE; > } else > g_warn_if_reached(); > @@ -1291,6 +1298,16 @@ static gboolean check_for_grab_key(SpiceDisplay > *display, int type, int keyval) > return FALSE; > } > > +static gboolean check_for_grab_key_pressed(SpiceDisplay *display, int type, > int keyval) > +{ > +return check_for_grab_key(display, type, keyval, GDK_KEY_PRESS, > GDK_KEY_RELEASE); > +} > + > +static gboolean check_for_grab_key_released(SpiceDisplay *display, int type, > int keyval) > +{ > +return check_for_grab_key(display, type, keyval, GDK_KEY_RELEASE, > GDK_KEY_PRESS); > +} > + > static void update_display(SpiceDisplay *display) > { > #ifdef G_OS_WIN32 > @@ -1321,7 +1338,7 @@ static gboolean key_event(GtkWidget *widget, > GdkEventKey *key) > __FUNCTION__, key->type == GDK_KEY_PRESS ? "press" : "release", > key->hardware_keycode, key->state, key->group, key->is_modifier); > > -if (check_for_grab_key(display, key->type, key->keyval)) { > +if (!d->seq_pressed && check_for_grab_key_pressed(display, key->type, > key->keyval)) { > g_sig
Re: [Spice-devel] [spice-gtk] Remove channels from SpiceSession::channels on session disconnect
On Fri, Nov 21, 2014 at 02:13:20PM +0100, Marc-André Lureau wrote: > Fundamentally, it doesn't change much from my patch, I guess removing > g_clear_object(channel->priv->session) and ignoring the case where the > channel isn't in the session (like you do here) would have been the > same. Yup, that's very likely, it was easier for me to restart from scratch (after learning from your patches) than to try to modify your patches, but they probably don't need to be much different indeed. > > I still think it's a better idea to clear the channel->session if the > channel has been disconnect from the session, although I agree that it > requires a bit more testing and fixing for some warnings/criticals. Well, clearing the session would indeed be cleaner, but it has its share of issues with existing code. Ideally it would be possible not to set the session to NULL for now, and warn when something tries to use the session after it has been disconnected. This would allow to fix the various issues that show up without risking a crash on a NULL session, but I don't really see how to achieve that except by adding some test+g_warning to all function acting on a session. Christophe pgpr22bl8yUIR.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel