[Spice-devel] [spice-gtk v1 1/3] channel-display: remove id parameter from helper function

Victor Toso victortoso at redhat.com
Tue Mar 13 08:59:58 UTC 2018


Hi,

On Mon, Mar 12, 2018 at 04:28:50PM +0100, Victor Toso wrote:
> On Mon, Mar 12, 2018 at 04:13:06PM +0100, Lukáš Hrázký wrote:
> > On Fri, 2018-03-02 at 12:39 +0100, Victor Toso wrote:
> > > From: Victor Toso <me at victortoso.com>
> > > 
> > > Instead of passing the id parameter for destroy_display_stream() which
> > > is only used for debug, let's store the id when creating the stream at
> > > display_stream_create().
> > > 
> > > Another benefit is that we can drop a helper function to get the id
> > > while holding display_stream structure.
> > > 
> > > This patch also renames destroy_display_stream() to
> > > display_stream_destroy() to keep compatibility with
> > > display_stream_create()
> > 
> > Seems this could go into it's own commit... Just sayin' :)
> 
> It would be two patches changing the same code, one renaming the
> function and all the callers and the second removing id parameter
> and again, in all its callers.
> 
> I don't mind splitting this in v2/v3 if you prefer, I just
> thought it made sense to combine those changes in one patch.
> 
> Thanks for the review,
>         toso

JFYI, split was done and currently at [0]. I'll wait some time
before resending this to see if anything else comes up.

[0] https://github.com/victortoso/spice-gtk/commits/tosend-use-ghashtable

Cheers,
        toso
> 
> > 
> > > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > > ---
> > >  src/channel-display-priv.h |  1 +
> > >  src/channel-display.c      | 23 +++++++++++------------
> > >  2 files changed, 12 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
> > > index 1389868..94c9913 100644
> > > --- a/src/channel-display-priv.h
> > > +++ b/src/channel-display-priv.h
> > > @@ -115,6 +115,7 @@ typedef struct drops_sequence_stats {
> > >  
> > >  struct display_stream {
> > >      /* from messages */
> > > +    uint32_t                    id;
> > >      uint32_t                    flags;
> > >      SpiceRect                   dest;
> > >      display_surface             *surface;
> > > diff --git a/src/channel-display.c b/src/channel-display.c
> > > index 45fe38c..1a79a8b 100644
> > > --- a/src/channel-display.c
> > > +++ b/src/channel-display.c
> > > @@ -106,7 +106,7 @@ static display_surface *find_surface(SpiceDisplayChannelPrivate *c, guint32 surf
> > >  static void spice_display_channel_reset(SpiceChannel *channel, gboolean migrating);
> > >  static void spice_display_channel_reset_capabilities(SpiceChannel *channel);
> > >  static void destroy_canvas(display_surface *surface);
> > > -static void destroy_display_stream(display_stream *st, int id);
> > > +static void display_stream_destroy(gpointer st);
> > >  static void display_session_mm_time_reset_cb(SpiceSession *session, gpointer data);
> > >  static SpiceGlScanout* spice_gl_scanout_copy(const SpiceGlScanout *scanout);
> > >  
> > > @@ -1236,13 +1236,15 @@ gint64 get_stream_id_by_stream(SpiceChannel *channel, display_stream *st)
> > >  }
> > >  
> > >  /* coroutine context */
> > > -static display_stream *display_stream_create(SpiceChannel *channel, uint32_t surface_id,
> > > +static display_stream *display_stream_create(SpiceChannel *channel,
> > > +                                             uint32_t id, uint32_t surface_id,
> > >                                               uint32_t flags, uint32_t codec_type,
> > >                                               const SpiceRect *dest, const SpiceClip *clip)
> > >  {
> > >      SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
> > >      display_stream *st = g_new0(display_stream, 1);
> > >  
> > > +    st->id = id;
> > >      st->flags = flags;
> > >      st->dest = *dest;
> > >      st->clip = *clip;
> > > @@ -1267,8 +1269,7 @@ static display_stream *display_stream_create(SpiceChannel *channel, uint32_t sur
> > >      }
> > >      if (st->video_decoder == NULL) {
> > >          spice_printerr("could not create a video decoder for codec %u", codec_type);
> > > -        destroy_display_stream(st, 0);
> > > -        st = NULL;
> > > +        g_clear_pointer(&st, display_stream_destroy);
> > >      }
> > >      return st;
> > >  }
> > > @@ -1281,10 +1282,7 @@ static void destroy_stream(SpiceChannel *channel, int id)
> > >      g_return_if_fail(c->streams != NULL);
> > >      g_return_if_fail(c->nstreams > id);
> > >  
> > > -    if (c->streams[id]) {
> > > -        destroy_display_stream(c->streams[id], id);
> > > -        c->streams[id] = NULL;
> > > -    }
> > > +    g_clear_pointer(&c->streams[id], display_stream_destroy);
> > >  }
> > >  
> > >  static void display_handle_stream_create(SpiceChannel *channel, SpiceMsgIn *in)
> > > @@ -1307,7 +1305,7 @@ static void display_handle_stream_create(SpiceChannel *channel, SpiceMsgIn *in)
> > >      }
> > >      g_return_if_fail(c->streams[op->id] == NULL);
> > >  
> > > -    c->streams[op->id] = display_stream_create(channel, op->surface_id,
> > > +    c->streams[op->id] = display_stream_create(channel, op->id, op->surface_id,
> > >                                                 op->flags, op->codec_type,
> > >                                                 &op->dest, &op->clip);
> > >      if (c->streams[op->id] == NULL) {
> > > @@ -1598,17 +1596,18 @@ static void display_handle_stream_clip(SpiceChannel *channel, SpiceMsgIn *in)
> > >      display_update_stream_region(st);
> > >  }
> > >  
> > > -static void destroy_display_stream(display_stream *st, int id)
> > > +static void display_stream_destroy(gpointer st_pointer)
> > >  {
> > >      int i;
> > > +    display_stream *st = st_pointer;
> > >  
> > >      if (st->num_input_frames > 0) {
> > >          guint64 drops_duration_total = 0;
> > >          guint32 num_out_frames = st->num_input_frames - st->arrive_late_count - st->num_drops_on_playback;
> > > -        CHANNEL_DEBUG(st->channel, "%s: id=%d #in-frames=%u out/in=%.2f "
> > > +        CHANNEL_DEBUG(st->channel, "%s: id=%u #in-frames=%u out/in=%.2f "
> > >              "#drops-on-receive=%u avg-late-time(ms)=%.2f "
> > >              "#drops-on-playback=%u", __FUNCTION__,
> > > -            id,
> > > +            st->id,
> > >              st->num_input_frames,
> > >              num_out_frames / (double)st->num_input_frames,
> > >              st->arrive_late_count,



> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180313/cb17d4fd/attachment-0001.sig>


More information about the Spice-devel mailing list