date:20160929

Re: [Spice-devel] spice performance tweaking

2016-09-29 Thread Rob Verduijn

Hi,

I'm not using TLS to connect the spice session, so unless there is another
encryption that I'm not aware of, it's already off
And I already tried switching of lzo compression, but if there was an
improvement I couldn't see it.

I'm not sure where the microsoft windows display driver entered the
discussion,
whenever I said window I was referring to one of the square boxes on my
linux desktop that contains the application that I'm lookinf at
(firefox/terminal/chrome).

Rob Verduijn





2016-09-29 13:21 GMT+02:00 Frediano Ziglio :

>
> Hi,
>
> I tried a virtual rhel7.3beta (server with gui) on a rhel7.3beta host.
>
> The host was a laptop to which I setup to use my old wifi router that only
> has 54Mbit so the bandwith was poor and unstable.
>
> Without the compression options the spice display was really bad.
> You could see the screen being rendered bit by bit.
>
> I tried this with the custom rpm and did not really see an improvement.
>
> I downgraded the spice-server and rebooted
>
> I added the compression options:
>   
>   
>   
>   
>
> Now the spice display was very usable and almost no artifacts.
>
> Only when moving the windows you can see tearing at the edges.
> Also started a glxgears window to see how it deals with that
>
> I applied the custom rpm and rebooted the host.
>
> After restarting the vm and logging into spice
> There is less tearing when moving the window.
>
>
> Good! Thanks for the test! I'll work to make these changes integrated in a
> future version.
>
>
>
> So there is an improvement, but it's hard to tell how much  by simply
> watching when draging the window.
>
>
> some details from our openvpn config
> the protocol has to be tcp.
>
> centos 7.2 openvpn server settings:
>
> port secret-port-number
> proto tcp
> dev tun
> tun-mtu 1500
> ca /path/to/ca
> cert /path/to/cert
> key /path/to/key
> dh /path/to/dh
> server some.ip.address some.subnet.mask
> ifconfig-pool-persist ipp.txt
> keepalive 10 120
> comp-lzo
> user nobody
> group nobody
> persist-key
> persist-tun
> status /path/to/log
> verb 3
> mute 20
>
> I would personally try different combination of compression/encryption.
> Obviously you want encryption on the VPN but if you have also encryption
> on Spice this
> is mainly wasting bandwidth and latency.
> Also depending on the type of data VPN compression can be a bad idea.
> I would try (if this is possible) to turn it off.
>
> On the Windows side I tried to see what could cause the flicker issue.
> It seems for some reason (unknown to me) some commands are trying to read
> some data back
> from the card causing a 0.01 seconds delay (which could be one cause of
> the delay). No much
> to suggest to improve this. Looks like the XPDM driver can be compiled
> using different way
> to do the synchronization and looks like the new WDDM driver does the
> synchronization in
> a different way (not clear why).
> I don't think I'll have much more time to investigate into this.
> I would suggest to open a ticket upstream so people don't forget this.
>
> Frediano
>
> fedora 24 NetworkManager settings:
> [connection]
> id=our id for our top secret vpn
> uuid=..xxx.xxx.xx
> type=vpn
> permissions=user:secretusernottellingyou:;
> secondaries=
>
> [vpn]
> remote-random=no
> connection-type=tls
> remote=remote.ip.address
> tunnel-mtu=1500
> comp-lzo=yes
> cert-pass-flags=1
> proto-tcp=yes
> port=
> mssfix=yes
> dev-type=tun
> ca=/path/to/ca
> key=/path/to/key
> cert=/path/to/crt
> service-type=org.freedesktop.NetworkManager.openvpn
>
> [ipv4]
> dns-search=
> ignore-auto-routes=true
> method=auto
> never-default=true
>
>
> route1=additional.route/mask
>
>
> route2=additional.route/mask
>
>
>
> [ipv6]
>
>
> addr-gen-mode=stable-privacy
>
>
> dns-search=
>
>
> method=auto
>
>
>
> Cheers
> Rob Verduijn.
>
>
>
>
> 2016-09-15 16:04 GMT+02:00 Rob Verduijn :
>
>> Hello,
>>
>> The performance fixes sound awesome.
>> I'm afraid I cannot test them in the environment with the low bandwith
>> setup soon (maybe next week, but don't hold your breath)
>> I'm going to build a local test setup to see if this is improving some of
>> the issues. I got a laptop running the rhel7.3 beta, let's see how that
>> performs.
>>
>> Rob Verduijn
>>
>>
>> 2016-09-15 13:31 GMT+02:00 Frediano Ziglio :
>>
>>> I saw you are using CentOS 7. I built the package with RHEL 7 (they are
>>> binary compatible).
>>> About the testing just which normal usage you should see improvements in
>>> bandwidth and
>>> reactivity.
>>>
>>> Changes from current CentOS package:
>>> - used a newer version, there are couple of changes that decrease
>>> latency;
>>> - additional patches to improve bandwidth usage (for small drawing this
>>> should decrease
>>>   bandwidth usage by a 15-20%);
>>> - additional patch to decrease a bandwidth limitation due to a peculiar
>>> half-duplex usage
>>>   of spice protocol (this is clearly visible with

[Spice-devel] [PATCH v2 0/4] Protocol file syntax documentation

2016-09-29 Thread Frediano Ziglio

Small update for this patchset:
- fix headers in "Extended protocol documentation";
- added some more documentation on attributes.

Frediano Ziglio (4):
  Start adding protocol file documentation
  Start writing some documentation on protocol
  Extended protocol documentation
  More work on attribute protocol documentation

 Makefile.am |   2 +-
 configure.ac|   2 +
 docs/Makefile.am|  17 ++
 docs/spice_protocol.txt | 423 
 m4/spice_manual.m4  |  32 
 5 files changed, 475 insertions(+), 1 deletion(-)
 create mode 100644 docs/Makefile.am
 create mode 100644 docs/spice_protocol.txt
 create mode 100644 m4/spice_manual.m4

-- 
2.7.4

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

[Spice-devel] [PATCH v2 3/4] Extended protocol documentation

2016-09-29 Thread Frediano Ziglio

Signed-off-by: Frediano Ziglio 
---
 docs/spice_protocol.txt | 360 
 1 file changed, 360 insertions(+)

diff --git a/docs/spice_protocol.txt b/docs/spice_protocol.txt
index 3406cb9..832ea73 100644
--- a/docs/spice_protocol.txt
+++ b/docs/spice_protocol.txt
@@ -23,6 +23,8 @@ It resemble the C format.
 
 (here BNF with some regular expression is used).
 
+It's used to generate automatically code to marshall/demarshall the network 
data.
+
 Example:
 
 channel ExampleChannel {
@@ -47,4 +49,362 @@ Both C and C++ style comments are supported
 /* this is a comment too
but can be split in multiple lines */
 
+Base types
+--
+
+All int from 8 to 64 bit (8, 16, 32 and 64) are supported either signed or 
unsigned.
+Also you can pass one unix descriptor.
+
+base_type ::= 
"int8"|"uint8"|"int16"|"uint16"|"int32"|"uint32"|"int64"|"uint64"|"unix_fd" ;
+
+Example:
+
+int16 x;
+
+Enumerations and flags
+--
+
+It's possible to specify enumerations and flags. The difference is that flags 
are defined as 2 power
+values and can combined. Enumerations and flags must have a size (`8`, `16` or 
`32`) specified.
+
+enum ::=  "{" [  ] "}"  ";" ;
+flag ::=  "{" [  ] "}"  ";" ;
+enum_type ::= "enum8"|"enum16"|"enum32" ;
+flag_type ::= "flag8"|"flag16"|"flag32" ;
+enumflag_items ::= |
+enumflag_item ::=  [ "="  ] [ "," ] ;
+enum_name ::= [a-z0-9_]* ;
+
+Example:
+
+enum16 Level {
+LOW = 0x100,
+MEDIUM,
+HIGH = 0x1000
+};
+
+Variables
+-
+
+As you should already have noted variables are similar to C syntax but there 
are
+some differences.
+
+variable ::=  [ "*" ]  [ "["  "]" ] 
;
+
+The `*` specify a pointer. This is quite different from C. For the protocol it
+specifies that in the protocol stream a relative offset is put that points to 
that
+variable usually after all defined fields. This happens even on arrays, so for 
instance
+
+int32 *n;
+
+containing a 0x12345678 `n` value could ended up coded as
+
+04 00 00 00 // 4 as offset
+78 56 34 12 // `n`
+
+(little endian). While an array of 2 items defined as
+
+int32 *n[2];
+
+and containing 0x12345678 and 0x9abcdef could end up with
+
+04 00 00 00 // 4 as offset
+78 56 34 12 // `n`[0]
+ef cd ab 09 // `n`[1]
+
+note that `int32 *n[2]` defined a pointer to an array of 2 items and not
+an array of pointers as C.
+
+*WARNING*: You should avoid using pointers on protocol if not necessary as 
they are complicated
+to handle not using autogenerated code and also use more space on the network.
+
+Arrays
+--
+
+As seen above the easiest way to define an array size is specifiying a 
constant value.
+However there are multiple way to specify the size
+
+array_size ::= 
||""|||
 ;
+array_size_image ::= "image_size" "("  ","  ")" ;
+array_size_bytes ::= "bytes" "("  ","  ")" ;
+array_size_cstring ::= "cstring()" ;
+
+We already seen integer.
+Specifying an identifier name instead (should be variable) indicate that the 
length is specified
+in another field, for instance
+
+uint8 name_len;
+int8 name[name_len];
+
+allows to put a name of `name_len` len.
+The empty value tell that the array will end when the containing message end 
so if we have
+
+int8 name[];
+
+and the message is
+
+66 6f 6f
+
+possibly the name we want is `foo` (66 6f 6f is the ASCII encoding for `foo`).
+
+TODO: what happen with two [] in the structure ??
+TODO: can a [] array not be the last and what happens ??
+
+`image_size` allow to specify an array holding an image, for instance
+
+uint16 width;
+uint16 heigth;
+uin18 raw_image[image_size(8, width, height)];
+
+could contain row data in raw_image. The constant `8` is the bit size of the 
image.
+
+TODO `bytes`
+
+`cstring` allows to specify NUL-terminated sequence so having
+
+int8 name[cstring()];
+
+and the message as
+
+66 6f 6f 00
+
+we'll have the `foo` name. Note that the field does not need to end the 
message as in `int8 name[]` example.
+
+Structures
+--
+
+The simpler coumpound type is the structure. As in C is defined as a list of 
fields (any variable or swicth).
+But as a protocol definition there are no alignment or padding and every field 
(beside pointer values) follow each other.
+
+ ::= "struct"  "{" [  ] "}"  ";" ;
+ ::= | ;
+ ::= |
+
+Example:
+
+struct Point {
+int32 x;
+int32 y;
+};
+
+Messages
+
+
+Messages have the same syntax of structure (beside `message`) with the 
different that they can
+be used directly inside channels.
+
+ ::= "message"  "{" [  ] "}"  ";" 
;
+
+Switches
+
+
+TODO
+
+Type definitions
+
+
+Like C type definition allow to short types defining new ones.
+
+ ::= "typedef"  ` ;
+
+note that unlike C name came before the type.
+
+Example:
+
+typedef XCoord int32;
+

[Spice-devel] [PATCH v2 4/4] More work on attribute protocol documentation

2016-09-29 Thread Frediano Ziglio

Signed-off-by: Frediano Ziglio 
---
 docs/spice_protocol.txt | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/docs/spice_protocol.txt b/docs/spice_protocol.txt
index 832ea73..13506f1 100644
--- a/docs/spice_protocol.txt
+++ b/docs/spice_protocol.txt
@@ -336,12 +336,21 @@ TODO
 nomarshal
 ~
 
-TODO
+Do not generate code for marshalling this variable.
+Usually used on last array element to make possible to manually feed data.
+
+Example:
+struct Data {
+uint32 data_size;
+uint8 data[data_size] @nomarshal;
+};
 
 zero_terminated
 ~~~
 
-TODO
+The field should terminated by zero.
+Actually it's not used by python code so it's not enforced and no
+code is generated.
 
 marshall
 
@@ -351,12 +360,16 @@ TODO
 nonnull
 ~~~
 
-TODO
+This pointer field cannot be NULL. This means that marshaller assume C 
structure
+contain not NULL pointer and demarshaller will fail to demarshall message if 
offset
+is 0.
 
 unique_flag
 ~~~
 
-TODO
+This flag field should contain just a single flag.
+Actually it's not used by python code so it's not enforced and no
+code is generated.
 
 ptr_array
 ~
-- 
2.7.4

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

[Spice-devel] [PATCH v2 1/4] Start adding protocol file documentation

2016-09-29 Thread Frediano Ziglio

The protocol file is not documented and people have to read code to
understand the specification.
This can lead to unexpected or not optimal results so it's better
to have it documented.
The m4/spice_manual.m4 came from spice server and is meant to be
reused.

Signed-off-by: Frediano Ziglio 
---
 Makefile.am |  2 +-
 configure.ac|  2 ++
 docs/Makefile.am| 17 +
 docs/spice_protocol.txt |  2 ++
 m4/spice_manual.m4  | 32 
 5 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100644 docs/Makefile.am
 create mode 100644 docs/spice_protocol.txt
 create mode 100644 m4/spice_manual.m4

diff --git a/Makefile.am b/Makefile.am
index 63d7956..ee0a1e2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,7 +1,7 @@
 NULL =
 ACLOCAL_AMFLAGS = -I m4
 
-SUBDIRS = python_modules common tests
+SUBDIRS = python_modules common tests docs
 
 EXTRA_DIST =   \
spice_codegen.py\
diff --git a/configure.ac b/configure.ac
index c3ad5a4..6d9f378 100644
--- a/configure.ac
+++ b/configure.ac
@@ -17,6 +17,7 @@ AM_INIT_AUTOMAKE([1.11 dist-xz no-dist-gzip tar-ustar foreign 
-Wall -Werror])
 AM_MAINTAINER_MODE
 AM_SILENT_RULES([yes])
 LT_INIT
+SPICE_MANUAL
 
 AC_PROG_CC
 AC_PROG_CC_C99
@@ -50,6 +51,7 @@ AC_CONFIG_FILES([
   common/Makefile
   python_modules/Makefile
   tests/Makefile
+  docs/Makefile
 ])
 
 AH_BOTTOM([
diff --git a/docs/Makefile.am b/docs/Makefile.am
new file mode 100644
index 000..47f40c3
--- /dev/null
+++ b/docs/Makefile.am
@@ -0,0 +1,17 @@
+NULL =
+ASCIIDOC_FLAGS = -a icons -a toc
+
+EXTRA_DIST =   \
+   spice_protocol.html \
+   spice_protocol.txt  \
+   $(NULL)
+
+if BUILD_HTML_MANUAL
+all-local: spice_protocol.html
+
+spice_protocol.html: spice_protocol.txt
+   $(AM_V_GEN) $(ASCIIDOC) -n $(ASCIIDOC_FLAGS) -o $@ $<
+endif
+
+clean-local:
+   rm -f spice_protocol.html
diff --git a/docs/spice_protocol.txt b/docs/spice_protocol.txt
new file mode 100644
index 000..b62da25
--- /dev/null
+++ b/docs/spice_protocol.txt
@@ -0,0 +1,2 @@
+Spice protocol format file
+==
diff --git a/m4/spice_manual.m4 b/m4/spice_manual.m4
new file mode 100644
index 000..c36f6f7
--- /dev/null
+++ b/m4/spice_manual.m4
@@ -0,0 +1,32 @@
+dnl SPICE_MANUAL
+dnl 
+dnl Check if user wants manuals to be compiled and
+dnl if all programs (asciidoc and a2x) are available
+dnl 
+dnl Shell defines:
+dnl - have_asciidoc yes or not is asciidoc program is available
+dnl Automake macros:
+dnl - A2X a2x program or empty
+dnl - ASCIIDOC asciidoc program or emtpy
+dnl - BUILD_MANUAL if asciidoc and a2x are available
+dnl - BUILD_HTML_MANUAL if asciidoc is available (html can be produced)
+dnl - BUILD_CHUNKED_MANUAL if a2x is available
+AC_DEFUN([SPICE_MANUAL],[
+AC_ARG_ENABLE([manual],
+   AS_HELP_STRING([--enable-manual=@<:@auto/yes/no@:>@],
+  [Build SPICE manual]),
+   [],
+   [enable_manual="auto"])
+if test "x$enable_manual" != "xno"; then
+AC_PATH_PROG([ASCIIDOC], [asciidoc])
+AS_IF([test -z "$ASCIIDOC" && test "x$enable_manual" = "xyes"],
+  [AC_MSG_ERROR([asciidoc is missing and build of manual was 
requested])])
+AC_PATH_PROG([A2X], [a2x])
+AS_IF([test -z "$A2X" && test "x$enable_manual" = "xyes"],
+  [AC_MSG_ERROR([a2x is missing and build of manual was 
requested])])
+fi
+AS_IF([test -n "$ASCIIDOC"], [have_asciidoc=yes], [have_asciidoc=no])
+AM_CONDITIONAL([BUILD_MANUAL], [test -n "$ASCIIDOC" || test -n "$A2X"])
+AM_CONDITIONAL([BUILD_HTML_MANUAL], [test -n "$ASCIIDOC"])
+AM_CONDITIONAL([BUILD_CHUNKED_MANUAL], [test -n "$A2X"])
+])
-- 
2.7.4

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

[Spice-devel] [PATCH v2 2/4] Start writing some documentation on protocol

2016-09-29 Thread Frediano Ziglio

Signed-off-by: Frediano Ziglio 
---
 docs/spice_protocol.txt | 48 
 1 file changed, 48 insertions(+)

diff --git a/docs/spice_protocol.txt b/docs/spice_protocol.txt
index b62da25..3406cb9 100644
--- a/docs/spice_protocol.txt
+++ b/docs/spice_protocol.txt
@@ -1,2 +1,50 @@
 Spice protocol format file
 ==
+
+Copyright (C) 2016 Red Hat, Inc.
+Licensed under a Creative Commons Attribution-Share Alike 3.0
+United States License (see 
http://creativecommons.org/licenses/by-sa/3.0/us/legalcode).
+
+Basic
+-
+The spice protocol format file defines the network protocol used by spice.
+It resemble the C format.
+
+file ::=   ;
+definitions ::= | ;
+definition ::= | ;
+protocol ::= "protocol"  "{"  "}" ";" ;
+protocol_channels ::= 
| ;
+protocol_channel ::=   [ "="  ] ";" ;
+integer ::= | ;
+dec ::= [+-][0-9]+ ;
+hex ::= "0x" [0-9a-f]+ ;
+identifier ::= [a-z][a-z0-9_]* ;
+
+(here BNF with some regular expression is used).
+
+Example:
+
+channel ExampleChannel {
+   message {
+  uint32 dummy;
+   } Dummy;
+};
+
+protocol Example {
+ExampleChannel first = 1001;
+};
+
+As you can see brackets like C are used and structures looks like C but you
+can also see that keyworks like `channel`, `protocol`, `message` or some
+predefined types like `uint32` are proper of the protocol.
+
+Comments
+
+Both C and C++ style comments are supported
+
+// this is a comment
+/* this is a comment too
+   but can be split in multiple lines */
+
+
-- 
2.7.4

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

Re: [Spice-devel] spice performance tweaking

2016-09-29 Thread Frediano Ziglio

> Hi,

> I tried a virtual rhel7.3beta (server with gui) on a rhel7.3beta host.

> The host was a laptop to which I setup to use my old wifi router that only
> has 54Mbit so the bandwith was poor and unstable.

> Without the compression options the spice display was really bad.
> You could see the screen being rendered bit by bit.

> I tried this with the custom rpm and did not really see an improvement.

> I downgraded the spice-server and rebooted

> I added the compression options:
> 
> 
> 
> 

> Now the spice display was very usable and almost no artifacts.

> Only when moving the windows you can see tearing at the edges.
> Also started a glxgears window to see how it deals with that

> I applied the custom rpm and rebooted the host.

> After restarting the vm and logging into spice
> There is less tearing when moving the window.

Good! Thanks for the test! I'll work to make these changes integrated in a 
future version. 

> So there is an improvement, but it's hard to tell how much by simply watching
> when draging the window.

> some details from our openvpn config
> the protocol has to be tcp.

> centos 7.2 openvpn server settings:

> port secret-port-number
> proto tcp
> dev tun
> tun-mtu 1500
> ca /path/to/ca
> cert /path/to/cert
> key /path/to/key
> dh /path/to/dh
> server some.ip.address some.subnet.mask
> ifconfig-pool-persist ipp.txt
> keepalive 10 120
> comp-lzo
> user nobody
> group nobody
> persist-key
> persist-tun
> status /path/to/log
> verb 3
> mute 20

I would personally try different combination of compression/encryption. 
Obviously you want encryption on the VPN but if you have also encryption on 
Spice this 
is mainly wasting bandwidth and latency. 
Also depending on the type of data VPN compression can be a bad idea. 
I would try (if this is possible) to turn it off. 

On the Windows side I tried to see what could cause the flicker issue. 
It seems for some reason (unknown to me) some commands are trying to read some 
data back 
from the card causing a 0.01 seconds delay (which could be one cause of the 
delay). No much 
to suggest to improve this. Looks like the XPDM driver can be compiled using 
different way 
to do the synchronization and looks like the new WDDM driver does the 
synchronization in 
a different way (not clear why). 
I don't think I'll have much more time to investigate into this. 
I would suggest to open a ticket upstream so people don't forget this. 

Frediano 

> fedora 24 NetworkManager settings:
> [connection]
> id=our id for our top secret vpn
> uuid=..xxx.xxx.xx
> type=vpn
> permissions=user:secretusernottellingyou:;
> secondaries=

> [vpn]
> remote-random=no
> connection-type=tls
> remote=remote.ip.address
> tunnel-mtu=1500
> comp-lzo=yes
> cert-pass-flags=1
> proto-tcp=yes
> port=
> mssfix=yes
> dev-type=tun
> ca=/path/to/ca
> key=/path/to/key
> cert=/path/to/crt
> service-type=org.freedesktop.NetworkManager.openvpn

> [ipv4]
> dns-search=
> ignore-auto-routes=true
> method=auto
> never-default=true
> route1=additional.route/mask
> route2=additional.route/mask

> [ipv6]
> addr-gen-mode=stable-privacy
> dns-search=
> method=auto

> Cheers
> Rob Verduijn.

> 2016-09-15 16:04 GMT+02:00 Rob Verduijn < rob.verdu...@gmail.com > :

> > Hello,
> 

> > The performance fixes sound awesome.
> 
> > I'm afraid I cannot test them in the environment with the low bandwith
> > setup
> > soon (maybe next week, but don't hold your breath)
> 
> > I'm going to build a local test setup to see if this is improving some of
> > the
> > issues. I got a laptop running the rhel7.3 beta, let's see how that
> > performs.
> 

> > Rob Verduijn
> 

> > 2016-09-15 13:31 GMT+02:00 Frediano Ziglio < fzig...@redhat.com > :
> 

> > > I saw you are using CentOS 7. I built the package with RHEL 7 (they are
> > > binary compatible).
> > 
> 
> > > About the testing just which normal usage you should see improvements in
> > > bandwidth and
> > 
> 
> > > reactivity.
> > 
> 

> > > Changes from current CentOS package:
> > 
> 
> > > - used a newer version, there are couple of changes that decrease
> > > latency;
> > 
> 
> > > - additional patches to improve bandwidth usage (for small drawing this
> > > should decrease
> > 
> 
> > > bandwidth usage by a 15-20%);
> > 
> 
> > > - additional patch to decrease a bandwidth limitation due to a peculiar
> > > half-duplex usage
> > 
> 
> > > of spice protocol (this is clearly visible with high latency
> > > connections);
> > 
> 
> > > - additional patch to decrease packet fragmentation due to TCP_NODELAY
> > > usage.
> > 
> 

> > > Alternatively would be helpful for us to get a local reproduction of the
> > > problem.
> > 
> 
> > > OpenVPN configuration files would be helpful (we don't need any security
> > 
> 
> > > detail like keys, ip, host or system names, just to understand the type
> > > of
> > > VPN,
> > 
> 
> > > encryption parameters, compression, additional latency introduced and so
> > > on).
> > 
> 

> > > The fact that you are

[Spice-devel] [PATCH xf86-video-qxl] Adjust xspice_wakeup_handler() prototype for building xspice with server 1.19

2016-09-29 Thread Hans de Goede

Signed-off-by: Hans de Goede 
---
 src/spiceqxl_main_loop.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/spiceqxl_main_loop.c b/src/spiceqxl_main_loop.c
index db89b6d..0ac1f3e 100644
--- a/src/spiceqxl_main_loop.c
+++ b/src/spiceqxl_main_loop.c
@@ -330,7 +330,11 @@ static int no_write_watches(Ring *w)
 return 1;
 }
 
+#if ABI_VIDEODRV_VERSION >= SET_ABI_VERSION(23, 0)
+static void xspice_wakeup_handler(void *data, int nfds)
+#else
 static void xspice_wakeup_handler(pointer data, int nfds, pointer readmask)
+#endif
 {
 if (!nfds && no_write_watches()) {
 return;
-- 
2.9.3

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

[Spice-devel] [PATCH] Cache field to make code easier

2016-09-29 Thread Frediano Ziglio

Signed-off-by: Frediano Ziglio 
---
 server/display-channel.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/server/display-channel.c b/server/display-channel.c
index d7ea7d5..32f8e67 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1976,22 +1976,23 @@ void display_channel_process_surface_cmd(DisplayChannel 
*display,
 
 switch (surface_cmd->type) {
 case QXL_SURFACE_CMD_CREATE: {
-uint32_t height = surface_cmd->u.surface_create.height;
-int32_t stride = surface_cmd->u.surface_create.stride;
+const RedSurfaceCreate *create = _cmd->u.surface_create;
+uint32_t height = create->height;
+int32_t stride = create->stride;
 int reloaded_surface = loadvm || (surface_cmd->flags & 
QXL_SURF_FLAG_KEEP_DATA);
 
 if (surface->refs) {
 spice_warning("avoiding creating a surface twice");
 break;
 }
-data = surface_cmd->u.surface_create.data;
+data = create->data;
 if (stride < 0) {
 /* No need to worry about overflow here, command should already be 
validated
  * when it is read, specifically red_get_surface_cmd */
 data -= (int32_t)(stride * (height - 1));
 }
-display_channel_create_surface(display, surface_id, 
surface_cmd->u.surface_create.width,
-   height, stride, 
surface_cmd->u.surface_create.format, data,
+display_channel_create_surface(display, surface_id, create->width,
+   height, stride, create->format, data,
reloaded_surface,
// reloaded surfaces will be sent on 
demand
!reloaded_surface);
-- 
2.7.4

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

[Spice-devel] [PATCH RFC 10/14] Pass surface directly to dcc_create_surface

2016-09-29 Thread Frediano Ziglio

Signed-off-by: Frediano Ziglio 
---
 server/dcc.c | 13 ++---
 server/dcc.h |  3 +--
 server/display-channel.c | 11 ++-
 3 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/server/dcc.c b/server/dcc.c
index c4510b0..8a63caf 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -127,10 +127,10 @@ int 
dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, RedSurface
 return red_channel_client_wait_outgoing_item(rcc, 
DISPLAY_CLIENT_SHORT_TIMEOUT);
 }
 
-void dcc_create_surface(DisplayChannelClient *dcc, int surface_id)
+void dcc_create_surface(DisplayChannelClient *dcc, RedSurface *surface)
 {
 DisplayChannel *display;
-RedSurface *surface;
+uint32_t surface_id = surface->id;
 RedSurfaceCreateItem *create;
 uint32_t flags;
 
@@ -139,14 +139,13 @@ void dcc_create_surface(DisplayChannelClient *dcc, int 
surface_id)
 }
 
 display = DCC_TO_DC(dcc);
-flags = is_primary_surface_id(DCC_TO_DC(dcc), surface_id) ? 
SPICE_SURFACE_FLAGS_PRIMARY : 0;
+flags = is_primary_surface(DCC_TO_DC(dcc), surface) ? 
SPICE_SURFACE_FLAGS_PRIMARY : 0;
 
 /* don't send redundant create surface commands to client */
 if (!dcc || display->common.during_target_migrate ||
 dcc->priv->surface_client_created[surface_id]) {
 return;
 }
-surface = >priv->surfaces[surface_id];
 create = red_surface_create_item_new(RED_CHANNEL(display),
  surface_id, surface->context.width,
  surface->context.height,
@@ -256,7 +255,7 @@ static void 
add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
 if (dcc->priv->surface_client_created[surface_id] == TRUE) {
 continue;
 }
-dcc_create_surface(dcc, surface_id);
+dcc_create_surface(dcc, drawable->surface_deps[x]);
 display_channel_current_flush(display, drawable->surface_deps[x]);
 dcc_push_surface_image(dcc, surface_id);
 }
@@ -266,7 +265,7 @@ static void 
add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
 return;
 }
 
-dcc_create_surface(dcc, drawable->surface->id);
+dcc_create_surface(dcc, drawable->surface);
 display_channel_current_flush(display, drawable->surface);
 dcc_push_surface_image(dcc, drawable->surface->id);
 }
@@ -439,7 +438,7 @@ void dcc_start(DisplayChannelClient *dcc)
 if (display->priv->surfaces[0].context.canvas) {
 display_channel_current_flush(display, >priv->surfaces[0]);
 red_channel_client_pipe_add_type(rcc, 
RED_PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE);
-dcc_create_surface(dcc, 0);
+dcc_create_surface(dcc, >priv->surfaces[0]);
 dcc_push_surface_image(dcc, 0);
 dcc_push_monitors_config(dcc);
 red_pipe_add_verb(rcc, SPICE_MSG_DISPLAY_MARK);
diff --git a/server/dcc.h b/server/dcc.h
index 47386ff..2068d78 100644
--- a/server/dcc.h
+++ b/server/dcc.h
@@ -122,8 +122,7 @@ void   dcc_stream_agent_clip
 (DisplayCha
   
StreamAgent *agent);
 void   dcc_create_stream 
(DisplayChannelClient *dcc,
   Stream 
*stream);
-void   dcc_create_surface
(DisplayChannelClient *dcc,
-  int 
surface_id);
+void dcc_create_surface(DisplayChannelClient *dcc, struct RedSurface *surface);
 void   dcc_push_surface_image
(DisplayChannelClient *dcc,
   int 
surface_id);
 RedImageItem * dcc_add_surface_area_image
(DisplayChannelClient *dcc,
diff --git a/server/display-channel.c b/server/display-channel.c
index e0a91a3..64a59de 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1773,15 +1773,15 @@ void display_channel_destroy_surfaces(DisplayChannel 
*display)
 display_channel_free_glz_drawables(display);
 }
 
-static void send_create_surface(DisplayChannel *display, int surface_id, int 
image_ready)
+static void send_create_surface(DisplayChannel *display, RedSurface *surface, 
int image_ready)
 {
 DisplayChannelClient *dcc;
 GListIter iter;
 
 FOREACH_DCC(display, iter, dcc) {
-dcc_create_surface(dcc, surface_id);
+dcc_create_surface(dcc, surface);
 if (image_ready)
-dcc_push_surface_image(dcc, surface_id);
+dcc_push_surface_image(dcc, surface->id);
 }
 }
 
@@ -1854,8 +1854,9 @@ void display_channel_create_surface(DisplayChannel 
*display, uint32_t surface_id
 }
 
 spice_return_if_fail(surface->context.canvas);
-if

[Spice-devel] [PATCH RFC 08/14] Pass surface directly calling display_channel_current_flush

2016-09-29 Thread Frediano Ziglio

Signed-off-by: Frediano Ziglio 
---
 server/dcc.c | 6 +++---
 server/display-channel.c | 8 
 server/display-channel.h | 2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/server/dcc.c b/server/dcc.c
index 3e597b8..c4510b0 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -257,7 +257,7 @@ static void 
add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
 continue;
 }
 dcc_create_surface(dcc, surface_id);
-display_channel_current_flush(display, surface_id);
+display_channel_current_flush(display, drawable->surface_deps[x]);
 dcc_push_surface_image(dcc, surface_id);
 }
 }
@@ -267,7 +267,7 @@ static void 
add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
 }
 
 dcc_create_surface(dcc, drawable->surface->id);
-display_channel_current_flush(display, drawable->surface->id);
+display_channel_current_flush(display, drawable->surface);
 dcc_push_surface_image(dcc, drawable->surface->id);
 }
 
@@ -437,7 +437,7 @@ void dcc_start(DisplayChannelClient *dcc)
 
 red_channel_client_ack_zero_messages_window(RED_CHANNEL_CLIENT(dcc));
 if (display->priv->surfaces[0].context.canvas) {
-display_channel_current_flush(display, 0);
+display_channel_current_flush(display, >priv->surfaces[0]);
 red_channel_client_pipe_add_type(rcc, 
RED_PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE);
 dcc_create_surface(dcc, 0);
 dcc_push_surface_image(dcc, 0);
diff --git a/server/display-channel.c b/server/display-channel.c
index 006b218..7627df9 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1152,7 +1152,7 @@ void display_channel_flush_all_surfaces(DisplayChannel 
*display)
 
 for (x = 0; x < NUM_SURFACES; ++x) {
 if (display->priv->surfaces[x].context.canvas) {
-display_channel_current_flush(display, x);
+display_channel_current_flush(display, 
>priv->surfaces[x]);
 }
 }
 }
@@ -1203,12 +1203,12 @@ static bool free_one_drawable(DisplayChannel *display, 
int force_glz_free)
 return TRUE;
 }
 
-void display_channel_current_flush(DisplayChannel *display, int surface_id)
+void display_channel_current_flush(DisplayChannel *display, RedSurface 
*surface)
 {
-while (!ring_is_empty(>priv->surfaces[surface_id].current_list)) {
+while (!ring_is_empty(>current_list)) {
 free_one_drawable(display, FALSE);
 }
-current_remove_all(display, >priv->surfaces[surface_id]);
+current_remove_all(display, surface);
 }
 
 void display_channel_free_some(DisplayChannel *display)
diff --git a/server/display-channel.h b/server/display-channel.h
index 59ec6a4..bd386a7 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -269,7 +269,7 @@ void   
display_channel_compress_stats_reset  (DisplayCha
 void   display_channel_surface_id_unref  
(DisplayChannel *display,
   uint32_t 
surface_id);
 void   display_channel_current_flush 
(DisplayChannel *display,
-  int 
surface_id);
+  
RedSurface *surface);
 intdisplay_channel_wait_for_migrate_data 
(DisplayChannel *display);
 void   display_channel_flush_all_surfaces
(DisplayChannel *display);
 void   
display_channel_free_glz_drawables_to_free(DisplayChannel *display);
-- 
2.7.4

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

[Spice-devel] [PATCH RFC 14/14] Reuse more validate_surface

2016-09-29 Thread Frediano Ziglio

Signed-off-by: Frediano Ziglio 
---
 server/display-channel.c | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/server/display-channel.c b/server/display-channel.c
index cf019fe..c4d07f7 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -968,15 +968,15 @@ static void draw_depend_on_me(DisplayChannel *display, 
RedSurface *surface)
 static int validate_drawable_bbox(DisplayChannel *display, RedDrawable 
*drawable)
 {
 DrawContext *context;
-uint32_t surface_id = drawable->surface_id;
 
 /* surface_id must be validated before calling into
  * validate_drawable_bbox
  */
-if (!display_channel_validate_surface(display, drawable->surface_id)) {
+RedSurface *surface = display_channel_validate_surface(display, 
drawable->surface_id);
+if (!surface) {
 return FALSE;
 }
-context = >priv->surfaces[surface_id].context;
+context = >context;
 
 if (drawable->bbox.top < 0)
 return FALSE;
@@ -1687,12 +1687,11 @@ void display_channel_update(DisplayChannel *display,
 QXLRect **qxl_dirty_rects, uint32_t 
*num_dirty_rects)
 {
 SpiceRect rect;
-RedSurface *surface;
+RedSurface *surface = display_channel_validate_surface(display, 
surface_id);
 
-spice_return_if_fail(display_channel_validate_surface(display, 
surface_id));
+spice_return_if_fail(surface);
 
 red_get_rect_ptr(, area);
-surface = >priv->surfaces[surface_id];
 display_channel_surface_draw(display, surface, );
 
 if (*qxl_dirty_rects == NULL) {
@@ -1732,11 +1731,10 @@ static void 
display_channel_destroy_surface(DisplayChannel *display, RedSurface
 
 void display_channel_destroy_surface_wait(DisplayChannel *display, uint32_t 
surface_id)
 {
-if (!display_channel_validate_surface(display, surface_id))
-return;
-RedSurface *surface = >priv->surfaces[surface_id];
-if (!surface->context.canvas)
+RedSurface *surface = display_channel_validate_surface(display, 
surface_id);
+if (!surface) {
 return;
+}
 
 draw_depend_on_me(display, surface);
 /* note that draw_depend_on_me must be called before current_remove_all.
-- 
2.7.4

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

[Spice-devel] [PATCH RFC 12/14] Use directly surface instead of id

2016-09-29 Thread Frediano Ziglio

Signed-off-by: Frediano Ziglio 
---
 server/dcc.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/server/dcc.c b/server/dcc.c
index a800aaa..eeb2867 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -243,28 +243,28 @@ static void 
add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
 {
 DisplayChannel *display = DCC_TO_DC(dcc);
 int x;
+RedSurface *surface;
 
 for (x = 0; x < 3; ++x) {
-int surface_id;
-
-if (drawable->surface_deps[x] != NULL) {
-surface_id = drawable->surface_deps[x]->id;
-if (dcc->priv->surface_client_created[surface_id] == TRUE) {
+surface = drawable->surface_deps[x];
+if (surface) {
+if (dcc->priv->surface_client_created[surface->id] == TRUE) {
 continue;
 }
-dcc_create_surface(dcc, drawable->surface_deps[x]);
-display_channel_current_flush(display, drawable->surface_deps[x]);
-dcc_push_surface_image(dcc, drawable->surface_deps[x]);
+dcc_create_surface(dcc, surface);
+display_channel_current_flush(display, surface);
+dcc_push_surface_image(dcc, surface);
 }
 }
 
-if (dcc->priv->surface_client_created[drawable->surface->id] == TRUE) {
+surface = drawable->surface;
+if (dcc->priv->surface_client_created[surface->id] == TRUE) {
 return;
 }
 
-dcc_create_surface(dcc, drawable->surface);
-display_channel_current_flush(display, drawable->surface);
-dcc_push_surface_image(dcc, drawable->surface);
+dcc_create_surface(dcc, surface);
+display_channel_current_flush(display, surface);
+dcc_push_surface_image(dcc, surface);
 }
 
 static void red_drawable_pipe_item_free(RedPipeItem *item)
-- 
2.7.4

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

[Spice-devel] [PATCH RFC 07/14] Pass surface directly calling dcc_clear_surface_drawables_from_pipe

2016-09-29 Thread Frediano Ziglio

Signed-off-by: Frediano Ziglio 
---
 server/dcc.c | 11 +++
 server/dcc.h |  3 ++-
 server/display-channel.c |  8 
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/server/dcc.c b/server/dcc.c
index be95e1b..3e597b8 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -65,22 +65,17 @@ int dcc_drawable_is_in_pipe(DisplayChannelClient *dcc, 
Drawable *drawable)
  * Return: TRUE if wait_if_used == FALSE, or otherwise, if all of the pipe 
items that
  * are related to the surface have been cleared (or sent) from the pipe.
  */
-int dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int 
surface_id,
+int dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, 
RedSurface *surface,
   int wait_if_used)
 {
 GList *l;
 int x;
 RedChannelClient *rcc;
-DisplayChannel *display;
-RedSurface *surface;
 
 spice_return_val_if_fail(dcc != NULL, TRUE);
-/* removing the newest drawables that their destination is surface_id and
+/* removing the newest drawables that their destination is surface and
no other drawable depends on them */
 
-display = DCC_TO_DC(dcc);
-surface = >priv->surfaces[surface_id];
-
 rcc = RED_CHANNEL_CLIENT(dcc);
 for (l = rcc->priv->pipe.head; l != NULL; ) {
 Drawable *drawable;
@@ -112,7 +107,7 @@ int 
dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface
 }
 
 if (depend_found) {
-spice_debug("surface %d dependent item found %p, %p", surface_id, 
drawable, item);
+spice_debug("surface %d dependent item found %p, %p", surface->id, 
drawable, item);
 if (!wait_if_used) {
 return TRUE;
 }
diff --git a/server/dcc.h b/server/dcc.h
index 932e051..47386ff 100644
--- a/server/dcc.h
+++ b/server/dcc.h
@@ -44,6 +44,7 @@
 typedef struct DisplayChannel DisplayChannel;
 typedef struct Stream Stream;
 typedef struct StreamAgent StreamAgent;
+struct RedSurface;
 
 typedef struct WaitForChannels {
 SpiceMsgWaitForChannels header;
@@ -146,7 +147,7 @@ void   dcc_add_drawable_after   
 (DisplayCha
 void   dcc_send_item 
(RedChannelClient *dcc,
   
RedPipeItem *item);
 intdcc_clear_surface_drawables_from_pipe 
(DisplayChannelClient *dcc,
-  int 
surface_id,
+  struct 
RedSurface *surface,
   int 
wait_if_used);
 intdcc_drawable_is_in_pipe   
(DisplayChannelClient *dcc,
   Drawable 
*drawable);
diff --git a/server/display-channel.c b/server/display-channel.c
index 87c801b..006b218 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1693,14 +1693,14 @@ void display_channel_update(DisplayChannel *display,
 region_clear(>draw_dirty_region);
 }
 
-static void clear_surface_drawables_from_pipes(DisplayChannel *display, int 
surface_id,
+static void clear_surface_drawables_from_pipes(DisplayChannel *display, 
RedSurface *surface,
int wait_if_used)
 {
 GListIter iter;
 DisplayChannelClient *dcc;
 
 FOREACH_DCC(display, iter, dcc) {
-if (!dcc_clear_surface_drawables_from_pipe(dcc, surface_id, 
wait_if_used)) {
+if (!dcc_clear_surface_drawables_from_pipe(dcc, surface, 
wait_if_used)) {
 red_channel_client_disconnect(RED_CHANNEL_CLIENT(dcc));
 }
 }
@@ -1714,7 +1714,7 @@ static void 
display_channel_destroy_surface(DisplayChannel *display, RedSurface
otherwise "current" will hold items that other drawables may depend on, 
and then
current_remove_all will remove them from the pipe. */
 current_remove_all(display, surface);
-clear_surface_drawables_from_pipes(display, surface->id, FALSE);
+clear_surface_drawables_from_pipes(display, surface, FALSE);
 display_channel_surface_unref(display, surface);
 }
 
@@ -1731,7 +1731,7 @@ void display_channel_destroy_surface_wait(DisplayChannel 
*display, uint32_t surf
otherwise "current" will hold items that other drawables may depend on, 
and then
current_remove_all will remove them from the pipe. */
 current_remove_all(display, surface);
-clear_surface_drawables_from_pipes(display, surface_id, TRUE);
+clear_surface_drawables_from_pipes(display, surface, TRUE);
 }
 
 /* called upon device reset */
-- 
2.7.4

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org

[Spice-devel] [PATCH RFC 11/14] Pass surface directly to dcc_push_surface_image

2016-09-29 Thread Frediano Ziglio

Signed-off-by: Frediano Ziglio 
---
 server/dcc.c | 13 +
 server/dcc.h |  3 +--
 server/display-channel.c |  5 +++--
 3 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/server/dcc.c b/server/dcc.c
index 8a63caf..a800aaa 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -217,18 +217,15 @@ RedImageItem 
*dcc_add_surface_area_image(DisplayChannelClient *dcc,
 return item;
 }
 
-void dcc_push_surface_image(DisplayChannelClient *dcc, int surface_id)
+void dcc_push_surface_image(DisplayChannelClient *dcc, RedSurface *surface)
 {
-DisplayChannel *display;
 SpiceRect area;
-RedSurface *surface;
+uint32_t surface_id = surface->id;
 
 if (!dcc) {
 return;
 }
 
-display = DCC_TO_DC(dcc);
-surface = >priv->surfaces[surface_id];
 if (!surface->context.canvas) {
 return;
 }
@@ -257,7 +254,7 @@ static void 
add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
 }
 dcc_create_surface(dcc, drawable->surface_deps[x]);
 display_channel_current_flush(display, drawable->surface_deps[x]);
-dcc_push_surface_image(dcc, surface_id);
+dcc_push_surface_image(dcc, drawable->surface_deps[x]);
 }
 }
 
@@ -267,7 +264,7 @@ static void 
add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
 
 dcc_create_surface(dcc, drawable->surface);
 display_channel_current_flush(display, drawable->surface);
-dcc_push_surface_image(dcc, drawable->surface->id);
+dcc_push_surface_image(dcc, drawable->surface);
 }
 
 static void red_drawable_pipe_item_free(RedPipeItem *item)
@@ -439,7 +436,7 @@ void dcc_start(DisplayChannelClient *dcc)
 display_channel_current_flush(display, >priv->surfaces[0]);
 red_channel_client_pipe_add_type(rcc, 
RED_PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE);
 dcc_create_surface(dcc, >priv->surfaces[0]);
-dcc_push_surface_image(dcc, 0);
+dcc_push_surface_image(dcc, >priv->surfaces[0]);
 dcc_push_monitors_config(dcc);
 red_pipe_add_verb(rcc, SPICE_MSG_DISPLAY_MARK);
 dcc_create_all_streams(dcc);
diff --git a/server/dcc.h b/server/dcc.h
index 2068d78..db48191 100644
--- a/server/dcc.h
+++ b/server/dcc.h
@@ -123,8 +123,7 @@ void   dcc_stream_agent_clip
 (DisplayCha
 void   dcc_create_stream 
(DisplayChannelClient *dcc,
   Stream 
*stream);
 void dcc_create_surface(DisplayChannelClient *dcc, struct RedSurface *surface);
-void   dcc_push_surface_image
(DisplayChannelClient *dcc,
-  int 
surface_id);
+void dcc_push_surface_image(DisplayChannelClient *dcc, struct RedSurface 
*surface);
 RedImageItem * dcc_add_surface_area_image
(DisplayChannelClient *dcc,
   int 
surface_id,
   
SpiceRect *area,
diff --git a/server/display-channel.c b/server/display-channel.c
index 64a59de..d0b4a25 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1780,8 +1780,9 @@ static void send_create_surface(DisplayChannel *display, 
RedSurface *surface, in
 
 FOREACH_DCC(display, iter, dcc) {
 dcc_create_surface(dcc, surface);
-if (image_ready)
-dcc_push_surface_image(dcc, surface->id);
+if (image_ready) {
+dcc_push_surface_image(dcc, surface);
+}
 }
 }
 
-- 
2.7.4

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

[Spice-devel] [PATCH RFC 09/14] New function to pass surface directly to display_channel_draw

2016-09-29 Thread Frediano Ziglio

Signed-off-by: Frediano Ziglio 
---
 server/display-channel.c | 28 
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/server/display-channel.c b/server/display-channel.c
index 7627df9..e0a91a3 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -25,6 +25,8 @@
 static void drawable_draw(DisplayChannel *display, Drawable *drawable);
 static Drawable *display_channel_drawable_try_new(DisplayChannel *display,
   uint32_t 
process_commands_generation);
+static void display_channel_surface_draw(DisplayChannel *display, RedSurface 
*surface,
+ const SpiceRect *area);
 
 uint32_t display_channel_generate_uid(DisplayChannel *display)
 {
@@ -895,7 +897,8 @@ static void handle_self_bitmap(DisplayChannel *display, 
Drawable *drawable)
 image->u.bitmap.data = spice_chunks_new_linear(dest, height * dest_stride);
 image->u.bitmap.data->flags |= SPICE_CHUNKS_FLAGS_FREE;
 
-display_channel_draw(display, _drawable->self_bitmap_area, 
drawable->surface->id);
+display_channel_surface_draw(display, drawable->surface,
+ _drawable->self_bitmap_area);
 surface_read_bits(display, drawable->surface,
 _drawable->self_bitmap_area, dest, dest_stride);
 
@@ -957,7 +960,8 @@ static void draw_depend_on_me(DisplayChannel *display, 
RedSurface *surface)
 Drawable *drawable;
 DependItem *depended_item = SPICE_CONTAINEROF(ring_item, DependItem, 
ring_item);
 drawable = depended_item->drawable;
-display_channel_draw(display, >red_drawable->bbox, 
drawable->surface->id);
+display_channel_surface_draw(display, drawable->surface,
+ >red_drawable->bbox);
 }
 }
 
@@ -1373,7 +1377,8 @@ static void drawable_deps_draw(DisplayChannel *display, 
Drawable *drawable)
 RedSurface *surface = drawable->surface_deps[x];
 if (surface != NULL && drawable->depend_items[x].drawable) {
 depended_item_remove(>depend_items[x]);
-display_channel_draw(display, 
>red_drawable->surfaces_rects[x], surface->id);
+display_channel_surface_draw(display, surface,
+ 
>red_drawable->surfaces_rects[x]);
 }
 }
 }
@@ -1635,7 +1640,6 @@ void display_channel_draw_until(DisplayChannel *display, 
const SpiceRect *area,
 void display_channel_draw(DisplayChannel *display, const SpiceRect *area, int 
surface_id)
 {
 RedSurface *surface;
-Drawable *last;
 
 spice_debug("surface %d: area ==>", surface_id);
 rect_debug(area);
@@ -1647,6 +1651,14 @@ void display_channel_draw(DisplayChannel *display, const 
SpiceRect *area, int su
 
 surface = >priv->surfaces[surface_id];
 
+display_channel_surface_draw(display, surface, area);
+}
+
+static void display_channel_surface_draw(DisplayChannel *display, RedSurface 
*surface,
+ const SpiceRect *area)
+{
+Drawable *last;
+
 last = current_find_intersects_rect(>current_list, NULL, area);
 if (last)
 draw_until(display, surface, last);
@@ -1680,9 +1692,9 @@ void display_channel_update(DisplayChannel *display,
 spice_return_if_fail(display_channel_validate_surface(display, 
surface_id));
 
 red_get_rect_ptr(, area);
-display_channel_draw(display, , surface_id);
-
 surface = >priv->surfaces[surface_id];
+display_channel_surface_draw(display, surface, );
+
 if (*qxl_dirty_rects == NULL) {
 *num_dirty_rects = 
pixman_region32_n_rects(>draw_dirty_region);
 *qxl_dirty_rects = spice_new0(QXLRect, *num_dirty_rects);
@@ -1722,10 +1734,10 @@ void 
display_channel_destroy_surface_wait(DisplayChannel *display, uint32_t surf
 {
 if (!display_channel_validate_surface(display, surface_id))
 return;
-if (!display->priv->surfaces[surface_id].context.canvas)
+RedSurface *surface = >priv->surfaces[surface_id];
+if (!surface->context.canvas)
 return;
 
-RedSurface *surface = >priv->surfaces[surface_id];
 draw_depend_on_me(display, surface);
 /* note that draw_depend_on_me must be called before current_remove_all.
otherwise "current" will hold items that other drawables may depend on, 
and then
-- 
2.7.4

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

[Spice-devel] [PATCH RFC 13/14] Change validate_surface to return surface pointer

2016-09-29 Thread Frediano Ziglio

Signed-off-by: Frediano Ziglio 
---
 server/dcc-send.c|  8 
 server/display-channel.c | 19 +++
 server/display-channel.h |  2 +-
 3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/server/dcc-send.c b/server/dcc-send.c
index 1e31584..d5ed97e 100644
--- a/server/dcc-send.c
+++ b/server/dcc-send.c
@@ -94,9 +94,9 @@ static int is_surface_area_lossy(DisplayChannelClient *dcc, 
uint32_t surface_id,
 QRegion lossy_region;
 DisplayChannel *display = DCC_TO_DC(dcc);
 
-spice_return_val_if_fail(display_channel_validate_surface(display, 
surface_id), FALSE);
+surface = display_channel_validate_surface(display, surface_id);
+spice_return_val_if_fail(surface, FALSE);
 
-surface = >priv->surfaces[surface_id];
 surface_lossy_region = >priv->surface_client_lossy_region[surface_id];
 
 if (!area) {
@@ -401,13 +401,13 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc, 
SpiceMarshaller *m,
 RedSurface *surface;
 
 surface_id = simage->u.surface.surface_id;
-if (!display_channel_validate_surface(display, surface_id)) {
+surface = display_channel_validate_surface(display, surface_id);
+if (!surface) {
 spice_warning("Invalid surface in SPICE_IMAGE_TYPE_SURFACE");
 pthread_mutex_unlock(>priv->pixmap_cache->lock);
 return FILL_BITS_TYPE_SURFACE;
 }
 
-surface = >priv->surfaces[surface_id];
 image.descriptor.type = SPICE_IMAGE_TYPE_SURFACE;
 image.descriptor.flags = 0;
 image.descriptor.width = surface->context.width;
diff --git a/server/display-channel.c b/server/display-channel.c
index d0b4a25..cf019fe 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -2073,19 +2073,20 @@ int display_channel_get_stream_id(DisplayChannel 
*display, Stream *stream)
 return (int)(stream - display->priv->streams_buf);
 }
 
-gboolean display_channel_validate_surface(DisplayChannel *display, uint32_t 
surface_id)
+RedSurface *display_channel_validate_surface(DisplayChannel *display, uint32_t 
surface_id)
 {
 if SPICE_UNLIKELY(surface_id >= display->priv->n_surfaces) {
 spice_warning("invalid surface_id %u", surface_id);
-return FALSE;
+return NULL;
 }
-if (!display->priv->surfaces[surface_id].context.canvas) {
+RedSurface *surface = >priv->surfaces[surface_id];
+if (!surface->context.canvas) {
 spice_warning("canvas address is %p for %d (and is NULL)\n",
&(display->priv->surfaces[surface_id].context.canvas), 
surface_id);
 spice_warning("failed on %d", surface_id);
-return FALSE;
+return NULL;
 }
-return TRUE;
+return surface;
 }
 
 void display_channel_update_monitors_config(DisplayChannel *display,
@@ -2102,14 +2103,16 @@ void 
display_channel_update_monitors_config(DisplayChannel *display,
 
 void display_channel_set_monitors_config_to_primary(DisplayChannel *display)
 {
-DrawContext *context = >priv->surfaces[0].context;
-QXLHead head = { 0, };
+RedSurface *surface = display_channel_validate_surface(display, 0);
 
-spice_return_if_fail(display->priv->surfaces[0].context.canvas);
+spice_return_if_fail(surface);
+
+DrawContext *context = >context;
 
 if (display->priv->monitors_config)
 monitors_config_unref(display->priv->monitors_config);
 
+QXLHead head = { 0, };
 head.width = context->width;
 head.height = context->height;
 display->priv->monitors_config = monitors_config_new(, 1, 1);
diff --git a/server/display-channel.h b/server/display-channel.h
index bd386a7..2b04bb6 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -295,7 +295,7 @@ void display_channel_update_monitors_config(DisplayChannel 
*display, QXLMonitors
 uint16_t count, uint16_t 
max_allowed);
 void display_channel_set_monitors_config_to_primary(DisplayChannel *display);
 
-gboolean display_channel_validate_surface(DisplayChannel *display, uint32_t 
surface_id);
+RedSurface *display_channel_validate_surface(DisplayChannel *display, uint32_t 
surface_id);
 gboolean display_channel_surface_has_canvas(DisplayChannel *display, uint32_t 
surface_id);
 void display_channel_reset_image_cache(DisplayChannel *self);
 
-- 
2.7.4

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

[Spice-devel] [PATCH RFC 04/14] Pass surface directly to display_channel_surface_unref

2016-09-29 Thread Frediano Ziglio

Signed-off-by: Frediano Ziglio 
---
 server/display-channel.c | 23 ++-
 server/display-channel.h |  2 +-
 server/red-worker.c  |  2 +-
 3 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/server/display-channel.c b/server/display-channel.c
index 5e9bc7d..5cdd0cb 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -169,9 +169,8 @@ static void stop_streams(DisplayChannel *display)
 memset(display->priv->items_trace, 0, sizeof(display->priv->items_trace));
 }
 
-void display_channel_surface_unref(DisplayChannel *display, uint32_t 
surface_id)
+static void display_channel_surface_unref(DisplayChannel *display, RedSurface 
*surface)
 {
-RedSurface *surface = >priv->surfaces[surface_id];
 QXLInstance *qxl = display->common.qxl;
 DisplayChannelClient *dcc;
 GListIter iter;
@@ -182,7 +181,7 @@ void display_channel_surface_unref(DisplayChannel *display, 
uint32_t surface_id)
 }
 
 // only primary surface streams are supported
-if (is_primary_surface(display, surface_id)) {
+if (is_primary_surface(display, surface->id)) {
 stop_streams(display);
 }
 spice_assert(surface->context.canvas);
@@ -198,7 +197,7 @@ void display_channel_surface_unref(DisplayChannel *display, 
uint32_t surface_id)
 region_destroy(>draw_dirty_region);
 surface->context.canvas = NULL;
 FOREACH_DCC(display, iter, dcc) {
-dcc_destroy_surface(dcc, surface_id);
+dcc_destroy_surface(dcc, surface->id);
 }
 
 spice_warn_if_fail(ring_is_empty(>depend_on_me));
@@ -211,6 +210,11 @@ gboolean display_channel_surface_has_canvas(DisplayChannel 
*display,
 return display->priv->surfaces[surface_id].context.canvas != NULL;
 }
 
+void display_channel_surface_id_unref(DisplayChannel *display, uint32_t 
surface_id)
+{
+display_channel_surface_unref(display, 
>priv->surfaces[surface_id]);
+}
+
 static void streams_update_visible_region(DisplayChannel *display, Drawable 
*drawable)
 {
 Ring *ring;
@@ -1333,7 +1337,7 @@ static void drawable_unref_surface_deps(DisplayChannel 
*display, Drawable *drawa
 if (surface == NULL) {
 continue;
 }
-display_channel_surface_unref(display, surface->id);
+display_channel_surface_unref(display, surface);
 }
 }
 
@@ -1354,7 +1358,7 @@ void drawable_unref(Drawable *drawable)
 
 drawable_remove_dependencies(display, drawable);
 drawable_unref_surface_deps(display, drawable);
-display_channel_surface_unref(display, drawable->surface->id);
+display_channel_surface_unref(display, drawable->surface);
 
 glz_retention_detach_drawables(>glz_retention);
 
@@ -1709,13 +1713,14 @@ static void 
clear_surface_drawables_from_pipes(DisplayChannel *display, int surf
 /* TODO: cleanup/refactor destroy functions */
 static void display_channel_destroy_surface(DisplayChannel *display, uint32_t 
surface_id)
 {
+RedSurface *surface = >priv->surfaces[surface_id];
 draw_depend_on_me(display, surface_id);
 /* note that draw_depend_on_me must be called before current_remove_all.
otherwise "current" will hold items that other drawables may depend on, 
and then
current_remove_all will remove them from the pipe. */
-current_remove_all(display, >priv->surfaces[surface_id]);
+current_remove_all(display, surface);
 clear_surface_drawables_from_pipes(display, surface_id, FALSE);
-display_channel_surface_unref(display, surface_id);
+display_channel_surface_unref(display, surface);
 }
 
 void display_channel_destroy_surface_wait(DisplayChannel *display, uint32_t 
surface_id)
@@ -1745,7 +1750,7 @@ void display_channel_destroy_surfaces(DisplayChannel 
*display)
 if (display->priv->surfaces[i].context.canvas) {
 display_channel_destroy_surface_wait(display, i);
 if (display->priv->surfaces[i].context.canvas) {
-display_channel_surface_unref(display, i);
+display_channel_surface_unref(display, 
>priv->surfaces[i]);
 }
 spice_assert(!display->priv->surfaces[i].context.canvas);
 }
diff --git a/server/display-channel.h b/server/display-channel.h
index 8918ccb..a425364 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -266,7 +266,7 @@ void   display_channel_set_video_codecs 
 (DisplayCha
 intdisplay_channel_get_streams_timeout   
(DisplayChannel *display);
 void   display_channel_compress_stats_print  (const 
DisplayChannel *display);
 void   display_channel_compress_stats_reset  
(DisplayChannel *display);
-void   display_channel_surface_unref 
(DisplayChannel *display,
+void   display_channel_surface_id_unref  
(DisplayChannel *display,

[Spice-devel] [PATCH RFC 06/14] Pass surface directly calling draw_depend_on_me and display_channel_destroy_surface

2016-09-29 Thread Frediano Ziglio

Signed-off-by: Frediano Ziglio 
---
 server/display-channel.c | 22 +-
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/server/display-channel.c b/server/display-channel.c
index dcbd250..87c801b 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -949,13 +949,10 @@ static int handle_surface_deps(DisplayChannel *display, 
Drawable *drawable)
 return TRUE;
 }
 
-static void draw_depend_on_me(DisplayChannel *display, uint32_t surface_id)
+static void draw_depend_on_me(DisplayChannel *display, RedSurface *surface)
 {
-RedSurface *surface;
 RingItem *ring_item;
 
-surface = >priv->surfaces[surface_id];
-
 while ((ring_item = ring_get_tail(>depend_on_me))) {
 Drawable *drawable;
 DependItem *depended_item = SPICE_CONTAINEROF(ring_item, DependItem, 
ring_item);
@@ -1048,7 +1045,6 @@ static Drawable 
*display_channel_get_drawable(DisplayChannel *display, uint8_t e
  */
 static void display_channel_add_drawable(DisplayChannel *display, Drawable 
*drawable)
 {
-int surface_id = drawable->surface->id;
 RedDrawable *red_drawable = drawable->red_drawable;
 
 red_drawable->mm_time = reds_get_mm_time();
@@ -1072,7 +1068,7 @@ static void display_channel_add_drawable(DisplayChannel 
*display, Drawable *draw
 handle_self_bitmap(display, drawable);
 }
 
-draw_depend_on_me(display, surface_id);
+draw_depend_on_me(display, drawable->surface);
 
 if (!handle_surface_deps(display, drawable)) {
 return;
@@ -1711,15 +1707,14 @@ static void 
clear_surface_drawables_from_pipes(DisplayChannel *display, int surf
 }
 
 /* TODO: cleanup/refactor destroy functions */
-static void display_channel_destroy_surface(DisplayChannel *display, uint32_t 
surface_id)
+static void display_channel_destroy_surface(DisplayChannel *display, 
RedSurface *surface)
 {
-RedSurface *surface = >priv->surfaces[surface_id];
-draw_depend_on_me(display, surface_id);
+draw_depend_on_me(display, surface);
 /* note that draw_depend_on_me must be called before current_remove_all.
otherwise "current" will hold items that other drawables may depend on, 
and then
current_remove_all will remove them from the pipe. */
 current_remove_all(display, surface);
-clear_surface_drawables_from_pipes(display, surface_id, FALSE);
+clear_surface_drawables_from_pipes(display, surface->id, FALSE);
 display_channel_surface_unref(display, surface);
 }
 
@@ -1730,11 +1725,12 @@ void 
display_channel_destroy_surface_wait(DisplayChannel *display, uint32_t surf
 if (!display->priv->surfaces[surface_id].context.canvas)
 return;
 
-draw_depend_on_me(display, surface_id);
+RedSurface *surface = >priv->surfaces[surface_id];
+draw_depend_on_me(display, surface);
 /* note that draw_depend_on_me must be called before current_remove_all.
otherwise "current" will hold items that other drawables may depend on, 
and then
current_remove_all will remove them from the pipe. */
-current_remove_all(display, >priv->surfaces[surface_id]);
+current_remove_all(display, surface);
 clear_surface_drawables_from_pipes(display, surface_id, TRUE);
 }
 
@@ -2003,7 +1999,7 @@ void display_channel_process_surface_cmd(DisplayChannel 
*display,
 break;
 }
 surface->destroy = surface_cmd->release_info_ext;
-display_channel_destroy_surface(display, surface_id);
+display_channel_destroy_surface(display, surface);
 break;
 default:
 spice_warn_if_reached();
-- 
2.7.4

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

[Spice-devel] [PATCH RFC 03/14] Pass surface directly to current_remove_all

2016-09-29 Thread Frediano Ziglio

Signed-off-by: Frediano Ziglio 
---
 server/display-channel.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/server/display-channel.c b/server/display-channel.c
index 18f31dc..5e9bc7d 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -382,9 +382,9 @@ static void current_remove(DisplayChannel *display, 
TreeItem *item)
 }
 }
 
-static void current_remove_all(DisplayChannel *display, int surface_id)
+static void current_remove_all(DisplayChannel *display, RedSurface *surface)
 {
-Ring *ring = >priv->surfaces[surface_id].current;
+Ring *ring = >current;
 RingItem *ring_item;
 
 while ((ring_item = ring_get_head(ring))) {
@@ -1208,7 +1208,7 @@ void display_channel_current_flush(DisplayChannel 
*display, int surface_id)
 while (!ring_is_empty(>priv->surfaces[surface_id].current_list)) {
 free_one_drawable(display, FALSE);
 }
-current_remove_all(display, surface_id);
+current_remove_all(display, >priv->surfaces[surface_id]);
 }
 
 void display_channel_free_some(DisplayChannel *display)
@@ -1713,7 +1713,7 @@ static void 
display_channel_destroy_surface(DisplayChannel *display, uint32_t su
 /* note that draw_depend_on_me must be called before current_remove_all.
otherwise "current" will hold items that other drawables may depend on, 
and then
current_remove_all will remove them from the pipe. */
-current_remove_all(display, surface_id);
+current_remove_all(display, >priv->surfaces[surface_id]);
 clear_surface_drawables_from_pipes(display, surface_id, FALSE);
 display_channel_surface_unref(display, surface_id);
 }
@@ -1729,7 +1729,7 @@ void display_channel_destroy_surface_wait(DisplayChannel 
*display, uint32_t surf
 /* note that draw_depend_on_me must be called before current_remove_all.
otherwise "current" will hold items that other drawables may depend on, 
and then
current_remove_all will remove them from the pipe. */
-current_remove_all(display, surface_id);
+current_remove_all(display, >priv->surfaces[surface_id]);
 clear_surface_drawables_from_pipes(display, surface_id, TRUE);
 }
 
-- 
2.7.4

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

[Spice-devel] [PATCH RFC 00/14] Change the way RedSurface is stored and handled

2016-09-29 Thread Frediano Ziglio

This series of patches attempt to use direct pointers to
surfaces.
The current code rely on the fact that surface pointer for a specific
surface_id does not change.
For this reason for instance surfaces are allocated in a static array.
On the other way surfaces have reference counting so why don't use
counters for life management instead of having complicated function
that attempt to do all possible checks in order to make sure they
are not used?
Mainly instead of using surface_id and every time having to ask
DisplayChannel to convert this to a RedSurface pass and store directly
the pointers.
On deletion you could simply remove the reference to the main list
(which can became something like RedSurface *surfaces[MAX_SURFACES])
and you can create a new RedSurface.
Also as usually multiple surfaces are disabled this will reduce
memory usage.

Frediano Ziglio (14):
  Use direct pointers for surface and surface dependencies from Drawable
  Pass surface directly for surface_read_bits
  Pass surface directly to current_remove_all
  Pass surface directly to display_channel_surface_unref
  Pass surface directly in is_primary_surface
  Pass surface directly calling draw_depend_on_me and
display_channel_destroy_surface
  Pass surface directly calling dcc_clear_surface_drawables_from_pipe
  Pass surface directly calling display_channel_current_flush
  New function to pass surface directly to display_channel_draw
  Pass surface directly to dcc_create_surface
  Pass surface directly to dcc_push_surface_image
  Use directly surface instead of id
  Change validate_surface to return surface pointer
  Reuse more validate_surface

 server/dcc-send.c|  29 ---
 server/dcc.c |  56 ++---
 server/dcc.h |   9 +-
 server/display-channel.c | 214 +--
 server/display-channel.h |  28 +--
 server/red-worker.c  |   2 +-
 6 files changed, 178 insertions(+), 160 deletions(-)

-- 
2.7.4

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

[Spice-devel] [PATCH RFC 02/14] Pass surface directly for surface_read_bits

2016-09-29 Thread Frediano Ziglio

Signed-off-by: Frediano Ziglio 
---
 server/display-channel.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/server/display-channel.c b/server/display-channel.c
index 99082e6..18f31dc 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -847,11 +847,10 @@ static void drawable_ref_surface_deps(DisplayChannel 
*display, Drawable *drawabl
 }
 }
 
-static void surface_read_bits(DisplayChannel *display, int surface_id,
+static void surface_read_bits(DisplayChannel *display, RedSurface *surface,
   const SpiceRect *area, uint8_t *dest, int 
dest_stride)
 {
 SpiceCanvas *canvas;
-RedSurface *surface = >priv->surfaces[surface_id];
 
 canvas = surface->context.canvas;
 canvas->ops->read_bits(canvas, dest, dest_stride, area);
@@ -893,7 +892,7 @@ static void handle_self_bitmap(DisplayChannel *display, 
Drawable *drawable)
 image->u.bitmap.data->flags |= SPICE_CHUNKS_FLAGS_FREE;
 
 display_channel_draw(display, _drawable->self_bitmap_area, 
drawable->surface->id);
-surface_read_bits(display, drawable->surface->id,
+surface_read_bits(display, drawable->surface,
 _drawable->self_bitmap_area, dest, dest_stride);
 
 /* For 32bit non-primary surfaces we need to keep any non-zero
-- 
2.7.4

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

[Spice-devel] [PATCH RFC 05/14] Pass surface directly in is_primary_surface

2016-09-29 Thread Frediano Ziglio

Signed-off-by: Frediano Ziglio 
---
 server/dcc.c |  4 ++--
 server/display-channel.c | 14 +++---
 server/display-channel.h |  7 ++-
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/server/dcc.c b/server/dcc.c
index af32de7..be95e1b 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -144,7 +144,7 @@ void dcc_create_surface(DisplayChannelClient *dcc, int 
surface_id)
 }
 
 display = DCC_TO_DC(dcc);
-flags = is_primary_surface(DCC_TO_DC(dcc), surface_id) ? 
SPICE_SURFACE_FLAGS_PRIMARY : 0;
+flags = is_primary_surface_id(DCC_TO_DC(dcc), surface_id) ? 
SPICE_SURFACE_FLAGS_PRIMARY : 0;
 
 /* don't send redundant create surface commands to client */
 if (!dcc || display->common.during_target_migrate ||
@@ -204,7 +204,7 @@ RedImageItem 
*dcc_add_surface_area_image(DisplayChannelClient *dcc,
 
 /* For 32bit non-primary surfaces we need to keep any non-zero
high bytes as the surface may be used as source to an alpha_blend */
-if (!is_primary_surface(display, surface_id) &&
+if (!is_primary_surface(display, surface) &&
 item->image_format == SPICE_BITMAP_FMT_32BIT &&
 rgb32_data_has_alpha(item->width, item->height, item->stride, 
item->data, _set)) {
 if (all_set) {
diff --git a/server/display-channel.c b/server/display-channel.c
index 5cdd0cb..dcbd250 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -181,7 +181,7 @@ static void display_channel_surface_unref(DisplayChannel 
*display, RedSurface *s
 }
 
 // only primary surface streams are supported
-if (is_primary_surface(display, surface->id)) {
+if (is_primary_surface(display, surface)) {
 stop_streams(display);
 }
 spice_assert(surface->context.canvas);
@@ -226,7 +226,7 @@ static void streams_update_visible_region(DisplayChannel 
*display, Drawable *dra
 return;
 }
 
-if (!is_primary_surface(display, drawable->surface->id)) {
+if (!is_primary_surface(display, drawable->surface)) {
 return;
 }
 
@@ -629,7 +629,7 @@ static int current_add_with_shadow(DisplayChannel *display, 
Ring *ring, Drawable
 // for now putting them on root.
 
 // only primary surface streams are supported
-if (is_primary_surface(display, item->surface->id)) {
+if (is_primary_surface(display, item->surface)) {
 stream_detach_behind(display, >base.rgn, NULL);
 }
 
@@ -642,7 +642,7 @@ static int current_add_with_shadow(DisplayChannel *display, 
Ring *ring, Drawable
 region_destroy(_rgn);
 streams_update_visible_region(display, item);
 } else {
-if (is_primary_surface(display, item->surface->id)) {
+if (is_primary_surface(display, item->surface)) {
 stream_detach_behind(display, >tree_item.base.rgn, item);
 }
 }
@@ -759,7 +759,7 @@ static int current_add(DisplayChannel *display, Ring *ring, 
Drawable *drawable)
  * stream_detach_behind
  */
 current_add_drawable(display, drawable, ring);
-if (is_primary_surface(display, drawable->surface->id)) {
+if (is_primary_surface(display, drawable->surface)) {
 stream_detach_behind(display, >tree_item.base.rgn, 
drawable);
 }
 }
@@ -777,7 +777,7 @@ static bool drawable_can_stream(DisplayChannel *display, 
Drawable *drawable)
 return FALSE;
 }
 
-if (!is_primary_surface(display, drawable->surface->id)) {
+if (!is_primary_surface(display, drawable->surface)) {
 return FALSE;
 }
 
@@ -901,7 +901,7 @@ static void handle_self_bitmap(DisplayChannel *display, 
Drawable *drawable)
 
 /* For 32bit non-primary surfaces we need to keep any non-zero
high bytes as the surface may be used as source to an alpha_blend */
-if (!is_primary_surface(display, drawable->surface->id) &&
+if (!is_primary_surface(display, drawable->surface) &&
 image->u.bitmap.format == SPICE_BITMAP_FMT_32BIT &&
 rgb32_data_has_alpha(width, height, dest_stride, dest, _set)) {
 if (all_set) {
diff --git a/server/display-channel.h b/server/display-channel.h
index a425364..59ec6a4 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -399,7 +399,7 @@ static inline int has_shadow(RedDrawable *drawable)
 return drawable->type == QXL_COPY_BITS;
 }
 
-static inline int is_primary_surface(DisplayChannel *display, uint32_t 
surface_id)
+static inline int is_primary_surface_id(DisplayChannel *display, uint32_t 
surface_id)
 {
 if (surface_id == 0) {
 return TRUE;
@@ -407,6 +407,11 @@ static inline int is_primary_surface(DisplayChannel 
*display, uint32_t surface_i
 return FALSE;
 }
 
+static inline int is_primary_surface(DisplayChannel *display, RedSurface 
*surface)
+{
+return surface->id == 0;
+}
+
 static inline void region_add_clip_rects(QRegion *rgn, SpiceClipRects *data)
 {
 int i;
-- 
2.7.4

[Spice-devel] [PATCH RFC 01/14] Use direct pointers for surface and surface dependencies from Drawable

2016-09-29 Thread Frediano Ziglio

As we use reference counting is more direct to use direct pointers.
Also this will allow to have a surface in a "released state"
reducing the complexity of code destroying a surface.

Signed-off-by: Frediano Ziglio 
---
 server/dcc-send.c| 21 ++-
 server/dcc.c | 21 ++-
 server/display-channel.c | 91 +++-
 server/display-channel.h | 15 +---
 4 files changed, 78 insertions(+), 70 deletions(-)

diff --git a/server/dcc-send.c b/server/dcc-send.c
index c49adab..1e31584 100644
--- a/server/dcc-send.c
+++ b/server/dcc-send.c
@@ -316,7 +316,7 @@ static void fill_base(SpiceMarshaller *base_marshaller, 
Drawable *drawable)
 {
 SpiceMsgDisplayBase base;
 
-base.surface_id = drawable->surface_id;
+base.surface_id = drawable->surface->id;
 base.box = drawable->red_drawable->bbox;
 base.clip = drawable->red_drawable->clip;
 
@@ -556,7 +556,7 @@ static void 
surface_lossy_region_update(DisplayChannelClient *dcc,
 return;
 }
 
-surface_lossy_region = 
>priv->surface_client_lossy_region[item->surface_id];
+surface_lossy_region = 
>priv->surface_client_lossy_region[item->surface->id];
 drawable = item->red_drawable;
 
 if (drawable->clip.type == SPICE_CLIP_TYPE_RECTS ) {
@@ -653,7 +653,10 @@ static int drawable_depends_on_areas(Drawable *drawable, 
int surface_ids[],
 int dep_surface_id;
 
  for (x = 0; x < 3; ++x) {
-dep_surface_id = drawable->surface_deps[x];
+if (drawable->surface_deps[x] == NULL) {
+continue;
+}
+dep_surface_id = drawable->surface_deps[x]->id;
 if (dep_surface_id == surface_ids[i]) {
 if (rect_intersects(_areas[i], 
_drawable->surfaces_rects[x])) {
 return TRUE;
@@ -828,7 +831,7 @@ static void 
red_lossy_marshall_qxl_draw_fill(RedChannelClient *rcc,
 brush_is_lossy = is_brush_lossy(rcc, >u.fill.brush,
 _bitmap_data);
 if (!dest_allowed_lossy) {
-dest_is_lossy = is_surface_area_lossy(dcc, item->surface_id, 
>bbox,
+dest_is_lossy = is_surface_area_lossy(dcc, item->surface->id, 
>bbox,
   _lossy_area);
 }
 
@@ -845,7 +848,7 @@ static void 
red_lossy_marshall_qxl_draw_fill(RedChannelClient *rcc,
 int num_resend = 0;
 
 if (dest_is_lossy) {
-resend_surface_ids[num_resend] = item->surface_id;
+resend_surface_ids[num_resend] = item->surface->id;
 resend_areas[num_resend] = _lossy_area;
 num_resend++;
 }
@@ -1148,7 +1151,7 @@ static void 
red_lossy_marshall_qxl_copy_bits(RedChannelClient *rcc,
 src_rect.right = drawable->bbox.right + horz_offset;
 src_rect.bottom = drawable->bbox.bottom + vert_offset;
 
-src_is_lossy = is_surface_area_lossy(dcc, item->surface_id,
+src_is_lossy = is_surface_area_lossy(dcc, item->surface->id,
  _rect, _lossy_area);
 
 surface_lossy_region_update(dcc, item, FALSE, src_is_lossy);
@@ -1210,7 +1213,7 @@ static void 
red_lossy_marshall_qxl_draw_blend(RedChannelClient *rcc,
 }
 
 if (dest_is_lossy) {
-resend_surface_ids[num_resend] = item->surface_id;
+resend_surface_ids[num_resend] = item->surface->id;
 resend_areas[num_resend] = _lossy_area;
 num_resend++;
 }
@@ -1388,7 +1391,7 @@ static void 
red_lossy_marshall_qxl_draw_rop3(RedChannelClient *rcc,
 }
 
 if (dest_is_lossy) {
-resend_surface_ids[num_resend] = item->surface_id;
+resend_surface_ids[num_resend] = item->surface->id;
 resend_areas[num_resend] = _lossy_area;
 num_resend++;
 }
@@ -1469,7 +1472,7 @@ static void 
red_lossy_marshall_qxl_draw_composite(RedChannelClient *rcc,
 }
 
 if (dest_is_lossy) {
-resend_surface_ids[num_resend] = item->surface_id;
+resend_surface_ids[num_resend] = item->surface->id;
 resend_areas[num_resend] = _lossy_area;
 num_resend++;
 }
diff --git a/server/dcc.c b/server/dcc.c
index ce8677d..af32de7 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -71,11 +71,16 @@ int 
dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface
 GList *l;
 int x;
 RedChannelClient *rcc;
+DisplayChannel *display;
+RedSurface *surface;
 
 spice_return_val_if_fail(dcc != NULL, TRUE);
 /* removing the newest drawables that their destination is surface_id and
no other drawable depends on them */
 
+display = DCC_TO_DC(dcc);
+surface = >priv->surfaces[surface_id];
+
 rcc = RED_CHANNEL_CLIENT(dcc);
 for (l = rcc->priv->pipe.head; l != NULL; ) {
 Drawable *drawable;
@@ -94,13 +99,13 @@ int

Re: [Spice-devel] [PATCH 2/2] SPICE port basic implementation

2016-09-29 Thread Oliver Gutierrez

Here is an small guide to test this for the ones that want to check it
faster:

Add a spiceport to a virtual machine adding this under devices in the XML


  
  
  



Boot machine, Connect to it as usual using spice-html5 and as root:

echo "CHILI" > /dev/virtio-ports/org.freedesktop.FleetCommander.0

In the browser console yo sould see the events for opening the port, the
message and the port closing.

If in a week or so there is no other feedback then I will contact you
Jeremy.

Thanks and cheers!

On Wed, Sep 28, 2016 at 10:10 PM, Jeremy White 
wrote:

> Looks good to me.  I don't have the time to test this myself, so I'll
> leave this open for others to kibitz for now.
>
> I don't think it will do harm, so if no one else expresses an opinion
> after a while, you can probably nudge me into pushing it.
>
> Reviewed-by: Jeremy White 
>
> Cheers,
>
> Jeremy
>
> On 09/27/2016 10:29 AM, Oliver Gutierrez wrote:
> > ---
> >  enums.js|  9 ++
> >  main.js |  3 ++
> >  port.js | 85 ++
> +++
> >  spice.html  | 10 +++
> >  spice_auto.html | 10 +++
> >  spicemsg.js | 18 
> >  utils.js|  7 +
> >  7 files changed, 142 insertions(+)
> >  create mode 100644 port.js
> >
> > diff --git a/enums.js b/enums.js
> > index 301fea0..b6e013c 100644
> > --- a/enums.js
> > +++ b/enums.js
> > @@ -166,6 +166,15 @@ var SPICE_MSG_PLAYBACK_VOLUME   = 105;
> >  var SPICE_MSG_PLAYBACK_MUTE = 106;
> >  var SPICE_MSG_PLAYBACK_LATENCY  = 107;
> >
> > +var SPICE_MSG_SPICEVMC_DATA = 101;
> > +var SPICE_MSG_PORT_INIT = 201;
> > +var SPICE_MSG_PORT_EVENT= 202;
> > +var SPICE_MSG_END_PORT  = 203;
> > +
> > +var SPICE_MSGC_SPICEVMC_DATA= 101;
> > +var SPICE_MSGC_PORT_EVENT   = 201;
> > +var SPICE_MSGC_END_PORT = 202;
> > +
> >  var SPICE_PLAYBACK_CAP_CELT_0_5_1   = 0;
> >  var SPICE_PLAYBACK_CAP_VOLUME   = 1;
> >  var SPICE_PLAYBACK_CAP_LATENCY  = 2;
> > diff --git a/main.js b/main.js
> > index 874a038..2d8a1ff 100644
> > --- a/main.js
> > +++ b/main.js
> > @@ -59,6 +59,7 @@ function SpiceMainConn()
> >  this.file_xfer_tasks = {};
> >  this.file_xfer_task_id = 0;
> >  this.file_xfer_read_queue = [];
> > +this.ports = [];
> >  }
> >
> >  SpiceMainConn.prototype = Object.create(SpiceConn.prototype);
> > @@ -154,6 +155,8 @@ SpiceMainConn.prototype.process_channel_message =
> function(msg)
> >  this.cursor = new SpiceCursorConn(conn);
> >  else if (chans.channels[i].type == SPICE_CHANNEL_PLAYBACK)
> >  this.cursor = new SpicePlaybackConn(conn);
> > +else if (chans.channels[i].type == SPICE_CHANNEL_PORT)
> > +this.ports.push(new SpicePortConn(conn));
> >  else
> >  {
> >  if (! ("extra_channels" in this))
> > diff --git a/port.js b/port.js
> > new file mode 100644
> > index 000..ee22073
> > --- /dev/null
> > +++ b/port.js
> > @@ -0,0 +1,85 @@
> > +"use strict";
> > +/*
> > +   Copyright (C) 2016 by Oliver Gutierrez 
> > + Miroslav Chodil 
> > +
> > +   This file is part of spice-html5.
> > +
> > +   spice-html5 is free software: you can redistribute it and/or modify
> > +   it under the terms of the GNU Lesser General Public License as
> published by
> > +   the Free Software Foundation, either version 3 of the License, or
> > +   (at your option) any later version.
> > +
> > +   spice-html5 is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +   GNU Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> License
> > +   along with spice-html5.  If not, see .
> > +*/
> > +
> > +/*-
> ---
> > +**  SpicePortConn
> > +**  Drive the Spice Port Channel
> > +**-
> -*/
> > +function SpicePortConn()
> > +{
> > +DEBUG > 0 && console.log('SPICE port: created SPICE port channel.
> Args:', arguments);
> > +SpiceConn.apply(this, arguments);
> > +this.port_name = null;
> > +}
> > +
> > +SpicePortConn.prototype = Object.create(SpiceConn.prototype);
> > +
> > +SpicePortConn.prototype.process_channel_message = function(msg)
> > +{
> > +if (msg.type == SPICE_MSG_PORT_INIT)
> > +{
> > +if (this.port_name === null)
> > +{
> > +var m = new SpiceMsgPortInit(msg.data);
> > +this.portName =

Re: [Spice-devel] spice performance tweaking

[Spice-devel] [PATCH v2 0/4] Protocol file syntax documentation

[Spice-devel] [PATCH v2 3/4] Extended protocol documentation

[Spice-devel] [PATCH v2 4/4] More work on attribute protocol documentation

[Spice-devel] [PATCH v2 1/4] Start adding protocol file documentation

[Spice-devel] [PATCH v2 2/4] Start writing some documentation on protocol

Re: [Spice-devel] spice performance tweaking

[Spice-devel] [PATCH xf86-video-qxl] Adjust xspice_wakeup_handler() prototype for building xspice with server 1.19

[Spice-devel] [PATCH] Cache field to make code easier

[Spice-devel] [PATCH RFC 10/14] Pass surface directly to dcc_create_surface

[Spice-devel] [PATCH RFC 08/14] Pass surface directly calling display_channel_current_flush

[Spice-devel] [PATCH RFC 14/14] Reuse more validate_surface

[Spice-devel] [PATCH RFC 12/14] Use directly surface instead of id

[Spice-devel] [PATCH RFC 07/14] Pass surface directly calling dcc_clear_surface_drawables_from_pipe

[Spice-devel] [PATCH RFC 11/14] Pass surface directly to dcc_push_surface_image

[Spice-devel] [PATCH RFC 09/14] New function to pass surface directly to display_channel_draw

[Spice-devel] [PATCH RFC 13/14] Change validate_surface to return surface pointer

[Spice-devel] [PATCH RFC 04/14] Pass surface directly to display_channel_surface_unref

[Spice-devel] [PATCH RFC 06/14] Pass surface directly calling draw_depend_on_me and display_channel_destroy_surface

[Spice-devel] [PATCH RFC 03/14] Pass surface directly to current_remove_all

[Spice-devel] [PATCH RFC 00/14] Change the way RedSurface is stored and handled

[Spice-devel] [PATCH RFC 02/14] Pass surface directly for surface_read_bits

[Spice-devel] [PATCH RFC 05/14] Pass surface directly in is_primary_surface

[Spice-devel] [PATCH RFC 01/14] Use direct pointers for surface and surface dependencies from Drawable

Re: [Spice-devel] [PATCH 2/2] SPICE port basic implementation

25 matches

Site Navigation

Mail list logo

Footer information