[Spice-devel] [PATCH 00/17] WIP: Refactor the streaming agent towards a more standard C++ style
Christophe de Dinechin
christophe at dinechin.org
Fri Feb 16 16:15:30 UTC 2018
From: Christophe de Dinechin <dinechin at redhat.com>
The streaming agent started as C code. This series converts
the style to something that is more usual for C++, notably:
- Adds encapsulation and RAII for resources such as the stream
- Splits functionality into several classes with clear roles
- Puts message formatting and writing in a reused based class
- Isolate what's specific to each message in three derived classes
- Isolate X11-specific code in separate classes, one for cursor messages,
one for the thread polling the data.
Reasons for marking this WIP:
1. This series is, unfortunately, not correctly tested, because on my
machine, I currently have a black screen with the 'master' streaming
agent, server, protocol, common, plugin and spicy. Fallback to MJPEG
leads to a large repetition of messages like:
(spicy:4217): GSpice-CRITICAL **: need more input data
What I have tested using the -d option is that the syslog output
from the agent looks similar (size of data captured, etc) relative
to master both for the MJPEG plugin and with fallback. But without
a picture, I am still concerned about the lack of testing.
2. The classes were isolated, but not moved in separate headers. This
is intentional. I prefer to make sure that the changes on the code
are agreed on before we move the individual classes to their own
headers. It thinks it will also faclitate history browsing
3. This series compiles without warnings both with GCC in C++11 mode
and by clang in gnu++11 mode. However, Frediano pointed out that it
uses a designated intiializer syntax that is presently a GNU
extension (the C99 designated initializer syntax). We may consider
it a problem or not. If it's a problem, it's sufficient to remove
the designators, but I think they add to readability.
4. Our current configure.ac requires a warning if all initializers are
not present. Since padding was made explicit in the protocol, this
requires the code to initialize padding fields, which I don't like.
5. The series integrates off-topic patches sent in a separate series, but
which are necessary to successfully build with both clang and gcc.
This series can also be browsed at
https://gitlab.com/c3d/spice-streaming-agent/merge_requests/1/commits
Christophe de Dinechin (17):
Add missing <string> header
log_binary is really a boolean
Eliminate signed/unsigned warning
Do not create std::string for constants
Use RAII to cleanup stream in case of exception or return
Replace inefficient C-style initialization with C++-style
Get rid of C-style memset initializations, use C++ style aggregates
Use C++ style for cursor message initialization instead of C style
Reorder headers according to style guide
Since we use a namespace, simplify name of local classes
Move read, write and locking into the 'Stream' class
Convert message writing from C style to C++ style
Add more meaningful syslog reporting
Create a class encapsulating the X11 display cursor capture
Create FrameLog class to abstract logging of frames
Remove client_codecs global variable, moved inside the 'Stream' class
Move the capture loop in the ConcreteAgent, get rid of global agent
variable
src/concrete-agent.cpp | 1 +
src/concrete-agent.hpp | 4 +
src/mjpeg-fallback.cpp | 2 +-
src/spice-streaming-agent.cpp | 521 +++++++++++++++++++++++++-----------------
4 files changed, 311 insertions(+), 217 deletions(-)
--
2.13.5 (Apple Git-94)
More information about the Spice-devel
mailing list