[Spice-devel] [spice-server v1 3/4] display-limits: use SPICE_MAX_NUM_STREAMS from protocol
Frediano Ziglio
fziglio at redhat.com
Thu Apr 19 12:35:26 UTC 2018
>
> On Thu, Apr 19, 2018 at 03:26:05AM -0400, Frediano Ziglio wrote:
> > >
> > > From: Victor Toso <me at victortoso.com>
> > >
> > > Using the limit SPICE_MAX_NUM_STREAMS introduced in spice-protocol
> > > 0.12.14 to fix the amount of streams we might be working currently.
> > >
> > > This change removes the NUM_STREAMS define in display-limits.h
> > >
> > > This is an ABI break patch as the size of some arrays will change such
> > > as:
> > > - stream_agents from DisplayChannelClientPrivate in dcc-private.h
> > > - streams_buf from DisplayChannelPrivate in display-channel-private.h
> > >
> > > Signed-off-by: Victor Toso <victortoso at redhat.com>
> >
> > I would prefer to keep the actual limit and check with a verify
> > that is below the protocol one.
>
> But why keeping two limits? One by protocol and one by server?
>
Because for the server that limit is enough. We never hit the actual limit.
> > The fact that previously the limit was 4 billions didn't means
> > that the arrays contained 4 billions elements.
>
> But a server could send a MAX_UINT as id, etc.
>
And can continue to do, that's why a limit is defined in the
protocol and checked by client. Let space for extension without
having to change the messages structures (well, usually you want
to define the limits before coding but was a mistake not defining
them).
> > About ABI is not an issue changing the internal ABI of a
> > library, basically most of the commits change it.
>
> Ok, I'll remove from commit log
>
> > > ---
> > > server/dcc-private.h | 2 +-
> > > server/dcc.c | 6 +++---
> > > server/display-channel-private.h | 2 +-
> > > server/display-channel.c | 2 +-
> > > server/display-limits.h | 3 ---
> > > server/stream-channel.c | 2 +-
> > > server/video-stream.c | 4 ++--
> > > 7 files changed, 9 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/server/dcc-private.h b/server/dcc-private.h
> > > index 848d4270..75eacc9c 100644
> > > --- a/server/dcc-private.h
> > > +++ b/server/dcc-private.h
> > > @@ -61,7 +61,7 @@ struct DisplayChannelClientPrivate
> > > uint8_t surface_client_created[NUM_SURFACES];
> > > QRegion surface_client_lossy_region[NUM_SURFACES];
> > >
> > > - VideoStreamAgent stream_agents[NUM_STREAMS];
> > > + VideoStreamAgent stream_agents[SPICE_MAX_NUM_STREAMS];
> > > uint32_t streams_max_latency;
> > > uint64_t streams_max_bit_rate;
> > > bool gl_draw_ongoing;
> > > diff --git a/server/dcc.c b/server/dcc.c
> > > index 15b65978..cfe0aae3 100644
> > > --- a/server/dcc.c
> > > +++ b/server/dcc.c
> > > @@ -486,7 +486,7 @@ static void
> > > dcc_init_stream_agents(DisplayChannelClient
> > > *dcc)
> > > int i;
> > > DisplayChannel *display = DCC_TO_DC(dcc);
> > >
> > > - for (i = 0; i < NUM_STREAMS; i++) {
> > > + for (i = 0; i < SPICE_MAX_NUM_STREAMS; i++) {
Maybe some could be replaced by G_N_ELEMENTS?
> > > VideoStreamAgent *agent = &dcc->priv->stream_agents[i];
> > > agent->stream = display_channel_get_nth_video_stream(display,
> > > i);
> > > region_init(&agent->vis_region);
> > > @@ -600,7 +600,7 @@ static void
> > > dcc_destroy_stream_agents(DisplayChannelClient *dcc)
> > > {
> > > int i;
> > >
> > > - for (i = 0; i < NUM_STREAMS; i++) {
> > > + for (i = 0; i < SPICE_MAX_NUM_STREAMS; i++) {
> > > VideoStreamAgent *agent = &dcc->priv->stream_agents[i];
> > > region_destroy(&agent->vis_region);
> > > region_destroy(&agent->clip);
> > > @@ -1032,7 +1032,7 @@ static bool
> > > dcc_handle_stream_report(DisplayChannelClient *dcc,
> > > {
> > > VideoStreamAgent *agent;
> > >
> > > - if (report->stream_id >= NUM_STREAMS) {
> > > + if (report->stream_id >= SPICE_MAX_NUM_STREAMS) {
> > > spice_warning("stream_report: invalid stream id %u",
> > > report->stream_id);
> > > return FALSE;
> > > diff --git a/server/display-channel-private.h
> > > b/server/display-channel-private.h
> > > index 617ce30d..3dd3e233 100644
> > > --- a/server/display-channel-private.h
> > > +++ b/server/display-channel-private.h
> > > @@ -98,7 +98,7 @@ struct DisplayChannelPrivate
> > > int stream_video;
> > > GArray *video_codecs;
> > > uint32_t stream_count;
> > > - VideoStream streams_buf[NUM_STREAMS];
> > > + VideoStream streams_buf[SPICE_MAX_NUM_STREAMS];
> > > VideoStream *free_streams;
> > > Ring streams;
> > > ItemTrace items_trace[NUM_TRACE_ITEMS];
> > > diff --git a/server/display-channel.c b/server/display-channel.c
> > > index 229f2c0f..39158154 100644
> > > --- a/server/display-channel.c
> > > +++ b/server/display-channel.c
> > > @@ -104,7 +104,7 @@ display_channel_finalize(GObject *object)
> > > for (stream = self->priv->free_streams; stream; stream =
> > > stream->next) {
> > > ++count;
> > > }
> > > - spice_assert(count == NUM_STREAMS);
> > > + spice_assert(count == SPICE_MAX_NUM_STREAMS);
> > > spice_assert(ring_is_empty(&self->priv->streams));
> > >
> > > for (count = 0; count < NUM_SURFACES; ++count) {
> > > diff --git a/server/display-limits.h b/server/display-limits.h
> > > index e875149b..e3adf691 100644
> > > --- a/server/display-limits.h
> > > +++ b/server/display-limits.h
> > > @@ -22,7 +22,4 @@
> > > /** Maximum number of surfaces a guest can create */
> > > #define NUM_SURFACES 1024
> > >
> > > -/** Maximum number of streams created by spice-server */
> > > -#define NUM_STREAMS 50
> > > -
> > > #endif /* DISPLAY_LIMITS_H_ */
> > > diff --git a/server/stream-channel.c b/server/stream-channel.c
> > > index 88f859f6..6c27d844 100644
> > > --- a/server/stream-channel.c
> > > +++ b/server/stream-channel.c
> > > @@ -467,7 +467,7 @@ stream_channel_change_format(StreamChannel *channel,
> > > const StreamMsgFormat *fmt)
> > > }
> > >
> > > // allocate a new stream id
> > > - channel->stream_id = (channel->stream_id + 1) % NUM_STREAMS;
> > > + channel->stream_id = (channel->stream_id + 1) %
> > > SPICE_MAX_NUM_STREAMS;
> > >
> > > // send create stream
> > > StreamCreateItem *item = g_new0(StreamCreateItem, 1);
> > > diff --git a/server/video-stream.c b/server/video-stream.c
> > > index a4b83b4f..368673a7 100644
> > > --- a/server/video-stream.c
> > > +++ b/server/video-stream.c
> > > @@ -140,7 +140,7 @@ void
> > > display_channel_init_video_streams(DisplayChannel
> > > *display)
> > >
> > > ring_init(&display->priv->streams);
> > > display->priv->free_streams = NULL;
> > > - for (i = 0; i < NUM_STREAMS; i++) {
> > > + for (i = 0; i < SPICE_MAX_NUM_STREAMS; i++) {
> > > VideoStream *stream =
> > > display_channel_get_nth_video_stream(display,
> > > i);
> > > ring_item_init(&stream->link);
> > > video_stream_free(display, stream);
> > > @@ -570,7 +570,7 @@ static void
> > > dcc_update_streams_max_latency(DisplayChannelClient *dcc,
> > > if (DCC_TO_DC(dcc)->priv->stream_count == 1) {
> > > return;
> > > }
> > > - for (i = 0; i < NUM_STREAMS; i++) {
> > > + for (i = 0; i < SPICE_MAX_NUM_STREAMS; i++) {
> > > VideoStreamAgent *other_agent = dcc_get_video_stream_agent(dcc,
> > > i);
> > > if (other_agent == remove_agent || !other_agent->video_encoder)
> > > {
> > > continue;
> >
> > Frediano
>
More information about the Spice-devel
mailing list