Re: [Spice-devel] [spice-gtk PATCH v3 1/5] giopipe: don't fail on create_source

2015-05-29 Thread Marc-André Lureau 
Hi

On Tue, May 26, 2015 at 3:35 PM, Victor Toso victort...@redhat.com wrote:

 PipeInputStream and PipeOutputStream should not fail when creating
 GPollableStream source. It is already checked and unref in case of
 existing source.

 In order to track possible issues, the g_return_val_if_fail was
 changed to a g_debug message;
 ---
  gtk/giopipe.c | 14 --
  1 file changed, 8 insertions(+), 6 deletions(-)

 diff --git a/gtk/giopipe.c b/gtk/giopipe.c
 index 50edb5b..86eaab6 100644
 --- a/gtk/giopipe.c
 +++ b/gtk/giopipe.c
 @@ -234,10 +234,11 @@ pipe_input_stream_create_source
 (GPollableInputStream *stream,
  PipeInputStream *self = PIPE_INPUT_STREAM(stream);
  GSource *pollable_source;

 -g_return_val_if_fail (self-source == NULL ||
 -  g_source_is_destroyed (self-source), NULL);
 +if (self-source != NULL  !g_source_is_destroyed (self-source))
 +g_debug (%s: GPollableSource already exists %p - This could lead
 to data loss (%ld),
 + G_STRFUNC, self-source, self-read);

 -if (self-source  g_source_is_destroyed (self-source))
 +if (self-source)
  g_source_unref (self-source);

  pollable_source = g_pollable_source_new_full (self, NULL,
 cancellable);
 @@ -416,10 +417,11 @@ pipe_output_stream_create_source
 (GPollableOutputStream *stream,
  PipeOutputStream *self = PIPE_OUTPUT_STREAM(stream);
  GSource *pollable_source;

 -g_return_val_if_fail (self-source == NULL ||
 -  g_source_is_destroyed (self-source), NULL);
 +if (self-source != NULL  !g_source_is_destroyed (self-source))
 +g_debug (%s: GPollableSource already exists %p - This could lead
 to data loss (%ld),
 + G_STRFUNC, self-source, self-count);

 -if (self-source  g_source_is_destroyed (self-source))
 +if (self-source)
  g_source_unref (self-source);

  pollable_source = g_pollable_source_new_full (self, NULL,
 cancellable);
 --


Your tests show that io stream prevents concurrent read / write, and that
self-source is going to be destroyed after the current tasks return.
However, if gpollable create_source is called seperately, it may create
zombie sources, they will never be dispatched. It is easy to show the
issue with a test like:

for (i = 0; i  1; i++) {
GSource *s = g_pollable_input_stream_create_source(f-ip1, NULL);

g_source_set_callback (s, NULL, NULL, NULL);
g_source_attach (s, NULL);

g_source_unref(s);
}

g_main_loop_run (f-loop);

After the source is attached, the context will keep a reference, but when
creating a new source, it is not removed from the context. giopipe could
also call g_source_destroy() before the unref, but I don't think this is a
good practice: destroyed source callbacks would never be called, and it may
be hard to understand why.

I suppose the best would be to mimic poll() behaviour, and dispatch with
set_ready_time(0) all the created sources (that are still active).

-- 
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH v2] Report invalid password as a special auth error

2015-05-29 Thread Christophe Fergeau 
Hey,

On Wed, May 27, 2015 at 04:05:16PM +0200, Cédric Bosdonnat wrote:
 Provide a special authentication error message for too long passwords.
 Also check for too long passwords before sending them over the wire.
 ---
 
  Diff to v1:
* Added a check in spice_channel_send_spice_ticket
* moved spice_channel_failed_authentication before 
 spice_channel_send_spice_ticket
  in order to reuse it there.
 
  gtk/spice-channel.c | 64 
 +
  gtk/spice-client.h  |  2 ++
  2 files changed, 42 insertions(+), 24 deletions(-)
 
 diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
 index 4e7d8b7..c4a18f5 100644
 --- a/gtk/spice-channel.c
 +++ b/gtk/spice-channel.c
 @@ -1010,6 +1010,33 @@ static int spice_channel_read(SpiceChannel *channel, 
 void *data, size_t length)
  }
  
  /* coroutine context */
 +static void spice_channel_failed_authentication(SpiceChannel *channel,
 +gboolean invalidPassword)
 +{
 +SpiceChannelPrivate *c = channel-priv;
 +
 +if (c-auth_needs_username_and_password)
 +g_set_error_literal(c-error,
 +SPICE_CLIENT_ERROR,
 +
 SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
 +_(Authentication failed: password and username 
 are required));
 +else if (invalidPassword)
 +g_set_error_literal(c-error,
 +SPICE_CLIENT_ERROR,
 +SPICE_CLIENT_ERROR_AUTH_INVALID_PASSWORD,
 +_(Authentication failed: password is too 
 long));
 +else
 +g_set_error_literal(c-error,
 +SPICE_CLIENT_ERROR,
 +SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,
 +_(Authentication failed: password is 
 required));
 +
 +c-event = SPICE_CHANNEL_ERROR_AUTH;
 +
 +c-has_error = TRUE; /* force disconnect */
 +}
 +
 +/* coroutine context */
  static void spice_channel_send_spice_ticket(SpiceChannel *channel)
  {
  SpiceChannelPrivate *c = channel-priv;
 @@ -1039,11 +1066,17 @@ static void 
 spice_channel_send_spice_ticket(SpiceChannel *channel)
  g_object_get(c-session, password, password, NULL);
  if (password == NULL)
  password = g_strdup();
 +if (strlen(password)  SPICE_MAX_PASSWORD_LENGTH) {
 +spice_channel_failed_authentication(channel, TRUE);
 +goto cleanup;
 +}


My feeling is that spice_channel_send_spice_ticket() should return
TRUE/FALSE to indicate whether it failed or not, so that
spice_channel_recv_link_msg() can then decide to jump to its error:
label.

Christophe


pgpEKBWeS0QFk.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 09/11] Move main reference counter to GEM object instead of TTM ones

2015-05-29 Thread Frediano Ziglio 

 On 27 May 2015 at 20:04, Frediano Ziglio fzig...@redhat.com wrote:
  qxl_bo structure has two reference counters, one in the GEM object and
  another in the TTM object. The GEM object keep a counter to the TTM object
  so when GEM counter reached zero the TTM counter (using qxl_bo_unref) was
  decremented. The qxl object is fully freed (both GEM and TTM part are
  cleaned)
  when the TTM counter reach zero.
  One issue was that surface idr structure has no owning on qxl_bo objects
  however
  it contains a pointer to qxl_bo object. This caused some nasty race
  condition
  for instance qxl_bo object was reaped even after counter was already zero.
  This patch fix these races moving main counter (the one used by
  qxl_bo_(un)ref)
  to GEM object which cleanup routine (qxl_gem_object_free) remove the idr
  pointer
  (using qxl_surface_evict) when the counters are still valid.
 
 Uggh, but yes, not sure I like this fix for the problem, but if it works,
 
 Reviewed-by: Dave Airlie airl...@redhat.com
 

Well, the patch does not surely looks very clear but I can assure the problems 
it fixes are much less clear to understand.
Having a pointer to a object the is going to be deleted whenever another thread 
decide to causes difficult races. I tried to avoid this kind of change and fix 
the races instead but was a nightmare.
My first experimental patch added an additional counter on top of GEM and TTM 
one as the main counter but at the end was much more complicated and result was 
similar to move the main counter to GEM.
Mainly external references (from userspace and kernel) are pointers to GEM. 
Pointers to TTM are from memory mapped files. Deleting the spice id after GEM 
object has no references assure the not owning reference from spice id still 
refer to a valid object. As user can't retrieve a pointer from a mapping (at 
most can remap it) so there are no risks counter to GEM is incremented again.

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 0/11] Add GStreamer and VP8 support

2015-05-29 Thread Fabio Fantoni 
Il 29/05/2015 04:29, Francois Gouget ha scritto:
 On Thu, 28 May 2015, Fabio Fantoni wrote:
 [...]
 Thanks for reply, I'm preparing spice-gtk with vp8 patches armhf for
 test it on arm thin client (for example raspberry 2), I want try to use
 gst-omx for hardware decoding of mjpeg and vp8, can I simply build and
 install gst-omx instead of gst-ffmpeg or a change in spice-gtk is needed
 for use gst-omx?
 I have patches that could probably be useful to you for this: a 
 simplification of the pipeline creation, optionally the use of 
 decoebin, and for the performance aspects zero-copy for both the input 
 buffer and the ouput frame. So I have created some github repositories 
 to make it easier to get these:


 https://github.com/fgouget/spice-gtk.git
  * gst branch
All the above except for decodebin.

  * decodebin branch
Uses decodebin instead of manually picking the decoders. However on 
my machine decodebin tries to use vaapi which, given my test 
configuration, immediately triggers an assert, thus killing the 
process. Uninstalling vaapi fixes that but it's annoying. So I don't 
really like the decodebin approach though it's probably more a vaapi 
bug than a decodebin issue.

Thanks, I'll try it.



 https://github.com/fgouget/spice.git
  * gst branch
The patches adding GStreamer and VP8 support.

  * gst-1.0 branch
Experimental branch with non-functional GStreamer 1.0 and h264 code. 
Note that GStreamer 1.0 support can be disabled using './configure 
--disable-gstreamer-1.0' so H264 support can be tested too.

I'm trying the gst1 support but build fails with this error:
gstreamer_encoder.c: In function 'construct_pipeline':
gstreamer_encoder.c:219:41: error: 'GstEncoder' has no member named 'frc'
  deadline, encoder-frc.period * 1000 / 2,

Why you tell that 1.0 is not functional? I suppose not only for the
build error I had.



 https://github.com/fgouget/xf86-video-qxl.git
  * gst branch
Has the patch to make it possible to specify the encoder:codec 
pairs in either SpiceVideoCodecs or XSPICE_VIDEO_CODECS.


 There are also spice-common and spice-protocol repositories but these 
 should get pulled automatically.






smime.p7s
Description: Firma crittografica S/MIME
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 00/11] Miscellaneous stability patches

2015-05-29 Thread Frans Klaver 
On Thu, May 28, 2015 at 4:10 PM, Frediano Ziglio fzig...@redhat.com wrote:

 also indicating in each patch what is a right now fix and what isn't.

 What do you mean by right fix or not ?

He probably meant indicating whether it is an urgent fix.

Frans
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 0/11] Add GStreamer and VP8 support

2015-05-29 Thread Francois Gouget 
On Fri, 29 May 2015, Fabio Fantoni wrote:
[...]
 I'm trying the gst1 support but build fails with this error:
 gstreamer_encoder.c: In function 'construct_pipeline':
 gstreamer_encoder.c:219:41: error: 'GstEncoder' has no member named 'frc'
   deadline, encoder-frc.period * 1000 / 2,
 
 Why you tell that 1.0 is not functional? I suppose not only for the
 build error I had.

It's a merge bug. Sorry. I have pushed an update that fixes this so you 
may want to rebase.

The initial push had caps issues, mostly it was missing format=BGRx, 
which caused GStreamer 1.0 not to work. Note that the failure mode 
caused the video to be sent through the regular non-stream channel so it 
looked like it worked.

Now the GStreamer 1.0 situation is as follows:
 * VP8 works.
 * MJPEG fails because of a caps negotiation error despite the 
   videoconvert element.
 * H264 seems to work but the client fails to display it. So I'm not 
   totally sure.

Also I finally found the right parameters to improve the VP8 encoder 
performance.

Cheers,

-- 
Francois Gouget fgou...@codeweavers.com
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 1/2] Use image compress constants from spice-protocol

2015-05-29 Thread Christophe Fergeau 
Looks good, ACK.

Christophe

On Fri, Apr 17, 2015 at 12:40:47PM +0200, Javier Celaya wrote:
 ---
  server/red_dispatcher.c |  2 +-
  server/red_dispatcher.h |  2 +-
  server/red_worker.c |  8 
  server/red_worker.h |  2 +-
  server/reds.c   |  8 
  server/spice-server.h   | 15 ++-
  spice-common|  2 +-
  7 files changed, 14 insertions(+), 25 deletions(-)
 
 diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
 index d8389bc..4965a97 100644
 --- a/server/red_dispatcher.c
 +++ b/server/red_dispatcher.c
 @@ -79,7 +79,7 @@ typedef struct RedWorkeState {
  } RedWorkeState;
  
  extern uint32_t streaming_video;
 -extern spice_image_compression_t image_compression;
 +extern SpiceImageCompress image_compression;
  extern spice_wan_compression_t jpeg_state;
  extern spice_wan_compression_t zlib_glz_state;
  
 diff --git a/server/red_dispatcher.h b/server/red_dispatcher.h
 index 907b7c7..2a1b446 100644
 --- a/server/red_dispatcher.h
 +++ b/server/red_dispatcher.h
 @@ -167,7 +167,7 @@ typedef struct RedWorkerMessageLoadvmCommands {
  } RedWorkerMessageLoadvmCommands;
  
  typedef struct RedWorkerMessageSetCompression {
 -spice_image_compression_t image_compression;
 +SpiceImageCompress image_compression;
  } RedWorkerMessageSetCompression;
  
  typedef struct RedWorkerMessageSetStreamingVideo {
 diff --git a/server/red_worker.c b/server/red_worker.c
 index 5deb30b..29d6d6d 100644
 --- a/server/red_worker.c
 +++ b/server/red_worker.c
 @@ -987,7 +987,7 @@ typedef struct RedWorker {
  
  ImageCache image_cache;
  
 -spice_image_compression_t image_compression;
 +SpiceImageCompress image_compression;
  spice_wan_compression_t jpeg_state;
  spice_wan_compression_t zlib_glz_state;
  
 @@ -6573,7 +6573,7 @@ static inline int 
 red_compress_image(DisplayChannelClient *dcc,
   compress_send_data_t* o_comp_data)
  {
  DisplayChannel *display_channel = DCC_TO_DC(dcc);
 -spice_image_compression_t image_compression =
 +SpiceImageCompress image_compression =
  display_channel-common.worker-image_compression;
  int quic_compress = FALSE;
  
 @@ -6878,7 +6878,7 @@ static void fill_mask(RedChannelClient *rcc, 
 SpiceMarshaller *m,
  
  if (mask_bitmap  m) {
  if (display_channel-common.worker-image_compression != 
 SPICE_IMAGE_COMPRESS_OFF) {
 -spice_image_compression_t save_img_comp =
 +SpiceImageCompress save_img_comp =
  display_channel-common.worker-image_compression;
  display_channel-common.worker-image_compression = 
 SPICE_IMAGE_COMPRESS_OFF;
  fill_bits(dcc, m, mask_bitmap, drawable, FALSE);
 @@ -8824,7 +8824,7 @@ static void red_marshall_image(RedChannelClient *rcc, 
 SpiceMarshaller *m, ImageI
  int comp_succeeded;
  int lossy_comp = FALSE;
  int lz_comp = FALSE;
 -spice_image_compression_t comp_mode;
 +SpiceImageCompress comp_mode;
  SpiceMsgDisplayDrawCopy copy;
  SpiceMarshaller *src_bitmap_out, *mask_bitmap_out;
  SpiceMarshaller *bitmap_palette_out, *lzplt_palette_out;
 diff --git a/server/red_worker.h b/server/red_worker.h
 index 272661f..2042b3d 100644
 --- a/server/red_worker.h
 +++ b/server/red_worker.h
 @@ -92,7 +92,7 @@ typedef struct WorkerInitData {
  uint32_t *pending;
  uint32_t num_renderers;
  uint32_t renderers[RED_MAX_RENDERERS];
 -spice_image_compression_t image_compression;
 +SpiceImageCompress image_compression;
  spice_wan_compression_t jpeg_state;
  spice_wan_compression_t zlib_glz_state;
  int streaming_video;
 diff --git a/server/reds.c b/server/reds.c
 index 6d70b68..d6d76b2 100644
 --- a/server/reds.c
 +++ b/server/reds.c
 @@ -111,7 +111,7 @@ static int ticketing_enabled = 1; //Ticketing is enabled 
 by default
  static pthread_mutex_t *lock_cs;
  static long *lock_count;
  uint32_t streaming_video = STREAM_VIDEO_FILTER;
 -spice_image_compression_t image_compression = SPICE_IMAGE_COMPRESS_AUTO_GLZ;
 +SpiceImageCompress image_compression = SPICE_IMAGE_COMPRESS_AUTO_GLZ;
  spice_wan_compression_t jpeg_state = SPICE_WAN_COMPRESSION_AUTO;
  spice_wan_compression_t zlib_glz_state = SPICE_WAN_COMPRESSION_AUTO;
  int agent_mouse = TRUE;
 @@ -2709,7 +2709,7 @@ static inline void on_activating_ticketing(void)
  }
  }
  
 -static void set_image_compression(spice_image_compression_t val)
 +static void set_image_compression(SpiceImageCompress val)
  {
  if (val == image_compression) {
  return;
 @@ -3550,14 +3550,14 @@ SPICE_GNUC_VISIBLE int 
 spice_server_set_tls(SpiceServer *s, int port,
  }
  
  SPICE_GNUC_VISIBLE int spice_server_set_image_compression(SpiceServer *s,
 -  
 spice_image_compression_t comp)
 +  SpiceImageCompress 
 comp)
  {
  spice_assert(reds == s);

Re: [Spice-devel] [PATCH 0/2] Set the image compression reported by client

2015-05-29 Thread Christophe Fergeau 
Hey,

I'm finally taking a closer look at these patches, sorry that it took
an awfully long time to get them reviewed :(

Christophe

On Fri, Apr 17, 2015 at 12:40:46PM +0200, Javier Celaya wrote:
 This patch set uses the SpiceImageCompress enum from spice-common
 instead of spice_image_compression_t, and handles the preferred
 image compression message from the client.
 
 Javier Celaya (2):
   Use image compress constants from spice-protocol
   Handle preferred image compression messages
 
  server/red_dispatcher.c |  2 +-
  server/red_dispatcher.h |  2 +-
  server/red_worker.c | 33 +
  server/red_worker.h |  2 +-
  server/reds.c   |  8 
  server/spice-server.h   | 15 ++-
  spice-common|  2 +-
  7 files changed, 39 insertions(+), 25 deletions(-)
 
 -- 
 1.9.3
 
 ___
 Spice-devel mailing list
 Spice-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/spice-devel


pgp6VPUhujymv.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 v2] Proto: Add preferred compression message and constants.

2015-05-29 Thread Christophe Fergeau 
ACK.

On Mon, Apr 27, 2015 at 10:01:00AM +0200, Javier Celaya wrote:
 When accessing a virtual desktop from different devices, some may have
 different image compression requirements, e.g. slow devices may prefer
 the faster LZ4 over GLZ. This message instructs the server to switch the
 image compression algorithm. This patch also promotes the
 SPICE_IMAGE_COMPRESS_* constants so that they are available from both
 the server and the client.
 ---
  common/client_marshallers.h |  1 +
  common/messages.h   |  4 
  spice.proto | 15 +++
  3 files changed, 20 insertions(+)
 
 diff --git a/common/client_marshallers.h b/common/client_marshallers.h
 index 85051a0..522f620 100644
 --- a/common/client_marshallers.h
 +++ b/common/client_marshallers.h
 @@ -72,6 +72,7 @@ typedef struct {
  void (*msgc_smartcard_reader_add)(SpiceMarshaller *m, VSCMsgReaderAdd 
 *msg);
  #endif
  void (*msgc_port_event)(SpiceMarshaller *m, SpiceMsgcPortEvent *msg);
 +void (*msgc_display_preferred_compression)(SpiceMarshaller *m, 
 SpiceMsgcDisplayPreferredCompression *msg);
  } SpiceMessageMarshallers;
  
  SpiceMessageMarshallers *spice_message_marshallers_get(void);
 diff --git a/common/messages.h b/common/messages.h
 index a8a0eee..5f5de54 100644
 --- a/common/messages.h
 +++ b/common/messages.h
 @@ -630,6 +630,10 @@ typedef struct SpiceMsgcPortEvent {
  uint8_t event;
  } SpiceMsgcPortEvent;
  
 +typedef struct SpiceMsgcDisplayPreferredCompression {
 +uint8_t image_compression;
 +} SpiceMsgcDisplayPreferredCompression;
 +
  SPICE_END_DECLS
  
  #endif /* _H_SPICE_PROTOCOL */
 diff --git a/spice.proto b/spice.proto
 index 01493c9..2889802 100644
 --- a/spice.proto
 +++ b/spice.proto
 @@ -361,6 +361,17 @@ enum8 image_type {
  LZ4,
  };
  
 +enum8 image_compress {
 +INVALID  = 0,
 +OFF,
 +AUTO_GLZ,
 +AUTO_LZ,
 +QUIC,
 +GLZ,
 +LZ,
 +LZ4,
 +};
 +
  flags8 image_flags {
  CACHE_ME,
  HIGH_BITS_SET,
 @@ -922,6 +933,10 @@ channel DisplayChannel : BaseChannel {
  int32 last_frame_delay;
  uint32 audio_delay;
  } stream_report;
 +
 +message {
 +uint8 image_compression;
 +} preferred_compression;
  };
  
  flags16 keyboard_modifier_flags {
 -- 
 1.9.3
 
 ___
 Spice-devel mailing list
 Spice-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/spice-devel


pgpf_SSwqVb_4.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 2/2] Handle preferred image compression messages

2015-05-29 Thread Christophe Fergeau 
ACK too, I was initially worried that
display_channel-common.worker-image_compression would be accessed from
different thread, but after adding a few printf, this looks fine.

Christophe

On Fri, Apr 17, 2015 at 12:40:48PM +0200, Javier Celaya wrote:
 ---
  server/red_worker.c | 25 +
  1 file changed, 25 insertions(+)
 
 diff --git a/server/red_worker.c b/server/red_worker.c
 index 29d6d6d..af2f1d6 100644
 --- a/server/red_worker.c
 +++ b/server/red_worker.c
 @@ -10276,6 +10276,27 @@ static int 
 display_channel_handle_stream_report(DisplayChannelClient *dcc,
  return TRUE;
  }
  
 +static int display_channel_handle_preferred_compression(DisplayChannelClient 
 *dcc,
 +SpiceMsgcDisplayPreferredCompression *pc) {
 +DisplayChannel *display_channel = DCC_TO_DC(dcc);
 +switch (pc-image_compression) {
 +case SPICE_IMAGE_COMPRESS_AUTO_LZ:
 +case SPICE_IMAGE_COMPRESS_AUTO_GLZ:
 +case SPICE_IMAGE_COMPRESS_QUIC:
 +#ifdef USE_LZ4
 +case SPICE_IMAGE_COMPRESS_LZ4:
 +#endif
 +case SPICE_IMAGE_COMPRESS_LZ:
 +case SPICE_IMAGE_COMPRESS_GLZ:
 +case SPICE_IMAGE_COMPRESS_OFF:
 +display_channel-common.worker-image_compression = 
 pc-image_compression;
 +return TRUE;
 +default:
 +spice_warning(preferred-compression: unsupported image compression 
 setting);
 +return FALSE;
 +}
 +}
 +
  static int display_channel_handle_message(RedChannelClient *rcc, uint32_t 
 size, uint16_t type,
void *message)
  {
 @@ -10292,6 +10313,10 @@ static int 
 display_channel_handle_message(RedChannelClient *rcc, uint32_t size,
  case SPICE_MSGC_DISPLAY_STREAM_REPORT:
  return display_channel_handle_stream_report(dcc,
  
 (SpiceMsgcDisplayStreamReport *)message);
 +case SPICE_MSGC_DISPLAY_PREFERRED_COMPRESSION:
 +return display_channel_handle_preferred_compression(dcc,
 +(SpiceMsgcDisplayPreferredCompression *)message);
 +
  default:
  return red_channel_client_handle_message(rcc, size, type, message);
  }
 -- 
 1.9.3
 
 ___
 Spice-devel mailing list
 Spice-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/spice-devel


pgpBPIOJGHvsm.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 1/3] Spice-session: Add preferred-compression property.

2015-05-29 Thread Christophe Fergeau 
On Fri, Apr 17, 2015 at 12:40:23PM +0200, Javier Celaya wrote:
 Also, depend on the spice-common commit that introduces the
 SpiceImageCompress enum.
 ---
  gtk/spice-session.c | 26 ++
  spice-common|  2 +-
  2 files changed, 27 insertions(+), 1 deletion(-)
 
 diff --git a/gtk/spice-session.c b/gtk/spice-session.c
 index 020a70e..c83d239 100644
 --- a/gtk/spice-session.c
 +++ b/gtk/spice-session.c
 @@ -117,6 +117,7 @@ struct _SpiceSessionPrivate {
  uint32_t  n_display_channels;
  guint8uuid[16];
  gchar *name;
 +int   preferred_compression;
  
  /* associated objects */
  SpiceAudio*audio_manager;
 @@ -203,6 +204,7 @@ enum {
  PROP_SHARE_DIR_RO,
  PROP_USERNAME,
  PROP_UNIX_PATH,
 +PROP_PREF_COMPRESS,
  };
  
  /* signals */
 @@ -658,6 +660,9 @@ static void spice_session_get_property(GObject
 *gobject,
  case PROP_SHARE_DIR_RO:
  g_value_set_boolean(value, s-share_dir_ro);
  break;
 +case PROP_PREF_COMPRESS:
 +g_value_set_int(value, s-preferred_compression);
 +break;
  default:
   G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
   break;
 @@ -794,6 +799,9 @@ static void spice_session_set_property(GObject  
 *gobject,
  case PROP_SHARE_DIR_RO:
  s-share_dir_ro = g_value_get_boolean(value);
  break;
 +case PROP_PREF_COMPRESS:
 +s-preferred_compression = g_value_get_int(value);
 +break;
  default:
  G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
  break;
 @@ -1403,6 +1411,24 @@ static void spice_session_class_init(SpiceSessionClass 
 *klass)
G_PARAM_CONSTRUCT |
G_PARAM_STATIC_STRINGS));
  
 +/**
 + * SpiceSession:preferred-compression:
 + *
 + * The image compression algorithm the client prefers to use. It is
 + * reported to the server.
 + *
 + * Since: 0.29
 + **/
 +g_object_class_install_property
 +(gobject_class, PROP_PREF_COMPRESS,
 + g_param_spec_int(preferred-compression,
 +  Preferred image compression algorithm,
 +  Preferred image compression algorithm,
 +  SPICE_IMAGE_COMPRESS_INVALID, 
 SPICE_IMAGE_COMPRESS_ENUM_END - 1,
 +  SPICE_IMAGE_COMPRESS_INVALID,
 +  G_PARAM_READWRITE |
 +  G_PARAM_STATIC_STRINGS));

I'd prefer if we used a g_param_spec_enum here, however this means doing
the registration by hand (see how it's done in gtk/spice-glib-enums.c)
as glib-mkenums does not like the generated enums.h (because of enum
name in typedef enum Foo {};)

This would look like:

#define SPICE_TYPE_CHANNEL_EVENT spice_channel_event_get_type()
GType spice_channel_event_get_type (void);

static const GEnumValue _spice_channel_event_values[] = {
  { SPICE_CHANNEL_NONE, SPICE_CHANNEL_NONE, none },
  { SPICE_CHANNEL_OPENED, SPICE_CHANNEL_OPENED, opened },
  { SPICE_CHANNEL_SWITCHING, SPICE_CHANNEL_SWITCHING, switching },
  { SPICE_CHANNEL_CLOSED, SPICE_CHANNEL_CLOSED, closed },
  { SPICE_CHANNEL_ERROR_CONNECT, SPICE_CHANNEL_ERROR_CONNECT, error-connect 
},
  { SPICE_CHANNEL_ERROR_TLS, SPICE_CHANNEL_ERROR_TLS, error-tls },
  { SPICE_CHANNEL_ERROR_LINK, SPICE_CHANNEL_ERROR_LINK, error-link },
  { SPICE_CHANNEL_ERROR_AUTH, SPICE_CHANNEL_ERROR_AUTH, error-auth },
  { SPICE_CHANNEL_ERROR_IO, SPICE_CHANNEL_ERROR_IO, error-io },
  { 0, NULL, NULL }
};

G_STATIC_ASSERT(G_N_ELEMENTS(_spice_channel_event_values) == 
SPICE_IMAGE_COMPRESS_ENUM_END - 1);

GType
spice_channel_event_get_type (void)
{
  static GType type = 0;
  static volatile gsize type_volatile = 0;

  if (g_once_init_enter(type_volatile)) {
type = g_enum_register_static (SpiceChannelEvent, 
_spice_channel_event_values);
g_once_init_leave(type_volatile, type);
  }

  return type;
}

Christophe


pgp_kZQKyBiU2.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 v2 2/3] Cmdline: Get the preferred-compression property

2015-05-29 Thread Christophe Fergeau 
ACK, but gtk/spice-option.c needs similar changes otherwise
remote-viewer won't pick up the new compression parameters.

Christophe

On Mon, Apr 27, 2015 at 10:04:35AM +0200, Javier Celaya wrote:
 ---
  gtk/spice-cmdline.c | 48 
  1 file changed, 48 insertions(+)
 
 diff --git a/gtk/spice-cmdline.c b/gtk/spice-cmdline.c
 index 8619b57..c52b1a2 100644
 --- a/gtk/spice-cmdline.c
 +++ b/gtk/spice-cmdline.c
 @@ -27,6 +27,39 @@ static char *port;
  static char *tls_port;
  static char *password;
  static char *uri;
 +static int preferred_compression;
 +
 +GQuark spice_cmdline_error_quark(void)
 +{
 +return g_quark_from_static_string(spice-cmdline-error);
 +}
 +
 +static gboolean parse_preferred_compression(const gchar *option_name, const 
 gchar *value,
 +gpointer data, GError **error) {
 +if (!strcmp(value, auto-glz)) {
 +preferred_compression = SPICE_IMAGE_COMPRESS_AUTO_GLZ;
 +} else if (!strcmp(value, auto-lz)) {
 +preferred_compression = SPICE_IMAGE_COMPRESS_AUTO_LZ;
 +} else if (!strcmp(value, quic)) {
 +preferred_compression = SPICE_IMAGE_COMPRESS_QUIC;
 +} else if (!strcmp(value, glz)) {
 +preferred_compression = SPICE_IMAGE_COMPRESS_GLZ;
 +} else if (!strcmp(value, lz)) {
 +preferred_compression = SPICE_IMAGE_COMPRESS_LZ;
 +#ifdef USE_LZ4
 +} else if (!strcmp(value, lz4)) {
 +preferred_compression = SPICE_IMAGE_COMPRESS_LZ4;
 +#endif
 +} else if (!strcmp(value, off)) {
 +preferred_compression = SPICE_IMAGE_COMPRESS_OFF;
 +} else {
 +preferred_compression = SPICE_IMAGE_COMPRESS_INVALID;
 +g_set_error(error, spice_cmdline_error_quark(), 
 G_OPTION_ERROR_FAILED,
 +_(Image compression algorithm %s not supported), 
 value);
 +return FALSE;
 +}
 +return TRUE;
 +}
  
  static GOptionEntry spice_entries[] = {
  {
 @@ -64,6 +97,17 @@ static GOptionEntry spice_entries[] = {
  .description  = N_(Server password),
  .arg_description  = N_(password),
  },{
 +.long_name= preferred-compression,
 +.short_name   = 'c',
 +.arg  = G_OPTION_ARG_CALLBACK,
 +.arg_data = parse_preferred_compression,
 +.description  = N_(Preferred image compression algorithm),
 +#ifdef USE_LZ4
 +.arg_description  = N_(auto-glz|auto-lz|quic|glz|lz|lz4|off),
 +#else
 +.arg_description  = N_(auto-glz|auto-lz|quic|glz|lz|off),
 +#endif
 +},{
  /* end of list */
  }
  };
 @@ -72,6 +116,8 @@ GOptionGroup *spice_cmdline_get_option_group(void)
  {
  GOptionGroup *grp;
  
 +preferred_compression = SPICE_IMAGE_COMPRESS_INVALID;
 +
  grp = g_option_group_new(spice,
   _(Spice connection options:),
   _(Show Spice options),
 @@ -95,4 +141,6 @@ void spice_cmdline_session_setup(SpiceSession *session)
  g_object_set(session, tls-port, tls_port, NULL);
  if (password)
  g_object_set(session, password, password, NULL);
 +if (preferred_compression != SPICE_IMAGE_COMPRESS_INVALID)
 +g_object_set(session, preferred-compression, 
 preferred_compression, NULL);
  }
 -- 
 1.9.3
 
 ___
 Spice-devel mailing list
 Spice-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/spice-devel


pgpv7Ljbz7CMX.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 v2 3/3] Display: Send a preferred compression message on init.

2015-05-29 Thread Christophe Fergeau 
On Mon, Apr 27, 2015 at 10:05:17AM +0200, Javier Celaya wrote:
 If the user prefers a specific compression algorithm, report it when
 setting up the display channel.

ACK.

Christophe


pgpjUItWxXEe2.pgp
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel