[17/17] Move the capture loop in the ConcreteAgent, get rid of global agent variable

Submitted by Christophe de Dinechin on Feb. 16, 2018, 4:15 p.m.

Details

Message ID 20180216161547.28110-18-christophe@dinechin.org
State New
Headers show
Series "WIP: Refactor the streaming agent towards a more standard C++ style" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Christophe de Dinechin Feb. 16, 2018, 4:15 p.m.
From: Christophe de Dinechin <dinechin@redhat.com>

Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
---
 src/concrete-agent.hpp        |  4 ++++
 src/spice-streaming-agent.cpp | 14 ++++++--------
 2 files changed, 10 insertions(+), 8 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/concrete-agent.hpp b/src/concrete-agent.hpp
index 5bca23b..346ba6c 100644
--- a/src/concrete-agent.hpp
+++ b/src/concrete-agent.hpp
@@ -14,6 +14,9 @@ 
 namespace spice {
 namespace streaming_agent {
 
+class Stream;
+class FrameLog;
+
 struct ConcreteConfigureOption: ConfigureOption
 {
     ConcreteConfigureOption(const char *name, const char *value)
@@ -33,6 +36,7 @@  public:
     void Register(Plugin& plugin) override;
     const ConfigureOption* Options() const override;
     void LoadPlugins(const std::string &directory);
+    void CaptureLoop(Stream &stream, FrameLog &frame_log);
     // pointer must remain valid
     void AddOption(const char *name, const char *value);
     FrameCapture *GetBestFrameCapture(const std::set<SpiceVideoCodecType>& codecs);
diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
index 19f3c07..fcb460c 100644
--- a/src/spice-streaming-agent.cpp
+++ b/src/spice-streaming-agent.cpp
@@ -41,8 +41,6 @@ 
 
 using namespace spice::streaming_agent;
 
-static ConcreteAgent agent;
-
 namespace spice
 {
 namespace streaming_agent
@@ -446,8 +444,8 @@  static void usage(const char *progname)
     exit(1);
 }
 
-static void
-do_capture(Stream &stream, const char *streamport, FrameLog &frame_log)
+
+void ConcreteAgent::CaptureLoop(Stream &stream, FrameLog &frame_log)
 {
     unsigned int frame_count = 0;
     while (!quit_requested) {
@@ -465,7 +463,7 @@  do_capture(Stream &stream, const char *streamport, FrameLog &frame_log)
         syslog(LOG_INFO, "streaming starts now\n");
         uint64_t time_last = 0;
 
-        std::unique_ptr<FrameCapture> capture(agent.GetBestFrameCapture(stream.client_codecs()));
+        std::unique_ptr<FrameCapture> capture(GetBestFrameCapture(stream.client_codecs()));
         if (!capture)
             throw std::runtime_error("cannot find a suitable capture system");
 
@@ -535,6 +533,7 @@  int main(int argc, char* argv[])
 
     setlogmask(logmask);
 
+    ConcreteAgent agent;
     while ((opt = getopt_long(argc, argv, "bhp:c:l:d", long_options, NULL)) != -1) {
         switch (opt) {
         case 0:
@@ -569,16 +568,15 @@  int main(int argc, char* argv[])
         }
     }
 
-    agent.LoadPlugins(PLUGINSDIR);
-
     register_interrupts();
 
     int ret = EXIT_SUCCESS;
     try {
+        agent.LoadPlugins(PLUGINSDIR);
         Stream streamfd(streamport);
         X11CursorThread cursor_thread(streamfd);
         FrameLog frame_log(log_filename, log_binary);
-        do_capture(streamfd, streamport, frame_log);
+        agent.CaptureLoop(streamfd, frame_log);
     }
     catch (std::runtime_error &err) {
         syslog(LOG_ERR, "%s\n", err.what());

Comments

On Fri, 2018-02-16 at 17:15 +0100, Christophe de Dinechin wrote:
> From: Christophe de Dinechin <dinechin@redhat.com>
> 
> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
> ---
>  src/concrete-agent.hpp        |  4 ++++
>  src/spice-streaming-agent.cpp | 14 ++++++--------
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/src/concrete-agent.hpp b/src/concrete-agent.hpp
> index 5bca23b..346ba6c 100644
> --- a/src/concrete-agent.hpp
> +++ b/src/concrete-agent.hpp
> @@ -14,6 +14,9 @@
>  namespace spice {
>  namespace streaming_agent {
>  
> +class Stream;
> +class FrameLog;
> +
>  struct ConcreteConfigureOption: ConfigureOption
>  {
>      ConcreteConfigureOption(const char *name, const char *value)
> @@ -33,6 +36,7 @@ public:
>      void Register(Plugin& plugin) override;
>      const ConfigureOption* Options() const override;
>      void LoadPlugins(const std::string &directory);
> +    void CaptureLoop(Stream &stream, FrameLog &frame_log);
>      // pointer must remain valid
>      void AddOption(const char *name, const char *value);
>      FrameCapture *GetBestFrameCapture(const std::set<SpiceVideoCodecType>& codecs);
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> index 19f3c07..fcb460c 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -41,8 +41,6 @@
>  
>  using namespace spice::streaming_agent;
>  
> -static ConcreteAgent agent;
> -
>  namespace spice
>  {
>  namespace streaming_agent
> @@ -446,8 +444,8 @@ static void usage(const char *progname)
>      exit(1);
>  }
>  
> -static void
> -do_capture(Stream &stream, const char *streamport, FrameLog &frame_log)
> +
> +void ConcreteAgent::CaptureLoop(Stream &stream, FrameLog &frame_log)

Should be "capture_loop"

>  {
>      unsigned int frame_count = 0;
>      while (!quit_requested) {
> @@ -465,7 +463,7 @@ do_capture(Stream &stream, const char *streamport, FrameLog &frame_log)
>          syslog(LOG_INFO, "streaming starts now\n");
>          uint64_t time_last = 0;
>  
> -        std::unique_ptr<FrameCapture> capture(agent.GetBestFrameCapture(stream.client_codecs()));
> +        std::unique_ptr<FrameCapture> capture(GetBestFrameCapture(stream.client_codecs()));
>          if (!capture)
>              throw std::runtime_error("cannot find a suitable capture system");
>  
> @@ -535,6 +533,7 @@ int main(int argc, char* argv[])
>  
>      setlogmask(logmask);
>  
> +    ConcreteAgent agent;
>      while ((opt = getopt_long(argc, argv, "bhp:c:l:d", long_options, NULL)) != -1) {
>          switch (opt) {
>          case 0:
> @@ -569,16 +568,15 @@ int main(int argc, char* argv[])
>          }
>      }
>  
> -    agent.LoadPlugins(PLUGINSDIR);
> -
>      register_interrupts();
>  
>      int ret = EXIT_SUCCESS;
>      try {
> +        agent.LoadPlugins(PLUGINSDIR);
>          Stream streamfd(streamport);
>          X11CursorThread cursor_thread(streamfd);
>          FrameLog frame_log(log_filename, log_binary);
> -        do_capture(streamfd, streamport, frame_log);
> +        agent.CaptureLoop(streamfd, frame_log);
>      }
>      catch (std::runtime_error &err) {
>          syslog(LOG_ERR, "%s\n", err.what());
> On 20 Feb 2018, at 16:53, Lukáš Hrázký <lhrazky@redhat.com> wrote:
> 
> On Fri, 2018-02-16 at 17:15 +0100, Christophe de Dinechin wrote:
>> From: Christophe de Dinechin <dinechin@redhat.com>
>> 
>> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
>> ---
>> src/concrete-agent.hpp        |  4 ++++
>> src/spice-streaming-agent.cpp | 14 ++++++--------
>> 2 files changed, 10 insertions(+), 8 deletions(-)
>> 
>> diff --git a/src/concrete-agent.hpp b/src/concrete-agent.hpp
>> index 5bca23b..346ba6c 100644
>> --- a/src/concrete-agent.hpp
>> +++ b/src/concrete-agent.hpp
>> @@ -14,6 +14,9 @@
>> namespace spice {
>> namespace streaming_agent {
>> 
>> +class Stream;
>> +class FrameLog;
>> +
>> struct ConcreteConfigureOption: ConfigureOption
>> {
>>     ConcreteConfigureOption(const char *name, const char *value)
>> @@ -33,6 +36,7 @@ public:
>>     void Register(Plugin& plugin) override;
>>     const ConfigureOption* Options() const override;
>>     void LoadPlugins(const std::string &directory);
>> +    void CaptureLoop(Stream &stream, FrameLog &frame_log);
>>     // pointer must remain valid
>>     void AddOption(const char *name, const char *value);
>>     FrameCapture *GetBestFrameCapture(const std::set<SpiceVideoCodecType>& codecs);
>> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
>> index 19f3c07..fcb460c 100644
>> --- a/src/spice-streaming-agent.cpp
>> +++ b/src/spice-streaming-agent.cpp
>> @@ -41,8 +41,6 @@
>> 
>> using namespace spice::streaming_agent;
>> 
>> -static ConcreteAgent agent;
>> -
>> namespace spice
>> {
>> namespace streaming_agent
>> @@ -446,8 +444,8 @@ static void usage(const char *progname)
>>     exit(1);
>> }
>> 
>> -static void
>> -do_capture(Stream &stream, const char *streamport, FrameLog &frame_log)
>> +
>> +void ConcreteAgent::CaptureLoop(Stream &stream, FrameLog &frame_log)
> 
> Should be “capture_loop”

Yes, but this is one case where local consistency trumped the new style guide :-)

> 

>> {
>>     unsigned int frame_count = 0;
>>     while (!quit_requested) {
>> @@ -465,7 +463,7 @@ do_capture(Stream &stream, const char *streamport, FrameLog &frame_log)
>>         syslog(LOG_INFO, "streaming starts now\n");
>>         uint64_t time_last = 0;
>> 
>> -        std::unique_ptr<FrameCapture> capture(agent.GetBestFrameCapture(stream.client_codecs()));
>> +        std::unique_ptr<FrameCapture> capture(GetBestFrameCapture(stream.client_codecs()));
>>         if (!capture)
>>             throw std::runtime_error("cannot find a suitable capture system");
>> 
>> @@ -535,6 +533,7 @@ int main(int argc, char* argv[])
>> 
>>     setlogmask(logmask);
>> 
>> +    ConcreteAgent agent;
>>     while ((opt = getopt_long(argc, argv, "bhp:c:l:d", long_options, NULL)) != -1) {
>>         switch (opt) {
>>         case 0:
>> @@ -569,16 +568,15 @@ int main(int argc, char* argv[])
>>         }
>>     }
>> 
>> -    agent.LoadPlugins(PLUGINSDIR);
>> -
>>     register_interrupts();
>> 
>>     int ret = EXIT_SUCCESS;
>>     try {
>> +        agent.LoadPlugins(PLUGINSDIR);
>>         Stream streamfd(streamport);
>>         X11CursorThread cursor_thread(streamfd);
>>         FrameLog frame_log(log_filename, log_binary);
>> -        do_capture(streamfd, streamport, frame_log);
>> +        agent.CaptureLoop(streamfd, frame_log);
>>     }
>>     catch (std::runtime_error &err) {
>>         syslog(LOG_ERR, "%s\n", err.what());