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 <fgouget@xxxxxxxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel