On Tue, Apr 17, 2018 at 07:10:43AM -0400, Frediano Ziglio wrote: > > > From: Victor Toso <[email protected]> > > > > As it makes easier to track what is allocated in its own function and > > ultimately what needs to be freed later on. > > > > Signed-off-by: Victor Toso <[email protected]> > > --- > > src/channel-display.c | 30 ++++++++++++++++++++++-------- > > 1 file changed, 22 insertions(+), 8 deletions(-) > > > > diff --git a/src/channel-display.c b/src/channel-display.c > > index a134516..2ee761e 100644 > > --- a/src/channel-display.c > > +++ b/src/channel-display.c > > @@ -1485,6 +1485,27 @@ static void > > display_session_mm_time_reset_cb(SpiceSession *session, gpointer dat > > > > #define STREAM_PLAYBACK_SYNC_DROP_SEQ_LEN_LIMIT 5 > > > > +static SpiceFrame *spice_frame_new(display_stream *st, > > + SpiceMsgIn *in, > > + guint32 server_mmtime) > > +{ > > + SpiceFrame *frame; > > + guint8 *data_ptr; > > + const SpiceRect *dest_rect = stream_get_dest(st, in); > > + guint32 data_size = spice_msg_in_frame_data(in, &data_ptr); > > + > > why all these variables? I prefer the old syntax.
1) I was actually thinking in changing the helper functions but
not super important and didn't do it.
2) Because this makes it easier to identify where data is coming
from? (a _new() function which needs to call 2 helpers to get
the right parameters for SpiceFrame initialization)
This is really person preference, not important. I'll change it.
> Also would be more a clean factorization.
Indeed
>
> > + frame = g_new(SpiceFrame, 1);
> > + frame->mm_time = server_mmtime;
> > + frame->dest = *dest_rect;
> > + frame->data = data_ptr;
> > + frame->size = data_size;
> > + frame->data_opaque = in;
> > + frame->ref_data = (void*)spice_msg_in_ref;
> > + frame->unref_data = (void*)spice_msg_in_unref;
> > + frame->free = (void*)g_free;
> > + return frame;
> > +}
> > +
> > G_GNUC_INTERNAL
> > void spice_frame_free(SpiceFrame *frame)
> > {
> > @@ -1551,14 +1572,7 @@ static void display_handle_stream_data(SpiceChannel
> > *channel, SpiceMsgIn *in)
> > * decoding and best decide if/when to drop them when they are late,
> > * taking into account the impact on later frames.
> > */
> > - frame = g_new(SpiceFrame, 1);
> > - frame->mm_time = op->multi_media_time;
> > - frame->dest = *stream_get_dest(st, in);
> > - frame->size = spice_msg_in_frame_data(in, &frame->data);
> > - frame->data_opaque = in;
> > - frame->ref_data = (void*)spice_msg_in_ref;
> > - frame->unref_data = (void*)spice_msg_in_unref;
> > - frame->free = (void*)g_free;
> > + frame = spice_frame_new(st, in, op->multi_media_time);
> > if (!st->video_decoder->queue_frame(st->video_decoder, frame, latency))
> > {
> > destroy_stream(channel, op->id);
> > report_invalid_stream(channel, op->id);
>
> Frediano
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/spice-devel
