[Spice-devel] [spice-server v1 3/4] display-limits: use SPICE_MAX_NUM_STREAMS from protocol

Victor Toso victortoso at redhat.com
Thu Apr 19 10:02:48 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?
 
> 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.

> 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++) {
> >          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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180419/ddd465a6/attachment.sig>


More information about the Spice-devel mailing list