[Spice-devel] [PATCH spice 2/11] server: Remove the rate_control_is_active field from MJpegEncoder.

[Marc-André Lureau]

ack

On Wed, May 13, 2015 at 10:26 PM, Francois Gouget <fgouget at codeweavers.com>
wrote:

> It is redundant with the corresponding callbacks.
> ---
>
> This patch only depends on patch 1/11 and is independent from the rest
> of the series.
>
>  server/mjpeg_encoder.c | 22 +++++++++++++---------
>  server/mjpeg_encoder.h |  2 +-
>  server/red_worker.c    |  4 ++--
>  3 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
> index 95d841f..9a41ef3 100644
> --- a/server/mjpeg_encoder.c
> +++ b/server/mjpeg_encoder.c
> @@ -164,7 +164,6 @@ struct MJpegEncoder {
>      unsigned int bytes_per_pixel; /* bytes per pixel of the input buffer
> */
>      void (*pixel_converter)(uint8_t *src, uint8_t *dest);
>
> -    int rate_control_is_active;
>      MJpegEncoderRateControl rate_control;
>      MJpegEncoderRateControlCbs cbs;
>      void *cbs_opaque;
> @@ -185,21 +184,25 @@ static uint32_t
> get_min_required_playback_delay(uint64_t frame_enc_size,
>                                                  uint64_t byte_rate,
>                                                  uint32_t latency);
>
> -MJpegEncoder *mjpeg_encoder_new(int bit_rate_control, uint64_t
> starting_bit_rate,
> +static inline int rate_control_is_active(MJpegEncoder* encoder)
> +{
> +    return encoder->cbs.get_roundtrip_ms != NULL;
> +}
> +
> +MJpegEncoder *mjpeg_encoder_new(uint64_t starting_bit_rate,
>                                  MJpegEncoderRateControlCbs *cbs, void
> *opaque)
>  {
>      MJpegEncoder *enc;
>
> -    spice_assert(!bit_rate_control || (cbs && cbs->get_roundtrip_ms &&
> cbs->get_source_fps));
> +    spice_assert(!cbs || (cbs && cbs->get_roundtrip_ms &&
> cbs->get_source_fps));
>
>      enc = spice_new0(MJpegEncoder, 1);
>
>      enc->first_frame = TRUE;
> -    enc->rate_control_is_active = bit_rate_control;
>      enc->rate_control.byte_rate = starting_bit_rate / 8;
>      enc->starting_bit_rate = starting_bit_rate;
>
> -    if (bit_rate_control) {
> +    if (cbs) {
>          struct timespec time;
>
>          clock_gettime(CLOCK_MONOTONIC, &time);
> @@ -211,6 +214,7 @@ MJpegEncoder *mjpeg_encoder_new(int bit_rate_control,
> uint64_t starting_bit_rate
>          enc->rate_control.quality_eval_data.reason =
> MJPEG_QUALITY_EVAL_REASON_RATE_CHANGE;
>          enc->rate_control.warmup_start_time = ((uint64_t) time.tv_sec) *
> 1000000000 + time.tv_nsec;
>      } else {
> +        enc->cbs.get_roundtrip_ms = NULL;
>          mjpeg_encoder_reset_quality(enc, MJPEG_LEGACY_STATIC_QUALITY_ID,
> MJPEG_MAX_FPS, 0);
>      }
>
> @@ -607,7 +611,7 @@ static void
> mjpeg_encoder_adjust_params_to_bit_rate(MJpegEncoder *encoder)
>      uint32_t latency = 0;
>      uint32_t src_fps;
>
> -    spice_assert(encoder->rate_control_is_active);
> +    spice_assert(rate_control_is_active(encoder));
>
>      rate_control = &encoder->rate_control;
>      quality_eval = &rate_control->quality_eval_data;
> @@ -692,7 +696,7 @@ static void mjpeg_encoder_adjust_fps(MJpegEncoder
> *encoder, uint64_t now)
>      MJpegEncoderRateControl *rate_control = &encoder->rate_control;
>      uint64_t adjusted_fps_time_passed;
>
> -    spice_assert(encoder->rate_control_is_active);
> +    spice_assert(rate_control_is_active(encoder));
>
>      adjusted_fps_time_passed = (now -
> rate_control->adjusted_fps_start_time) / 1000 / 1000;
>
> @@ -734,7 +738,7 @@ int mjpeg_encoder_start_frame(MJpegEncoder *encoder,
> SpiceBitmapFmt format,
>  {
>      uint32_t quality;
>
> -    if (encoder->rate_control_is_active) {
> +    if (rate_control_is_active(encoder)) {
>          MJpegEncoderRateControl *rate_control = &encoder->rate_control;
>          struct timespec time;
>          uint64_t now;
> @@ -1131,7 +1135,7 @@ void mjpeg_encoder_client_stream_report(MJpegEncoder
> *encoder,
>                  end_frame_mm_time - start_frame_mm_time,
>                  end_frame_delay, audio_delay);
>
> -    if (!encoder->rate_control_is_active) {
> +    if (!rate_control_is_active(encoder)) {
>          spice_debug("rate control was not activated: ignoring");
>          return;
>      }
> diff --git a/server/mjpeg_encoder.h b/server/mjpeg_encoder.h
> index 741ea1c..d584b92 100644
> --- a/server/mjpeg_encoder.h
> +++ b/server/mjpeg_encoder.h
> @@ -49,7 +49,7 @@ typedef struct MJpegEncoderStats {
>      double avg_quality;
>  } MJpegEncoderStats;
>
> -MJpegEncoder *mjpeg_encoder_new(int bit_rate_control, uint64_t
> starting_bit_rate,
> +MJpegEncoder *mjpeg_encoder_new(uint64_t starting_bit_rate,
>                                  MJpegEncoderRateControlCbs *cbs, void
> *opaque);
>  void mjpeg_encoder_destroy(MJpegEncoder *encoder);
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 5deb30b..e0ff8e9 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -3100,9 +3100,9 @@ static void
> red_display_create_stream(DisplayChannelClient *dcc, Stream *stream)
>          mjpeg_cbs.update_client_playback_delay =
> red_stream_update_client_playback_latency;
>
>          initial_bit_rate = red_stream_get_initial_bit_rate(dcc, stream);
> -        agent->mjpeg_encoder = mjpeg_encoder_new(TRUE, initial_bit_rate,
> &mjpeg_cbs, agent);
> +        agent->mjpeg_encoder = mjpeg_encoder_new(initial_bit_rate,
> &mjpeg_cbs, agent);
>      } else {
> -        agent->mjpeg_encoder = mjpeg_encoder_new(FALSE, 0, NULL, NULL);
> +        agent->mjpeg_encoder = mjpeg_encoder_new(0, NULL, NULL);
>      }
>      red_channel_client_pipe_add(&dcc->common.base, &agent->create_item);
>
> --
> 2.1.4
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>



-- 
Marc-André Lureau
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150519/3bb9b3de/attachment.html>