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

[Jonathon Jongsma]

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.

(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? )



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