Re: [Spice-devel] [PATCH spice-gtk] meson: Pass --msgid-bugs-address to PO generation

2019-02-19 Thread Marc-André Lureau
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

2019-02-19 Thread Jonathon Jongsma
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

2019-02-19 Thread Uri Lublin

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

2019-02-19 Thread Frediano Ziglio
> 
> 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

2019-02-19 Thread Margit Meyer

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

2019-02-19 Thread Uri Lublin
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

2019-02-19 Thread Marc-André Lureau
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

2019-02-19 Thread Frediano Ziglio
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

2019-02-19 Thread Victor Toso
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

2019-02-19 Thread Marc-André Lureau
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

2019-02-19 Thread Frediano Ziglio
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

2019-02-19 Thread Frediano Ziglio
> 
> 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

2019-02-19 Thread Marc-André Lureau
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

2019-02-19 Thread Victor Toso
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

2019-02-19 Thread Victor Toso
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

2019-02-19 Thread Frediano Ziglio
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

2019-02-19 Thread Frediano Ziglio
> 
> 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

2019-02-19 Thread marcandre . lureau
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