[Spice-devel] [PATCH spice-streaming-agent 1/2] Wait to have some client before initialising capture

[Frediano Ziglio]

> 
> On Tue, 2017-11-21 at 09:49 +0000, Frediano Ziglio wrote:
> > This saves some resources if no client are connected.
> > Also will allow to get a better capture engine taking into account
> > client supported codecs.
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  src/spice-streaming-agent.cpp | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-
> > agent.cpp
> > index 23b9768..20f2925 100644
> > --- a/src/spice-streaming-agent.cpp
> > +++ b/src/spice-streaming-agent.cpp
> > @@ -343,10 +343,6 @@ static void cursor_changes(Display *display, int
> > event_base)
> >  static void
> >  do_capture(const char *streamport, FILE *f_log)
> >  {
> > -    std::unique_ptr<FrameCapture>
> > capture(agent.GetBestFrameCapture());
> > -    if (!capture)
> > -        throw std::runtime_error("cannot find a suitable capture
> > system");
> > -
> >      streamfd = open(streamport, O_RDWR);
> >      if (streamfd < 0)
> >          // TODO was syslog(LOG_ERR, "Failed to open %s: %s\n",
> > streamport, strerror(errno));
> > @@ -361,9 +357,16 @@ do_capture(const char *streamport, FILE *f_log)
> >              }
> >          }
> >  
> > +        if (quit)
> > +            return;
> > +
> 
> 
> This hunk looks unrelated, no? Also, returning directly here bypasses
> the 'done:' section at the end of the function. That doesn't seem
> correct.
> 

It is, previously the capture was build at the beginning so
when quit was set code exited directly, now it won't.

> (by the way, since we're using c++ here, couldn't we do the cleanup in
> some sort of destructor rather than using a goto? )
> 

In the current code there's no much difference, the device is closed
anyway. There's another series (which currently seems stuck) that is
providing the proper destructor.

> 
> 
> >          syslog(LOG_INFO, "streaming starts now\n");
> >          uint64_t time_last = 0;
> >  
> > +        std::unique_ptr<FrameCapture>
> > capture(agent.GetBestFrameCapture());
> > +        if (!capture)
> > +            throw std::runtime_error("cannot find a suitable capture
> > system");
> > +
> >          while (!quit && streaming_requested) {
> >              if (++frame_count % 100 == 0) {
> >                  syslog(LOG_DEBUG, "SENT %d frames\n", frame_count);
> > @@ -410,9 +413,6 @@ do_capture(const char *streamport, FILE *f_log)
> >                  syslog(LOG_ERR, "FAILED to read command\n");
> >                  goto done;
> >              }
> > -            if (!streaming_requested) {
> > -                capture->Reset();
> > -            }
> >          }
> >      }
> >  
> 
> 
> Otherwise seems fine to me
> 
> Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>
> 

Frediano