[Spice-devel] [PATCH 2/2] server: Store the opaque pointer in VideoEncoderRateControlCbs

[Frediano Ziglio]

> 
> It has the same lifetime as the corresponding structure so this
> simplifies keeping track of it.
> 
> Signed-off-by: Francois Gouget <fgouget at codeweavers.com>

This it's quite new in the callback style but I quite like.
Jonathon pointed out similar issue yesterday (opaque is separate from callbacks).

Reviewed-by: Frediano Ziglio <fziglio at redhat.com>

Frediano

> ---
>  server/mjpeg-encoder.c | 13 +++++--------
>  server/mjpeg-encoder.h |  4 +++-
>  server/stream.c        |  5 +++--
>  3 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/server/mjpeg-encoder.c b/server/mjpeg-encoder.c
> index 1086b53..5c9629a 100644
> --- a/server/mjpeg-encoder.c
> +++ b/server/mjpeg-encoder.c
> @@ -167,7 +167,6 @@ struct MJpegEncoder {
>  
>      MJpegEncoderRateControl rate_control;
>      MJpegEncoderRateControlCbs cbs;
> -    void *cbs_opaque;
>  
>      /* stats */
>      uint64_t starting_bit_rate;
> @@ -330,13 +329,13 @@ spice_jpeg_mem_dest(j_compress_ptr cinfo,
>  static inline uint32_t mjpeg_encoder_get_source_fps(MJpegEncoder *encoder)
>  {
>      return encoder->cbs.get_source_fps ?
> -        encoder->cbs.get_source_fps(encoder->cbs_opaque) : MJPEG_MAX_FPS;
> +        encoder->cbs.get_source_fps(encoder->cbs.opaque) : MJPEG_MAX_FPS;
>  }
>  
>  static inline uint32_t mjpeg_encoder_get_latency(MJpegEncoder *encoder)
>  {
>      return encoder->cbs.get_roundtrip_ms ?
> -        encoder->cbs.get_roundtrip_ms(encoder->cbs_opaque) / 2 : 0;
> +        encoder->cbs.get_roundtrip_ms(encoder->cbs.opaque) / 2 : 0;
>  }
>  
>  static uint32_t get_max_fps(uint64_t frame_size, uint64_t bytes_per_sec)
> @@ -531,7 +530,7 @@ complete_sample:
>                                                               rate_control->byte_rate,
>                                                               latency);
>  
> -        encoder->cbs.update_client_playback_delay(encoder->cbs_opaque,
> min_delay);
> +        encoder->cbs.update_client_playback_delay(encoder->cbs.opaque,
> min_delay);
>      }
>  }
>  
> @@ -1227,7 +1226,7 @@ void mjpeg_encoder_client_stream_report(MJpegEncoder
> *encoder,
>              rate_control->fps < MIN(src_fps, MJPEG_MAX_FPS) ||
>              end_frame_delay < 0) {
>              is_video_delay_small = TRUE;
>              if (encoder->cbs.update_client_playback_delay) {
> -
> encoder->cbs.update_client_playback_delay(encoder->cbs_opaque,
> +
> encoder->cbs.update_client_playback_delay(encoder->cbs.opaque,
>                                                            min_playback_delay);
>              }
>          }
> @@ -1337,8 +1336,7 @@ void mjpeg_encoder_get_stats(MJpegEncoder *encoder,
> MJpegEncoderStats *stats)
>  }
>  
>  MJpegEncoder *mjpeg_encoder_new(uint64_t starting_bit_rate,
> -                                MJpegEncoderRateControlCbs *cbs,
> -                                void *cbs_opaque)
> +                                MJpegEncoderRateControlCbs *cbs)
>  {
>      MJpegEncoder *encoder = spice_new0(MJpegEncoder, 1);
>  
> @@ -1348,7 +1346,6 @@ MJpegEncoder *mjpeg_encoder_new(uint64_t
> starting_bit_rate,
>  
>      if (cbs) {
>          encoder->cbs = *cbs;
> -        encoder->cbs_opaque = cbs_opaque;
>          mjpeg_encoder_reset_quality(encoder, MJPEG_QUALITY_SAMPLE_NUM / 2,
>          5, 0);
>          encoder->rate_control.during_quality_eval = TRUE;
>          encoder->rate_control.quality_eval_data.type =
>          MJPEG_QUALITY_EVAL_TYPE_SET;
> diff --git a/server/mjpeg-encoder.h b/server/mjpeg-encoder.h
> index 31e7cb3..6771dd5 100644
> --- a/server/mjpeg-encoder.h
> +++ b/server/mjpeg-encoder.h
> @@ -32,12 +32,14 @@ typedef struct MJpegEncoder MJpegEncoder;
>  /*
>   * Callbacks required for controling and adjusting
>   * the stream bit rate:
> + * @opaque: a pointer to be passed to the rate control callbacks.
>   * get_roundtrip_ms: roundtrip time in milliseconds
>   * get_source_fps: the input frame rate (#frames per second), i.e.,
>   * the rate of frames arriving from the guest to spice-server,
>   * before any drops.
>   */
>  typedef struct MJpegEncoderRateControlCbs {
> +    void *opaque;
>      uint32_t (*get_roundtrip_ms)(void *opaque);
>      uint32_t (*get_source_fps)(void *opaque);
>      void (*update_client_playback_delay)(void *opaque, uint32_t delay_ms);
> @@ -50,7 +52,7 @@ typedef struct MJpegEncoderStats {
>  } MJpegEncoderStats;
>  
>  MJpegEncoder *mjpeg_encoder_new(uint64_t starting_bit_rate,
> -                                MJpegEncoderRateControlCbs *cbs, void
> *opaque);
> +                                MJpegEncoderRateControlCbs *cbs);
>  void mjpeg_encoder_destroy(MJpegEncoder *encoder);
>  
>  int mjpeg_encoder_encode_frame(MJpegEncoder *encoder,
> diff --git a/server/stream.c b/server/stream.c
> index 2bfb993..e6da168 100644
> --- a/server/stream.c
> +++ b/server/stream.c
> @@ -719,14 +719,15 @@ void dcc_create_stream(DisplayChannelClient *dcc,
> Stream *stream)
>          MJpegEncoderRateControlCbs mjpeg_cbs;
>          uint64_t initial_bit_rate;
>  
> +        mjpeg_cbs.opaque = agent;
>          mjpeg_cbs.get_roundtrip_ms = get_roundtrip_ms;
>          mjpeg_cbs.get_source_fps = get_source_fps;
>          mjpeg_cbs.update_client_playback_delay =
>          update_client_playback_delay;
>  
>          initial_bit_rate = get_initial_bit_rate(dcc, stream);
> -        agent->mjpeg_encoder = mjpeg_encoder_new(initial_bit_rate,
> &mjpeg_cbs, agent);
> +        agent->mjpeg_encoder = mjpeg_encoder_new(initial_bit_rate,
> &mjpeg_cbs);
>      } else {
> -        agent->mjpeg_encoder = mjpeg_encoder_new(0, NULL, NULL);
> +        agent->mjpeg_encoder = mjpeg_encoder_new(0, NULL);
>      }
>      red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc),
>      &agent->create_item);
>  
> --
> 2.7.0
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>