Re: [Spice-devel] [PATCH] Remove goto within switch statement
Hi Jonathon,
On Tue, 2016-06-14 at 16:04 -0500, Jonathon Jongsma wrote:
> Having a goto label in the middle of a switch/case statement is a bit
> confusing. But the same behavior can be achieved
it changes the behavior - if LZ4 compression is selected on server but it is not
supported on the client, then LZ compression should be used.
With your patch it skips compression and fallback to spice_error() call.
Pavel
> by simply rearranging
> the cases so that we fall through to the one that we wanted to jump to.
> ---
>
> This should apply on top of frediano's encapsulation patch series.
>
> server/dcc.c | 15 +++
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/server/dcc.c b/server/dcc.c
> index ca5569e..cf4fc77 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -747,7 +747,13 @@ int dcc_compress_image(DisplayChannelClient *dcc,
> if (success) {
> break;
> }
> -goto lz_compress;
> +/* on failure, fall through to compress with LZ */
> +case SPICE_IMAGE_COMPRESSION_LZ:
> +success = image_encoders_compress_lz(&dcc->encoders, dest, src,
> o_comp_data);
> +if (success && !bitmap_fmt_is_rgb(src->format)) {
> +dcc_palette_cache_palette(dcc, dest->u.lz_plt.palette, &(dest-
> >u.lz_plt.flags));
> +}
> +break;
> #ifdef USE_LZ4
> case SPICE_IMAGE_COMPRESSION_LZ4:
> if (red_channel_client_test_remote_cap(&dcc->common.base,
> @@ -756,13 +762,6 @@ int dcc_compress_image(DisplayChannelClient *dcc,
> break;
> }
> #endif
> -lz_compress:
> -case SPICE_IMAGE_COMPRESSION_LZ:
> -success = image_encoders_compress_lz(&dcc->encoders, dest, src,
> o_comp_data);
> -if (success && !bitmap_fmt_is_rgb(src->format)) {
> -dcc_palette_cache_palette(dcc, dest->u.lz_plt.palette, &(dest-
> >u.lz_plt.flags));
> -}
> -break;
> default:
> spice_error("invalid image compression type %u", image_compression);
> }
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v4 13/19] Change RedGlzDrawable::drawable from pointer to boolean
On Tue, 2016-06-14 at 10:33 +0100, Frediano Ziglio wrote:
> The field was used just as a flag.
> This has the advantage to make clear to not use the pointer as we don't
> have ownership.
> Also many the structure a bit smaller.
s/many/makes/
Acked-by: Jonathon Jongsma
>
> Signed-off-by: Frediano Ziglio
> ---
> server/dcc-encoders.c | 12 +---
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index 22cfcf3..00349fe 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -48,10 +48,10 @@ struct RedGlzDrawable {
> RingItem link;// ordered by the time it was encoded
> RingItem drawable_link;
> RedDrawable *red_drawable;
> -struct Drawable*drawable;
> GlzDrawableInstanceItem instances_pool[MAX_GLZ_DRAWABLE_INSTANCES];
> Ring instances;
> uint8_t instances_count;
> +gboolean has_drawable;
> ImageEncoders *encoders;
> };
>
> @@ -505,9 +505,7 @@ static void
> image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
> if (ring_is_empty(&glz_drawable->instances)) {
> spice_assert(glz_drawable->instances_count == 0);
>
> -Drawable *drawable = glz_drawable->drawable;
> -
> -if (drawable) {
> +if (glz_drawable->has_drawable) {
> ring_remove(&glz_drawable->drawable_link);
> }
> red_drawable_unref(glz_drawable->red_drawable);
> @@ -569,7 +567,7 @@ int
> image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc)
> while ((n < RED_RELEASE_BUNCH_SIZE) && (ring_link != NULL)) {
> RedGlzDrawable *glz_drawable = SPICE_CONTAINEROF(ring_link,
> RedGlzDrawable, link);
> ring_link = ring_next(&enc->glz_drawables, ring_link);
> -if (!glz_drawable->drawable) {
> +if (!glz_drawable->has_drawable) {
> image_encoders_free_glz_drawable(enc, glz_drawable);
> n++;
> }
> @@ -630,7 +628,7 @@ void image_encoders_glz_detach_from_drawable(struct
> Drawable *drawable)
> RingItem *item, *next;
>
> RING_FOREACH_SAFE(item, next, &drawable->glz_ring) {
> -SPICE_CONTAINEROF(item, RedGlzDrawable, drawable_link)->drawable =
> NULL;
> +SPICE_CONTAINEROF(item, RedGlzDrawable, drawable_link)->has_drawable
> = FALSE;
> ring_remove(item);
> }
> }
> @@ -1143,7 +1141,7 @@ static RedGlzDrawable *get_glz_drawable(ImageEncoders
> *enc, Drawable *drawable)
>
> ret->encoders = enc;
> ret->red_drawable = red_drawable_ref(drawable->red_drawable);
> -ret->drawable = drawable;
> +ret->has_drawable = TRUE;
> ret->instances_count = 0;
> ring_init(&ret->instances);
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v4 12/19] Better encoders encapsulation
Acked-by: Jonathon Jongsma
On Tue, 2016-06-14 at 10:33 +0100, Frediano Ziglio wrote:
> Avoid to access some fields from dcc.c
>
> Signed-off-by: Frediano Ziglio
> ---
> server/dcc-encoders.c | 4
> server/dcc.c | 3 ---
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index 3547adc..22cfcf3 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -710,6 +710,8 @@ gboolean image_encoders_get_glz_dictionary(ImageEncoders
> *enc,
> {
> GlzSharedDictionary *shared_dict;
>
> +spice_return_val_if_fail(!enc->glz_dict, FALSE);
> +
> pthread_mutex_lock(&glz_dictionary_list_lock);
>
> shared_dict = find_glz_dictionary(client, id);
> @@ -743,6 +745,8 @@ gboolean
> image_encoders_restore_glz_dictionary(ImageEncoders *enc,
> {
> GlzSharedDictionary *shared_dict = NULL;
>
> +spice_return_val_if_fail(!enc->glz_dict, FALSE);
> +
> pthread_mutex_lock(&glz_dictionary_list_lock);
>
> shared_dict = find_glz_dictionary(client, id);
> diff --git a/server/dcc.c b/server/dcc.c
> index 3190c94..039229d 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -903,7 +903,6 @@ static int dcc_handle_init(DisplayChannelClient *dcc,
> SpiceMsgcDisplayInit *init
> init->pixmap_cache_size);
> spice_return_val_if_fail(dcc->pixmap_cache, FALSE);
>
> -spice_return_val_if_fail(!dcc->encoders.glz_dict, FALSE);
> success = image_encoders_get_glz_dictionary(&dcc->encoders,
> RED_CHANNEL_CLIENT(dcc)-
> >client,
> init->glz_dictionary_id,
> @@ -1008,8 +1007,6 @@ int dcc_handle_message(RedChannelClient *rcc, uint32_t
> size, uint16_t type, void
> static int dcc_handle_migrate_glz_dictionary(DisplayChannelClient *dcc,
> SpiceMigrateDataDisplay
> *migrate)
> {
> -spice_return_val_if_fail(!dcc->encoders.glz_dict, FALSE);
> -
> return image_encoders_restore_glz_dictionary(&dcc->encoders,
> RED_CHANNEL_CLIENT(dcc)-
> >client,
> migrate->glz_dict_id,
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH] Remove goto within switch statement
Having a goto label in the middle of a switch/case statement is a bit
confusing. But the same behavior can be achieved by simply rearranging
the cases so that we fall through to the one that we wanted to jump to.
---
This should apply on top of frediano's encapsulation patch series.
server/dcc.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/server/dcc.c b/server/dcc.c
index ca5569e..cf4fc77 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -747,7 +747,13 @@ int dcc_compress_image(DisplayChannelClient *dcc,
if (success) {
break;
}
-goto lz_compress;
+/* on failure, fall through to compress with LZ */
+case SPICE_IMAGE_COMPRESSION_LZ:
+success = image_encoders_compress_lz(&dcc->encoders, dest, src,
o_comp_data);
+if (success && !bitmap_fmt_is_rgb(src->format)) {
+dcc_palette_cache_palette(dcc, dest->u.lz_plt.palette,
&(dest->u.lz_plt.flags));
+}
+break;
#ifdef USE_LZ4
case SPICE_IMAGE_COMPRESSION_LZ4:
if (red_channel_client_test_remote_cap(&dcc->common.base,
@@ -756,13 +762,6 @@ int dcc_compress_image(DisplayChannelClient *dcc,
break;
}
#endif
-lz_compress:
-case SPICE_IMAGE_COMPRESSION_LZ:
-success = image_encoders_compress_lz(&dcc->encoders, dest, src,
o_comp_data);
-if (success && !bitmap_fmt_is_rgb(src->format)) {
-dcc_palette_cache_palette(dcc, dest->u.lz_plt.palette,
&(dest->u.lz_plt.flags));
-}
-break;
default:
spice_error("invalid image compression type %u", image_compression);
}
--
2.5.5
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v4 11/19] Better encapsulation for image_encoders_compress_glz call
On Tue, 2016-06-14 at 10:33 +0100, Frediano Ziglio wrote:
> Do not access too much encoders data.
> Slightly different as now if glz is frozen lz compression is used.
This *seems* safe, though I don't know the code enough to know what
side-effects
it might have. Would like a second opinion.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/dcc-encoders.c | 15 +++
> server/dcc.c | 17 -
> 2 files changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index 6668dff..3547adc 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -1191,6 +1191,17 @@ int image_encoders_compress_glz(ImageEncoders *enc,
> spice_info("LZ global compress fmt=%d", src->format);
> #endif
>
> +if ((src->x * src->y) >= glz_enc_dictionary_get_size(enc->glz_dict-
> >dict)) {
> +return FALSE;
> +}
> +
> +pthread_rwlock_rdlock(&enc->glz_dict->encode_lock);
> +/* using the global dictionary only if it is not frozen */
> +if (enc->glz_dict->migrate_freeze) {
> +pthread_rwlock_unlock(&enc->glz_dict->encode_lock);
> +return FALSE;
> +}
> +
> encoder_data_init(&glz_data->data);
>
> glz_drawable = get_glz_drawable(enc, drawable);
> @@ -1241,8 +1252,12 @@ int image_encoders_compress_glz(ImageEncoders *enc,
> o_comp_data->comp_buf_size = zlib_size;
>
> stat_compress_add(&enc->shared_data->zlib_glz_stat, start_time, glz_size,
> zlib_size);
> +pthread_rwlock_unlock(&enc->glz_dict->encode_lock);
> return TRUE;
> +
> glz:
> +pthread_rwlock_unlock(&enc->glz_dict->encode_lock);
> +
> dest->descriptor.type = SPICE_IMAGE_TYPE_GLZ_RGB;
> dest->u.lz_rgb.data_size = glz_size;
>
> diff --git a/server/dcc.c b/server/dcc.c
> index e44feb7..3190c94 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -740,19 +740,10 @@ int dcc_compress_image(DisplayChannelClient *dcc,
> success = image_encoders_compress_quic(&dcc->encoders, dest, src,
> o_comp_data);
> break;
> case SPICE_IMAGE_COMPRESSION_GLZ:
> -if ((src->x * src->y) < glz_enc_dictionary_get_size(dcc-
> >encoders.glz_dict->dict)) {
> -int frozen;
> -/* using the global dictionary only if it is not frozen */
> -pthread_rwlock_rdlock(&dcc->encoders.glz_dict->encode_lock);
> -frozen = dcc->encoders.glz_dict->migrate_freeze;
> -if (!frozen) {
> -success = image_encoders_compress_glz(&dcc->encoders, dest,
> src, drawable, o_comp_data,
> - display_channel-
> >enable_zlib_glz_wrap);
> -}
> -pthread_rwlock_unlock(&dcc->encoders.glz_dict->encode_lock);
> -if (!frozen) {
> -break;
> -}
> +success = image_encoders_compress_glz(&dcc->encoders, dest, src,
> drawable, o_comp_data,
> + display_channel-
> >enable_zlib_glz_wrap);
> +if (success) {
> +break;
> }
> goto lz_compress;
> #ifdef USE_LZ4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v4 10/19] Encapsulate dcc_release_glz
Acked-by: Jonathon Jongsma
On Tue, 2016-06-14 at 10:33 +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio
> ---
> server/dcc-encoders.c | 4 +++-
> server/dcc-encoders.h | 1 -
> server/dcc.c | 1 -
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index f423f22..6668dff 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -64,6 +64,7 @@ static void
> image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
> GlzDrawableInstanceItem
> *instance);
> static void encoder_data_init(EncoderData *data);
> static void encoder_data_reset(EncoderData *data);
> +static void image_encoders_release_glz(ImageEncoders *enc);
>
>
> static SPICE_GNUC_NORETURN SPICE_GNUC_PRINTF(2, 3) void
> @@ -459,6 +460,7 @@ void image_encoders_init(ImageEncoders *enc,
> ImageEncoderSharedData *shared_data
>
> void image_encoders_free(ImageEncoders *enc)
> {
> +image_encoders_release_glz(enc);
> quic_destroy(enc->quic);
> enc->quic = NULL;
> lz_destroy(enc->lz);
> @@ -764,7 +766,7 @@ gboolean image_encoders_glz_create(ImageEncoders *enc,
> uint8_t id)
> }
>
> /* destroy encoder, and dictionary if no one uses it*/
> -void image_encoders_release_glz(ImageEncoders *enc)
> +static void image_encoders_release_glz(ImageEncoders *enc)
> {
> GlzSharedDictionary *shared_dict;
>
> diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
> index 9674465..bdfffe7 100644
> --- a/server/dcc-encoders.h
> +++ b/server/dcc-encoders.h
> @@ -46,7 +46,6 @@ int
> image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc);
> void image_encoders_free_glz_drawables(ImageEncoders *enc);
> void image_encoders_free_glz_drawables_to_free(ImageEncoders* enc);
> gboolean image_encoders_glz_create(ImageEncoders *enc, uint8_t id);
> -void image_encoders_release_glz(ImageEncoders *enc);
> void image_encoders_glz_free_from_drawable(struct Drawable *drawable);
> void image_encoders_glz_detach_from_drawable(struct Drawable *drawable);
> void image_encoders_glz_get_restore_data(ImageEncoders *enc,
> diff --git a/server/dcc.c b/server/dcc.c
> index 6511ce5..e44feb7 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -489,7 +489,6 @@ void dcc_stop(DisplayChannelClient *dcc)
>
> pixmap_cache_unref(dcc->pixmap_cache);
> dcc->pixmap_cache = NULL;
> -image_encoders_release_glz(&dcc->encoders);
> dcc_palette_cache_reset(dcc);
> free(dcc->send_data.stream_outbuf);
> free(dcc->send_data.free_list.res);
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v4 08/19] Encapsulate code to save glz state
Acked-by: Jonathon Jongsma
On Tue, 2016-06-14 at 10:33 +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio
> ---
> server/dcc-encoders.c | 10 ++
> server/dcc-encoders.h | 2 ++
> server/dcc-send.c | 8 ++--
> 3 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index 13dbff5..2e5984a 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -640,6 +640,16 @@ void image_encoders_freeze_glz(ImageEncoders *enc)
> pthread_rwlock_unlock(&enc->glz_dict->encode_lock);
> }
>
> +void image_encoders_glz_get_restore_data(ImageEncoders *enc,
> + uint8_t *out_id,
> GlzEncDictRestoreData *out_data)
> +{
> +spice_assert(enc->glz_dict);
> +image_encoders_freeze_glz(enc);
> +*out_id = enc->glz_dict->id;
> +glz_enc_dictionary_get_restore_data(enc->glz_dict->dict, out_data,
> +&enc->glz_data.usr);
> +}
> +
> static GlzSharedDictionary *glz_shared_dictionary_new(RedClient *client,
> uint8_t id,
> GlzEncDictContext
> *dict)
> {
> diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
> index 68fd5a5..ce8010c 100644
> --- a/server/dcc-encoders.h
> +++ b/server/dcc-encoders.h
> @@ -51,6 +51,8 @@ void image_encoders_freeze_glz(ImageEncoders *enc);
> void image_encoders_release_glz(ImageEncoders *enc);
> void image_encoders_glz_free_from_drawable(struct Drawable *drawable);
> void image_encoders_glz_detach_from_drawable(struct Drawable *drawable);
> +void image_encoders_glz_get_restore_data(ImageEncoders *enc,
> + uint8_t *out_id,
> GlzEncDictRestoreData *out_data);
>
> #define RED_COMPRESS_BUF_SIZE (1024 * 64)
> struct RedCompressBuf {
> diff --git a/server/dcc-send.c b/server/dcc-send.c
> index c0c7573..ba695ef 100644
> --- a/server/dcc-send.c
> +++ b/server/dcc-send.c
> @@ -1859,12 +1859,8 @@ static void
> display_channel_marshall_migrate_data(RedChannelClient *rcc,
> memcpy(display_data.pixmap_cache_clients, dcc->pixmap_cache->sync,
> sizeof(display_data.pixmap_cache_clients));
>
> -spice_assert(dcc->encoders.glz_dict);
> -image_encoders_freeze_glz(&dcc->encoders);
> -display_data.glz_dict_id = dcc->encoders.glz_dict->id;
> -glz_enc_dictionary_get_restore_data(dcc->encoders.glz_dict->dict,
> -&display_data.glz_dict_data,
> -&dcc->encoders.glz_data.usr);
> +image_encoders_glz_get_restore_data(&dcc->encoders,
> &display_data.glz_dict_id,
> +&display_data.glz_dict_data);
>
> /* all data besided the surfaces ref */
> spice_marshaller_add(base_marshaller,
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [streaming v16 00/23] Add GStreamer support for video streaming
On 06/14/2016 10:24 AM, Christophe Fergeau wrote: > Hey, > > So I've finally pushed all of the spice-server bits save for the > patch trying to guess the number of CPUs to use for the vp8 encoder > (hopefully I did not introduce rebase issues). > > Thanks a lot (again) for all the work, and sorry for the time it took to > get this in :( Hurray! Thanks, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk PATCH] Handle single headed monitors that have a non-zero x, y config offset
Hi On Tue, Jun 14, 2016 at 10:46 AM, Christophe Fergeau wrote: >> Imho, the fix should be on server or guest/driver side, it shouldn't >> use a display monitor config with a scanout offset if there is none to >> be applied by the client. > > Regarding fixing this on the server, isn't it going to have similar > issues? Ie once we get QXL_IO_MONITORS_CONFIG_ASYNC in QEMU, we could > make sure that display channels with just one monitor will get (0, 0) as > their primary position, but this is not correct with virgl, so we'd need > some gl check in the server before doing the zero-ing out? virtio-gpu/virgl do not use QXL_IO_MONITORS (or any QXL io/device). It's not an issue if a QXL driver want to use a single monitor with a position different from +0+0 imho. Apparently, this change was made by Sandy to fix #1202419, although I fail to reproduce the problem (I am still using older drivers/agents). I think we can just use the solution I proposed for the virgl case, until someone else is hurt by this again and we can find a proper solution. -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [streaming v16 00/23] Add GStreamer support for video streaming
Great job both of you! Frediano > > Hey, > > So I've finally pushed all of the spice-server bits save for the > patch trying to guess the number of CPUs to use for the vp8 encoder > (hopefully I did not introduce rebase issues). > > Thanks a lot (again) for all the work, and sorry for the time it took to > get this in :( > > Christophe > > > On Tue, Jun 07, 2016 at 03:57:21PM +0200, Francois Gouget wrote: > > > > This patch series adds support for using GStreamer to encode the video > > streams, adding support for VP8 and h264 codecs. > > > > As before the patches can also be grabbed from the gst branch of the > > repositories below. Note that this time around the required support is > > already present in the client. > > > > spice: https://github.com/fgouget/spice > > xf86-video-qxl: https://github.com/fgouget/xf86-video-qxl > > spice-protocol: https://github.com/fgouget/spice-protocol > > > > > > There's also 'extras' branches with more experimental/future patches for > > the curious, and gst-vNN branches to help comparing the various > > revisions. > > > > Note that to test GStreamer support with QEMU you probably need to grab > > the patch below and set the video-codecs option to something like > > gstreamer:h264 or gstreamer:vp8. > > > > https://lists.freedesktop.org/archives/spice-devel/2015-May/019771.html > > > > > > Changes from v15: > > * Added a patch to better check for sized frames. > > > > * Added a patch to get rid of RedDrawable.sized_stream to simplify the > > code. > > > > > > Changes from v14: > > > > * In this round the width / height removal is back because I think it > > is relevant to the discussion around the gstreamer-encoder code for > > copying the frames. > > > > * I also split that change in two parts : one removing the use of the > > width / height parameters in the MJPEG encoder, and another removing > > them from the video-encoder API. > > > > * Added a comment and tweaked the warning in > > is_chunk_stride_aligned(). > > > > * Added a comment in line_copy() wrt 0-byte chunks. > > > > * Renamed max_mem to max_block_count in zero_copy(). > > > > * I also changed the patchset prefix to 'streaming' as suggested by > > Pavel Grunt. > > > > > > Changes from v13: > > > > Rebased + some changes suggested by Christophe Fergeau. > > > > > > -- > > Francois Gouget > > ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [streaming v16 00/23] Add GStreamer support for video streaming
Hey, So I've finally pushed all of the spice-server bits save for the patch trying to guess the number of CPUs to use for the vp8 encoder (hopefully I did not introduce rebase issues). Thanks a lot (again) for all the work, and sorry for the time it took to get this in :( Christophe On Tue, Jun 07, 2016 at 03:57:21PM +0200, Francois Gouget wrote: > > This patch series adds support for using GStreamer to encode the video > streams, adding support for VP8 and h264 codecs. > > As before the patches can also be grabbed from the gst branch of the > repositories below. Note that this time around the required support is > already present in the client. > > spice: https://github.com/fgouget/spice > xf86-video-qxl: https://github.com/fgouget/xf86-video-qxl > spice-protocol: https://github.com/fgouget/spice-protocol > > > There's also 'extras' branches with more experimental/future patches for > the curious, and gst-vNN branches to help comparing the various > revisions. > > Note that to test GStreamer support with QEMU you probably need to grab > the patch below and set the video-codecs option to something like > gstreamer:h264 or gstreamer:vp8. > > https://lists.freedesktop.org/archives/spice-devel/2015-May/019771.html > > > Changes from v15: > * Added a patch to better check for sized frames. > > * Added a patch to get rid of RedDrawable.sized_stream to simplify the > code. > > > Changes from v14: > > * In this round the width / height removal is back because I think it > is relevant to the discussion around the gstreamer-encoder code for > copying the frames. > > * I also split that change in two parts : one removing the use of the > width / height parameters in the MJPEG encoder, and another removing > them from the video-encoder API. > > * Added a comment and tweaked the warning in > is_chunk_stride_aligned(). > > * Added a comment in line_copy() wrt 0-byte chunks. > > * Renamed max_mem to max_block_count in zero_copy(). > > * I also changed the patchset prefix to 'streaming' as suggested by > Pavel Grunt. > > > Changes from v13: > > Rebased + some changes suggested by Christophe Fergeau. > > > -- > Francois Gouget > > ___ > 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 v4 09/19] Make some function static
Acked-by: Jonathon Jongsma
On Tue, 2016-06-14 at 10:33 +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio
> ---
> server/dcc-encoders.c | 4 ++--
> server/dcc-encoders.h | 2 --
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index 2e5984a..f423f22 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -523,7 +523,7 @@ static void
> image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
> * if possible.
> * NOTE - the caller should prevent encoding using the dictionary during this
> operation
> */
> -void image_encoders_free_glz_drawable(ImageEncoders *enc, RedGlzDrawable
> *drawable)
> +static void image_encoders_free_glz_drawable(ImageEncoders *enc,
> RedGlzDrawable *drawable)
> {
> RingItem *head_instance = ring_get_head(&drawable->instances);
> int cont = (head_instance != NULL);
> @@ -633,7 +633,7 @@ void image_encoders_glz_detach_from_drawable(struct
> Drawable *drawable)
> }
> }
>
> -void image_encoders_freeze_glz(ImageEncoders *enc)
> +static void image_encoders_freeze_glz(ImageEncoders *enc)
> {
> pthread_rwlock_wrlock(&enc->glz_dict->encode_lock);
> enc->glz_dict->migrate_freeze = TRUE;
> diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
> index ce8010c..9674465 100644
> --- a/server/dcc-encoders.h
> +++ b/server/dcc-encoders.h
> @@ -42,12 +42,10 @@ void image_encoder_shared_stat_print(const
> ImageEncoderSharedData *shared_data);
>
> void image_encoders_init(ImageEncoders *enc, ImageEncoderSharedData
> *shared_data);
> void image_encoders_free(ImageEncoders *enc);
> -void image_encoders_free_glz_drawable(ImageEncoders *enc, RedGlzDrawable
> *drawable);
> int image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc);
> void image_encoders_free_glz_drawables(ImageEncoders *enc);
> void image_encoders_free_glz_drawables_to_free(ImageEncoders* enc);
> gboolean image_encoders_glz_create(ImageEncoders *enc, uint8_t id);
> -void image_encoders_freeze_glz(ImageEncoders *enc);
> void image_encoders_release_glz(ImageEncoders *enc);
> void image_encoders_glz_free_from_drawable(struct Drawable *drawable);
> void image_encoders_glz_detach_from_drawable(struct Drawable *drawable);
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 2/2] Rename image_encoders_free_glz_drawable()
On Tue, 2016-06-14 at 10:14 -0500, Jonathon Jongsma wrote:
> Rename this function to red_glz_drawable_free() and remove the
> ImageEncoders argument since the RedGlzDrawable already holds a pointer
> to the ImageEncoders structure
> ---
> server/dcc-encoders.c | 17 +
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index 32908d0..4049eec 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -534,14 +534,15 @@ static void
> glz_drawable_instance_item_free(GlzDrawableInstanceItem *instance)
> * if possible.
> * NOTE - the caller should prevent encoding using the dictionary during this
> operation
> */
> -static void image_encoders_free_glz_drawable(ImageEncoders *enc,
> RedGlzDrawable *drawable)
> +static void red_glz_drawable_free(RedGlzDrawable *glz_drawable)
I renamed the argument from drawable to glz_drawable to avoid confusion between
Drawable and RedGlzDrawable types, but perhaps that change should not be
included in this patch as it's unrelated the the main goal of the patch.
> {
> -RingItem *head_instance = ring_get_head(&drawable->instances);
> +ImageEncoders *enc = glz_drawable->encoders;
> +RingItem *head_instance = ring_get_head(&glz_drawable->instances);
> int cont = (head_instance != NULL);
>
> while (cont) {
> -if (drawable->instances_count == 1) {
> -/* Last instance: image_encoders_free_glz_drawable_instance will
> free the drawable */
> +if (glz_drawable->instances_count == 1) {
> +/* Last instance: glz_drawable_instance_item_free will free the
> glz_drawable */
> cont = FALSE;
> }
> GlzDrawableInstanceItem *instance = SPICE_CONTAINEROF(head_instance,
> @@ -556,7 +557,7 @@ static void image_encoders_free_glz_drawable(ImageEncoders
> *enc, RedGlzDrawable
> glz_drawable_instance_item_free(instance);
>
> if (cont) {
> -head_instance = ring_get_head(&drawable->instances);
> +head_instance = ring_get_head(&glz_drawable->instances);
> }
> }
> }
> @@ -593,7 +594,7 @@ int
> image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc)
> RedGlzDrawable *glz_drawable = SPICE_CONTAINEROF(ring_link,
> RedGlzDrawable, link);
> ring_link = ring_next(&enc->glz_drawables, ring_link);
> if (!glz_drawable->has_drawable) {
> -image_encoders_free_glz_drawable(enc, glz_drawable);
> +red_glz_drawable_free(glz_drawable);
> n++;
> }
> }
> @@ -634,7 +635,7 @@ void image_encoders_free_glz_drawables(ImageEncoders *enc)
> RedGlzDrawable *drawable = SPICE_CONTAINEROF(ring_link,
> RedGlzDrawable, link);
> // no need to lock the to_free list, since we assured no other thread
> is encoding and
> // thus not other thread access the to_free list of the channel
> -image_encoders_free_glz_drawable(enc, drawable);
> +red_glz_drawable_free(drawable);
> }
> pthread_rwlock_unlock(&glz_dict->encode_lock);
> }
> @@ -644,7 +645,7 @@ void image_encoders_glz_free_from_drawable_ring(Ring
> *drawable_ring)
> RingItem *glz_item, *next_item;
> RedGlzDrawable *glz;
> SAFE_FOREACH(glz_item, next_item, TRUE, drawable_ring, glz,
> LINK_TO_GLZ(glz_item)) {
> -image_encoders_free_glz_drawable(glz->encoders, glz);
> +red_glz_drawable_free(glz);
> }
> }
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 2/2] Rename image_encoders_free_glz_drawable()
Rename this function to red_glz_drawable_free() and remove the
ImageEncoders argument since the RedGlzDrawable already holds a pointer
to the ImageEncoders structure
---
server/dcc-encoders.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index 32908d0..4049eec 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -534,14 +534,15 @@ static void
glz_drawable_instance_item_free(GlzDrawableInstanceItem *instance)
* if possible.
* NOTE - the caller should prevent encoding using the dictionary during this
operation
*/
-static void image_encoders_free_glz_drawable(ImageEncoders *enc,
RedGlzDrawable *drawable)
+static void red_glz_drawable_free(RedGlzDrawable *glz_drawable)
{
-RingItem *head_instance = ring_get_head(&drawable->instances);
+ImageEncoders *enc = glz_drawable->encoders;
+RingItem *head_instance = ring_get_head(&glz_drawable->instances);
int cont = (head_instance != NULL);
while (cont) {
-if (drawable->instances_count == 1) {
-/* Last instance: image_encoders_free_glz_drawable_instance will
free the drawable */
+if (glz_drawable->instances_count == 1) {
+/* Last instance: glz_drawable_instance_item_free will free the
glz_drawable */
cont = FALSE;
}
GlzDrawableInstanceItem *instance = SPICE_CONTAINEROF(head_instance,
@@ -556,7 +557,7 @@ static void image_encoders_free_glz_drawable(ImageEncoders
*enc, RedGlzDrawable
glz_drawable_instance_item_free(instance);
if (cont) {
-head_instance = ring_get_head(&drawable->instances);
+head_instance = ring_get_head(&glz_drawable->instances);
}
}
}
@@ -593,7 +594,7 @@ int
image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc)
RedGlzDrawable *glz_drawable = SPICE_CONTAINEROF(ring_link,
RedGlzDrawable, link);
ring_link = ring_next(&enc->glz_drawables, ring_link);
if (!glz_drawable->has_drawable) {
-image_encoders_free_glz_drawable(enc, glz_drawable);
+red_glz_drawable_free(glz_drawable);
n++;
}
}
@@ -634,7 +635,7 @@ void image_encoders_free_glz_drawables(ImageEncoders *enc)
RedGlzDrawable *drawable = SPICE_CONTAINEROF(ring_link,
RedGlzDrawable, link);
// no need to lock the to_free list, since we assured no other thread
is encoding and
// thus not other thread access the to_free list of the channel
-image_encoders_free_glz_drawable(enc, drawable);
+red_glz_drawable_free(drawable);
}
pthread_rwlock_unlock(&glz_dict->encode_lock);
}
@@ -644,7 +645,7 @@ void image_encoders_glz_free_from_drawable_ring(Ring
*drawable_ring)
RingItem *glz_item, *next_item;
RedGlzDrawable *glz;
SAFE_FOREACH(glz_item, next_item, TRUE, drawable_ring, glz,
LINK_TO_GLZ(glz_item)) {
-image_encoders_free_glz_drawable(glz->encoders, glz);
+red_glz_drawable_free(glz);
}
}
--
2.5.5
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 1/2] Rename image_encoders_free_glz_drawable_instance()
Rename this function to glz_drawable_instance_item_free() and remove the
ImageEncoders argument since the RedGlzDrawable already holds a pointer
to the ImageEncoders structure.
---
server/dcc-encoders.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index e18dadb..32908d0 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -72,8 +72,7 @@ struct RedGlzDrawable {
#define DRAWABLE_FOREACH_GLZ_SAFE(drawable, link, next, glz) \
SAFE_FOREACH(link, next, drawable, &(drawable)->glz_ring, glz,
LINK_TO_GLZ(link))
-static void image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
- GlzDrawableInstanceItem
*instance);
+static void glz_drawable_instance_item_free(GlzDrawableInstanceItem *instance);
static void encoder_data_init(EncoderData *data);
static void encoder_data_reset(EncoderData *data);
static void image_encoders_release_glz(ImageEncoders *enc);
@@ -384,7 +383,7 @@ static void glz_usr_free_image(GlzEncoderUsrContext *usr,
GlzUsrImageContext *im
ImageEncoders *drawable_enc =
glz_drawable_instance->glz_drawable->encoders;
ImageEncoders *this_enc = SPICE_CONTAINEROF(lz_data, ImageEncoders,
glz_data);
if (this_enc == drawable_enc) {
-image_encoders_free_glz_drawable_instance(drawable_enc,
glz_drawable_instance);
+glz_drawable_instance_item_free(glz_drawable_instance);
} else {
/* The glz dictionary is shared between all DisplayChannelClient
* instances that belong to the same client, and glz_usr_free_image
@@ -494,8 +493,7 @@ void image_encoders_free(ImageEncoders *enc)
it is not used by Drawable).
NOTE - 1) can be called only by the display channel that created the
drawable
2) it is assumed that the instance was already removed from the
dictionary*/
-static void image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
- GlzDrawableInstanceItem
*instance)
+static void glz_drawable_instance_item_free(GlzDrawableInstanceItem *instance)
{
RedGlzDrawable *glz_drawable;
@@ -504,7 +502,6 @@ static void
image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
glz_drawable = instance->glz_drawable;
-spice_assert(glz_drawable->encoders == enc);
spice_assert(glz_drawable->instances_count > 0);
ring_remove(&instance->glz_link);
@@ -523,7 +520,7 @@ static void
image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
ring_remove(&glz_drawable->drawable_link);
}
red_drawable_unref(glz_drawable->red_drawable);
-enc->shared_data->glz_drawable_count--;
+glz_drawable->encoders->shared_data->glz_drawable_count--;
if (ring_item_is_linked(&glz_drawable->link)) {
ring_remove(&glz_drawable->link);
}
@@ -556,7 +553,7 @@ static void image_encoders_free_glz_drawable(ImageEncoders
*enc, RedGlzDrawable
instance->context,
&enc->glz_data.usr);
}
-image_encoders_free_glz_drawable_instance(enc, instance);
+glz_drawable_instance_item_free(instance);
if (cont) {
head_instance = ring_get_head(&drawable->instances);
@@ -615,7 +612,7 @@ void
image_encoders_free_glz_drawables_to_free(ImageEncoders* enc)
GlzDrawableInstanceItem *drawable_instance =
SPICE_CONTAINEROF(ring_link,
GlzDrawableInstanceItem,
free_link);
-image_encoders_free_glz_drawable_instance(enc, drawable_instance);
+glz_drawable_instance_item_free(drawable_instance);
}
pthread_mutex_unlock(&enc->glz_drawables_inst_to_free_lock);
}
--
2.5.5
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 0/2] RFC: cleanup glz drawable stuff
These patches should apply on top of Frediano's "Better encapsulation of image encoding stuff" patch series. Jonathon Jongsma (2): Rename image_encoders_free_glz_drawable_instance() Rename image_encoders_free_glz_drawable() server/dcc-encoders.c | 32 +++- 1 file changed, 15 insertions(+), 17 deletions(-) -- 2.5.5 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v4 07/19] Encapsulate some data in dcc-encoders
On Tue, 2016-06-14 at 10:32 +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio
> ---
> server/dcc-encoders.c| 60 ++-
> -
> server/dcc-encoders.h| 33 ++
> server/display-channel.c | 13 +++
> server/display-channel.h | 5
> 4 files changed, 63 insertions(+), 48 deletions(-)
>
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index 4de780d..13dbff5 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -26,8 +26,45 @@
>
> #define ZLIB_DEFAULT_COMPRESSION_LEVEL 3
>
> +#define MAX_GLZ_DRAWABLE_INSTANCES 2
> +
> +typedef struct GlzDrawableInstanceItem GlzDrawableInstanceItem;
> +
> +/* for each qxl drawable, there may be several instances of lz drawables */
> +/* TODO - reuse this stuff for the top level. I just added a second level of
> multiplicity
> + * at the Drawable by keeping a ring, so:
> + * Drawable -> (ring of) RedGlzDrawable -> (up to 2) GlzDrawableInstanceItem
> + * and it should probably (but need to be sure...) be
> + * Drawable -> ring of GlzDrawableInstanceItem.
> + */
> +struct GlzDrawableInstanceItem {
> +RingItem glz_link;
> +RingItem free_link;
> +GlzEncDictImageContext *context;
> +RedGlzDrawable *glz_drawable;
> +};
> +
> +struct RedGlzDrawable {
> +RingItem link;// ordered by the time it was encoded
> +RingItem drawable_link;
> +RedDrawable *red_drawable;
> +struct Drawable*drawable;
> +GlzDrawableInstanceItem instances_pool[MAX_GLZ_DRAWABLE_INSTANCES];
> +Ring instances;
> +uint8_t instances_count;
> +ImageEncoders *encoders;
> +};
> +
> +#define LINK_TO_GLZ(ptr) SPICE_CONTAINEROF((ptr), RedGlzDrawable, \
> + drawable_link)
> +#define DRAWABLE_FOREACH_GLZ_SAFE(drawable, link, next, glz) \
> +SAFE_FOREACH(link, next, drawable, &(drawable)->glz_ring, glz,
> LINK_TO_GLZ(link))
> +
> static void image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
> GlzDrawableInstanceItem
> *instance);
> +static void encoder_data_init(EncoderData *data);
> +static void encoder_data_reset(EncoderData *data);
> +
>
> static SPICE_GNUC_NORETURN SPICE_GNUC_PRINTF(2, 3) void
> quic_usr_error(QuicUsrContext *usr, const char *fmt, ...)
> @@ -139,14 +176,14 @@ static void glz_usr_free(GlzEncoderUsrContext *usr, void
> *ptr)
> free(ptr);
> }
>
> -void encoder_data_init(EncoderData *data)
> +static void encoder_data_init(EncoderData *data)
> {
> data->bufs_tail = g_new(RedCompressBuf, 1);
> data->bufs_head = data->bufs_tail;
> data->bufs_head->send_next = NULL;
> }
>
> -void encoder_data_reset(EncoderData *data)
> +static void encoder_data_reset(EncoderData *data)
> {
> RedCompressBuf *buf = data->bufs_head;
> while (buf) {
> @@ -577,6 +614,25 @@ void image_encoders_free_glz_drawables(ImageEncoders
> *enc)
> pthread_rwlock_unlock(&glz_dict->encode_lock);
> }
>
> +void image_encoders_glz_free_from_drawable(struct Drawable *drawable)
> +{
> +RingItem *glz_item, *next_item;
> +RedGlzDrawable *glz;
> +DRAWABLE_FOREACH_GLZ_SAFE(drawable, glz_item, next_item, glz) {
> +image_encoders_free_glz_drawable(glz->encoders, glz);
> +}
> +}
I find that the name of this function is not very clear. Basically, it is
freeing and removing all of the RedGlzDrawables attached to the given Drawable.
In my opinion, the fact that ImageEncoders is used internally should not
necessarily be relevant to the name of the function. I think something like
drawable_free_glz_drawables(Drawable*) would be a lot easier to understand.
Digging a bit deeper, it seems that the only reason that
image_encoders_free_glz_drawable needs ImageEncoders here is to update the glz
dictionary and decrement glz_drawable_count. I'll send a couple additional
patches that refactor this to remove the ImageEncoders parameter from this
function.
> +
> +void image_encoders_glz_detach_from_drawable(struct Drawable *drawable)
> +{
> +RingItem *item, *next;
> +
> +RING_FOREACH_SAFE(item, next, &drawable->glz_ring) {
> +SPICE_CONTAINEROF(item, RedGlzDrawable, drawable_link)->drawable =
> NULL;
> +ring_remove(item);
> +}
> +}
Here again, I think that the image_encoders_ function prefix confuses
things slightly. The ImageEncoders structure is not used at all in this
function, even internally. So, I think a more fitting name might be something
like drawable_detach_glz_drawables(Drawable*)...
(Long-term, I think the 'Drawable' type badly needs to be renamed as there's
just way too much confusion between Drawable, QXLDrawable, RedGlzDrawable,
RedDrawable, etc. It's impossible for me to keep straight which struct is for
what.)
Also, it's not clear why this function uses RING_FOREACH_SAFE() when the
previous one uses DRAWABLE_FOREACH_GLZ_SAFE(). I realize that this w
Re: [Spice-devel] [PATCH] Remove a warning compiling under Windows
Acked-by: Jonathon Jongsma
On Mon, 2016-06-13 at 12:31 +0100, Frediano Ziglio wrote:
> On Windows long is always 32 bit so under x64 the cast from pointer to
> "unsigned long" cause a warning.
>
> Signed-off-by: Frediano Ziglio
> ---
> common/quic.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/common/quic.c b/common/quic.c
> index f014a2a..5b00d65 100644
> --- a/common/quic.c
> +++ b/common/quic.c
> @@ -1073,7 +1073,7 @@ static int init_encoder(Encoder *encoder, QuicUsrContext
> *usr)
>
> static int encoder_reste(Encoder *encoder, uint32_t *io_ptr, uint32_t
> *io_ptr_end)
> {
> -spice_assert(((unsigned long)io_ptr % 4) == ((unsigned long)io_ptr_end %
> 4));
> +spice_assert(((uintptr_t)io_ptr % 4) == ((uintptr_t)io_ptr_end % 4));
> spice_assert(io_ptr <= io_ptr_end);
>
> encoder->rgb_state.waitcnt = 0;
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v4 03/19] Change dcc_encoders_init to take ImageEncoders instead of DisplayChannelClient
Acked-by: Jonathon Jongsma
On Tue, 2016-06-14 at 10:32 +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio
> ---
> server/dcc-encoders.c | 4 +---
> server/dcc-encoders.h | 2 +-
> server/dcc.c | 2 +-
> 3 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index c03409c..7497029 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -398,10 +398,8 @@ static void image_encoders_init_zlib(ImageEncoders *enc)
> }
> }
>
> -void dcc_encoders_init(DisplayChannelClient *dcc, ImageEncoderSharedData
> *shared_data)
> +void image_encoders_init(ImageEncoders *enc, ImageEncoderSharedData
> *shared_data)
> {
> -ImageEncoders *enc = &dcc->encoders;
> -
> spice_assert(shared_data);
> enc->shared_data = shared_data;
>
> diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
> index 596429c..2b4d556 100644
> --- a/server/dcc-encoders.h
> +++ b/server/dcc-encoders.h
> @@ -41,7 +41,7 @@ void image_encoder_shared_init(ImageEncoderSharedData
> *shared_data);
> void image_encoder_shared_stat_reset(ImageEncoderSharedData *shared_data);
> void image_encoder_shared_stat_print(const ImageEncoderSharedData
> *shared_data);
>
> -void dcc_encoders_init(DisplayChannelClient *dcc, ImageEncoderSharedData
> *shared_data);
> +void image_encoders_init(ImageEncoders *enc, ImageEncoderSharedData
> *shared_data);
> void image_encoders_free(ImageEncoders *enc);
> void dcc_free_glz_drawable (DisplayChannelC
> lient *dcc,
> RedGlzDrawable
> *drawable);
> diff --git a/server/dcc.c b/server/dcc.c
> index d73b742..1cc85b5 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -396,7 +396,7 @@ DisplayChannelClient *dcc_new(DisplayChannel *display,
> ring_init(&dcc->glz_drawables_inst_to_free);
> pthread_mutex_init(&dcc->glz_drawables_inst_to_free_lock, NULL);
>
> -dcc_encoders_init(dcc, &display->encoder_globals);
> +image_encoders_init(&dcc->encoders, &display->encoder_globals);
>
> return dcc;
> }
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v4 01/19] Add a structure to hold ImageEncoders shared data
Acked-by: Jonathon Jongsma
On Tue, 2016-06-14 at 10:32 +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio
> ---
> server/dcc-encoders.c| 121 +++---
> -
> server/dcc-encoders.h| 33 +
> server/dcc.c | 26 +-
> server/display-channel.c | 73 ++--
> server/display-channel.h | 9 +---
> 5 files changed, 141 insertions(+), 121 deletions(-)
>
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index 6aaf954..c79cf58 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -398,10 +398,13 @@ static void image_encoders_init_zlib(ImageEncoders *enc)
> }
> }
>
> -void dcc_encoders_init(DisplayChannelClient *dcc)
> +void dcc_encoders_init(DisplayChannelClient *dcc, ImageEncoderSharedData
> *shared_data)
> {
> ImageEncoders *enc = &dcc->encoders;
>
> +spice_assert(shared_data);
> +enc->shared_data = shared_data;
> +
> dcc_init_glz_data(dcc);
> image_encoders_init_quic(enc);
> image_encoders_init_lz(enc);
> @@ -708,15 +711,14 @@ void dcc_release_glz(DisplayChannelClient *dcc)
> }
>
> int image_encoders_compress_quic(ImageEncoders *enc, SpiceImage *dest,
> - SpiceBitmap *src, compress_send_data_t*
> o_comp_data,
> - stat_info_t *stats)
> + SpiceBitmap *src, compress_send_data_t*
> o_comp_data)
> {
> QuicData *quic_data = &enc->quic_data;
> QuicContext *quic = enc->quic;
> volatile QuicImageType type;
> int size, stride;
> stat_start_time_t start_time;
> -stat_start_time_init(&start_time, stats);
> +stat_start_time_init(&start_time, &enc->shared_data->quic_stat);
>
> #ifdef COMPRESS_DEBUG
> spice_info("QUIC compress");
> @@ -776,7 +778,7 @@ int image_encoders_compress_quic(ImageEncoders *enc,
> SpiceImage *dest,
> o_comp_data->comp_buf = quic_data->data.bufs_head;
> o_comp_data->comp_buf_size = size << 2;
>
> -stat_compress_add(stats, start_time, src->stride * src->y,
> +stat_compress_add(&enc->shared_data->quic_stat, start_time, src->stride *
> src->y,
> o_comp_data->comp_buf_size);
> return TRUE;
> }
> @@ -797,8 +799,7 @@ static const LzImageType bitmap_fmt_to_lz_image_type[] = {
>
> int image_encoders_compress_lz(ImageEncoders *enc,
> SpiceImage *dest, SpiceBitmap *src,
> - compress_send_data_t* o_comp_data,
> - stat_info_t *stats)
> + compress_send_data_t* o_comp_data)
> {
> LzData *lz_data = &enc->lz_data;
> LzContext *lz = enc->lz;
> @@ -806,7 +807,7 @@ int image_encoders_compress_lz(ImageEncoders *enc,
> int size;// size of the compressed data
>
> stat_start_time_t start_time;
> -stat_start_time_init(&start_time, stats);
> +stat_start_time_init(&start_time, &enc->shared_data->lz_stat);
>
> #ifdef COMPRESS_DEBUG
> spice_info("LZ LOCAL compress");
> @@ -856,15 +857,13 @@ int image_encoders_compress_lz(ImageEncoders *enc,
> o_comp_data->lzplt_palette = dest->u.lz_plt.palette;
> }
>
> -stat_compress_add(stats, start_time, src->stride * src->y,
> +stat_compress_add(&enc->shared_data->lz_stat, start_time, src->stride *
> src->y,
> o_comp_data->comp_buf_size);
> return TRUE;
> }
>
> int image_encoders_compress_jpeg(ImageEncoders *enc, SpiceImage *dest,
> - SpiceBitmap *src, compress_send_data_t*
> o_comp_data,
> - stat_info_t *jpeg_stats, // FIXME put all
> stats in a structure
> - stat_info_t *jpeg_alpha_stats)
> + SpiceBitmap *src, compress_send_data_t*
> o_comp_data)
> {
> JpegData *jpeg_data = &enc->jpeg_data;
> LzData *lz_data = &enc->lz_data;
> @@ -879,7 +878,7 @@ int image_encoders_compress_jpeg(ImageEncoders *enc,
> SpiceImage *dest,
> int stride;
> uint8_t *lz_out_start_byte;
> stat_start_time_t start_time;
> -stat_start_time_init(&start_time, jpeg_alpha_stats);
> +stat_start_time_init(&start_time, &enc->shared_data->jpeg_alpha_stat);
>
> #ifdef COMPRESS_DEBUG
> spice_info("JPEG compress");
> @@ -943,7 +942,7 @@ int image_encoders_compress_jpeg(ImageEncoders *enc,
> SpiceImage *dest,
> o_comp_data->comp_buf_size = jpeg_size;
> o_comp_data->is_lossy = TRUE;
>
> -stat_compress_add(jpeg_stats, start_time, src->stride * src->y,
> +stat_compress_add(&enc->shared_data->jpeg_stat, start_time, src-
> >stride * src->y,
> o_comp_data->comp_buf_size);
> return TRUE;
> }
> @@ -983,21 +982,20 @@ int image_encoders_compress_jpeg(ImageEncoders *enc,
> SpiceImage *dest,
> o_c
[Spice-devel] [spice-gtk] main: channel-main to increase file-transfer reference
This is a minor fix in the logic as in both situations (with or without the patch) the reference count for the SpiceFileTransferTask object is the same. The change is interesting as SpiceFileTransferTask is created but on g_file_read_async() it increases its reference count while c->file_xfer_tasks keeps the original one. It should be the other way around. --- src/channel-main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/channel-main.c b/src/channel-main.c index 4736e13..7731943 100644 --- a/src/channel-main.c +++ b/src/channel-main.c @@ -3112,7 +3112,7 @@ static void file_xfer_send_start_msg_async(SpiceMainChannel *channel, CHANNEL_DEBUG(channel, "Insert a xfer task:%u to task list", task->id); g_hash_table_insert(c->file_xfer_tasks, GUINT_TO_POINTER(task->id), -task); +g_object_ref(task)); g_signal_connect(task, "finished", G_CALLBACK(task_finished), channel); g_signal_emit(channel, signals[SPICE_MAIN_NEW_FILE_TRANSFER], 0, task); @@ -3120,7 +3120,7 @@ static void file_xfer_send_start_msg_async(SpiceMainChannel *channel, G_PRIORITY_DEFAULT, cancellable, file_xfer_read_async_cb, - g_object_ref(task)); + task); task->pending = TRUE; /* if we created a per-task cancellable above, free it */ -- 2.5.5 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk] main: assign variable after check for null
On Tue, 2016-06-14 at 14:28 +0200, Victor Toso wrote:
Acked-by: Pavel Grunt
> ---
> src/channel-main.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/channel-main.c b/src/channel-main.c
> index 89675d5..e7171c1 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -3181,12 +3181,13 @@ void spice_main_file_copy_async(SpiceMainChannel
> *channel,
> GAsyncReadyCallback callback,
> gpointer user_data)
> {
> -SpiceMainChannelPrivate *c = channel->priv;
> +SpiceMainChannelPrivate *c;
>
> g_return_if_fail(channel != NULL);
> g_return_if_fail(SPICE_IS_MAIN_CHANNEL(channel));
> g_return_if_fail(sources != NULL);
>
> +c = channel->priv;
> if (!c->agent_connected) {
> g_task_report_new_error(channel,
> callback,
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk] main: assign variable after check for null
---
src/channel-main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/channel-main.c b/src/channel-main.c
index 89675d5..e7171c1 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -3181,12 +3181,13 @@ void spice_main_file_copy_async(SpiceMainChannel
*channel,
GAsyncReadyCallback callback,
gpointer user_data)
{
-SpiceMainChannelPrivate *c = channel->priv;
+SpiceMainChannelPrivate *c;
g_return_if_fail(channel != NULL);
g_return_if_fail(SPICE_IS_MAIN_CHANNEL(channel));
g_return_if_fail(sources != NULL);
+c = channel->priv;
if (!c->agent_connected) {
g_task_report_new_error(channel,
callback,
--
2.5.5
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 10/19] Encapsulate dcc_release_glz
Signed-off-by: Frediano Ziglio
---
server/dcc-encoders.c | 4 +++-
server/dcc-encoders.h | 1 -
server/dcc.c | 1 -
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index f423f22..6668dff 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -64,6 +64,7 @@ static void
image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
GlzDrawableInstanceItem
*instance);
static void encoder_data_init(EncoderData *data);
static void encoder_data_reset(EncoderData *data);
+static void image_encoders_release_glz(ImageEncoders *enc);
static SPICE_GNUC_NORETURN SPICE_GNUC_PRINTF(2, 3) void
@@ -459,6 +460,7 @@ void image_encoders_init(ImageEncoders *enc,
ImageEncoderSharedData *shared_data
void image_encoders_free(ImageEncoders *enc)
{
+image_encoders_release_glz(enc);
quic_destroy(enc->quic);
enc->quic = NULL;
lz_destroy(enc->lz);
@@ -764,7 +766,7 @@ gboolean image_encoders_glz_create(ImageEncoders *enc,
uint8_t id)
}
/* destroy encoder, and dictionary if no one uses it*/
-void image_encoders_release_glz(ImageEncoders *enc)
+static void image_encoders_release_glz(ImageEncoders *enc)
{
GlzSharedDictionary *shared_dict;
diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
index 9674465..bdfffe7 100644
--- a/server/dcc-encoders.h
+++ b/server/dcc-encoders.h
@@ -46,7 +46,6 @@ int
image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc);
void image_encoders_free_glz_drawables(ImageEncoders *enc);
void image_encoders_free_glz_drawables_to_free(ImageEncoders* enc);
gboolean image_encoders_glz_create(ImageEncoders *enc, uint8_t id);
-void image_encoders_release_glz(ImageEncoders *enc);
void image_encoders_glz_free_from_drawable(struct Drawable *drawable);
void image_encoders_glz_detach_from_drawable(struct Drawable *drawable);
void image_encoders_glz_get_restore_data(ImageEncoders *enc,
diff --git a/server/dcc.c b/server/dcc.c
index 6511ce5..e44feb7 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -489,7 +489,6 @@ void dcc_stop(DisplayChannelClient *dcc)
pixmap_cache_unref(dcc->pixmap_cache);
dcc->pixmap_cache = NULL;
-image_encoders_release_glz(&dcc->encoders);
dcc_palette_cache_reset(dcc);
free(dcc->send_data.stream_outbuf);
free(dcc->send_data.free_list.res);
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 14/19] Use Ring instead of accessing Drawable internals
Remove some coupling, we mainly need to store a list of RedGlzDrawables.
Signed-off-by: Frediano Ziglio
---
server/dcc-encoders.c| 8
server/dcc-encoders.h| 4 ++--
server/display-channel.c | 4 ++--
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index 00349fe..a83794e 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -614,20 +614,20 @@ void image_encoders_free_glz_drawables(ImageEncoders *enc)
pthread_rwlock_unlock(&glz_dict->encode_lock);
}
-void image_encoders_glz_free_from_drawable(struct Drawable *drawable)
+void image_encoders_glz_free_from_drawable_ring(Ring *drawable_ring)
{
RingItem *glz_item, *next_item;
RedGlzDrawable *glz;
-DRAWABLE_FOREACH_GLZ_SAFE(drawable, glz_item, next_item, glz) {
+SAFE_FOREACH(glz_item, next_item, TRUE, drawable_ring, glz,
LINK_TO_GLZ(glz_item)) {
image_encoders_free_glz_drawable(glz->encoders, glz);
}
}
-void image_encoders_glz_detach_from_drawable(struct Drawable *drawable)
+void image_encoders_glz_detach_from_drawable_ring(Ring *drawable_ring)
{
RingItem *item, *next;
-RING_FOREACH_SAFE(item, next, &drawable->glz_ring) {
+RING_FOREACH_SAFE(item, next, drawable_ring) {
SPICE_CONTAINEROF(item, RedGlzDrawable, drawable_link)->has_drawable =
FALSE;
ring_remove(item);
}
diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
index bdfffe7..ac61a70 100644
--- a/server/dcc-encoders.h
+++ b/server/dcc-encoders.h
@@ -46,8 +46,8 @@ int
image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc);
void image_encoders_free_glz_drawables(ImageEncoders *enc);
void image_encoders_free_glz_drawables_to_free(ImageEncoders* enc);
gboolean image_encoders_glz_create(ImageEncoders *enc, uint8_t id);
-void image_encoders_glz_free_from_drawable(struct Drawable *drawable);
-void image_encoders_glz_detach_from_drawable(struct Drawable *drawable);
+void image_encoders_glz_free_from_drawable_ring(Ring *drawable_ring);
+void image_encoders_glz_detach_from_drawable_ring(Ring *drawable_ring);
void image_encoders_glz_get_restore_data(ImageEncoders *enc,
uint8_t *out_id,
GlzEncDictRestoreData *out_data);
diff --git a/server/display-channel.c b/server/display-channel.c
index f923be8..bddedd9 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1171,7 +1171,7 @@ static bool free_one_drawable(DisplayChannel *display,
int force_glz_free)
drawable = SPICE_CONTAINEROF(ring_item, Drawable, list_link);
if (force_glz_free) {
-image_encoders_glz_free_from_drawable(drawable);
+image_encoders_glz_free_from_drawable_ring(&drawable->glz_ring);
}
drawable_draw(display, drawable);
container = drawable->tree_item.base.container;
@@ -1340,7 +1340,7 @@ void drawable_unref(Drawable *drawable)
drawable_unref_surface_deps(display, drawable);
display_channel_surface_unref(display, drawable->surface_id);
-image_encoders_glz_detach_from_drawable(drawable);
+image_encoders_glz_detach_from_drawable_ring(&drawable->glz_ring);
if (drawable->red_drawable) {
red_drawable_unref(drawable->red_drawable);
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 07/19] Encapsulate some data in dcc-encoders
Signed-off-by: Frediano Ziglio
---
server/dcc-encoders.c| 60 ++--
server/dcc-encoders.h| 33 ++
server/display-channel.c | 13 +++
server/display-channel.h | 5
4 files changed, 63 insertions(+), 48 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index 4de780d..13dbff5 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -26,8 +26,45 @@
#define ZLIB_DEFAULT_COMPRESSION_LEVEL 3
+#define MAX_GLZ_DRAWABLE_INSTANCES 2
+
+typedef struct GlzDrawableInstanceItem GlzDrawableInstanceItem;
+
+/* for each qxl drawable, there may be several instances of lz drawables */
+/* TODO - reuse this stuff for the top level. I just added a second level of
multiplicity
+ * at the Drawable by keeping a ring, so:
+ * Drawable -> (ring of) RedGlzDrawable -> (up to 2) GlzDrawableInstanceItem
+ * and it should probably (but need to be sure...) be
+ * Drawable -> ring of GlzDrawableInstanceItem.
+ */
+struct GlzDrawableInstanceItem {
+RingItem glz_link;
+RingItem free_link;
+GlzEncDictImageContext *context;
+RedGlzDrawable *glz_drawable;
+};
+
+struct RedGlzDrawable {
+RingItem link;// ordered by the time it was encoded
+RingItem drawable_link;
+RedDrawable *red_drawable;
+struct Drawable*drawable;
+GlzDrawableInstanceItem instances_pool[MAX_GLZ_DRAWABLE_INSTANCES];
+Ring instances;
+uint8_t instances_count;
+ImageEncoders *encoders;
+};
+
+#define LINK_TO_GLZ(ptr) SPICE_CONTAINEROF((ptr), RedGlzDrawable, \
+ drawable_link)
+#define DRAWABLE_FOREACH_GLZ_SAFE(drawable, link, next, glz) \
+SAFE_FOREACH(link, next, drawable, &(drawable)->glz_ring, glz,
LINK_TO_GLZ(link))
+
static void image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
GlzDrawableInstanceItem
*instance);
+static void encoder_data_init(EncoderData *data);
+static void encoder_data_reset(EncoderData *data);
+
static SPICE_GNUC_NORETURN SPICE_GNUC_PRINTF(2, 3) void
quic_usr_error(QuicUsrContext *usr, const char *fmt, ...)
@@ -139,14 +176,14 @@ static void glz_usr_free(GlzEncoderUsrContext *usr, void
*ptr)
free(ptr);
}
-void encoder_data_init(EncoderData *data)
+static void encoder_data_init(EncoderData *data)
{
data->bufs_tail = g_new(RedCompressBuf, 1);
data->bufs_head = data->bufs_tail;
data->bufs_head->send_next = NULL;
}
-void encoder_data_reset(EncoderData *data)
+static void encoder_data_reset(EncoderData *data)
{
RedCompressBuf *buf = data->bufs_head;
while (buf) {
@@ -577,6 +614,25 @@ void image_encoders_free_glz_drawables(ImageEncoders *enc)
pthread_rwlock_unlock(&glz_dict->encode_lock);
}
+void image_encoders_glz_free_from_drawable(struct Drawable *drawable)
+{
+RingItem *glz_item, *next_item;
+RedGlzDrawable *glz;
+DRAWABLE_FOREACH_GLZ_SAFE(drawable, glz_item, next_item, glz) {
+image_encoders_free_glz_drawable(glz->encoders, glz);
+}
+}
+
+void image_encoders_glz_detach_from_drawable(struct Drawable *drawable)
+{
+RingItem *item, *next;
+
+RING_FOREACH_SAFE(item, next, &drawable->glz_ring) {
+SPICE_CONTAINEROF(item, RedGlzDrawable, drawable_link)->drawable =
NULL;
+ring_remove(item);
+}
+}
+
void image_encoders_freeze_glz(ImageEncoders *enc)
{
pthread_rwlock_wrlock(&enc->glz_dict->encode_lock);
diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
index 655c1b3..68fd5a5 100644
--- a/server/dcc-encoders.h
+++ b/server/dcc-encoders.h
@@ -32,7 +32,6 @@
#include "zlib-encoder.h"
typedef struct RedCompressBuf RedCompressBuf;
-typedef struct GlzDrawableInstanceItem GlzDrawableInstanceItem;
typedef struct RedGlzDrawable RedGlzDrawable;
typedef struct ImageEncoders ImageEncoders;
typedef struct ImageEncoderSharedData ImageEncoderSharedData;
@@ -50,6 +49,8 @@ void image_encoders_free_glz_drawables_to_free(ImageEncoders*
enc);
gboolean image_encoders_glz_create(ImageEncoders *enc, uint8_t id);
void image_encoders_freeze_glz(ImageEncoders *enc);
void image_encoders_release_glz(ImageEncoders *enc);
+void image_encoders_glz_free_from_drawable(struct Drawable *drawable);
+void image_encoders_glz_detach_from_drawable(struct Drawable *drawable);
#define RED_COMPRESS_BUF_SIZE (1024 * 64)
struct RedCompressBuf {
@@ -106,9 +107,6 @@ typedef struct {
char message_buf[512];
} EncoderData;
-void encoder_data_init(EncoderData *data);
-void encoder_data_reset(EncoderData *data);
-
typedef struct {
QuicUsrContext usr;
EncoderData data;
@@ -141,33 +139,6 @@ typedef struct {
EncoderData data;
} GlzData;
-#define MAX_GLZ_DRAWABLE_INSTANCES 2
-
-/* for each qxl drawable, there may be several instances of lz drawables */
-/* TODO - reuse this stuff for the top level. I just added a second level of
multiplicity
- * at the Draw
[Spice-devel] [PATCH v4 19/19] Remove message_buf from EncoderData
This buffer was just written and then used, no reason to store into
a more persistent structure.
Signed-off-by: Frediano Ziglio
---
server/dcc-encoders.c | 34 ++
server/dcc-encoders.h | 1 -
2 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index beacb30..e18dadb 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -84,11 +84,12 @@ quic_usr_error(QuicUsrContext *usr, const char *fmt, ...)
{
EncoderData *usr_data = &(((QuicData *)usr)->data);
va_list ap;
+char message_buf[512];
va_start(ap, fmt);
-vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt, ap);
+vsnprintf(message_buf, sizeof(message_buf), fmt, ap);
va_end(ap);
-spice_critical("%s", usr_data->message_buf);
+spice_critical("%s", message_buf);
longjmp(usr_data->jmp_env, 1);
}
@@ -98,11 +99,12 @@ lz_usr_error(LzUsrContext *usr, const char *fmt, ...)
{
EncoderData *usr_data = &(((LzData *)usr)->data);
va_list ap;
+char message_buf[512];
va_start(ap, fmt);
-vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt, ap);
+vsnprintf(message_buf, sizeof(message_buf), fmt, ap);
va_end(ap);
-spice_critical("%s", usr_data->message_buf);
+spice_critical("%s", message_buf);
longjmp(usr_data->jmp_env, 1);
}
@@ -110,14 +112,14 @@ lz_usr_error(LzUsrContext *usr, const char *fmt, ...)
static SPICE_GNUC_PRINTF(2, 3) void
glz_usr_error(GlzEncoderUsrContext *usr, const char *fmt, ...)
{
-EncoderData *usr_data = &(((GlzData *)usr)->data);
va_list ap;
+char message_buf[512];
va_start(ap, fmt);
-vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt, ap);
+vsnprintf(message_buf, sizeof(message_buf), fmt, ap);
va_end(ap);
-spice_critical("%s", usr_data->message_buf); // if global lz fails in the
middle
+spice_critical("%s", message_buf); // if global lz fails in the middle
// the consequences are not
predictable since the window
// can turn to be unsynchronized
between the server and
// and the client
@@ -126,37 +128,37 @@ glz_usr_error(GlzEncoderUsrContext *usr, const char *fmt,
...)
static SPICE_GNUC_PRINTF(2, 3) void
quic_usr_warn(QuicUsrContext *usr, const char *fmt, ...)
{
-EncoderData *usr_data = &(((QuicData *)usr)->data);
va_list ap;
+char message_buf[512];
va_start(ap, fmt);
-vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt, ap);
+vsnprintf(message_buf, sizeof(message_buf), fmt, ap);
va_end(ap);
-spice_warning("%s", usr_data->message_buf);
+spice_warning("%s", message_buf);
}
static SPICE_GNUC_PRINTF(2, 3) void
lz_usr_warn(LzUsrContext *usr, const char *fmt, ...)
{
-EncoderData *usr_data = &(((LzData *)usr)->data);
va_list ap;
+char message_buf[512];
va_start(ap, fmt);
-vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt, ap);
+vsnprintf(message_buf, sizeof(message_buf), fmt, ap);
va_end(ap);
-spice_warning("%s", usr_data->message_buf);
+spice_warning("%s", message_buf);
}
static SPICE_GNUC_PRINTF(2, 3) void
glz_usr_warn(GlzEncoderUsrContext *usr, const char *fmt, ...)
{
-EncoderData *usr_data = &(((GlzData *)usr)->data);
va_list ap;
+char message_buf[512];
va_start(ap, fmt);
-vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt, ap);
+vsnprintf(message_buf, sizeof(message_buf), fmt, ap);
va_end(ap);
-spice_warning("%s", usr_data->message_buf);
+spice_warning("%s", message_buf);
}
static void *quic_usr_malloc(QuicUsrContext *usr, int size)
diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
index 75962ad..0e072b5 100644
--- a/server/dcc-encoders.h
+++ b/server/dcc-encoders.h
@@ -98,7 +98,6 @@ typedef struct {
int size_left;
} compressed_data; // for encoding data that was already compressed by
another method
} u;
-char message_buf[512];
} EncoderData;
typedef struct {
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 08/19] Encapsulate code to save glz state
Signed-off-by: Frediano Ziglio
---
server/dcc-encoders.c | 10 ++
server/dcc-encoders.h | 2 ++
server/dcc-send.c | 8 ++--
3 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index 13dbff5..2e5984a 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -640,6 +640,16 @@ void image_encoders_freeze_glz(ImageEncoders *enc)
pthread_rwlock_unlock(&enc->glz_dict->encode_lock);
}
+void image_encoders_glz_get_restore_data(ImageEncoders *enc,
+ uint8_t *out_id,
GlzEncDictRestoreData *out_data)
+{
+spice_assert(enc->glz_dict);
+image_encoders_freeze_glz(enc);
+*out_id = enc->glz_dict->id;
+glz_enc_dictionary_get_restore_data(enc->glz_dict->dict, out_data,
+&enc->glz_data.usr);
+}
+
static GlzSharedDictionary *glz_shared_dictionary_new(RedClient *client,
uint8_t id,
GlzEncDictContext *dict)
{
diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
index 68fd5a5..ce8010c 100644
--- a/server/dcc-encoders.h
+++ b/server/dcc-encoders.h
@@ -51,6 +51,8 @@ void image_encoders_freeze_glz(ImageEncoders *enc);
void image_encoders_release_glz(ImageEncoders *enc);
void image_encoders_glz_free_from_drawable(struct Drawable *drawable);
void image_encoders_glz_detach_from_drawable(struct Drawable *drawable);
+void image_encoders_glz_get_restore_data(ImageEncoders *enc,
+ uint8_t *out_id,
GlzEncDictRestoreData *out_data);
#define RED_COMPRESS_BUF_SIZE (1024 * 64)
struct RedCompressBuf {
diff --git a/server/dcc-send.c b/server/dcc-send.c
index c0c7573..ba695ef 100644
--- a/server/dcc-send.c
+++ b/server/dcc-send.c
@@ -1859,12 +1859,8 @@ static void
display_channel_marshall_migrate_data(RedChannelClient *rcc,
memcpy(display_data.pixmap_cache_clients, dcc->pixmap_cache->sync,
sizeof(display_data.pixmap_cache_clients));
-spice_assert(dcc->encoders.glz_dict);
-image_encoders_freeze_glz(&dcc->encoders);
-display_data.glz_dict_id = dcc->encoders.glz_dict->id;
-glz_enc_dictionary_get_restore_data(dcc->encoders.glz_dict->dict,
-&display_data.glz_dict_data,
-&dcc->encoders.glz_data.usr);
+image_encoders_glz_get_restore_data(&dcc->encoders,
&display_data.glz_dict_id,
+&display_data.glz_dict_data);
/* all data besided the surfaces ref */
spice_marshaller_add(base_marshaller,
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 11/19] Better encapsulation for image_encoders_compress_glz call
Do not access too much encoders data.
Slightly different as now if glz is frozen lz compression is used.
Signed-off-by: Frediano Ziglio
---
server/dcc-encoders.c | 15 +++
server/dcc.c | 17 -
2 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index 6668dff..3547adc 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -1191,6 +1191,17 @@ int image_encoders_compress_glz(ImageEncoders *enc,
spice_info("LZ global compress fmt=%d", src->format);
#endif
+if ((src->x * src->y) >= glz_enc_dictionary_get_size(enc->glz_dict->dict))
{
+return FALSE;
+}
+
+pthread_rwlock_rdlock(&enc->glz_dict->encode_lock);
+/* using the global dictionary only if it is not frozen */
+if (enc->glz_dict->migrate_freeze) {
+pthread_rwlock_unlock(&enc->glz_dict->encode_lock);
+return FALSE;
+}
+
encoder_data_init(&glz_data->data);
glz_drawable = get_glz_drawable(enc, drawable);
@@ -1241,8 +1252,12 @@ int image_encoders_compress_glz(ImageEncoders *enc,
o_comp_data->comp_buf_size = zlib_size;
stat_compress_add(&enc->shared_data->zlib_glz_stat, start_time, glz_size,
zlib_size);
+pthread_rwlock_unlock(&enc->glz_dict->encode_lock);
return TRUE;
+
glz:
+pthread_rwlock_unlock(&enc->glz_dict->encode_lock);
+
dest->descriptor.type = SPICE_IMAGE_TYPE_GLZ_RGB;
dest->u.lz_rgb.data_size = glz_size;
diff --git a/server/dcc.c b/server/dcc.c
index e44feb7..3190c94 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -740,19 +740,10 @@ int dcc_compress_image(DisplayChannelClient *dcc,
success = image_encoders_compress_quic(&dcc->encoders, dest, src,
o_comp_data);
break;
case SPICE_IMAGE_COMPRESSION_GLZ:
-if ((src->x * src->y) <
glz_enc_dictionary_get_size(dcc->encoders.glz_dict->dict)) {
-int frozen;
-/* using the global dictionary only if it is not frozen */
-pthread_rwlock_rdlock(&dcc->encoders.glz_dict->encode_lock);
-frozen = dcc->encoders.glz_dict->migrate_freeze;
-if (!frozen) {
-success = image_encoders_compress_glz(&dcc->encoders, dest,
src, drawable, o_comp_data,
-
display_channel->enable_zlib_glz_wrap);
-}
-pthread_rwlock_unlock(&dcc->encoders.glz_dict->encode_lock);
-if (!frozen) {
-break;
-}
+success = image_encoders_compress_glz(&dcc->encoders, dest, src,
drawable, o_comp_data,
+
display_channel->enable_zlib_glz_wrap);
+if (success) {
+break;
}
goto lz_compress;
#ifdef USE_LZ4
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 05/19] Make dcc_compress_image_glz independent to DisplayChannelClient
Also rename to image_encoders_compress_glz
Signed-off-by: Frediano Ziglio
Acked-by: Jonathon Jongsma
---
server/dcc.c | 31 ---
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/server/dcc.c b/server/dcc.c
index 6060b7d..9007afb 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -698,15 +698,15 @@ static const LzImageType bitmap_fmt_to_lz_image_type[] = {
#define MIN_GLZ_SIZE_FOR_ZLIB 100
-static int dcc_compress_image_glz(DisplayChannelClient *dcc,
- SpiceImage *dest, SpiceBitmap *src, Drawable
*drawable,
- compress_send_data_t* o_comp_data)
+static int image_encoders_compress_glz(ImageEncoders *enc,
+ SpiceImage *dest, SpiceBitmap *src,
Drawable *drawable,
+ compress_send_data_t* o_comp_data,
+ gboolean enable_zlib_glz_wrap)
{
-DisplayChannel *display_channel = DCC_TO_DC(dcc);
stat_start_time_t start_time;
-stat_start_time_init(&start_time,
&display_channel->encoder_globals.zlib_glz_stat);
+stat_start_time_init(&start_time, &enc->shared_data->zlib_glz_stat);
spice_assert(bitmap_fmt_is_rgb(src->format));
-GlzData *glz_data = &dcc->encoders.glz_data;
+GlzData *glz_data = &enc->glz_data;
ZlibData *zlib_data;
LzImageType type = bitmap_fmt_to_lz_image_type[src->format];
RedGlzDrawable *glz_drawable;
@@ -720,7 +720,7 @@ static int dcc_compress_image_glz(DisplayChannelClient *dcc,
encoder_data_init(&glz_data->data);
-glz_drawable = get_glz_drawable(&dcc->encoders, drawable);
+glz_drawable = get_glz_drawable(enc, drawable);
glz_drawable_instance = add_glz_drawable_instance(glz_drawable);
glz_data->data.u.lines_data.chunks = src->data;
@@ -728,27 +728,27 @@ static int dcc_compress_image_glz(DisplayChannelClient
*dcc,
glz_data->data.u.lines_data.next = 0;
glz_data->data.u.lines_data.reverse = 0;
-glz_size = glz_encode(dcc->encoders.glz, type, src->x, src->y,
+glz_size = glz_encode(enc->glz, type, src->x, src->y,
(src->flags & SPICE_BITMAP_FLAGS_TOP_DOWN), NULL, 0,
src->stride, glz_data->data.bufs_head->buf.bytes,
sizeof(glz_data->data.bufs_head->buf),
glz_drawable_instance,
&glz_drawable_instance->context);
-stat_compress_add(&display_channel->encoder_globals.glz_stat, start_time,
src->stride * src->y, glz_size);
+stat_compress_add(&enc->shared_data->glz_stat, start_time, src->stride *
src->y, glz_size);
-if (!display_channel->enable_zlib_glz_wrap || (glz_size <
MIN_GLZ_SIZE_FOR_ZLIB)) {
+if (!enable_zlib_glz_wrap || (glz_size < MIN_GLZ_SIZE_FOR_ZLIB)) {
goto glz;
}
-stat_start_time_init(&start_time,
&display_channel->encoder_globals.zlib_glz_stat);
-zlib_data = &dcc->encoders.zlib_data;
+stat_start_time_init(&start_time, &enc->shared_data->zlib_glz_stat);
+zlib_data = &enc->zlib_data;
encoder_data_init(&zlib_data->data);
zlib_data->data.u.compressed_data.next = glz_data->data.bufs_head;
zlib_data->data.u.compressed_data.size_left = glz_size;
-zlib_size = zlib_encode(dcc->encoders.zlib, dcc->encoders.zlib_level,
+zlib_size = zlib_encode(enc->zlib, enc->zlib_level,
glz_size, zlib_data->data.bufs_head->buf.bytes,
sizeof(zlib_data->data.bufs_head->buf));
@@ -767,7 +767,7 @@ static int dcc_compress_image_glz(DisplayChannelClient *dcc,
o_comp_data->comp_buf = zlib_data->data.bufs_head;
o_comp_data->comp_buf_size = zlib_size;
-stat_compress_add(&display_channel->encoder_globals.zlib_glz_stat,
start_time, glz_size, zlib_size);
+stat_compress_add(&enc->shared_data->zlib_glz_stat, start_time, glz_size,
zlib_size);
return TRUE;
glz:
dest->descriptor.type = SPICE_IMAGE_TYPE_GLZ_RGB;
@@ -894,7 +894,8 @@ int dcc_compress_image(DisplayChannelClient *dcc,
pthread_rwlock_rdlock(&dcc->encoders.glz_dict->encode_lock);
frozen = dcc->encoders.glz_dict->migrate_freeze;
if (!frozen) {
-success = dcc_compress_image_glz(dcc, dest, src, drawable,
o_comp_data);
+success = image_encoders_compress_glz(&dcc->encoders, dest,
src, drawable, o_comp_data,
+
display_channel->enable_zlib_glz_wrap);
}
pthread_rwlock_unlock(&dcc->encoders.glz_dict->encode_lock);
if (!frozen) {
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 04/19] Move others glz fields to dcc-encoders
Signed-off-by: Frediano Ziglio
Acked-by: Jonathon Jongsma
---
server/dcc-encoders.c| 76 +---
server/dcc-encoders.h| 19 +++-
server/dcc.c | 18 +---
server/dcc.h | 5
server/display-channel.c | 12
server/display-channel.h | 2 --
server/red-worker.c | 4 +--
7 files changed, 66 insertions(+), 70 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index 7497029..d33ddbd 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -26,8 +26,8 @@
#define ZLIB_DEFAULT_COMPRESSION_LEVEL 3
-static void dcc_free_glz_drawable_instance(DisplayChannelClient *dcc,
- GlzDrawableInstanceItem *item);
+static void image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
+ GlzDrawableInstanceItem
*instance);
static SPICE_GNUC_NORETURN SPICE_GNUC_PRINTF(2, 3) void
quic_usr_error(QuicUsrContext *usr, const char *fmt, ...)
@@ -329,10 +329,10 @@ static void glz_usr_free_image(GlzEncoderUsrContext *usr,
GlzUsrImageContext *im
{
GlzData *lz_data = (GlzData *)usr;
GlzDrawableInstanceItem *glz_drawable_instance = (GlzDrawableInstanceItem
*)image;
-DisplayChannelClient *drawable_cc =
glz_drawable_instance->glz_drawable->dcc;
-DisplayChannelClient *this_cc = SPICE_CONTAINEROF(lz_data,
DisplayChannelClient, encoders.glz_data);
-if (this_cc == drawable_cc) {
-dcc_free_glz_drawable_instance(drawable_cc, glz_drawable_instance);
+ImageEncoders *drawable_enc =
glz_drawable_instance->glz_drawable->encoders;
+ImageEncoders *this_enc = SPICE_CONTAINEROF(lz_data, ImageEncoders,
glz_data);
+if (this_enc == drawable_enc) {
+image_encoders_free_glz_drawable_instance(drawable_enc,
glz_drawable_instance);
} else {
/* The glz dictionary is shared between all DisplayChannelClient
* instances that belong to the same client, and glz_usr_free_image
@@ -341,10 +341,10 @@ static void glz_usr_free_image(GlzEncoderUsrContext *usr,
GlzUsrImageContext *im
* called from any DisplayChannelClient thread, hence the need for
* this check.
*/
-pthread_mutex_lock(&drawable_cc->glz_drawables_inst_to_free_lock);
+pthread_mutex_lock(&drawable_enc->glz_drawables_inst_to_free_lock);
ring_add_before(&glz_drawable_instance->free_link,
-&drawable_cc->glz_drawables_inst_to_free);
-pthread_mutex_unlock(&drawable_cc->glz_drawables_inst_to_free_lock);
+&drawable_enc->glz_drawables_inst_to_free);
+pthread_mutex_unlock(&drawable_enc->glz_drawables_inst_to_free_lock);
}
}
@@ -403,6 +403,10 @@ void image_encoders_init(ImageEncoders *enc,
ImageEncoderSharedData *shared_data
spice_assert(shared_data);
enc->shared_data = shared_data;
+ring_init(&enc->glz_drawables);
+ring_init(&enc->glz_drawables_inst_to_free);
+pthread_mutex_init(&enc->glz_drawables_inst_to_free_lock, NULL);
+
image_encoders_init_glz_data(enc);
image_encoders_init_quic(enc);
image_encoders_init_lz(enc);
@@ -437,10 +441,9 @@ void image_encoders_free(ImageEncoders *enc)
it is not used by Drawable).
NOTE - 1) can be called only by the display channel that created the
drawable
2) it is assumed that the instance was already removed from the
dictionary*/
-static void dcc_free_glz_drawable_instance(DisplayChannelClient *dcc,
- GlzDrawableInstanceItem *instance)
+static void image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
+ GlzDrawableInstanceItem
*instance)
{
-DisplayChannel *display_channel = DCC_TO_DC(dcc);
RedGlzDrawable *glz_drawable;
spice_assert(instance);
@@ -448,7 +451,7 @@ static void
dcc_free_glz_drawable_instance(DisplayChannelClient *dcc,
glz_drawable = instance->glz_drawable;
-spice_assert(glz_drawable->dcc == dcc);
+spice_assert(glz_drawable->encoders == enc);
spice_assert(glz_drawable->instances_count > 0);
ring_remove(&instance->glz_link);
@@ -469,7 +472,7 @@ static void
dcc_free_glz_drawable_instance(DisplayChannelClient *dcc,
ring_remove(&glz_drawable->drawable_link);
}
red_drawable_unref(glz_drawable->red_drawable);
-display_channel->glz_drawable_count--;
+enc->shared_data->glz_drawable_count--;
if (ring_item_is_linked(&glz_drawable->link)) {
ring_remove(&glz_drawable->link);
}
@@ -483,14 +486,14 @@ static void
dcc_free_glz_drawable_instance(DisplayChannelClient *dcc,
* if possible.
* NOTE - the caller should prevent encoding using the dictionary during this
operation
*/
-void dcc_free_glz_drawable(DisplayChannelClient *dcc, RedGlzDrawable *drawabl
[Spice-devel] [PATCH v4 09/19] Make some function static
Signed-off-by: Frediano Ziglio
---
server/dcc-encoders.c | 4 ++--
server/dcc-encoders.h | 2 --
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index 2e5984a..f423f22 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -523,7 +523,7 @@ static void
image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
* if possible.
* NOTE - the caller should prevent encoding using the dictionary during this
operation
*/
-void image_encoders_free_glz_drawable(ImageEncoders *enc, RedGlzDrawable
*drawable)
+static void image_encoders_free_glz_drawable(ImageEncoders *enc,
RedGlzDrawable *drawable)
{
RingItem *head_instance = ring_get_head(&drawable->instances);
int cont = (head_instance != NULL);
@@ -633,7 +633,7 @@ void image_encoders_glz_detach_from_drawable(struct
Drawable *drawable)
}
}
-void image_encoders_freeze_glz(ImageEncoders *enc)
+static void image_encoders_freeze_glz(ImageEncoders *enc)
{
pthread_rwlock_wrlock(&enc->glz_dict->encode_lock);
enc->glz_dict->migrate_freeze = TRUE;
diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
index ce8010c..9674465 100644
--- a/server/dcc-encoders.h
+++ b/server/dcc-encoders.h
@@ -42,12 +42,10 @@ void image_encoder_shared_stat_print(const
ImageEncoderSharedData *shared_data);
void image_encoders_init(ImageEncoders *enc, ImageEncoderSharedData
*shared_data);
void image_encoders_free(ImageEncoders *enc);
-void image_encoders_free_glz_drawable(ImageEncoders *enc, RedGlzDrawable
*drawable);
int image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc);
void image_encoders_free_glz_drawables(ImageEncoders *enc);
void image_encoders_free_glz_drawables_to_free(ImageEncoders* enc);
gboolean image_encoders_glz_create(ImageEncoders *enc, uint8_t id);
-void image_encoders_freeze_glz(ImageEncoders *enc);
void image_encoders_release_glz(ImageEncoders *enc);
void image_encoders_glz_free_from_drawable(struct Drawable *drawable);
void image_encoders_glz_detach_from_drawable(struct Drawable *drawable);
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 12/19] Better encoders encapsulation
Avoid to access some fields from dcc.c
Signed-off-by: Frediano Ziglio
---
server/dcc-encoders.c | 4
server/dcc.c | 3 ---
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index 3547adc..22cfcf3 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -710,6 +710,8 @@ gboolean image_encoders_get_glz_dictionary(ImageEncoders
*enc,
{
GlzSharedDictionary *shared_dict;
+spice_return_val_if_fail(!enc->glz_dict, FALSE);
+
pthread_mutex_lock(&glz_dictionary_list_lock);
shared_dict = find_glz_dictionary(client, id);
@@ -743,6 +745,8 @@ gboolean
image_encoders_restore_glz_dictionary(ImageEncoders *enc,
{
GlzSharedDictionary *shared_dict = NULL;
+spice_return_val_if_fail(!enc->glz_dict, FALSE);
+
pthread_mutex_lock(&glz_dictionary_list_lock);
shared_dict = find_glz_dictionary(client, id);
diff --git a/server/dcc.c b/server/dcc.c
index 3190c94..039229d 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -903,7 +903,6 @@ static int dcc_handle_init(DisplayChannelClient *dcc,
SpiceMsgcDisplayInit *init
init->pixmap_cache_size);
spice_return_val_if_fail(dcc->pixmap_cache, FALSE);
-spice_return_val_if_fail(!dcc->encoders.glz_dict, FALSE);
success = image_encoders_get_glz_dictionary(&dcc->encoders,
RED_CHANNEL_CLIENT(dcc)->client,
init->glz_dictionary_id,
@@ -1008,8 +1007,6 @@ int dcc_handle_message(RedChannelClient *rcc, uint32_t
size, uint16_t type, void
static int dcc_handle_migrate_glz_dictionary(DisplayChannelClient *dcc,
SpiceMigrateDataDisplay *migrate)
{
-spice_return_val_if_fail(!dcc->encoders.glz_dict, FALSE);
-
return image_encoders_restore_glz_dictionary(&dcc->encoders,
RED_CHANNEL_CLIENT(dcc)->client,
migrate->glz_dict_id,
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 06/19] Move image_encoders_compress_glz to dcc-encoders.c
Signed-off-by: Frediano Ziglio
Acked-by: Jonathon Jongsma
---
server/dcc-encoders.c | 133 +
server/dcc-encoders.h | 4 ++
server/dcc.c | 147 --
3 files changed, 137 insertions(+), 147 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index d33ddbd..4de780d 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -1051,6 +1051,139 @@ int image_encoders_compress_lz4(ImageEncoders *enc,
SpiceImage *dest,
}
#endif
+/* if already exists, returns it. Otherwise allocates and adds it (1) to the
ring tail
+ in the channel (2) to the Drawable*/
+static RedGlzDrawable *get_glz_drawable(ImageEncoders *enc, Drawable *drawable)
+{
+RedGlzDrawable *ret;
+RingItem *item, *next;
+
+// TODO - I don't really understand what's going on here, so doing the
technical equivalent
+// now that we have multiple glz_dicts, so the only way to go from dcc to
drawable glz is to go
+// over the glz_ring (unless adding some better data structure then a ring)
+DRAWABLE_FOREACH_GLZ_SAFE(drawable, item, next, ret) {
+if (ret->encoders == enc) {
+return ret;
+}
+}
+
+ret = spice_new(RedGlzDrawable, 1);
+
+ret->encoders = enc;
+ret->red_drawable = red_drawable_ref(drawable->red_drawable);
+ret->drawable = drawable;
+ret->instances_count = 0;
+ring_init(&ret->instances);
+
+ring_item_init(&ret->link);
+ring_item_init(&ret->drawable_link);
+ring_add_before(&ret->link, &enc->glz_drawables);
+ring_add(&drawable->glz_ring, &ret->drawable_link);
+enc->shared_data->glz_drawable_count++;
+return ret;
+}
+
+/* allocates new instance and adds it to instances in the given drawable.
+ NOTE - the caller should set the glz_instance returned by the encoder by
itself.*/
+static GlzDrawableInstanceItem *add_glz_drawable_instance(RedGlzDrawable
*glz_drawable)
+{
+spice_assert(glz_drawable->instances_count < MAX_GLZ_DRAWABLE_INSTANCES);
+// NOTE: We assume the additions are performed consecutively, without
removals in the middle
+GlzDrawableInstanceItem *ret = glz_drawable->instances_pool +
glz_drawable->instances_count;
+glz_drawable->instances_count++;
+
+ring_item_init(&ret->free_link);
+ring_item_init(&ret->glz_link);
+ring_add(&glz_drawable->instances, &ret->glz_link);
+ret->context = NULL;
+ret->glz_drawable = glz_drawable;
+
+return ret;
+}
+
+#define MIN_GLZ_SIZE_FOR_ZLIB 100
+
+int image_encoders_compress_glz(ImageEncoders *enc,
+SpiceImage *dest, SpiceBitmap *src, Drawable
*drawable,
+compress_send_data_t* o_comp_data,
+gboolean enable_zlib_glz_wrap)
+{
+stat_start_time_t start_time;
+stat_start_time_init(&start_time, &enc->shared_data->zlib_glz_stat);
+spice_assert(bitmap_fmt_is_rgb(src->format));
+GlzData *glz_data = &enc->glz_data;
+ZlibData *zlib_data;
+LzImageType type = bitmap_fmt_to_lz_image_type[src->format];
+RedGlzDrawable *glz_drawable;
+GlzDrawableInstanceItem *glz_drawable_instance;
+int glz_size;
+int zlib_size;
+
+#ifdef COMPRESS_DEBUG
+spice_info("LZ global compress fmt=%d", src->format);
+#endif
+
+encoder_data_init(&glz_data->data);
+
+glz_drawable = get_glz_drawable(enc, drawable);
+glz_drawable_instance = add_glz_drawable_instance(glz_drawable);
+
+glz_data->data.u.lines_data.chunks = src->data;
+glz_data->data.u.lines_data.stride = src->stride;
+glz_data->data.u.lines_data.next = 0;
+glz_data->data.u.lines_data.reverse = 0;
+
+glz_size = glz_encode(enc->glz, type, src->x, src->y,
+ (src->flags & SPICE_BITMAP_FLAGS_TOP_DOWN), NULL, 0,
+ src->stride, glz_data->data.bufs_head->buf.bytes,
+ sizeof(glz_data->data.bufs_head->buf),
+ glz_drawable_instance,
+ &glz_drawable_instance->context);
+
+stat_compress_add(&enc->shared_data->glz_stat, start_time, src->stride *
src->y, glz_size);
+
+if (!enable_zlib_glz_wrap || (glz_size < MIN_GLZ_SIZE_FOR_ZLIB)) {
+goto glz;
+}
+stat_start_time_init(&start_time, &enc->shared_data->zlib_glz_stat);
+zlib_data = &enc->zlib_data;
+
+encoder_data_init(&zlib_data->data);
+
+zlib_data->data.u.compressed_data.next = glz_data->data.bufs_head;
+zlib_data->data.u.compressed_data.size_left = glz_size;
+
+zlib_size = zlib_encode(enc->zlib, enc->zlib_level,
+glz_size, zlib_data->data.bufs_head->buf.bytes,
+sizeof(zlib_data->data.bufs_head->buf));
+
+// the compressed buffer is bigger than the original data
+if (zlib_size >= glz_size) {
+encoder_data_reset(&zlib_data->data);
+goto glz;
+} else {
+
[Spice-devel] [PATCH v4 18/19] Reduce header dependency
Avoid dependencies from RedChannel stuff. Signed-off-by: Frediano Ziglio --- server/dcc-encoders.h | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h index 47051b4..75962ad 100644 --- a/server/dcc-encoders.h +++ b/server/dcc-encoders.h @@ -20,10 +20,10 @@ #include #include +#include -#include "red-channel.h" +#include "stat.h" #include "red-parse-qxl.h" -#include "image-cache.h" #include "glz-encoder.h" #include "jpeg-encoder.h" #ifdef USE_LZ4 @@ -31,6 +31,8 @@ #endif #include "zlib-encoder.h" +struct RedClient; + typedef struct RedCompressBuf RedCompressBuf; typedef struct RedGlzDrawable RedGlzDrawable; typedef struct ImageEncoders ImageEncoders; -- 2.7.4 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 13/19] Change RedGlzDrawable::drawable from pointer to boolean
The field was used just as a flag.
This has the advantage to make clear to not use the pointer as we don't
have ownership.
Also many the structure a bit smaller.
Signed-off-by: Frediano Ziglio
---
server/dcc-encoders.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index 22cfcf3..00349fe 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -48,10 +48,10 @@ struct RedGlzDrawable {
RingItem link;// ordered by the time it was encoded
RingItem drawable_link;
RedDrawable *red_drawable;
-struct Drawable*drawable;
GlzDrawableInstanceItem instances_pool[MAX_GLZ_DRAWABLE_INSTANCES];
Ring instances;
uint8_t instances_count;
+gboolean has_drawable;
ImageEncoders *encoders;
};
@@ -505,9 +505,7 @@ static void
image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
if (ring_is_empty(&glz_drawable->instances)) {
spice_assert(glz_drawable->instances_count == 0);
-Drawable *drawable = glz_drawable->drawable;
-
-if (drawable) {
+if (glz_drawable->has_drawable) {
ring_remove(&glz_drawable->drawable_link);
}
red_drawable_unref(glz_drawable->red_drawable);
@@ -569,7 +567,7 @@ int
image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc)
while ((n < RED_RELEASE_BUNCH_SIZE) && (ring_link != NULL)) {
RedGlzDrawable *glz_drawable = SPICE_CONTAINEROF(ring_link,
RedGlzDrawable, link);
ring_link = ring_next(&enc->glz_drawables, ring_link);
-if (!glz_drawable->drawable) {
+if (!glz_drawable->has_drawable) {
image_encoders_free_glz_drawable(enc, glz_drawable);
n++;
}
@@ -630,7 +628,7 @@ void image_encoders_glz_detach_from_drawable(struct
Drawable *drawable)
RingItem *item, *next;
RING_FOREACH_SAFE(item, next, &drawable->glz_ring) {
-SPICE_CONTAINEROF(item, RedGlzDrawable, drawable_link)->drawable =
NULL;
+SPICE_CONTAINEROF(item, RedGlzDrawable, drawable_link)->has_drawable =
FALSE;
ring_remove(item);
}
}
@@ -1143,7 +1141,7 @@ static RedGlzDrawable *get_glz_drawable(ImageEncoders
*enc, Drawable *drawable)
ret->encoders = enc;
ret->red_drawable = red_drawable_ref(drawable->red_drawable);
-ret->drawable = drawable;
+ret->has_drawable = TRUE;
ret->instances_count = 0;
ring_init(&ret->instances);
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 15/19] Do not access ImageEncoders internal to lock/unlock glz encoding
Signed-off-by: Frediano Ziglio
---
server/dcc-encoders.c| 14 ++
server/dcc-encoders.h| 2 ++
server/display-channel.c | 18 +-
3 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index a83794e..7b129e4 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -550,6 +550,20 @@ static void image_encoders_free_glz_drawable(ImageEncoders
*enc, RedGlzDrawable
}
}
+void image_encoders_glz_encode_lock(ImageEncoders *enc)
+{
+if (enc->glz_dict) {
+pthread_rwlock_wrlock(&enc->glz_dict->encode_lock);
+}
+}
+
+void image_encoders_glz_encode_unlock(ImageEncoders *enc)
+{
+if (enc->glz_dict) {
+pthread_rwlock_unlock(&enc->glz_dict->encode_lock);
+}
+}
+
/*
* Remove from the global lz dictionary some glz_drawables that have no
reference to
* Drawable (their qxl drawables are released too).
diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
index ac61a70..4587a0b 100644
--- a/server/dcc-encoders.h
+++ b/server/dcc-encoders.h
@@ -50,6 +50,8 @@ void image_encoders_glz_free_from_drawable_ring(Ring
*drawable_ring);
void image_encoders_glz_detach_from_drawable_ring(Ring *drawable_ring);
void image_encoders_glz_get_restore_data(ImageEncoders *enc,
uint8_t *out_id,
GlzEncDictRestoreData *out_data);
+void image_encoders_glz_encode_lock(ImageEncoders *enc);
+void image_encoders_glz_encode_unlock(ImageEncoders *enc);
#define RED_COMPRESS_BUF_SIZE (1024 * 64)
struct RedCompressBuf {
diff --git a/server/display-channel.c b/server/display-channel.c
index bddedd9..15d69c4 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1198,14 +1198,10 @@ void display_channel_free_some(DisplayChannel *display)
spice_debug("#draw=%d, #glz_draw=%d", display->drawable_count,
display->encoder_globals.glz_drawable_count);
FOREACH_CLIENT(display, link, next, dcc) {
-GlzSharedDictionary *glz_dict = dcc->encoders.glz_dict;
-
-if (glz_dict) {
-// encoding using the dictionary is prevented since the following
operations might
-// change the dictionary
-pthread_rwlock_wrlock(&glz_dict->encode_lock);
-n =
image_encoders_free_some_independent_glz_drawables(&dcc->encoders);
-}
+// encoding using the dictionary is prevented since the following
operations might
+// change the dictionary
+image_encoders_glz_encode_lock(&dcc->encoders);
+n = image_encoders_free_some_independent_glz_drawables(&dcc->encoders);
}
while (!ring_is_empty(&display->current_list) && n++ <
RED_RELEASE_BUNCH_SIZE) {
@@ -1213,11 +1209,7 @@ void display_channel_free_some(DisplayChannel *display)
}
FOREACH_CLIENT(display, link, next, dcc) {
-GlzSharedDictionary *glz_dict = dcc->encoders.glz_dict;
-
-if (glz_dict) {
-pthread_rwlock_unlock(&glz_dict->encode_lock);
-}
+image_encoders_glz_encode_unlock(&dcc->encoders);
}
}
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 01/19] Add a structure to hold ImageEncoders shared data
Signed-off-by: Frediano Ziglio
---
server/dcc-encoders.c| 121 +++
server/dcc-encoders.h| 33 +
server/dcc.c | 26 +-
server/display-channel.c | 73 ++--
server/display-channel.h | 9 +---
5 files changed, 141 insertions(+), 121 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index 6aaf954..c79cf58 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -398,10 +398,13 @@ static void image_encoders_init_zlib(ImageEncoders *enc)
}
}
-void dcc_encoders_init(DisplayChannelClient *dcc)
+void dcc_encoders_init(DisplayChannelClient *dcc, ImageEncoderSharedData
*shared_data)
{
ImageEncoders *enc = &dcc->encoders;
+spice_assert(shared_data);
+enc->shared_data = shared_data;
+
dcc_init_glz_data(dcc);
image_encoders_init_quic(enc);
image_encoders_init_lz(enc);
@@ -708,15 +711,14 @@ void dcc_release_glz(DisplayChannelClient *dcc)
}
int image_encoders_compress_quic(ImageEncoders *enc, SpiceImage *dest,
- SpiceBitmap *src, compress_send_data_t*
o_comp_data,
- stat_info_t *stats)
+ SpiceBitmap *src, compress_send_data_t*
o_comp_data)
{
QuicData *quic_data = &enc->quic_data;
QuicContext *quic = enc->quic;
volatile QuicImageType type;
int size, stride;
stat_start_time_t start_time;
-stat_start_time_init(&start_time, stats);
+stat_start_time_init(&start_time, &enc->shared_data->quic_stat);
#ifdef COMPRESS_DEBUG
spice_info("QUIC compress");
@@ -776,7 +778,7 @@ int image_encoders_compress_quic(ImageEncoders *enc,
SpiceImage *dest,
o_comp_data->comp_buf = quic_data->data.bufs_head;
o_comp_data->comp_buf_size = size << 2;
-stat_compress_add(stats, start_time, src->stride * src->y,
+stat_compress_add(&enc->shared_data->quic_stat, start_time, src->stride *
src->y,
o_comp_data->comp_buf_size);
return TRUE;
}
@@ -797,8 +799,7 @@ static const LzImageType bitmap_fmt_to_lz_image_type[] = {
int image_encoders_compress_lz(ImageEncoders *enc,
SpiceImage *dest, SpiceBitmap *src,
- compress_send_data_t* o_comp_data,
- stat_info_t *stats)
+ compress_send_data_t* o_comp_data)
{
LzData *lz_data = &enc->lz_data;
LzContext *lz = enc->lz;
@@ -806,7 +807,7 @@ int image_encoders_compress_lz(ImageEncoders *enc,
int size;// size of the compressed data
stat_start_time_t start_time;
-stat_start_time_init(&start_time, stats);
+stat_start_time_init(&start_time, &enc->shared_data->lz_stat);
#ifdef COMPRESS_DEBUG
spice_info("LZ LOCAL compress");
@@ -856,15 +857,13 @@ int image_encoders_compress_lz(ImageEncoders *enc,
o_comp_data->lzplt_palette = dest->u.lz_plt.palette;
}
-stat_compress_add(stats, start_time, src->stride * src->y,
+stat_compress_add(&enc->shared_data->lz_stat, start_time, src->stride *
src->y,
o_comp_data->comp_buf_size);
return TRUE;
}
int image_encoders_compress_jpeg(ImageEncoders *enc, SpiceImage *dest,
- SpiceBitmap *src, compress_send_data_t*
o_comp_data,
- stat_info_t *jpeg_stats, // FIXME put all
stats in a structure
- stat_info_t *jpeg_alpha_stats)
+ SpiceBitmap *src, compress_send_data_t*
o_comp_data)
{
JpegData *jpeg_data = &enc->jpeg_data;
LzData *lz_data = &enc->lz_data;
@@ -879,7 +878,7 @@ int image_encoders_compress_jpeg(ImageEncoders *enc,
SpiceImage *dest,
int stride;
uint8_t *lz_out_start_byte;
stat_start_time_t start_time;
-stat_start_time_init(&start_time, jpeg_alpha_stats);
+stat_start_time_init(&start_time, &enc->shared_data->jpeg_alpha_stat);
#ifdef COMPRESS_DEBUG
spice_info("JPEG compress");
@@ -943,7 +942,7 @@ int image_encoders_compress_jpeg(ImageEncoders *enc,
SpiceImage *dest,
o_comp_data->comp_buf_size = jpeg_size;
o_comp_data->is_lossy = TRUE;
-stat_compress_add(jpeg_stats, start_time, src->stride * src->y,
+stat_compress_add(&enc->shared_data->jpeg_stat, start_time,
src->stride * src->y,
o_comp_data->comp_buf_size);
return TRUE;
}
@@ -983,21 +982,20 @@ int image_encoders_compress_jpeg(ImageEncoders *enc,
SpiceImage *dest,
o_comp_data->comp_buf = jpeg_data->data.bufs_head;
o_comp_data->comp_buf_size = jpeg_size + alpha_lz_size;
o_comp_data->is_lossy = TRUE;
-stat_compress_add(jpeg_alpha_stats, start_time, src->stride * src->y,
+stat_compress_add(&enc->shared_data->jpeg_alpha_stat, start_time,
src->stride * src->y,
[Spice-devel] [PATCH v4 17/19] Remove dependency from dcc-encoders to Drawable
Encoding image requires a RedDrawable (where the data is stored) and
a Ring where to store information to free Glz structures.
Signed-off-by: Frediano Ziglio
---
server/dcc-encoders.c | 17 ++---
server/dcc-encoders.h | 3 ++-
server/dcc.c | 4 +++-
3 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index 4c7d73e..beacb30 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -22,7 +22,9 @@
#include
#include "dcc-encoders.h"
-#include "display-channel.h"
+#include "spice-bitmap-utils.h"
+#include "red-worker.h" // red_drawable_unref
+#include "pixmap-cache.h" // MAX_CACHE_CLIENTS
#define ZLIB_DEFAULT_COMPRESSION_LEVEL 3
@@ -1147,7 +1149,7 @@ int image_encoders_compress_lz4(ImageEncoders *enc,
SpiceImage *dest,
/* if already exists, returns it. Otherwise allocates and adds it (1) to the
ring tail
in the channel (2) to the Drawable*/
-static RedGlzDrawable *get_glz_drawable(ImageEncoders *enc, Drawable *drawable)
+static RedGlzDrawable *get_glz_drawable(ImageEncoders *enc, RedDrawable
*red_drawable, Ring *drawable_ring)
{
RedGlzDrawable *ret;
RingItem *item, *next;
@@ -1155,7 +1157,7 @@ static RedGlzDrawable *get_glz_drawable(ImageEncoders
*enc, Drawable *drawable)
// TODO - I don't really understand what's going on here, so doing the
technical equivalent
// now that we have multiple glz_dicts, so the only way to go from dcc to
drawable glz is to go
// over the glz_ring (unless adding some better data structure then a ring)
-DRAWABLE_FOREACH_GLZ_SAFE(drawable, item, next, ret) {
+SAFE_FOREACH(item, next, TRUE, drawable_ring, ret, LINK_TO_GLZ(item)) {
if (ret->encoders == enc) {
return ret;
}
@@ -1164,7 +1166,7 @@ static RedGlzDrawable *get_glz_drawable(ImageEncoders
*enc, Drawable *drawable)
ret = spice_new(RedGlzDrawable, 1);
ret->encoders = enc;
-ret->red_drawable = red_drawable_ref(drawable->red_drawable);
+ret->red_drawable = red_drawable_ref(red_drawable);
ret->has_drawable = TRUE;
ret->instances_count = 0;
ring_init(&ret->instances);
@@ -1172,7 +1174,7 @@ static RedGlzDrawable *get_glz_drawable(ImageEncoders
*enc, Drawable *drawable)
ring_item_init(&ret->link);
ring_item_init(&ret->drawable_link);
ring_add_before(&ret->link, &enc->glz_drawables);
-ring_add(&drawable->glz_ring, &ret->drawable_link);
+ring_add(drawable_ring, &ret->drawable_link);
enc->shared_data->glz_drawable_count++;
return ret;
}
@@ -1198,7 +1200,8 @@ static GlzDrawableInstanceItem
*add_glz_drawable_instance(RedGlzDrawable *glz_dr
#define MIN_GLZ_SIZE_FOR_ZLIB 100
int image_encoders_compress_glz(ImageEncoders *enc,
-SpiceImage *dest, SpiceBitmap *src, Drawable
*drawable,
+SpiceImage *dest, SpiceBitmap *src,
+RedDrawable *red_drawable, Ring *drawable_ring,
compress_send_data_t* o_comp_data,
gboolean enable_zlib_glz_wrap)
{
@@ -1230,7 +1233,7 @@ int image_encoders_compress_glz(ImageEncoders *enc,
encoder_data_init(&glz_data->data);
-glz_drawable = get_glz_drawable(enc, drawable);
+glz_drawable = get_glz_drawable(enc, red_drawable, drawable_ring);
glz_drawable_instance = add_glz_drawable_instance(glz_drawable);
glz_data->data.u.lines_data.chunks = src->data;
diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
index 5966b59..47051b4 100644
--- a/server/dcc-encoders.h
+++ b/server/dcc-encoders.h
@@ -195,7 +195,8 @@ int image_encoders_compress_jpeg(ImageEncoders *enc,
SpiceImage *dest,
int image_encoders_compress_lz4(ImageEncoders *enc, SpiceImage *dest,
SpiceBitmap *src, compress_send_data_t*
o_comp_data);
int image_encoders_compress_glz(ImageEncoders *enc,
-SpiceImage *dest, SpiceBitmap *src, struct
Drawable *drawable,
+SpiceImage *dest, SpiceBitmap *src,
+RedDrawable *red_drawable, Ring *drawable_ring,
compress_send_data_t* o_comp_data,
gboolean enable_zlib_glz_wrap);
diff --git a/server/dcc.c b/server/dcc.c
index 039229d..ca5569e 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -740,7 +740,9 @@ int dcc_compress_image(DisplayChannelClient *dcc,
success = image_encoders_compress_quic(&dcc->encoders, dest, src,
o_comp_data);
break;
case SPICE_IMAGE_COMPRESSION_GLZ:
-success = image_encoders_compress_glz(&dcc->encoders, dest, src,
drawable, o_comp_data,
+success = image_encoders_compress_glz(&dcc->encoders, dest, src,
+ drawable->red_drawable,
&drawable->glz_ring,
+
[Spice-devel] [PATCH v4 16/19] Make GlzSharedDictionary structure private in dcc-encoders.c
Signed-off-by: Frediano Ziglio
---
server/dcc-encoders.c | 10 ++
server/dcc-encoders.h | 11 +--
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index 7b129e4..4c7d73e 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -30,6 +30,16 @@
typedef struct GlzDrawableInstanceItem GlzDrawableInstanceItem;
+struct GlzSharedDictionary {
+RingItem base;
+GlzEncDictContext *dict;
+uint32_t refs;
+uint8_t id;
+pthread_rwlock_t encode_lock;
+int migrate_freeze;
+RedClient *client; // channel clients of the same client share the dict
+};
+
/* for each qxl drawable, there may be several instances of lz drawables */
/* TODO - reuse this stuff for the top level. I just added a second level of
multiplicity
* at the Drawable by keeping a ring, so:
diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
index 4587a0b..5966b59 100644
--- a/server/dcc-encoders.h
+++ b/server/dcc-encoders.h
@@ -35,6 +35,7 @@ typedef struct RedCompressBuf RedCompressBuf;
typedef struct RedGlzDrawable RedGlzDrawable;
typedef struct ImageEncoders ImageEncoders;
typedef struct ImageEncoderSharedData ImageEncoderSharedData;
+typedef struct GlzSharedDictionary GlzSharedDictionary;
void image_encoder_shared_init(ImageEncoderSharedData *shared_data);
void image_encoder_shared_stat_reset(ImageEncoderSharedData *shared_data);
@@ -71,16 +72,6 @@ static inline void compress_buf_free(RedCompressBuf *buf)
g_free(buf);
}
-typedef struct GlzSharedDictionary {
-RingItem base;
-GlzEncDictContext *dict;
-uint32_t refs;
-uint8_t id;
-pthread_rwlock_t encode_lock;
-int migrate_freeze;
-RedClient *client; // channel clients of the same client share the dict
-} GlzSharedDictionary;
-
gboolean image_encoders_get_glz_dictionary(ImageEncoders *enc,
struct RedClient *client,
uint8_t id, int window_size);
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 00/19] Better encapsulation of image encoding stuff
Hi, after the discussion and tests to reduce typedef mess I realized the code lack some proper encapsulation. In DisplayChannel there are multiple files to handle different stuff but all code needs to know all structures and basically include all stuff. Looking at code one stuff that could/should be separate is the image encoding. There is no high level reason why to compress an image a connection to a client is required. So I started moving encoding fields into a structure and encapsulating code reducing access and knowledge about image encoding. Results: - dcc-encoders.c does not include or use DisplayChannel stuff; - more structures are now defined in dcc-encoders.c; - dcc-encoders does not use Drawable but just a Ring to store internal data. Future improvements (not strictly related to this patchset): - dcc_compress_image should be moved to dcc-encoders.c; - there is no reason dcc-encoders.h should use RedClient, this is just as the Glz dictionaries are bound to client; - Glz code is still messy although more encapsulated; - ImageEncoders is quite huge, could be optimized; - dcc-encoders.[ch] should be renamed to image-encoders.[ch]; - implements lazy allocation for encoders to reduce memory footprint. Changes from v3: - renamed ImageEncoderGlobals to ImageEncoderSharedData (and globals variables to shared_data); - split a rename patch; - added small optimization for ImageEncoders. Changes from v2: - minor comment fixes; - added a structure to store global data. This allowed to store the old "glz_drawable_count". Also this reduce the number or parameters to pass to image_encoders_compress_XX functions. Changes from v1: - EncodersData structure renamed to ImageEncoders; - all encoders function have the image_encoders_ prefix; - split patches that move encoder fields from part that prepare dcc_compress_XX to be moved; - merged some not strictly related patches. Frediano Ziglio (19): Add a structure to hold ImageEncoders shared data Move some glz fields to ImageEncoders Change dcc_encoders_init to take ImageEncoders instead of DisplayChannelClient Move others glz fields to dcc-encoders Make dcc_compress_image_glz independent to DisplayChannelClient Move image_encoders_compress_glz to dcc-encoders.c Encapsulate some data in dcc-encoders Encapsulate code to save glz state Make some function static Encapsulate dcc_release_glz Better encapsulation for image_encoders_compress_glz call Better encoders encapsulation Change RedGlzDrawable::drawable from pointer to boolean Use Ring instead of accessing Drawable internals Do not access ImageEncoders internal to lock/unlock glz encoding Make GlzSharedDictionary structure private in dcc-encoders.c Remove dependency from dcc-encoders to Drawable Reduce header dependency Remove message_buf from EncoderData server/dcc-encoders.c| 573 +-- server/dcc-encoders.h| 123 +- server/dcc-send.c| 8 +- server/dcc.c | 218 +++--- server/dcc.h | 9 - server/display-channel.c | 112 ++--- server/display-channel.h | 16 +- server/red-worker.c | 4 +- 8 files changed, 565 insertions(+), 498 deletions(-) -- 2.7.4 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 03/19] Change dcc_encoders_init to take ImageEncoders instead of DisplayChannelClient
Signed-off-by: Frediano Ziglio
---
server/dcc-encoders.c | 4 +---
server/dcc-encoders.h | 2 +-
server/dcc.c | 2 +-
3 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index c03409c..7497029 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -398,10 +398,8 @@ static void image_encoders_init_zlib(ImageEncoders *enc)
}
}
-void dcc_encoders_init(DisplayChannelClient *dcc, ImageEncoderSharedData
*shared_data)
+void image_encoders_init(ImageEncoders *enc, ImageEncoderSharedData
*shared_data)
{
-ImageEncoders *enc = &dcc->encoders;
-
spice_assert(shared_data);
enc->shared_data = shared_data;
diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
index 596429c..2b4d556 100644
--- a/server/dcc-encoders.h
+++ b/server/dcc-encoders.h
@@ -41,7 +41,7 @@ void image_encoder_shared_init(ImageEncoderSharedData
*shared_data);
void image_encoder_shared_stat_reset(ImageEncoderSharedData *shared_data);
void image_encoder_shared_stat_print(const ImageEncoderSharedData
*shared_data);
-void dcc_encoders_init(DisplayChannelClient *dcc, ImageEncoderSharedData
*shared_data);
+void image_encoders_init(ImageEncoders *enc, ImageEncoderSharedData
*shared_data);
void image_encoders_free(ImageEncoders *enc);
void dcc_free_glz_drawable
(DisplayChannelClient *dcc,
RedGlzDrawable
*drawable);
diff --git a/server/dcc.c b/server/dcc.c
index d73b742..1cc85b5 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -396,7 +396,7 @@ DisplayChannelClient *dcc_new(DisplayChannel *display,
ring_init(&dcc->glz_drawables_inst_to_free);
pthread_mutex_init(&dcc->glz_drawables_inst_to_free_lock, NULL);
-dcc_encoders_init(dcc, &display->encoder_globals);
+image_encoders_init(&dcc->encoders, &display->encoder_globals);
return dcc;
}
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 02/19] Move some glz fields to ImageEncoders
Signed-off-by: Frediano Ziglio
Acked-by: Jonathon Jongsma
---
server/dcc-encoders.c| 95 +++-
server/dcc-encoders.h| 20 +++---
server/dcc-send.c| 10 ++---
server/dcc.c | 43 +++---
server/dcc.h | 4 --
server/display-channel.c | 4 +-
6 files changed, 97 insertions(+), 79 deletions(-)
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index c79cf58..c03409c 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -330,7 +330,7 @@ static void glz_usr_free_image(GlzEncoderUsrContext *usr,
GlzUsrImageContext *im
GlzData *lz_data = (GlzData *)usr;
GlzDrawableInstanceItem *glz_drawable_instance = (GlzDrawableInstanceItem
*)image;
DisplayChannelClient *drawable_cc =
glz_drawable_instance->glz_drawable->dcc;
-DisplayChannelClient *this_cc = SPICE_CONTAINEROF(lz_data,
DisplayChannelClient, glz_data);
+DisplayChannelClient *this_cc = SPICE_CONTAINEROF(lz_data,
DisplayChannelClient, encoders.glz_data);
if (this_cc == drawable_cc) {
dcc_free_glz_drawable_instance(drawable_cc, glz_drawable_instance);
} else {
@@ -348,16 +348,16 @@ static void glz_usr_free_image(GlzEncoderUsrContext *usr,
GlzUsrImageContext *im
}
}
-static void dcc_init_glz_data(DisplayChannelClient *dcc)
+static void image_encoders_init_glz_data(ImageEncoders *enc)
{
-dcc->glz_data.usr.error = glz_usr_error;
-dcc->glz_data.usr.warn = glz_usr_warn;
-dcc->glz_data.usr.info = glz_usr_warn;
-dcc->glz_data.usr.malloc = glz_usr_malloc;
-dcc->glz_data.usr.free = glz_usr_free;
-dcc->glz_data.usr.more_space = glz_usr_more_space;
-dcc->glz_data.usr.more_lines = glz_usr_more_lines;
-dcc->glz_data.usr.free_image = glz_usr_free_image;
+enc->glz_data.usr.error = glz_usr_error;
+enc->glz_data.usr.warn = glz_usr_warn;
+enc->glz_data.usr.info = glz_usr_warn;
+enc->glz_data.usr.malloc = glz_usr_malloc;
+enc->glz_data.usr.free = glz_usr_free;
+enc->glz_data.usr.more_space = glz_usr_more_space;
+enc->glz_data.usr.more_lines = glz_usr_more_lines;
+enc->glz_data.usr.free_image = glz_usr_free_image;
}
static void image_encoders_init_jpeg(ImageEncoders *enc)
@@ -405,7 +405,7 @@ void dcc_encoders_init(DisplayChannelClient *dcc,
ImageEncoderSharedData *shared
spice_assert(shared_data);
enc->shared_data = shared_data;
-dcc_init_glz_data(dcc);
+image_encoders_init_glz_data(enc);
image_encoders_init_quic(enc);
image_encoders_init_lz(enc);
image_encoders_init_jpeg(enc);
@@ -500,9 +500,9 @@ void dcc_free_glz_drawable(DisplayChannelClient *dcc,
RedGlzDrawable *drawable)
glz_link);
if (!ring_item_is_linked(&instance->free_link)) {
// the instance didn't get out from window yet
-glz_enc_dictionary_remove_image(dcc->glz_dict->dict,
+glz_enc_dictionary_remove_image(dcc->encoders.glz_dict->dict,
instance->context,
-&dcc->glz_data.usr);
+&dcc->encoders.glz_data.usr);
}
dcc_free_glz_drawable_instance(dcc, instance);
@@ -541,7 +541,7 @@ void dcc_free_glz_drawables_to_free(DisplayChannelClient*
dcc)
{
RingItem *ring_link;
-if (!dcc->glz_dict) {
+if (!dcc->encoders.glz_dict) {
return;
}
pthread_mutex_lock(&dcc->glz_drawables_inst_to_free_lock);
@@ -559,7 +559,7 @@ void dcc_free_glz_drawables_to_free(DisplayChannelClient*
dcc)
void dcc_free_glz_drawables(DisplayChannelClient *dcc)
{
RingItem *ring_link;
-GlzSharedDictionary *glz_dict = dcc ? dcc->glz_dict : NULL;
+GlzSharedDictionary *glz_dict = dcc ? dcc->encoders.glz_dict : NULL;
if (!glz_dict) {
return;
@@ -576,11 +576,11 @@ void dcc_free_glz_drawables(DisplayChannelClient *dcc)
pthread_rwlock_unlock(&glz_dict->encode_lock);
}
-void dcc_freeze_glz(DisplayChannelClient *dcc)
+void image_encoders_freeze_glz(ImageEncoders *enc)
{
-pthread_rwlock_wrlock(&dcc->glz_dict->encode_lock);
-dcc->glz_dict->migrate_freeze = TRUE;
-pthread_rwlock_unlock(&dcc->glz_dict->encode_lock);
+pthread_rwlock_wrlock(&enc->glz_dict->encode_lock);
+enc->glz_dict->migrate_freeze = TRUE;
+pthread_rwlock_unlock(&enc->glz_dict->encode_lock);
}
static GlzSharedDictionary *glz_shared_dictionary_new(RedClient *client,
uint8_t id,
@@ -623,82 +623,95 @@ static GlzSharedDictionary *find_glz_dictionary(RedClient
*client, uint8_t dict_
#define MAX_LZ_ENCODERS MAX_CACHE_CLIENTS
-static GlzSharedDictionary *create_glz_dictionary(DisplayChannelClient *dcc,
+static GlzSharedDictionary *create_glz_dictionary(ImageEncoders *enc,
+ RedClient *client,
Re: [Spice-devel] [spice-gtk PATCH] Handle single headed monitors that have a non-zero x, y config offset
On Mon, Jun 13, 2016 at 07:09:29PM +0200, Marc-André Lureau wrote: > My understanding is that there is some confusing between monitor > configuration position (the monitor config on main channel), and the > scanout/primary position (the monitor config on display channel). > > Imho, the fix should be on server or guest/driver side, it shouldn't > use a display monitor config with a scanout offset if there is none to > be applied by the client. Regarding fixing this on the server, isn't it going to have similar issues? Ie once we get QXL_IO_MONITORS_CONFIG_ASYNC in QEMU, we could make sure that display channels with just one monitor will get (0, 0) as their primary position, but this is not correct with virgl, so we'd need some gl check in the server before doing the zero-ing out? Christophe 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] [spice-gtk] widget: Disable IME context on main widget
Hey, On Mon, Jun 13, 2016 at 06:07:53AM -0400, Frediano Ziglio wrote: > > > > On Fri, Jun 10, 2016 at 08:45:56AM -0400, Frediano Ziglio wrote: > > > > > > This seems quite strong! I think this will remove the IME context > > > even from the Windows desktop :) > > > > Oh? I just assumed that the win32 hierarchy was more or less the same as > > the GTK+ hierarchy, and that iterating win32 windows between the current > > GTK+ widget and it's GTK+ parent would let us not hardcode just one > > GetParent() > > > > Forget it if it's all wrong ;) > > > > Christophe > > > > Added some debug to the code to understand the hierarchy. > The HWND parent of the widget if the HWND of GTK parent so at > least for these 2 classes there is a 1-to-1 relationship. > These first 2 windows have the same exact size and position > taking all client area (without title and borders) while the > grand parent take all window (so even title and borders). > > The Gtk parent of SpiceDisplay, at least used in remote-viewer > is a GtkBin created in virt-viewer. > So in the end we are changing a property of another Gtk > window. I don't know how this would work with spicy or other > users of this class. I think the reason of this is the way Gtk > handle the events basically taking all events from all windows > and dispatching with Gtk logic. This is different from classic > Windows programs where the dispatching is done by Windows itself. In short, ImmAssociateContext needs to be called both on the SpiceDisplay widget (handled by spice-gtk), and on the GTK+ container it's in, which is owned by the application using spice-gtk? I think I might be fine with calling ImmAssociateContext on gtk_widget_get_parent(SpiceDisplay) from within spice-gtk as this is the more generic solution (ie will do what is needed for every application using spice-gtk). Christophe 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] [spice-gtk PATCH] Handle single headed monitors that have a non-zero x, y config offset
Hey, On Mon, Jun 13, 2016 at 07:09:29PM +0200, Marc-André Lureau wrote: > >> On Thu, Jun 18, 2015 at 07:14:13PM -0400, sstut...@redhat.com wrote: > >> > From: Sandy Stutsman > >> > > >> > Each monitor on a Windows guest is represented as a separate, > >> > single-headed > >> > device with its own framebuffer. When there are multiple monitors, all > >> > monitors but one will have a non-zero xy config position. But even in > >> > these cases the whole area (frame-buffer) of each monitor should be > >> > updated. > >> > > >> > Addresses: https://bugzilla.redhat.com/show_bug.cgi?id=1202419 > > That's not compatible with virgl, which may use a monitor scanout > different than +0+0 (in multimonitor for ex, but one monitor per > display channel). > > I could probably workaround the issue with an extra test such as if > (monitors->len == 1 && !d->egl.enabled)... But that condition looks > wrong to me anyway. > > My understanding is that there is some confusing between monitor > configuration position (the monitor config on main channel), and the > scanout/primary position (the monitor config on display channel). To be honest, I'm not sure about the difference between these... Is this documented anywhere? If you mean that the main channel monitor configuration message should describe the overall monitor layout (including the (x, y) position) of all monitors across all display channels, and then the display channel monitor configuration message only describes the offset of the monitor(s) within the scanout of the current display channel, then yes, the agent changes corresponding to that patch were probably wrong. Christophe 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 v7 server] LZ4 compression is now available at the Spicevmc channel
>
> Compressed message type is CompressedData which contains compression
> type (1 byte) followed by the uncompressed data size (4 bytes - exists
> only if data was compressed) followed by the compressed data
>
> If SPICE_USBREDIR_CAP_DATA_COMPRESS_LZ4 capability is available &&
> data_size > COMPRESS_THRESHOLD && !AF_LOCAL data will be sent
> compressed otherwise data will be sent uncompressed (also if
> compression has failed)
>
> Update the required protocol to 0.12.12
>
> Signed-off-by: Frediano Ziglio
> Signed-off-by: Snir Sheriber
> ---
> configure.ac | 2 +-
> server/spicevmc.c | 140
> ++
> 2 files changed, 132 insertions(+), 10 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index c743875..856833b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -98,7 +98,7 @@ AS_IF([test x"$have_smartcard" = "xyes"], [
> AS_VAR_APPEND([SPICE_REQUIRES], [" libcacard >= 0.1.2"])
> ])
>
> -SPICE_PROTOCOL_MIN_VER=0.12.11
> +SPICE_PROTOCOL_MIN_VER=0.12.12
> PKG_CHECK_MODULES([SPICE_PROTOCOL], [spice-protocol >=
> $SPICE_PROTOCOL_MIN_VER])
> AC_SUBST([SPICE_PROTOCOL_MIN_VER])
>
> diff --git a/server/spicevmc.c b/server/spicevmc.c
> index b662d94..908ad5f 100644
> --- a/server/spicevmc.c
> +++ b/server/spicevmc.c
> @@ -34,6 +34,9 @@
> #include "red-channel.h"
> #include "reds.h"
> #include "migration-protocol.h"
> +#ifdef USE_LZ4
> +#include
> +#endif
>
> /* todo: add flow control. i.e.,
> * (a) limit the tokens available for the client
> @@ -41,10 +44,13 @@
> */
> /* 64K should be enough for all but the largest writes + 32 bytes hdr */
> #define BUF_SIZE (64 * 1024 + 32)
> +#define COMPRESS_THRESHOLD 1000
>
> typedef struct RedVmcPipeItem {
> RedPipeItem base;
>
> +SpiceDataCompressionType type;
> +uint32_t uncompressed_data_size;
> /* writes which don't fit this will get split, this is not a problem */
> uint8_t buf[BUF_SIZE];
> uint32_t buf_used;
> @@ -105,6 +111,53 @@ enum {
> RED_PIPE_ITEM_TYPE_PORT_EVENT,
> };
>
> +static void spicevmc_red_channel_release_msg_rcv_buf(RedChannelClient *rcc,
> + uint16_t type,
> + uint32_t size,
> + uint8_t *msg);
> +/* n is the data size (uncompressed)
> + * msg_item -- the current pipe item with the uncompressed data
> + * This function returns:
> + * - NULL upon failure.
> + * - a new pipe item with the compressed data in it upon success
> + */
> +static RedVmcPipeItem* try_compress_lz4(SpiceVmcState *state, int n,
> RedVmcPipeItem *msg_item)
> +{
> +RedVmcPipeItem *msg_item_compressed;
> +int compressed_data_count;
> +
> +if (reds_stream_get_family(state->rcc->stream) == AF_UNIX) {
> +/* AF_LOCAL - data will not be compressed */
> +return NULL;
> +}
> +if (n <= COMPRESS_THRESHOLD) {
> +/* n <= threshold - data will not be compressed */
> +return NULL;
> +}
> +if (!red_channel_test_remote_cap(&state->channel,
> SPICE_SPICEVMC_CAP_DATA_COMPRESS_LZ4)) {
> +/* Client doesn't have compression cap - data will not be compressed
> */
> +return NULL;
> +}
> +msg_item_compressed = spice_new0(RedVmcPipeItem, 1);
> +red_pipe_item_init(&msg_item_compressed->base,
> RED_PIPE_ITEM_TYPE_SPICEVMC_DATA);
> +compressed_data_count = LZ4_compress_default((char*)&msg_item->buf,
> +
> (char*)&msg_item_compressed->buf,
> + n,
> + BUF_SIZE);
> +
> +if (compressed_data_count > 0 && compressed_data_count < n) {
> +msg_item_compressed->type = SPICE_DATA_COMPRESSION_TYPE_LZ4;
> +msg_item_compressed->uncompressed_data_size = n;
> +msg_item_compressed->buf_used = compressed_data_count;
> +free(msg_item);
> +return msg_item_compressed;
> +}
> +
> +/* LZ4 compression failed or did non compress, fallback a non-compressed
> data is to be sent */
> +free(msg_item_compressed);
> +return NULL;
> +}
> +
> static RedPipeItem
> *spicevmc_chardev_read_msg_from_dev(SpiceCharDeviceInstance *sin,
> void *opaque)
> {
> @@ -121,6 +174,7 @@ static RedPipeItem
> *spicevmc_chardev_read_msg_from_dev(SpiceCharDeviceInstance *
>
> if (!state->pipe_item) {
> msg_item = spice_new0(RedVmcPipeItem, 1);
> +msg_item->type = SPICE_DATA_COMPRESSION_TYPE_NONE;
> red_pipe_item_init(&msg_item->base,
> RED_PIPE_ITEM_TYPE_SPICEVMC_DATA);
> } else {
> spice_assert(state->pipe_item->buf_used == 0);
> @@ -132,6 +186,15 @@ static RedPipeItem
> *spicevmc_chardev_read_msg_from_dev(SpiceCharDeviceInstance *
>sizeof(msg_item->buf));
> if (n > 0) {
> spice_de
Re: [Spice-devel] [PATCH v7 spice-common 3/3] Add LZ4 data compression and use it in spicevmc channel
>
> Compressed message type is CompressedData which contains compression
> type (1 byte) followed by the uncompressed data size (4 bytes-exists
> only if data was compressed) followed by the compressed data
>
> Update the required protocol to 0.12.12:
>
> Signed-off-by: Frediano Ziglio
> Signed-off-by: Snir Sheriber
> ---
> common/client_marshallers.h | 1 +
> common/messages.h | 7 +++
> configure.ac| 2 +-
> spice.proto | 24
> 4 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/common/client_marshallers.h b/common/client_marshallers.h
> index 728987e..2074323 100644
> --- a/common/client_marshallers.h
> +++ b/common/client_marshallers.h
> @@ -33,6 +33,7 @@ SPICE_BEGIN_DECLS
> typedef struct {
> void (*msg_SpiceMsgEmpty)(SpiceMarshaller *m, SpiceMsgEmpty *msg);
> void (*msg_SpiceMsgData)(SpiceMarshaller *m, SpiceMsgData *msg);
> +void (*msg_SpiceMsgCompressedData)(SpiceMarshaller *m,
> SpiceMsgCompressedData *msg);
> void (*msgc_ack_sync)(SpiceMarshaller *m, SpiceMsgcAckSync *msg);
> void (*msgc_pong)(SpiceMarshaller *m, SpiceMsgPing *msg);
> void (*msgc_disconnecting)(SpiceMarshaller *m, SpiceMsgDisconnect *msg);
> diff --git a/common/messages.h b/common/messages.h
> index f537950..516a345 100644
> --- a/common/messages.h
> +++ b/common/messages.h
> @@ -55,6 +55,13 @@ typedef struct SpiceMsgData {
> uint8_t data[0];
> } SpiceMsgData;
>
> +typedef struct SpiceMsgCompressedData {
> +uint8_t type;
> +uint32_t uncompressed_size;
> +uint32_t compressed_size;
> +uint8_t *compressed_data;
> +} SpiceMsgCompressedData;
> +
> typedef struct SpiceMsgEmpty {
> uint8_t padding;
> } SpiceMsgEmpty;
> diff --git a/configure.ac b/configure.ac
> index f8ff024..c3ad5a4 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -28,7 +28,7 @@ AM_PROG_CC_C_O
> SPICE_CHECK_SYSDEPS
>
> # Checks for libraries
> -PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.10])
> +PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.12])
>
> SPICE_CHECK_PYTHON_MODULES()
>
> diff --git a/spice.proto b/spice.proto
> index d21510d..0bfc515 100644
> --- a/spice.proto
> +++ b/spice.proto
> @@ -120,6 +120,28 @@ message Data {
> uint8 data[] @end @ctype(uint8_t);
> } @nocopy;
>
> +enum8 data_compression_type {
> +NONE,
> +LZ4,
> +};
> +
> +struct EmptyStructure {
> +};
> +
> +message CompressedData {
> +data_compression_type type;
> +switch (type) {
> +/* we cannot use !NONE (works only with flags) */
> +case NONE:
> +/* due to the way cases are defined after NONE we must have
> something */
> +/* due to a bug we cannot use @virtual to write 0 to compressed_size
> */
> +EmptyStructure empty;
> +default:
> +uint32 uncompressed_size;
> +} u @anon;
> +uint8 compressed_data[] @as_ptr(compressed_size);
> +} @ctype(SpiceMsgCompressedData);
> +
> struct ChannelWait {
> uint8 channel_type;
> uint8 channel_id;
> @@ -1373,8 +1395,10 @@ channel SmartcardChannel : BaseChannel {
> channel SpicevmcChannel : BaseChannel {
> server:
> Data data = 101;
> +CompressedData compressed_data = 102;
> client:
> Data data = 101;
> +CompressedData compressed_data = 102;
> };
>
> channel UsbredirChannel : SpicevmcChannel {
Acked-by: Frediano Ziglio
Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v7 spice-protocol] LZ4 compression is now available at the Spicevmc channel
>
> -New message type for compressed messages has been added to the protocol
>
> Compressed message type is CompressedData which contains compression
> type (1 byte) followed by the uncompressed data size (4 bytes -exists
> only if data was compressed) followed by the compressed data
>
> -SPICE_SPICEVMC_CAP_DATA_COMPRESS_LZ4 capability has been added
>
> When both sides has the capabilty all usb redirected data will be compressed
> (only if size > threshold and AF_LOCAL socket is not in use)
>
> To send compressed msg use SpiceMsgCompressedData type with
> spice_marshall_SpiceMsgCompressedData and then use add_ref to add the
> compressed data
>
> Signed-off-by: Snir Sheriber
> ---
> spice/enums.h| 9 +
> spice/protocol.h | 4
> 2 files changed, 13 insertions(+)
>
> diff --git a/spice/enums.h b/spice/enums.h
> index c6e9840..1d0c2db 100644
> --- a/spice/enums.h
> +++ b/spice/enums.h
> @@ -106,6 +106,13 @@ typedef enum SpiceMouseMode {
> SPICE_MOUSE_MODE_MASK = 0x3
> } SpiceMouseMode;
>
> +typedef enum SpiceDataCompressionType {
> +SPICE_DATA_COMPRESSION_TYPE_NONE,
> +SPICE_DATA_COMPRESSION_TYPE_LZ4,
> +
> +SPICE_DATA_COMPRESSION_TYPE_ENUM_END
> +} SpiceDataCompressionType;
> +
> typedef enum SpicePubkeyType {
> SPICE_PUBKEY_TYPE_INVALID,
> SPICE_PUBKEY_TYPE_RSA,
> @@ -634,12 +641,14 @@ enum {
>
> enum {
> SPICE_MSG_SPICEVMC_DATA = 101,
> +SPICE_MSG_SPICEVMC_COMPRESSED_DATA,
>
> SPICE_MSG_END_SPICEVMC
> };
>
> enum {
> SPICE_MSGC_SPICEVMC_DATA = 101,
> +SPICE_MSGC_SPICEVMC_COMPRESSED_DATA,
>
> SPICE_MSGC_END_SPICEVMC
> };
> diff --git a/spice/protocol.h b/spice/protocol.h
> index f4a2259..d742eda 100644
> --- a/spice/protocol.h
> +++ b/spice/protocol.h
> @@ -148,6 +148,10 @@ enum {
> };
>
> enum {
> +SPICE_SPICEVMC_CAP_DATA_COMPRESS_LZ4,
> +};
> +
> +enum {
> SPICE_PORT_EVENT_OPENED,
> SPICE_PORT_EVENT_CLOSED,
> SPICE_PORT_EVENT_BREAK,
Acked-by: Frediano Ziglio
Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
