Hi,

On 01/24/2012 09:01 AM, Yonit Halperin wrote:
Hi,

On 01/23/2012 12:36 PM, Hans de Goede wrote:
Hi,

I've various remarks, see my comments inline.

On 01/22/2012 09:12 AM, Yonit Halperin wrote:
Set the default sizes to be the same as in the old linux spice client.
cache_size=20M pixels (instead of 32M), window_size=8M pixels for a 64MB
dev ram (instead of 16M pixels).
---
gtk/channel-display-priv.h | 2 -
gtk/channel-display.c | 22 +++++++---
gtk/channel-main.c | 1 +
gtk/spice-session-priv.h | 12 ++++++
gtk/spice-session.c | 90 +++++++++++++++++++++++++++++++++++++++++++-
5 files changed, 117 insertions(+), 10 deletions(-)

diff --git a/gtk/channel-display-priv.h b/gtk/channel-display-priv.h
index 332d271..eca1787 100644
--- a/gtk/channel-display-priv.h
+++ b/gtk/channel-display-priv.h
@@ -36,8 +36,6 @@

G_BEGIN_DECLS

-#define DISPLAY_PIXMAP_CACHE (1024 * 1024 * 32)
-#define GLZ_WINDOW_SIZE (1024 * 1024 * 16)

typedef struct display_surface {
RingItem link;
diff --git a/gtk/channel-display.c b/gtk/channel-display.c
index ec42829..2473ac6 100644
--- a/gtk/channel-display.c
+++ b/gtk/channel-display.c
@@ -766,13 +766,21 @@ static void emit_invalidate(SpiceChannel
*channel, SpiceRect *bbox)
static void spice_display_channel_up(SpiceChannel *channel)
{
SpiceMsgOut *out;
- SpiceMsgcDisplayInit init = {
- .pixmap_cache_id = 1,
- .pixmap_cache_size = DISPLAY_PIXMAP_CACHE,
- .glz_dictionary_id = 1,
- .glz_dictionary_window_size = GLZ_WINDOW_SIZE,
- };
-
+ SpiceSession *s = spice_channel_get_session(channel);
+ SpiceMsgcDisplayInit init;
+ int cache_size;
+ int glz_window_size;
+
+ g_object_get(s,
+ "cache-size",&cache_size,
+ "glz-window-size",&glz_window_size,
+ NULL);
+ SPICE_DEBUG("%s: cache_size %d, glz_window_size %d (bytes)",
__FUNCTION__,
+ cache_size, glz_window_size);
+ init.pixmap_cache_id = 1;
+ init.glz_dictionary_id = 1;
+ init.pixmap_cache_size = cache_size / 4; /* pixels */
+ init.glz_dictionary_window_size = glz_window_size / 4; /* pixels */
out = spice_msg_out_new(channel, SPICE_MSGC_DISPLAY_INIT);
out->marshallers->msgc_display_init(out->marshaller,&init);
spice_msg_out_send_internal(out);
diff --git a/gtk/channel-main.c b/gtk/channel-main.c
index ebf660f..b2df547 100644
--- a/gtk/channel-main.c
+++ b/gtk/channel-main.c
@@ -1128,6 +1128,7 @@ static void main_handle_init(SpiceChannel
*channel, SpiceMsgIn *in)
init->current_mouse_mode);

spice_session_set_mm_time(session, init->multi_media_time);
+ spice_session_set_caches_hints(session, init->ram_hint,
init->display_channels_hint);

c->agent_tokens = init->agent_tokens;
if (init->agent_connected)
diff --git a/gtk/spice-session-priv.h b/gtk/spice-session-priv.h
index 5df1182..a814cfe 100644
--- a/gtk/spice-session-priv.h
+++ b/gtk/spice-session-priv.h
@@ -27,6 +27,10 @@

G_BEGIN_DECLS

+#define IMAGES_CACHE_SIZE_DEFAULT (1024 * 1024 * 80)
+#define MIN_GLZ_WINDOW_SIZE_DEFAULT (1024 * 1024 * 12)
+#define MAX_GLZ_WINDOW_SIZE_DEFAULT MIN((LZ_MAX_WINDOW_SIZE * 4),
1024 * 1024 * 64)
+

Are these glz window size min and max values only for the default
calculation, or
should the same min / max also be applied in
spice_session_set_property() ? If they
also should be applied in spice_session_set_property, I suggest dropping
the
_DEFAULT, and adding checks for them there. If they should not be
applied in
spice_session_set_property(), I would like to see some additional
MIN/MAX defines
which are used to clamp the input value in spice_session_set_property(),
likewise I
would like to see some min / max defines for the image cache size and the
PROP_CACHE_SIZE input value clamped to these in
spice_session_set_property().

The min/max default values are used only for the default calculation.
In g_object_class_install_property I set the min/max value for both the cache 
and dictionary. Their minimal size is 0. The maximal size for the cache is 
G_MAXINT and for the dictionary it is the maximal value that can be processed 
by the lz code: LZ_MAX_WINDOW_SIZE * 4 ( LZ_MAX_WINDOW_SIZE is in pixels). 
These limits will allow us to test a variety of values. Once we have conclusion 
for the optimal sizes (depending on different conditions), we can set stricter 
limits.
Do you want me to add explicit defines for these limits?

No, my bad, I forgot about the setting of min / max on the g_param_spec, so
wrt min/max things are fine, if you drop the 2 unnecessary get functions
this patch is good to go.

Regards,

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

Reply via email to