[Spice-devel] [PATCH spice-server] Do not compute stream_id more that necessary

[Frediano Ziglio]

> 
> Hi,
> 
> On Thu, Dec 07, 2017 at 12:26:43PM +0000, Frediano Ziglio wrote:
> > Reuse already computed value and avoid to compute in every
> > iterations.
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/dcc-send.c     | 4 ++--
> >  server/video-stream.c | 2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/server/dcc-send.c b/server/dcc-send.c
> > index 975b222a..828a472f 100644
> > --- a/server/dcc-send.c
> > +++ b/server/dcc-send.c
> > @@ -1743,7 +1743,7 @@ static bool red_marshall_stream_data(RedChannelClient
> > *rcc,
> >  
> >          red_channel_client_init_send_data(rcc,
> >          SPICE_MSG_DISPLAY_STREAM_DATA);
> >  
> > -        stream_data.base.id = display_channel_get_video_stream_id(display,
> > stream);
> > +        stream_data.base.id = stream_id;
> >          stream_data.base.multi_media_time = frame_mm_time;
> >          stream_data.data_size = outbuf->size;
> >  
> > @@ -1753,7 +1753,7 @@ static bool red_marshall_stream_data(RedChannelClient
> > *rcc,
> >  
> >          red_channel_client_init_send_data(rcc,
> >          SPICE_MSG_DISPLAY_STREAM_DATA_SIZED);
> >  
> > -        stream_data.base.id = display_channel_get_video_stream_id(display,
> > stream);
> > +        stream_data.base.id = stream_id;
> >          stream_data.base.multi_media_time = frame_mm_time;
> >          stream_data.data_size = outbuf->size;
> >          stream_data.width = copy->src_area.right - copy->src_area.left;
> > diff --git a/server/video-stream.c b/server/video-stream.c
> > index 619e5ed3..b45c318b 100644
> > --- a/server/video-stream.c
> > +++ b/server/video-stream.c
> > @@ -298,10 +298,10 @@ static void attach_stream(DisplayChannel *display,
> > Drawable *drawable, VideoStre
> >          stream->num_input_frames++;
> >      }
> >  
> > +    int stream_id = display_channel_get_video_stream_id(display, stream);
> 
> Sorry if I missed it but declarations are not in the top of block
> anymore?
> 

There's nothing about in out style document. The all at the beginning style
is C89 and former, from configure.ac

    AC_MSG_ERROR([C99 compiler is required.])

There are pro and cons about this style.
Pro:
- allows smaller diffs;
- easier to match type with initialization;
- a bit easier to factor out functions;
- avoid some minor user before initialization issue.
Cons:
- you can't jump (goto) all over the places. I don't see much a cons
  personally, I don't like code that jumps all other the places;
- can make some lines longer.

Personally I prefer this style. There are multiple occurrences in the
current code.

> Anyway,
> Acked-by: Victor Toso <victortoso at redhat.com>
> 
> >      FOREACH_DCC(display, dcc) {
> >          VideoStreamAgent *agent;
> >          QRegion clip_in_draw_dest;
> > -        int stream_id = display_channel_get_video_stream_id(display,
> > stream);
> >  
> >          agent = dcc_get_video_stream_agent(dcc, stream_id);
> >          region_or(&agent->vis_region, &drawable->tree_item.base.rgn);

Frediano