[Spice-devel] [spice v10 02/27] server: Store the opaque pointer in VideoEncoderRateControlCbs

[Frediano Ziglio]

> 
> I believe this was already reviewed separately, in a way it's odd to
> store instance data along with vfunc (ie class) data, but why not, makes
> more sense this way indeed.
> 
> Acked-by: Christophe Fergeau <cfergeau at redhat.com>
> 

Merged.

It's actually not so different that passing a pointer to a real
interface object. Just that in C you can implement it in different
way.

Frediano

> On Tue, Mar 01, 2016 at 04:50:32PM +0100, Francois Gouget wrote:
> > 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>
> > ---
> >  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 3bd1a04..48de21e 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);
> >      }
> >  }
> >  
> > @@ -1225,7 +1224,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);
> >              }
> >          }
> > @@ -1335,8 +1334,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);
> >  
> > @@ -1346,7 +1344,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 8223a7f..4d871ff 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
> >