Hey, On Tue, Mar 01, 2016 at 04:51:14PM +0100, Francois Gouget wrote: > Signed-off-by: Francois Gouget <[email protected]> > --- > server/dcc.c | 5 ++++- > server/dcc.h | 2 +- > server/stream.c | 41 +++++++++++++++++++++++++++++------------ > 3 files changed, 34 insertions(+), 14 deletions(-) > > diff --git a/server/dcc.c b/server/dcc.c > index d4a6c7c..df650c9 100644 > --- a/server/dcc.c > +++ b/server/dcc.c > @@ -408,7 +408,10 @@ static void dcc_create_all_streams(DisplayChannelClient > *dcc) > > while ((item = ring_next(ring, item))) { > Stream *stream = SPICE_CONTAINEROF(item, Stream, link); > - dcc_create_stream(dcc, stream); > + if (!dcc_create_stream(dcc, stream)) { > + stream_stop(DCC_TO_DC(dcc), stream); > + return; > + } > } > } > > diff --git a/server/dcc.h b/server/dcc.h > index 436d0be..c2ef871 100644 > --- a/server/dcc.h > +++ b/server/dcc.h > @@ -184,7 +184,7 @@ void dcc_destroy_surface > (DisplayCha > > uint32_t surface_id); > void dcc_stream_agent_clip > (DisplayChannelClient* dcc, > > StreamAgent *agent); > -void dcc_create_stream > (DisplayChannelClient *dcc, > +gboolean dcc_create_stream > (DisplayChannelClient *dcc, > Stream > *stream); > void dcc_create_surface > (DisplayChannelClient *dcc, > int > surface_id); > diff --git a/server/stream.c b/server/stream.c > index 9fae3ef..c2898ca 100644 > --- a/server/stream.c > +++ b/server/stream.c > @@ -440,7 +440,12 @@ static void display_channel_create_stream(DisplayChannel > *display, Drawable *dra > display->streams_size_total += stream->width * stream->height; > display->stream_count++; > FOREACH_DCC(display, dcc_ring_item, next, dcc) { > - dcc_create_stream(dcc, stream); > + if (!dcc_create_stream(dcc, stream)) { > + drawable->stream = NULL; > + stream->current = NULL; > + stream_stop(display, stream); > + return; > + } > }
I tried to test these error paths, and things mostly behave as expected
when they are triggered, that's good :)
> spice_debug("stream %d %dx%d (%d, %d) (%d, %d) %u fps",
> (int)(stream - display->streams_buf), stream->width,
> @@ -698,24 +703,31 @@ static void update_client_playback_delay(void *opaque,
> uint32_t delay_ms)
> }
>
> /* A helper for dcc_create_stream(). */
> -static VideoEncoder* dcc_create_video_encoder(uint64_t starting_bit_rate,
> +static VideoEncoder* dcc_create_video_encoder(DisplayChannelClient *dcc,
> + uint64_t starting_bit_rate,
> VideoEncoderRateControlCbs
> *cbs)
> {
> + RedChannelClient *rcc = RED_CHANNEL_CLIENT(dcc);
> + int client_has_multi_codec = red_channel_client_test_remote_cap(rcc,
> SPICE_DISPLAY_CAP_MULTI_CODEC);
> + if (!client_has_multi_codec || red_channel_client_test_remote_cap(rcc,
> SPICE_DISPLAY_CAP_CODEC_MJPEG)) {
> #ifdef HAVE_GSTREAMER_1_0
> - VideoEncoder* video_encoder = gstreamer_encoder_new(starting_bit_rate,
> cbs);
> - if (video_encoder) {
> - return video_encoder;
> - }
> + VideoEncoder* video_encoder =
> gstreamer_encoder_new(starting_bit_rate, cbs);
> + if (video_encoder) {
> + return video_encoder;
> + }
> #endif
> - /* Use the builtin MJPEG video encoder as a fallback */
> - return mjpeg_encoder_new(starting_bit_rate, cbs);
> + /* Use the builtin MJPEG video encoder as a fallback */
> + return mjpeg_encoder_new(starting_bit_rate, cbs);
> + }
> +
> + return NULL;
> }
>
> -void dcc_create_stream(DisplayChannelClient *dcc, Stream *stream)
> +gboolean dcc_create_stream(DisplayChannelClient *dcc, Stream *stream)
> {
> StreamAgent *agent = &dcc->stream_agents[get_stream_id(DCC_TO_DC(dcc),
> stream)];
>
> - spice_return_if_fail(region_is_empty(&agent->vis_region));
> + spice_return_val_if_fail(region_is_empty(&agent->vis_region), FALSE);
>
> stream->refs++;
> if (stream->current) {
> @@ -739,9 +751,13 @@ void dcc_create_stream(DisplayChannelClient *dcc, Stream
> *stream)
> video_cbs.update_client_playback_delay =
> update_client_playback_delay;
>
> initial_bit_rate = get_initial_bit_rate(dcc, stream);
> - agent->video_encoder = dcc_create_video_encoder(initial_bit_rate,
> &video_cbs);
> + agent->video_encoder = dcc_create_video_encoder(dcc,
> initial_bit_rate, &video_cbs);
> } else {
> - agent->video_encoder = dcc_create_video_encoder(0, NULL);
> + agent->video_encoder = dcc_create_video_encoder(dcc, 0, NULL);
> + }
> + if (agent->video_encoder == NULL) {
> + stream->refs--;
> + return FALSE;
> }
> red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc),
> &agent->create_item);
Note that here we haven't sent a "create-stream" message yet when we
return early because agent->video_encoder is NULL. Then in the caller
you call stream_stop(display, stream); as part of the cleanup, which
sends a "stop-stream" message. This triggers a runtime warning in the
client. Would be nicer to avoid this :)
Looks good otherwise.
Christophe
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/spice-devel
