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@xxxxxxxxxx>
> ---
> 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 (DisplayChannelC
> lient *dcc);
> void dcc_encoders_free (DisplayChannelC
> 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).
> #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@xxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel