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

[Frediano Ziglio]

ping

> 
> > 
> > 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>
> > 
>