Hi,
Just replying here what we talked in IRC
On Mon, Aug 08, 2016 at 04:36:46PM +0200, Francois Gouget wrote:
> Signed-off-by: Francois Gouget <fgouget@xxxxxxxxxxxxxxx>
> ---
> src/channel-display.c | 53 ++++++++++++++++++++++++++++-----------------------
> 1 file changed, 29 insertions(+), 24 deletions(-)
>
> diff --git a/src/channel-display.c b/src/channel-display.c
> index b4c9ec0..f5dbf89 100644
> --- a/src/channel-display.c
> +++ b/src/channel-display.c
> @@ -109,6 +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 display_session_mm_time_reset_cb(SpiceSession *session, gpointer data);
> static SpiceGlScanout* spice_gl_scanout_copy(const SpiceGlScanout *scanout);
>
> @@ -1114,18 +1115,18 @@ static void display_handle_stream_create(SpiceChannel *channel, SpiceMsgIn *in)
> #ifdef HAVE_BUILTIN_MJPEG
> case SPICE_VIDEO_CODEC_TYPE_MJPEG:
> st->video_decoder = create_mjpeg_decoder(op->codec_type, st);
> - break;
> + return;
> #endif
> default:
> #ifdef HAVE_GSTVIDEO
> st->video_decoder = create_gstreamer_decoder(op->codec_type, st);
> + return;
As create_gstreamer_decoder() could fail (returning NULL) we should
check it and call destroy_stream() ...
> #else
> - st->video_decoder = NULL;
> + break;
> #endif
> }
> - if (st->video_decoder == NULL) {
> - spice_printerr("could not create a video decoder for codec %u", op->codec_type);
> - }
> + spice_printerr("could not create a video decoder for codec %u", op->codec_type);
> + destroy_stream(channel, op->id);
Maybe by keeping the if (st->video_decoder == NULL) and moving
destroy_stream() inside it would be enough.
Cheers,
toso
> }
>
> static const SpiceRect *stream_get_dest(display_stream *st, SpiceMsgIn *frame_msg)
> @@ -1231,6 +1232,7 @@ static void display_update_stream_report(SpiceDisplayChannel *channel, uint32_t
> g_return_if_fail(c->nstreams > stream_id);
>
> st = channel->priv->streams[stream_id];
> + g_return_if_fail(st != NULL);
>
> if (!st->report_is_active) {
> return;
> @@ -1353,6 +1355,7 @@ static void display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn *in)
> g_return_if_fail(c->nstreams > op->id);
>
> st = c->streams[op->id];
> + g_return_if_fail(st != NULL);
> mmtime = stream_get_time(st);
>
> if (spice_msg_in_type(in) == SPICE_MSG_DISPLAY_STREAM_DATA_SIZED) {
> @@ -1420,6 +1423,7 @@ static void display_handle_stream_clip(SpiceChannel *channel, SpiceMsgIn *in)
> g_return_if_fail(c->nstreams > op->id);
>
> st = c->streams[op->id];
> + g_return_if_fail(st != NULL);
>
> if (st->msg_clip) {
> spice_msg_in_unref(st->msg_clip);
> @@ -1434,8 +1438,6 @@ static void destroy_stream(SpiceChannel *channel, int id)
> {
> SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
> display_stream *st;
> - guint64 drops_duration_total = 0;
> - guint32 num_out_frames;
> int i;
>
> g_return_if_fail(c != NULL);
> @@ -1446,20 +1448,22 @@ static void destroy_stream(SpiceChannel *channel, int id)
> if (!st)
> return;
>
> - num_out_frames = st->num_input_frames - st->arrive_late_count - st->num_drops_on_playback;
> - CHANNEL_DEBUG(channel, "%s: id=%d #in-frames=%u out/in=%.2f "
> - "#drops-on-receive=%u avg-late-time(ms)=%.2f "
> - "#drops-on-playback=%u", __FUNCTION__,
> - id,
> - st->num_input_frames,
> - num_out_frames / (double)st->num_input_frames,
> - st->arrive_late_count,
> - st->arrive_late_count ? st->arrive_late_time / ((double)st->arrive_late_count): 0,
> - st->num_drops_on_playback);
> - if (st->num_drops_seqs) {
> - CHANNEL_DEBUG(channel, "%s: #drops-sequences=%u ==>", __FUNCTION__, st->num_drops_seqs);
> - }
> - for (i = 0; i < st->num_drops_seqs; i++) {
> + 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(channel, "%s: id=%d #in-frames=%u out/in=%.2f "
> + "#drops-on-receive=%u avg-late-time(ms)=%.2f "
> + "#drops-on-playback=%u", __FUNCTION__,
> + id,
> + st->num_input_frames,
> + num_out_frames / (double)st->num_input_frames,
> + st->arrive_late_count,
> + st->arrive_late_count ? st->arrive_late_time / ((double)st->arrive_late_count): 0,
> + st->num_drops_on_playback);
> + if (st->num_drops_seqs) {
> + CHANNEL_DEBUG(channel, "%s: #drops-sequences=%u ==>", __FUNCTION__, st->num_drops_seqs);
> + }
> + for (i = 0; i < st->num_drops_seqs; i++) {
> drops_sequence_stats *stats = &g_array_index(st->drops_seqs_stats_arr,
> drops_sequence_stats,
> i);
> @@ -1468,9 +1472,10 @@ static void destroy_stream(SpiceChannel *channel, int id)
> stats->len,
> stats->start_mm_time - st->first_frame_mm_time,
> stats->duration);
> - }
> - if (st->num_drops_seqs) {
> - CHANNEL_DEBUG(channel, "%s: drops-total-duration=%"G_GUINT64_FORMAT" ==>", __FUNCTION__, drops_duration_total);
> + }
> + if (st->num_drops_seqs) {
> + CHANNEL_DEBUG(channel, "%s: drops-total-duration=%"G_GUINT64_FORMAT" ==>", __FUNCTION__, drops_duration_total);
> + }
> }
>
> g_array_free(st->drops_seqs_stats_arr, TRUE);
> --
> 2.8.1
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel