[Spice-devel] [PATCH 05/30] Encapsulate quic information in a new EncodersData structure
Jonathon Jongsma
jjongsma at redhat.com
Wed Jun 8 20:37:04 UTC 2016
On Wed, 2016-06-08 at 15:32 -0500, Jonathon Jongsma wrote:
> On Tue, 2016-06-07 at 11:17 +0100, Frediano Ziglio wrote:
> >
> > Start putting all encoding code into dcc-encoders.c.
> >
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> > server/dcc-encoders.c | 27 ++++++++++++++-------------
> > server/dcc-encoders.h | 6 ++++++
> > server/dcc.c | 14 +++++++-------
> > server/dcc.h | 5 +++--
> > 4 files changed, 30 insertions(+), 22 deletions(-)
> >
> > diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> > index 2bf68d0..7d317d5 100644
> > --- a/server/dcc-encoders.c
> > +++ b/server/dcc-encoders.c
> > @@ -291,19 +291,19 @@ static int zlib_usr_more_input(ZlibEncoderUsrContext
> > *usr, uint8_t** input)
> > return buf_size;
> > }
> >
> > -static void dcc_init_quic(DisplayChannelClient *dcc)
> > +static void dcc_init_quic(EncodersData *enc)
> If these are no longer accepting a dcc argument, it seems to me that they're
> no
> longer DisplayChannelClient, so it doesn't really make sense to name them
> dcc_*.
> Something like encoders_data_init_quic() would seem more appropriate.
>
> >
> > {
> > - dcc->quic_data.usr.error = quic_usr_error;
> > - dcc->quic_data.usr.warn = quic_usr_warn;
> > - dcc->quic_data.usr.info = quic_usr_warn;
> > - dcc->quic_data.usr.malloc = quic_usr_malloc;
> > - dcc->quic_data.usr.free = quic_usr_free;
> > - dcc->quic_data.usr.more_space = quic_usr_more_space;
> > - dcc->quic_data.usr.more_lines = quic_usr_more_lines;
> > + enc->quic_data.usr.error = quic_usr_error;
> > + enc->quic_data.usr.warn = quic_usr_warn;
> > + enc->quic_data.usr.info = quic_usr_warn;
> > + enc->quic_data.usr.malloc = quic_usr_malloc;
> > + enc->quic_data.usr.free = quic_usr_free;
> > + enc->quic_data.usr.more_space = quic_usr_more_space;
> > + enc->quic_data.usr.more_lines = quic_usr_more_lines;
> >
> > - dcc->quic = quic_create(&dcc->quic_data.usr);
> > + enc->quic = quic_create(&enc->quic_data.usr);
> >
> > - if (!dcc->quic) {
> > + if (!enc->quic) {
> > spice_critical("create quic failed");
> > }
> > }
> > @@ -401,7 +401,7 @@ static void dcc_init_zlib(DisplayChannelClient *dcc)
> > void dcc_encoders_init(DisplayChannelClient *dcc)
> > {
> > dcc_init_glz_data(dcc);
> > - dcc_init_quic(dcc);
> > + dcc_init_quic(&dcc->encoders);
> > dcc_init_lz(dcc);
> > dcc_init_jpeg(dcc);
> > #ifdef USE_LZ4
> > @@ -415,8 +415,9 @@ void dcc_encoders_init(DisplayChannelClient *dcc)
> >
> > void dcc_encoders_free(DisplayChannelClient *dcc)
> > {
> > - quic_destroy(dcc->quic);
> > - dcc->quic = NULL;
> > + EncodersData *enc = &dcc->encoders;
> > + quic_destroy(enc->quic);
> > + enc->quic = NULL;
> > lz_destroy(dcc->lz);
> > dcc->lz = NULL;
> > jpeg_encoder_destroy(dcc->jpeg);
> > diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
> > index 95790b0..6bed548 100644
> > --- a/server/dcc-encoders.h
> > +++ b/server/dcc-encoders.h
> > @@ -34,6 +34,7 @@
> > typedef struct RedCompressBuf RedCompressBuf;
> > typedef struct GlzDrawableInstanceItem GlzDrawableInstanceItem;
> > typedef struct RedGlzDrawable RedGlzDrawable;
> > +typedef struct EncodersData EncodersData;
> >
> > void dcc_encoders_init (DisplayChanne
> > lC
> > lient *dcc);
> > void dcc_encoders_free (DisplayChanne
> > lC
> > lient *dcc);
> > @@ -161,6 +162,11 @@ struct RedGlzDrawable {
> > DisplayChannelClient *dcc;
> > };
> >
> > +struct EncodersData {
> > + QuicData quic_data;
> > + QuicContext *quic;
> > +};
> > +
> Will this struct eventually contain other encoder-related data? It seems weird
> to have the name be EncodersData (plural encoders) when it only contains data
> for a single encoder (quic).
I should have just looked a couple patches ahead. I see that all encoder data is
moving here. Fine.
>
> >
> > #define RED_RELEASE_BUNCH_SIZE 64
> >
> > #endif /* DCC_ENCODERS_H_ */
> > diff --git a/server/dcc.c b/server/dcc.c
> > index a88f8e6..27e668c 100644
> > --- a/server/dcc.c
> > +++ b/server/dcc.c
> > @@ -1026,15 +1026,15 @@ static int
> > dcc_compress_image_lz4(DisplayChannelClient
> > *dcc, SpiceImage *dest,
> > }
> > #endif
> >
> > -static int dcc_compress_image_quic(DisplayChannelClient *dcc, SpiceImage
> > *dest,
> > - SpiceBitmap *src, compress_send_data_t*
> > o_comp_data)
> > +static int dcc_compress_image_quic(EncodersData *enc, SpiceImage *dest,
> > + SpiceBitmap *src, compress_send_data_t*
> > o_comp_data, stat_info_t *stats)
> again, the dcc_* prefix here is no longer really appropriate.
>
> >
> > {
> > - QuicData *quic_data = &dcc->quic_data;
> > - QuicContext *quic = dcc->quic;
> > + QuicData *quic_data = &enc->quic_data;
> > + QuicContext *quic = enc->quic;
> > volatile QuicImageType type;
> > int size, stride;
> > stat_start_time_t start_time;
> > - stat_start_time_init(&start_time, &DCC_TO_DC(dcc)->quic_stat);
> > + stat_start_time_init(&start_time, stats);
> This looks like an improvment to avoid poking into the DisplayChannel from
> this
> function, but it's unrelated to the EncodersData change. It should be done in
> a
> separate commit.
>
> >
> >
> > #ifdef COMPRESS_DEBUG
> > spice_info("QUIC compress");
> > @@ -1094,7 +1094,7 @@ static int
> > dcc_compress_image_quic(DisplayChannelClient
> > *dcc, SpiceImage *dest,
> > o_comp_data->comp_buf = quic_data->data.bufs_head;
> > o_comp_data->comp_buf_size = size << 2;
> >
> > - stat_compress_add(&DCC_TO_DC(dcc)->quic_stat, start_time, src->stride *
> > src->y,
> > + stat_compress_add(stats, start_time, src->stride * src->y,
> > o_comp_data->comp_buf_size);
> > return TRUE;
> > }
> > @@ -1205,7 +1205,7 @@ int dcc_compress_image(DisplayChannelClient *dcc,
> > success = dcc_compress_image_jpeg(dcc, dest, src, o_comp_data);
> > break;
> > }
> > - success = dcc_compress_image_quic(dcc, dest, src, o_comp_data);
> > + success = dcc_compress_image_quic(&dcc->encoders, dest, src,
> > o_comp_data, &display_channel->quic_stat);
> > break;
> > case SPICE_IMAGE_COMPRESSION_GLZ:
> > if ((src->x * src->y) < glz_enc_dictionary_get_size(dcc->glz_dict-
> > >
> > > dict)) {
> > diff --git a/server/dcc.h b/server/dcc.h
> > index a11d25a..0ac5f5a 100644
> > --- a/server/dcc.h
> > +++ b/server/dcc.h
> > @@ -61,10 +61,11 @@ struct DisplayChannelClient {
> > spice_wan_compression_t jpeg_state;
> > spice_wan_compression_t zlib_glz_state;
> > int jpeg_quality;
> > +
> > + EncodersData encoders;
> > +
> > int zlib_level;
> >
> > - QuicData quic_data;
> > - QuicContext *quic;
> > LzData lz_data;
> > LzContext *lz;
> > JpegData jpeg_data;
> Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>
>
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
More information about the Spice-devel
mailing list