[Spice-devel] [PATCH spice-streaming-agent 1/2] Wait to have some client before initialising capture
Frediano Ziglio
fziglio at redhat.com
Mon Jan 29 09:54:01 UTC 2018
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>
> >
>
More information about the Spice-devel
mailing list