[Spice-devel] [PATCH v5 0/9] Better encapsulation of image encoding stuff

Frediano Ziglio fziglio at redhat.com
Wed Jun 15 09:37:50 UTC 2016


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 v4:
- renamed image_encoders_glz_free_from_drawable_ring to
  drawable_ring_free_glz_drawables and
  image_encoders_glz_detach_from_drawable_ring to
  drawable_ring_detach_glz_drawables;
- merged some patches;
- rebased on master.

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 (9):
  Encapsulate some data in dcc-encoders
  Make red_glz_drawable_free() static
  Better encapsulation for image_encoders_compress_glz call
  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    | 151 +++++++++++++++++++++++++++++++++++++++--------
 server/dcc-encoders.h    |  57 ++++--------------
 server/dcc.c             |  19 ++----
 server/display-channel.c |  31 +++-------
 server/display-channel.h |   5 --
 5 files changed, 150 insertions(+), 113 deletions(-)

-- 
2.7.4



More information about the Spice-devel mailing list