Re: [Spice-devel] [PATCH spice-gtk] meson: Pass --msgid-bugs-address to PO generation
Hi On Tue, Feb 19, 2019 at 6:11 PM Frediano Ziglio wrote: > > As autoconf provide e-mail address in the .po files. > The change in main meson.build is just a move to allow the > other meson.buuild to see the setting. > > Signed-off-by: Frediano Ziglio ack (pretty useless, even glib/gtk doesn't need it...) > --- > meson.build| 38 +++--- > po/meson.build | 1 + > 2 files changed, 20 insertions(+), 19 deletions(-) > > diff --git a/meson.build b/meson.build > index c015de7f..1863c52a 100644 > --- a/meson.build > +++ b/meson.build > @@ -339,6 +339,25 @@ endif > > add_project_arguments(compiler.get_supported_arguments(spice_gtk_global_cflags), >language : 'c') > > +# > +# write config.h > +# > +proj_version = meson.project_version() > +proj_name = meson.project_name() > +config_data = {'VERSION' : proj_version, > + 'PACKAGE_VERSION' : proj_version, > + 'GETTEXT_PACKAGE' : proj_name, > + 'LOCALE_DIR' : spice_gtk_localedir, > + 'PACKAGE_STRING' : '@0@ @1@'.format(proj_name, proj_version), > + 'PACKAGE_BUGREPORT' : 'spice-devel@lists.freedesktop.org'} > +foreach key, value : config_data > + spice_gtk_config_data.set_quoted(key, value) > +endforeach > + > +configure_file(output : 'config.h', > + install : false, > + configuration : spice_gtk_config_data) > + > # > # Subdirectories > # > @@ -359,25 +378,6 @@ subdir('man') > subdir('po') > subdir('vapi') > > -# > -# write config.h > -# > -proj_version = meson.project_version() > -proj_name = meson.project_name() > -config_data = {'VERSION' : proj_version, > - 'PACKAGE_VERSION' : proj_version, > - 'GETTEXT_PACKAGE' : proj_name, > - 'LOCALE_DIR' : spice_gtk_localedir, > - 'PACKAGE_STRING' : '@0@ @1@'.format(proj_name, proj_version), > - 'PACKAGE_BUGREPORT' : 'spice-devel@lists.freedesktop.org'} > -foreach key, value : config_data > - spice_gtk_config_data.set_quoted(key, value) > -endforeach > - > -configure_file(output : 'config.h', > - install : false, > - configuration : spice_gtk_config_data) > - > # > # write spice-client-glib.pc > # > diff --git a/po/meson.build b/po/meson.build > index fb3c3957..da459f2f 100644 > --- a/po/meson.build > +++ b/po/meson.build > @@ -1,3 +1,4 @@ > i18n = import('i18n') > i18n.gettext(meson.project_name(), > + args : '--msgid-bugs-address=' + > config_data['PACKAGE_BUGREPORT'], > preset : 'glib') > -- > 2.20.1 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-server] Move channel registration to constructed vfunc
For the Display Channel and the Cursor channel, move the call to reds_register_channel() to the _constructed() vfunc rather than calling it explicitly from RedWorker. This matches what other channels do. --- server/cursor-channel.c | 12 server/display-channel.c | 2 ++ server/red-worker.c | 2 -- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/server/cursor-channel.c b/server/cursor-channel.c index 9dd69fa25..4220084f5 100644 --- a/server/cursor-channel.c +++ b/server/cursor-channel.c @@ -366,12 +366,24 @@ cursor_channel_finalize(GObject *object) G_OBJECT_CLASS(cursor_channel_parent_class)->finalize(object); } +static void +cursor_channel_constructed(GObject *object) +{ +RedChannel *red_channel = RED_CHANNEL(object); +RedsState *reds = red_channel_get_server(red_channel); + +G_OBJECT_CLASS(cursor_channel_parent_class)->constructed(object); + +reds_register_channel(reds, red_channel); +} + static void cursor_channel_class_init(CursorChannelClass *klass) { GObjectClass *object_class = G_OBJECT_CLASS(klass); RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass); +object_class->constructed = cursor_channel_constructed; object_class->finalize = cursor_channel_finalize; channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_CURSOR, NULL); diff --git a/server/display-channel.c b/server/display-channel.c index e68ed10f8..6684135eb 100644 --- a/server/display-channel.c +++ b/server/display-channel.c @@ -2304,6 +2304,8 @@ display_channel_constructed(GObject *object) red_channel_set_cap(channel, SPICE_DISPLAY_CAP_PREF_COMPRESSION); red_channel_set_cap(channel, SPICE_DISPLAY_CAP_PREF_VIDEO_CODEC_TYPE); red_channel_set_cap(channel, SPICE_DISPLAY_CAP_STREAM_REPORT); + +reds_register_channel(reds, channel); } void display_channel_process_surface_cmd(DisplayChannel *display, diff --git a/server/red-worker.c b/server/red-worker.c index c74ae8886..3cb12b9c2 100644 --- a/server/red-worker.c +++ b/server/red-worker.c @@ -1322,7 +1322,6 @@ RedWorker* red_worker_new(QXLInstance *qxl, red_channel_init_stat_node(channel, >stat, "cursor_channel"); red_channel_register_client_cbs(channel, client_cursor_cbs); g_object_set_data(G_OBJECT(channel), "dispatcher", dispatcher); -reds_register_channel(reds, channel); // TODO: handle seamless migration. Temp, setting migrate to FALSE worker->display_channel = display_channel_new(reds, qxl, >core, FALSE, @@ -1333,7 +1332,6 @@ RedWorker* red_worker_new(QXLInstance *qxl, red_channel_init_stat_node(channel, >stat, "display_channel"); red_channel_register_client_cbs(channel, client_display_cbs); g_object_set_data(G_OBJECT(channel), "dispatcher", dispatcher); -reds_register_channel(reds, channel); return worker; } -- 2.17.2 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [vdagent-linux PATCH v1] Makefile.am: define check_PROGRAMS once
On 2/19/19 7:39 PM, Frediano Ziglio wrote: Also define TESTS once autoreconf complains when there are multiple definitions of a variable. In this case (partial error message follows): warning: check_PROGRAMS multiply defined Signed-off-by: Uri Lublin --- Makefile.am | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Makefile.am b/Makefile.am index c2e9668..d1afae2 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3,7 +3,12 @@ NULL = bin_PROGRAMS = src/spice-vdagent sbin_PROGRAMS = src/spice-vdagentd -check_PROGRAMS = tests/test-file-xfers + +check_PROGRAMS = \ + tests/test-file-xfers \ + tests/test-device-info \ + $(NULL) + Why instead using += on the others so to keep the future changes together? See below TESTS = $(check_PROGRAMS) common_sources =\ @@ -167,9 +172,3 @@ DISTCHECK_CONFIGURE_FLAGS = \ tests_test_device_info_LDADD = $(src_spice_vdagent_LDADD) tests_test_device_info_CFLAGS = $(src_spice_vdagent_CFLAGS) -check_PROGRAMS = \ - tests/test-device-info \ - $(NULL) Here it would be just check_PROGRAMS += tests/test-device-info Of course that's also possible. We can do it that way. - -TESTS = $(check_PROGRAMS) Surely this must go, no reason to define it again This has the advantage that to add a test you just need to add some lines like: tests_test_xxx_LDADD = $(src_spice_vdagent_LDADD) tests_test_xxx_CFLAGS = $(src_spice_vdagent_CFLAGS) check_PROGRAMS += tests/test-xxx The difference is that this change is contained in one diff hunk while having to add the test on the top check_PROGRAMS macro would need 2 hunks OK, I'll send a V2. Maybe I'll keep the TESTS at the bottom, since check_PROGRAMS keeps changing (not sure it makes a difference - I can check but for sure keeping it at the bottom is "safe"). Thanks, Uri. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [vdagent-linux PATCH v1] Makefile.am: define check_PROGRAMS once
> > Also define TESTS once > > autoreconf complains when there are multiple definitions of > a variable. In this case (partial error message follows): > warning: check_PROGRAMS multiply defined > > Signed-off-by: Uri Lublin > --- > Makefile.am | 13 ++--- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/Makefile.am b/Makefile.am > index c2e9668..d1afae2 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -3,7 +3,12 @@ NULL = > > bin_PROGRAMS = src/spice-vdagent > sbin_PROGRAMS = src/spice-vdagentd > -check_PROGRAMS = tests/test-file-xfers > + > +check_PROGRAMS = \ > + tests/test-file-xfers \ > + tests/test-device-info \ > + $(NULL) > + Why instead using += on the others so to keep the future changes together? See below > TESTS = $(check_PROGRAMS) > > common_sources = \ > @@ -167,9 +172,3 @@ DISTCHECK_CONFIGURE_FLAGS = \ > tests_test_device_info_LDADD = $(src_spice_vdagent_LDADD) > tests_test_device_info_CFLAGS = $(src_spice_vdagent_CFLAGS) > > -check_PROGRAMS = \ > - tests/test-device-info \ > - $(NULL) Here it would be just check_PROGRAMS += tests/test-device-info > - > -TESTS = $(check_PROGRAMS) Surely this must go, no reason to define it again This has the advantage that to add a test you just need to add some lines like: tests_test_xxx_LDADD = $(src_spice_vdagent_LDADD) tests_test_xxx_CFLAGS = $(src_spice_vdagent_CFLAGS) check_PROGRAMS += tests/test-xxx The difference is that this change is contained in one diff hunk while having to add the test on the top check_PROGRAMS macro would need 2 hunks Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] spice-vdagent and FreeBSD
Hi together, has anyone compiled / used spice-vdagent under FreeBSD? Kind regards Margit -- *ingenieur wissenschaften* *htw saar* *Hochschule für Technik und Wirtschaft des Saarlandes* University of Applied Sciences *Fakultät für Ingenieurwissenschaften* School of Engineering Dipl. Inf. Margit Meyer Informatik/Systemtechniklabor (STL) Campus Alt-Saarbrücken Goebenstraße 40 66117 Saarbrücken +49 (0) 681 58 67 - 145 margit.me...@htwsaar.de stl.htwsaar.de www.htwsaar.de/stl smime.p7s Description: S/MIME Cryptographic Signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [vdagent-linux PATCH v1] Makefile.am: define check_PROGRAMS once
Also define TESTS once autoreconf complains when there are multiple definitions of a variable. In this case (partial error message follows): warning: check_PROGRAMS multiply defined Signed-off-by: Uri Lublin --- Makefile.am | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Makefile.am b/Makefile.am index c2e9668..d1afae2 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3,7 +3,12 @@ NULL = bin_PROGRAMS = src/spice-vdagent sbin_PROGRAMS = src/spice-vdagentd -check_PROGRAMS = tests/test-file-xfers + +check_PROGRAMS = \ + tests/test-file-xfers \ + tests/test-device-info \ + $(NULL) + TESTS = $(check_PROGRAMS) common_sources = \ @@ -167,9 +172,3 @@ DISTCHECK_CONFIGURE_FLAGS = \ tests_test_device_info_LDADD = $(src_spice_vdagent_LDADD) tests_test_device_info_CFLAGS = $(src_spice_vdagent_CFLAGS) -check_PROGRAMS = \ - tests/test-device-info \ - $(NULL) - -TESTS = $(check_PROGRAMS) - -- 2.20.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk v2] channel-display-gst: Use recorder for frame statistics
Hi On Tue, Feb 19, 2019 at 6:02 PM Victor Toso wrote: > > Hi, > > On Tue, Feb 19, 2019 at 05:37:38PM +0100, Marc-André Lureau wrote: > > > > > -SPICE_DEBUG("frame mm_time %u size %u creation time %" > > > > > G_GINT64_FORMAT > > > > > -" decoded time %" G_GINT64_FORMAT " queue %u", > > > > > -frame->mm_time, frame->size, > > > > > frame->creation_time, > > > > > -duration, decoder->decoding_queue->length); > > > > > +record(frames_stats, > > > > > + "frame mm_time %u size %u creation time %" PRId64 > > > > > + " decoded time %" PRId64 " queue %u", > > > > > + frame->mm_time, frame->size, frame->creation_time, > > > > > + duration, decoder->decoding_queue->length); > > > > > > > > Why SPICE_DEBUG log is removed? > > > > > > > > Why is the "recorder" stuff necessary here? > > > > > > > > > > Measurement instrumentation > > > > Ok, but can we get have the regular log as well? > > This really is measurement type of logs. We don't have structured And "trace"(?) are better than "log" for "measurements"? > logging so I do prefer this kind of logging being enabled by some > dynamic tweaking instead of seeing it all the time. You can filter it out with grep, fairly easily. I proposed a series using structured logging and categories for SPICE_DEBUG= in the past iirc. I guess I should revisit it. -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-gtk] meson: Pass --msgid-bugs-address to PO generation
As autoconf provide e-mail address in the .po files. The change in main meson.build is just a move to allow the other meson.buuild to see the setting. Signed-off-by: Frediano Ziglio --- meson.build| 38 +++--- po/meson.build | 1 + 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/meson.build b/meson.build index c015de7f..1863c52a 100644 --- a/meson.build +++ b/meson.build @@ -339,6 +339,25 @@ endif add_project_arguments(compiler.get_supported_arguments(spice_gtk_global_cflags), language : 'c') +# +# write config.h +# +proj_version = meson.project_version() +proj_name = meson.project_name() +config_data = {'VERSION' : proj_version, + 'PACKAGE_VERSION' : proj_version, + 'GETTEXT_PACKAGE' : proj_name, + 'LOCALE_DIR' : spice_gtk_localedir, + 'PACKAGE_STRING' : '@0@ @1@'.format(proj_name, proj_version), + 'PACKAGE_BUGREPORT' : 'spice-devel@lists.freedesktop.org'} +foreach key, value : config_data + spice_gtk_config_data.set_quoted(key, value) +endforeach + +configure_file(output : 'config.h', + install : false, + configuration : spice_gtk_config_data) + # # Subdirectories # @@ -359,25 +378,6 @@ subdir('man') subdir('po') subdir('vapi') -# -# write config.h -# -proj_version = meson.project_version() -proj_name = meson.project_name() -config_data = {'VERSION' : proj_version, - 'PACKAGE_VERSION' : proj_version, - 'GETTEXT_PACKAGE' : proj_name, - 'LOCALE_DIR' : spice_gtk_localedir, - 'PACKAGE_STRING' : '@0@ @1@'.format(proj_name, proj_version), - 'PACKAGE_BUGREPORT' : 'spice-devel@lists.freedesktop.org'} -foreach key, value : config_data - spice_gtk_config_data.set_quoted(key, value) -endforeach - -configure_file(output : 'config.h', - install : false, - configuration : spice_gtk_config_data) - # # write spice-client-glib.pc # diff --git a/po/meson.build b/po/meson.build index fb3c3957..da459f2f 100644 --- a/po/meson.build +++ b/po/meson.build @@ -1,3 +1,4 @@ i18n = import('i18n') i18n.gettext(meson.project_name(), + args : '--msgid-bugs-address=' + config_data['PACKAGE_BUGREPORT'], preset : 'glib') -- 2.20.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk v2] channel-display-gst: Use recorder for frame statistics
Hi, On Tue, Feb 19, 2019 at 05:37:38PM +0100, Marc-André Lureau wrote: > > > > -SPICE_DEBUG("frame mm_time %u size %u creation time %" > > > > G_GINT64_FORMAT > > > > -" decoded time %" G_GINT64_FORMAT " queue %u", > > > > -frame->mm_time, frame->size, frame->creation_time, > > > > -duration, decoder->decoding_queue->length); > > > > +record(frames_stats, > > > > + "frame mm_time %u size %u creation time %" PRId64 > > > > + " decoded time %" PRId64 " queue %u", > > > > + frame->mm_time, frame->size, frame->creation_time, > > > > + duration, decoder->decoding_queue->length); > > > > > > Why SPICE_DEBUG log is removed? > > > > > > Why is the "recorder" stuff necessary here? > > > > > > > Measurement instrumentation > > Ok, but can we get have the regular log as well? This really is measurement type of logs. We don't have structured logging so I do prefer this kind of logging being enabled by some dynamic tweaking instead of seeing it all the time. signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk v2] channel-display-gst: Use recorder for frame statistics
Hi On Tue, Feb 19, 2019 at 5:19 PM Frediano Ziglio wrote: > > > > > Hi > > > > On Wed, Jan 30, 2019 at 4:00 PM Frediano Ziglio wrote: > > > > > > Allows to handle these statistics in a more flexible way. > > > > > > Signed-off-by: Frediano Ziglio > > > --- > > > src/channel-display-gst.c | 12 > > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > > > Changes since v1: > > > - change formatting constants to system one instead of GLib > > > to fix build on Windows. > > > > > > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c > > > index 4272ade8..5bb53b6f 100644 > > > --- a/src/channel-display-gst.c > > > +++ b/src/channel-display-gst.c > > > @@ -20,6 +20,7 @@ > > > #include "spice-client.h" > > > #include "spice-common.h" > > > #include "spice-channel-priv.h" > > > +#include "common/recorder.h" > > > > > > #include "channel-display-priv.h" > > > > > > @@ -109,6 +110,8 @@ static void schedule_frame(SpiceGstDecoder *decoder); > > > static void fetch_pending_sample(SpiceGstDecoder *decoder); > > > static SpiceGstFrame *get_decoded_frame(SpiceGstDecoder *decoder, > > > GstBuffer *buffer); > > > > > > +RECORDER(frames_stats, 64, "Frames statistics"); > > > + > > > > What is this 64 value for? > > > > It's the ring size. Ok, but why 64? > > > > static int spice_gst_buffer_get_stride(GstBuffer *buffer) > > > { > > > GstVideoMeta *video = gst_buffer_get_video_meta(buffer); > > > @@ -248,10 +251,11 @@ static SpiceGstFrame > > > *get_decoded_frame(SpiceGstDecoder *decoder, GstBuffer *buf > > > > > > const SpiceFrame *frame = gstframe->encoded_frame; > > > int64_t duration = g_get_monotonic_time() - frame->creation_time; > > > -SPICE_DEBUG("frame mm_time %u size %u creation time %" > > > G_GINT64_FORMAT > > > -" decoded time %" G_GINT64_FORMAT " queue %u", > > > -frame->mm_time, frame->size, frame->creation_time, > > > -duration, decoder->decoding_queue->length); > > > +record(frames_stats, > > > + "frame mm_time %u size %u creation time %" PRId64 > > > + " decoded time %" PRId64 " queue %u", > > > + frame->mm_time, frame->size, frame->creation_time, > > > + duration, decoder->decoding_queue->length); > > > > Why SPICE_DEBUG log is removed? > > > > Why is the "recorder" stuff necessary here? > > > > Measurement instrumentation Ok, but can we get have the regular log as well? > > > > } > > > return gstframe; > > > } > > Frediano -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-common] ci: Remove dependencies from copr build
Signed-off-by: Frediano Ziglio --- .gitlab-ci.yml | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) This patch also fixes the CI which currently is failing. https://gitlab.freedesktop.org/fziglio/spice-common/pipelines/19951 diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 92df9b5..43f09cb 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -2,12 +2,15 @@ image: fedora:latest before_script: - > -dnf install 'dnf-command(copr)' git libtool make libasan +dnf install git libtool make libasan python3 python3-six python3-pyparsing glib-networking meson ninja-build gdk-pixbuf2-devel +glib2-devel celt051-devel pixman-devel openssl-devel libjpeg-devel +libcacard-devel cyrus-sasl-devel lz4-devel opus-devel +gstreamer1-devel gstreamer1-plugins-base-devel -y - - dnf copr enable @spice/nightly -y - - dnf builddep spice -y + - git clone ${CI_REPOSITORY_URL/spice-common.git/spice-protocol.git} + - (cd spice-protocol && ./autogen.sh --prefix=/usr && make install) makecheck: script: -- 2.20.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk v2] channel-display-gst: Use recorder for frame statistics
> > Hi > > On Wed, Jan 30, 2019 at 4:00 PM Frediano Ziglio wrote: > > > > Allows to handle these statistics in a more flexible way. > > > > Signed-off-by: Frediano Ziglio > > --- > > src/channel-display-gst.c | 12 > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > Changes since v1: > > - change formatting constants to system one instead of GLib > > to fix build on Windows. > > > > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c > > index 4272ade8..5bb53b6f 100644 > > --- a/src/channel-display-gst.c > > +++ b/src/channel-display-gst.c > > @@ -20,6 +20,7 @@ > > #include "spice-client.h" > > #include "spice-common.h" > > #include "spice-channel-priv.h" > > +#include "common/recorder.h" > > > > #include "channel-display-priv.h" > > > > @@ -109,6 +110,8 @@ static void schedule_frame(SpiceGstDecoder *decoder); > > static void fetch_pending_sample(SpiceGstDecoder *decoder); > > static SpiceGstFrame *get_decoded_frame(SpiceGstDecoder *decoder, > > GstBuffer *buffer); > > > > +RECORDER(frames_stats, 64, "Frames statistics"); > > + > > What is this 64 value for? > It's the ring size. > > static int spice_gst_buffer_get_stride(GstBuffer *buffer) > > { > > GstVideoMeta *video = gst_buffer_get_video_meta(buffer); > > @@ -248,10 +251,11 @@ static SpiceGstFrame > > *get_decoded_frame(SpiceGstDecoder *decoder, GstBuffer *buf > > > > const SpiceFrame *frame = gstframe->encoded_frame; > > int64_t duration = g_get_monotonic_time() - frame->creation_time; > > -SPICE_DEBUG("frame mm_time %u size %u creation time %" > > G_GINT64_FORMAT > > -" decoded time %" G_GINT64_FORMAT " queue %u", > > -frame->mm_time, frame->size, frame->creation_time, > > -duration, decoder->decoding_queue->length); > > +record(frames_stats, > > + "frame mm_time %u size %u creation time %" PRId64 > > + " decoded time %" PRId64 " queue %u", > > + frame->mm_time, frame->size, frame->creation_time, > > + duration, decoder->decoding_queue->length); > > Why SPICE_DEBUG log is removed? > > Why is the "recorder" stuff necessary here? > Measurement instrumentation > > } > > return gstframe; > > } Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk v2] channel-display-gst: Use recorder for frame statistics
Hi On Wed, Jan 30, 2019 at 4:00 PM Frediano Ziglio wrote: > > Allows to handle these statistics in a more flexible way. > > Signed-off-by: Frediano Ziglio > --- > src/channel-display-gst.c | 12 > 1 file changed, 8 insertions(+), 4 deletions(-) > > Changes since v1: > - change formatting constants to system one instead of GLib > to fix build on Windows. > > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c > index 4272ade8..5bb53b6f 100644 > --- a/src/channel-display-gst.c > +++ b/src/channel-display-gst.c > @@ -20,6 +20,7 @@ > #include "spice-client.h" > #include "spice-common.h" > #include "spice-channel-priv.h" > +#include "common/recorder.h" > > #include "channel-display-priv.h" > > @@ -109,6 +110,8 @@ static void schedule_frame(SpiceGstDecoder *decoder); > static void fetch_pending_sample(SpiceGstDecoder *decoder); > static SpiceGstFrame *get_decoded_frame(SpiceGstDecoder *decoder, GstBuffer > *buffer); > > +RECORDER(frames_stats, 64, "Frames statistics"); > + What is this 64 value for? > static int spice_gst_buffer_get_stride(GstBuffer *buffer) > { > GstVideoMeta *video = gst_buffer_get_video_meta(buffer); > @@ -248,10 +251,11 @@ static SpiceGstFrame *get_decoded_frame(SpiceGstDecoder > *decoder, GstBuffer *buf > > const SpiceFrame *frame = gstframe->encoded_frame; > int64_t duration = g_get_monotonic_time() - frame->creation_time; > -SPICE_DEBUG("frame mm_time %u size %u creation time %" > G_GINT64_FORMAT > -" decoded time %" G_GINT64_FORMAT " queue %u", > -frame->mm_time, frame->size, frame->creation_time, > -duration, decoder->decoding_queue->length); > +record(frames_stats, > + "frame mm_time %u size %u creation time %" PRId64 > + " decoded time %" PRId64 " queue %u", > + frame->mm_time, frame->size, frame->creation_time, > + duration, decoder->decoding_queue->length); Why SPICE_DEBUG log is removed? Why is the "recorder" stuff necessary here? > } > return gstframe; > } > -- > 2.20.1 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH vdagent-linux] clipboard: cancel request for targets on grab from client
Hi, On Sun, Feb 17, 2019 at 09:21:11PM +0100, Jakub Janků wrote: > If gtk_clipboard_set_with_data() is invoked between > gtk_clipboard_request_targets() and the > GtkClipboardTargetsReceivedFunc callback, > the targets we eventually receive are no longer valid. > > To solve this, cancel the request in vdagent_clipboard_grab(). > > Otherwise we end up in a situation when vdagent holds > clipboard grab in the guest but cannot provide data to the > apps that request it - this can be observed in the log: > > CRITICAL **: 20:48:55.782: clipboard_get_cb: assertion > 'c->selections[sel_id].owner == OWNER_CLIENT' failed > > Signed-off-by: Jakub Janků > --- > > The same bug is present in spice-gtk as well as > in the X11 implementation of clipboard in vdagent. We will need the fix there as well, considering that we have some systems that will not get the gtk backend as default. > I'm planing on sending a fix for spice-gtk later > (possibly with other patches). jjanku++ > I decided not to fix the X11 clipboard in vdagent as I'm hoping > it could be removed with the next release. If you think it > should get fixed too, let me know. Please do fix X11 too, if I'm not asking too much. Preferably in a different patch... Acked-by: Victor Toso > --- > src/vdagent/clipboard.c | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/src/vdagent/clipboard.c b/src/vdagent/clipboard.c > index 6b01a7b..a8d2e91 100644 > --- a/src/vdagent/clipboard.c > +++ b/src/vdagent/clipboard.c > @@ -304,6 +304,7 @@ void vdagent_clipboard_grab(VDAgentClipboards *c, guint > sel_id, > vdagent_x11_clipboard_grab(c->x11, sel_id, types, n_types); > #else > GtkTargetEntry targets[G_N_ELEMENTS(atom2agent)]; > +Selection *sel; > guint n_targets, i, t; > > g_return_if_fail(sel_id < SELECTION_COUNT); > @@ -322,7 +323,13 @@ void vdagent_clipboard_grab(VDAgentClipboards *c, guint > sel_id, > return; > } > > -if (gtk_clipboard_set_with_data(c->selections[sel_id].clipboard, > +sel = >selections[sel_id]; > + > +if (sel->last_targets_req) { > +g_clear_pointer(>last_targets_req, request_ref_cancel); > +} > + > +if (gtk_clipboard_set_with_data(sel->clipboard, > targets, n_targets, > clipboard_get_cb, clipboard_clear_cb, c)) > clipboard_new_owner(c, sel_id, OWNER_CLIENT); > -- > 2.20.1 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk 1/2] Allows to enable recorder integration
Hi, On Tue, Feb 19, 2019 at 10:36:38AM -0500, Frediano Ziglio wrote: > ping the series In my mind, I've acked this spice-gtk series too, pretty sure I tested it before as well. Acked-by: Victor Toso > > > > recorder library will be used to collect some statistics during > > development. > > > > Signed-off-by: Frediano Ziglio > > --- > > configure.ac | 1 + > > meson_options.txt| 5 + > > subprojects/spice-common | 2 +- > > 3 files changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/configure.ac b/configure.ac > > index 7dab0be0..2e6f7e78 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -120,6 +120,7 @@ PKG_CHECK_EXISTS(openssl, > >[SPICE_GLIB_REQUIRES="${SPICE_GLIB_REQUIRES} openssl"], > >[SPICE_GLIB_LIBS="${SPICE_GLIB_LIBS} ${SSL_LIBS}"]) > > > > +SPICE_CHECK_RECORDER > > SPICE_CHECK_SASL > > > > AC_MSG_CHECKING([which gtk+ version to compile against]) > > diff --git a/meson_options.txt b/meson_options.txt > > index 2e605b77..98042171 100644 > > --- a/meson_options.txt > > +++ b/meson_options.txt > > @@ -80,3 +80,8 @@ option('smartcard', > > option('gtk_doc', > > type : 'feature', > > description : 'Generate API documentation with gtk-doc') > > + > > +option('recorder', > > +type : 'boolean', > > +value : false, > > +description: 'Enable recorder instrumentation') > > diff --git a/subprojects/spice-common b/subprojects/spice-common > > index 0a753b93..2e914f33 16 > > --- a/subprojects/spice-common > > +++ b/subprojects/spice-common > > @@ -1 +1 @@ > > -Subproject commit 0a753b93b50b548a2ac04023c96a7cda83f204bf > > +Subproject commit 2e914f3305f685c82143fa04067ce9f57fbfd602 > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk 1/2] Allows to enable recorder integration
ping the series > > recorder library will be used to collect some statistics during > development. > > Signed-off-by: Frediano Ziglio > --- > configure.ac | 1 + > meson_options.txt| 5 + > subprojects/spice-common | 2 +- > 3 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/configure.ac b/configure.ac > index 7dab0be0..2e6f7e78 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -120,6 +120,7 @@ PKG_CHECK_EXISTS(openssl, >[SPICE_GLIB_REQUIRES="${SPICE_GLIB_REQUIRES} openssl"], >[SPICE_GLIB_LIBS="${SPICE_GLIB_LIBS} ${SSL_LIBS}"]) > > +SPICE_CHECK_RECORDER > SPICE_CHECK_SASL > > AC_MSG_CHECKING([which gtk+ version to compile against]) > diff --git a/meson_options.txt b/meson_options.txt > index 2e605b77..98042171 100644 > --- a/meson_options.txt > +++ b/meson_options.txt > @@ -80,3 +80,8 @@ option('smartcard', > option('gtk_doc', > type : 'feature', > description : 'Generate API documentation with gtk-doc') > + > +option('recorder', > +type : 'boolean', > +value : false, > +description: 'Enable recorder instrumentation') > diff --git a/subprojects/spice-common b/subprojects/spice-common > index 0a753b93..2e914f33 16 > --- a/subprojects/spice-common > +++ b/subprojects/spice-common > @@ -1 +1 @@ > -Subproject commit 0a753b93b50b548a2ac04023c96a7cda83f204bf > +Subproject commit 2e914f3305f685c82143fa04067ce9f57fbfd602 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] build-sys: remove meson/check-spice-common
> > From: Marc-André Lureau > > This is not necessary, as meson already checks out the git submodules. > > Signed-off-by: Marc-André Lureau But Meson do this only once while current code to it every time. It can work for me, if you need to work on some different branch you can do a checkout and git submodules update. I think, like autogen script, it would be good to do automatically an update when Meson is called to generate (not update!) the build files again. If nobody complain I'll ack and merge in a week. > --- > build-aux/meson/check-spice-common | 5 - > meson.build| 3 --- > 2 files changed, 8 deletions(-) > delete mode 100755 build-aux/meson/check-spice-common > > diff --git a/build-aux/meson/check-spice-common > b/build-aux/meson/check-spice-common > deleted file mode 100755 > index a0d03a6..000 > --- a/build-aux/meson/check-spice-common > +++ /dev/null > @@ -1,5 +0,0 @@ > -#!/bin/sh > -set -e > -if git ls-files --error-unmatch > ${MESON_SOURCE_ROOT}/subprojects/spice-common > /dev/null 2>&1; then > -git --git-dir="${MESON_SOURCE_ROOT}/.git" submodule update --init > --recursive > -fi > diff --git a/meson.build b/meson.build > index c015de7..197789a 100644 > --- a/meson.build > +++ b/meson.build > @@ -6,9 +6,6 @@ project('spice-gtk', 'c', > license : 'LGPLv2.1', > meson_version : '>= 0.49') > > -message('Updating submodules') > -run_command('build-aux/meson/check-spice-common', check : true) > - > meson.add_dist_script('sh', '-c', 'echo > @0@>"$MESON_DIST_ROOT/.tarball-version"'.format(meson.project_version())) > > # Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH] build-sys: remove meson/check-spice-common
From: Marc-André Lureau This is not necessary, as meson already checks out the git submodules. Signed-off-by: Marc-André Lureau --- build-aux/meson/check-spice-common | 5 - meson.build| 3 --- 2 files changed, 8 deletions(-) delete mode 100755 build-aux/meson/check-spice-common diff --git a/build-aux/meson/check-spice-common b/build-aux/meson/check-spice-common deleted file mode 100755 index a0d03a6..000 --- a/build-aux/meson/check-spice-common +++ /dev/null @@ -1,5 +0,0 @@ -#!/bin/sh -set -e -if git ls-files --error-unmatch ${MESON_SOURCE_ROOT}/subprojects/spice-common > /dev/null 2>&1; then -git --git-dir="${MESON_SOURCE_ROOT}/.git" submodule update --init --recursive -fi diff --git a/meson.build b/meson.build index c015de7..197789a 100644 --- a/meson.build +++ b/meson.build @@ -6,9 +6,6 @@ project('spice-gtk', 'c', license : 'LGPLv2.1', meson_version : '>= 0.49') -message('Updating submodules') -run_command('build-aux/meson/check-spice-common', check : true) - meson.add_dist_script('sh', '-c', 'echo @0@>"$MESON_DIST_ROOT/.tarball-version"'.format(meson.project_version())) # -- 2.21.0.rc1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel