On Tue, Apr 17, 2018 at 07:10:43AM -0400, Frediano Ziglio wrote:
>
> > From: Victor Toso <me@xxxxxxxxxxxxxx>
> >
> > 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 <victortoso@xxxxxxxxxx>
> > ---
> > 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
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel