[Spice-devel] [RFC spice-streaming-agent 2/4] spice-streaming-agent: fully reset the capture loop on start/stop requests

[Kevin Pouget]

On Sun, Aug 11, 2019 at 4:42 PM Snir Sheriber <ssheribe at redhat.com> wrote:
>
> Hi,
>
>
> On 8/6/19 6:34 PM, Kevin Pouget wrote:
> > With this patch, spice-streaming-agent exits the frame-sending loop
> > when START/STOP requests are received. This allows the recomputation
> > of the most suitable capture/encoding plugin, that may have been
> > updated with START/STOP message.
> >
> > Signed-off-by: Kevin Pouget <kpouget at redhat.com>
> > ---
> >   src/spice-streaming-agent.cpp | 19 +++++++++++++------
> >   1 file changed, 13 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> > index 49f5dc4..d274b5f 100644
> > --- a/src/spice-streaming-agent.cpp
> > +++ b/src/spice-streaming-agent.cpp
> > @@ -125,7 +125,8 @@ private:
> >       static constexpr uint32_t max_device_address_len = 255;
> >   };
> >
> > -static bool streaming_requested = false;
> > +static bool capture_in_progress = false;
> > +static bool reset_requested = false;
> >   static bool quit_requested = false;
> >   static std::set<SpiceVideoCodecType> client_codecs;
> >
> > @@ -167,11 +168,12 @@ static void read_command_from_device(StreamPort &stream_port)
> >       }
> >       case STREAM_TYPE_START_STOP: {
> >           StartStopMessage msg = in_message.get_payload<StartStopMessage>();
> > -        streaming_requested = msg.start_streaming;
> > +        capture_in_progress = msg.start_streaming;
> >           client_codecs = msg.client_codecs;
> > +        reset_requested = true;
> >
> >           syslog(LOG_INFO, "GOT START_STOP message -- request to %s streaming",
> > -               streaming_requested ? "START" : "STOP");
> > +               capture_in_progress ? "START" : "STOP");
> >           return;
> >       }}
> >
> > @@ -233,20 +235,25 @@ do_capture(StreamPort &stream_port, FrameLog &frame_log, ConcreteAgent &agent)
> >   {
> >       unsigned int frame_count = 0;
> >       while (!quit_requested) {
> > -        while (!quit_requested && !streaming_requested) {
> > +        while (!quit_requested && !capture_in_progress) {
> >               read_command(stream_port, true);
> >           }
> >
> >           if (quit_requested) {
> >               return;
> >           }
> > +        reset_requested = false;
> >
> >           syslog(LOG_INFO, "streaming starts now");
> >           uint64_t time_last = 0;
> >
> >           std::unique_ptr<FrameCapture> capture(agent.GetBestFrameCapture(client_codecs));
> >           if (!capture) {
> > -            throw std::runtime_error("cannot find a suitable capture system");
> > +            syslog(LOG_ERR, "Error cannot find a suitable capture system");
> > +
> > +            // wait until a new start/stop request arrives with a new list of codecs
>
>
> Can you explain why you change it to a log message? in case of failure
> how you'll get out of this loop? client codec types availability can change
> in run time? or it can change only their preference?

this if-branch will be executed if no plugin matching the requested
codecs could be found. For instance, the streaming agent is configured
to work only with VP8, and the server wants only H264 encoding.
If the server asks for VP8, then the streaming can restart.
More generally, spice-streaming-agent is a daemon, so it should not
crash when there is no serious issue and the streaming can be easily
restarted.

With the previous plugin selection algorithm, I think that the only to
reach this point was if really no plugin could be instantiated, as the
codec type had less importance

> client codec types availability can change
> in run time? or it can change only their preference?

for a given client, the "supported" list should not change, but the
preferred list may, And the (future) server-side list of codecs
allowed for the guest may change, tha'ts already the case for host
encoding

> > +            capture_in_progress = false;
> > +            continue;
> >           }
> >
> >           std::vector<DeviceDisplayInfo> display_info;
> > @@ -275,7 +282,7 @@ do_capture(StreamPort &stream_port, FrameLog &frame_log, ConcreteAgent &agent)
> >               syslog(LOG_ERR, "Empty device display info from the plugin");
> >           }
> >
> > -        while (!quit_requested && streaming_requested) {
> > +        while (!quit_requested && !reset_requested && capture_in_progress) {
> >               if (++frame_count % 100 == 0) {
> >                   syslog(LOG_DEBUG, "SENT %d frames", frame_count);
> >               }
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel