On Thu, Apr 06, 2017 at 04:03:01PM +0200, Francois Gouget wrote: > This makes it easier to reuse display_streams for other types of > video streams should the need arise. > > Signed-off-by: Francois Gouget <[email protected]> > --- > src/channel-display.c | 110 > ++++++++++++++++++++++++++++---------------------- > 1 file changed, 62 insertions(+), 48 deletions(-) > > diff --git a/src/channel-display.c b/src/channel-display.c > index 00b54406..6b0efaff 100644 > --- a/src/channel-display.c > +++ b/src/channel-display.c > @@ -109,7 +109,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_stream(SpiceChannel *channel, int id); > +static void destroy_display_stream(display_stream *st, int id); > static void display_session_mm_time_reset_cb(SpiceSession *session, gpointer > data); > static SpiceGlScanout* spice_gl_scanout_copy(const SpiceGlScanout *scanout); > > @@ -1168,11 +1168,57 @@ static display_stream *get_stream_by_id(SpiceChannel > *channel, uint32_t id) > } > > /* coroutine context */ > +static display_stream *display_stream_create(SpiceChannel *channel, uint32_t > surface_id, uint32_t flags, uint32_t codec_type) > +{ > + SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv; > + display_stream *st = g_new0(display_stream, 1); > + > + st->flags = flags; > + st->surface = find_surface(c, surface_id); > + st->channel = channel; > + st->drops_seqs_stats_arr = g_array_new(FALSE, FALSE, > sizeof(drops_sequence_stats)); > + > + region_init(&st->region); > + display_update_stream_region(st); > + > + switch (codec_type) { > +#ifdef HAVE_BUILTIN_MJPEG > + case SPICE_VIDEO_CODEC_TYPE_MJPEG: > + st->video_decoder = create_mjpeg_decoder(codec_type, st); > + break; > +#endif > + default: > +#ifdef HAVE_GSTVIDEO > + st->video_decoder = create_gstreamer_decoder(codec_type, st); > +#endif > + break; > + } > + 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; > + } > + return st; > +} > + > +static void destroy_stream(SpiceChannel *channel, int id) > +{ > + SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv; > + > + g_return_if_fail(c != NULL); > + 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; > + } > +} > + > static void display_handle_stream_create(SpiceChannel *channel, SpiceMsgIn > *in) > { > SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv; > SpiceMsgDisplayStreamCreate *op = spice_msg_in_parsed(in); > - display_stream *st; > > CHANNEL_DEBUG(channel, "%s: id %u", __FUNCTION__, op->id); > > @@ -1188,36 +1234,15 @@ static void display_handle_stream_create(SpiceChannel > *channel, SpiceMsgIn *in) > memset(c->streams + n, 0, (c->nstreams - n) * sizeof(c->streams[0])); > } > g_return_if_fail(c->streams[op->id] == NULL); > - c->streams[op->id] = g_new0(display_stream, 1); > - st = c->streams[op->id]; > - > - st->flags = op->flags; > - st->dest = op->dest; > - st->clip = op->clip; > - st->surface = find_surface(c, op->surface_id); > - st->channel = channel; > - st->drops_seqs_stats_arr = g_array_new(FALSE, FALSE, > sizeof(drops_sequence_stats)); > > - region_init(&st->region); > - display_update_stream_region(st); > - > - switch (op->codec_type) { > -#ifdef HAVE_BUILTIN_MJPEG > - case SPICE_VIDEO_CODEC_TYPE_MJPEG: > - st->video_decoder = create_mjpeg_decoder(op->codec_type, st); > - break; > -#endif > - default: > -#ifdef HAVE_GSTVIDEO > - st->video_decoder = create_gstreamer_decoder(op->codec_type, st); > -#else > - st->video_decoder = NULL; > -#endif > - } > - if (st->video_decoder == NULL) { > - spice_printerr("could not create a video decoder for codec %u", > op->codec_type); > + c->streams[op->id] = display_stream_create(channel, op->surface_id, > op->flags, op->codec_type); > + if (c->streams[op->id] == NULL) { > + spice_printerr("could not create the %u video stream", op->id); > destroy_stream(channel, op->id); > report_invalid_stream(channel, op->id); > + } else { > + c->streams[op->id]->dest = op->dest; > + c->streams[op->id]->clip = op->clip;
Any reason why you initialize everything in display_stream_create()
except these SpiceRect/SpiceClip instances? I'd just squash this in:
diff --git a/src/channel-display.c b/src/channel-display.c
index 6b0efaff..ccaa7477 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -1168,12 +1168,16 @@ static display_stream *get_stream_by_id(SpiceChannel
*channel, uint32_t id)
}
/* coroutine context */
-static display_stream *display_stream_create(SpiceChannel *channel, uint32_t
surface_id, uint32_t flags, uint32_t codec_type)
+static display_stream *display_stream_create(SpiceChannel *channel, 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->flags = flags;
+ st->dest = *dest;
+ st->clip = *clip;
st->surface = find_surface(c, surface_id);
st->channel = channel;
st->drops_seqs_stats_arr = g_array_new(FALSE, FALSE,
sizeof(drops_sequence_stats));
@@ -1235,14 +1239,13 @@ 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,
op->flags, op->codec_type);
+ c->streams[op->id] = display_stream_create(channel, op->surface_id,
+ op->flags, op->codec_type,
+ &op->dest, &op->clip);
if (c->streams[op->id] == NULL) {
spice_printerr("could not create the %u video stream", op->id);
destroy_stream(channel, op->id);
report_invalid_stream(channel, op->id);
- } else {
- c->streams[op->id]->dest = op->dest;
- c->streams[op->id]->clip = op->clip;
}
}
Apart from this, looks good to me.
Christophe
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/spice-devel
