> 
> Create an AgentRunner (TODO: needs a better name) class to encapsulate
> the streaming and communication with the server. The basic setup (cmd

Also encapsulate command line interface (quite odd, does it make still
much sense?)

> arg parsing, signal handling, ...) is moved to main.cpp. The rest of the
> functions is moved to the AgentRunner class and modified as little as
> possible:
> - The cursor updating code is moved into a functor called CursorThread
> - Some initialization and cleanup is moved to AgentRunner's constructor
>   and destructor
> - Some error handling moved over to exceptions, mainly what was in
>   main() and do_capture()
> - A couple of variables gently renamed.
> 
> Signed-off-by: Lukáš Hrázký <lhra...@redhat.com>
> ---
> Changes since v1:
> - update according to Frediano's namespace changes
> 
>  src/Makefile.am               |   2 +
>  src/main.cpp                  | 126 ++++++++++++
>  src/spice-streaming-agent.cpp | 458
>  +++++++++++++++++-------------------------
>  src/spice-streaming-agent.hpp |  57 ++++++
>  4 files changed, 372 insertions(+), 271 deletions(-)
>  create mode 100644 src/main.cpp
>  create mode 100644 src/spice-streaming-agent.hpp
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 6d37274..42bb10f 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -49,6 +49,7 @@ spice_streaming_agent_LDADD = \
>  
>  spice_streaming_agent_SOURCES = \
>       spice-streaming-agent.cpp \
> +     spice-streaming-agent.hpp \
>       static-plugin.cpp \
>       static-plugin.hpp \
>       concrete-agent.cpp \
> @@ -57,4 +58,5 @@ spice_streaming_agent_SOURCES = \
>       mjpeg-fallback.hpp \
>       jpeg.cpp \
>       jpeg.hpp \
> +     main.cpp \
>       $(NULL)
> diff --git a/src/main.cpp b/src/main.cpp
> new file mode 100644
> index 0000000..46eda85
> --- /dev/null
> +++ b/src/main.cpp
> @@ -0,0 +1,126 @@
> +/* An implementation of a SPICE streaming agent
> + *
> + * \copyright
> + * Copyright 2016-2018 Red Hat Inc. All rights reserved.
> + */
> +
> +#include <config.h>
> +#include "spice-streaming-agent.hpp"
> +
> +#include <string.h>
> +#include <getopt.h>
> +#include <unistd.h>
> +#include <syslog.h>
> +#include <signal.h>
> +
> +
> +namespace ssa = spice::streaming_agent;
> +
> +static void usage(const char *progname)
> +{
> +    printf("usage: %s <options>\n", progname);
> +    printf("options are:\n");
> +    printf("\t-p portname  -- virtio-serial port to use\n");
> +    printf("\t-i accept commands from stdin\n");
> +    printf("\t-l file -- log frames to file\n");
> +    printf("\t--log-binary -- log binary frames (following -l)\n");
> +    printf("\t-d -- enable debug logs\n");
> +    printf("\t-c variable=value -- change settings\n");
> +    printf("\t\tframerate = 1-100 (check 10,20,30,40,50,60)\n");
> +    printf("\n");
> +    printf("\t-h or --help     -- print this help message\n");
> +
> +    exit(1);
> +}
> +
> +void handle_interrupt(int intr)
> +{
> +    syslog(LOG_INFO, "Got signal %d, exiting", intr);
> +    ssa::AgentRunner::quit = true;
> +}
> +
> +void register_interrupts(void)
> +{
> +    struct sigaction sa;
> +
> +    memset(&sa, 0, sizeof(sa));
> +    sa.sa_handler = handle_interrupt;
> +    if ((sigaction(SIGINT, &sa, NULL) != 0) &&
> +        (sigaction(SIGTERM, &sa, NULL) != 0)) {
> +        syslog(LOG_WARNING, "failed to register signal handler %m");
> +    }
> +}
> +
> +int main(int argc, char* argv[])
> +{
> +    std::string stream_port = "/dev/virtio-ports/com.redhat.stream.0";
> +    char opt;
> +    std::string log_filename;
> +    int log_binary = 0;
> +    bool stdin_ok = false;
> +    int logmask = LOG_UPTO(LOG_WARNING);
> +    struct option long_options[] = {
> +        { "log-binary", no_argument, &log_binary, 1},
> +        { "help", no_argument, NULL, 'h'},
> +        { 0, 0, 0, 0}
> +    };
> +
> +    if (isatty(fileno(stderr)) && isatty(fileno(stdin))) {
> +        stdin_ok = true;
> +    }
> +
> +    openlog("spice-streaming-agent", stdin_ok? (LOG_PERROR|LOG_PID) :
> LOG_PID, LOG_USER);
> +    setlogmask(logmask);
> +
> +    std::vector<std::pair<std::string, std::string>> options;
> +
> +    while ((opt = getopt_long(argc, argv, "hip:c:l:d", long_options, NULL))
> != -1) {
> +        switch (opt) {
> +        case 0:
> +            /* Handle long options if needed */
> +            break;
> +        case 'i':
> +            stdin_ok = true;
> +            openlog("spice-streaming-agent", LOG_PERROR|LOG_PID, LOG_USER);
> +            break;
> +        case 'p':
> +            stream_port = optarg;
> +            break;
> +        case 'c': {
> +            char *p = strchr(optarg, '=');
> +            if (p == NULL) {
> +                syslog(LOG_ERR, "wrong 'c' argument %s\n", optarg);
> +                usage(argv[0]);
> +            }
> +            *p++ = '\0';
> +            options.push_back(std::make_pair(optarg, p));
> +            break;
> +        }
> +        case 'l':
> +            log_filename = optarg;
> +            break;
> +        case 'd':
> +            logmask = LOG_UPTO(LOG_DEBUG);
> +            setlogmask(logmask);
> +            break;
> +        case 'h':
> +            usage(argv[0]);
> +            break;
> +        }
> +    }
> +
> +    register_interrupts();
> +
> +    try {
> +        ssa::AgentRunner runner(stream_port, log_filename, log_binary != 0,
> stdin_ok);
> +        // TODO fix overcomplicated passing of options to the agent
> +        runner.add_options(options);
> +        runner.run();
> +    } catch (const std::exception &e) {
> +        syslog(LOG_ERR, "Error: %s\n", e.what());
> +        return EXIT_FAILURE;
> +    }
> +
> +    closelog();
> +    return EXIT_SUCCESS;
> +}
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> index 0e7641e..f062ebe 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -1,44 +1,37 @@
>  /* An implementation of a SPICE streaming agent
>   *
>   * \copyright
> - * Copyright 2016-2017 Red Hat Inc. All rights reserved.
> + * Copyright 2016-2018 Red Hat Inc. All rights reserved.
>   */
>  
> -#include <stdio.h>
> -#include <stdlib.h>
> -#include <stdint.h>
> +#include <config.h>
> +#include "spice-streaming-agent.hpp"
> +
> +#include "concrete-agent.hpp"
> +#include "hexdump.h"
> +
> +#include <spice-streaming-agent/frame-capture.hpp>
> +#include <spice-streaming-agent/plugin.hpp>
> +#include <spice/stream-device.h>
> +#include <spice/enums.h>
> +#include <X11/Xlib.h>
> +#include <X11/extensions/Xfixes.h>
>  #include <string.h>
> -#include <getopt.h>
>  #include <unistd.h>
> -#include <errno.h>
>  #include <fcntl.h>
>  #include <sys/time.h>
>  #include <poll.h>
>  #include <syslog.h>
> -#include <signal.h>
> -#include <exception>
> -#include <stdexcept>
> -#include <memory>
> -#include <mutex>
>  #include <thread>
> -#include <vector>
>  #include <functional>
> -#include <X11/Xlib.h>
> -#include <X11/extensions/Xfixes.h>
> -
> -#include <spice/stream-device.h>
> -#include <spice/enums.h>
>  
> -#include <spice-streaming-agent/frame-capture.hpp>
> -#include <spice-streaming-agent/plugin.hpp>
>  
> -#include "hexdump.h"
> -#include "concrete-agent.hpp"
> +namespace spice {
> +namespace streaming_agent {
>  
> -using namespace std;
> -using namespace spice::streaming_agent;
> +bool AgentRunner::quit = false;
>  
> -static ConcreteAgent agent;
> +namespace {
>  
>  struct SpiceStreamFormatMessage
>  {
> @@ -52,15 +45,155 @@ struct SpiceStreamDataMessage
>      StreamMsgData msg;
>  };
>  
> -static int streaming_requested;
> -static std::set<SpiceVideoCodecType> client_codecs;
> -static bool quit;
> -static int streamfd = -1;
> -static bool stdin_ok;
> -static int log_binary = 0;
> -static std::mutex stream_mtx;
> +size_t write_all(int fd, const void *buf, const size_t len)
> +{
> +    size_t written = 0;
> +    while (written < len) {
> +        int l = write(fd, (const char *) buf + written, len - written);
> +        if (l < 0 && errno == EINTR) {
> +            continue;
> +        }
> +        if (l < 0) {
> +            syslog(LOG_ERR, "write failed - %m");
> +            return l;
> +        }
> +        written += l;
> +    }
> +    syslog(LOG_DEBUG, "write_all -- %u bytes written\n", (unsigned)written);
> +    return written;
> +}
> +
> +class CursorThread {
> +public:
> +    CursorThread(int streamfd, std::mutex &stream_mtx) :
> +        streamfd(streamfd),
> +        stream_mtx(stream_mtx)
> +    {
> +        display = XOpenDisplay(NULL);
> +        if (display == NULL) {
> +            throw std::runtime_error("Failed to open display.");
> +        }
> +        int error_base;
> +        if (!XFixesQueryExtension(display, &event_base, &error_base)) {
> +            throw std::runtime_error("XFixesQueryExtension failed");
> +        }
> +        Window rootwindow = DefaultRootWindow(display);
> +        XFixesSelectCursorInput(display, rootwindow,
> XFixesDisplayCursorNotifyMask);
> +    }
> +

I personally prefer long function to be defined outside the class
so is easier to see the interface. Also this hint the compiler
to inline long functions.

> +    void operator()()
> +    {
> +        unsigned long last_serial = 0;
> +
> +        while (1) {
> +            syslog(LOG_ERR, "CURSOR LOOP");
> +            XEvent event;
> +            XNextEvent(display, &event);
> +            if (event.type != event_base + 1)
> +                continue;
> +
> +            XFixesCursorImage *cursor = XFixesGetCursorImage(display);
> +            if (!cursor)
> +                continue;
> +
> +            if (cursor->cursor_serial == last_serial)
> +                continue;
> +
> +            last_serial = cursor->cursor_serial;
> +            auto fill_cursor = [&](uint32_t *pixels) {
> +                for (unsigned i = 0; i < cursor->width * cursor->height;
> ++i)
> +                    pixels[i] = cursor->pixels[i];
> +            };
> +            send_cursor(cursor->width, cursor->height, cursor->xhot,
> cursor->yhot,
> +                        fill_cursor, streamfd, stream_mtx);
> +        }
> +    }
> +
> +private:
> +    void send_cursor(unsigned width, unsigned height, int hotspot_x, int
> hotspot_y,
> +                     std::function<void(uint32_t *)> fill_cursor,
> +                     int streamfd, std::mutex &stream_mtx)
> +    {
> +        if (width >= STREAM_MSG_CURSOR_SET_MAX_WIDTH ||
> +            height >= STREAM_MSG_CURSOR_SET_MAX_HEIGHT)
> +            return;
> +
> +        size_t cursor_size =
> +            sizeof(StreamDevHeader) + sizeof(StreamMsgCursorSet) +
> +            width * height * sizeof(uint32_t);
> +        std::unique_ptr<uint8_t[]> msg(new uint8_t[cursor_size]);
> +
> +        StreamDevHeader
> &dev_hdr(*reinterpret_cast<StreamDevHeader*>(msg.get()));
> +        memset(&dev_hdr, 0, sizeof(dev_hdr));
> +        dev_hdr.protocol_version = STREAM_DEVICE_PROTOCOL;
> +        dev_hdr.type = STREAM_TYPE_CURSOR_SET;
> +        dev_hdr.size = cursor_size - sizeof(StreamDevHeader);
> +
> +        StreamMsgCursorSet &cursor_msg(
> +            *reinterpret_cast<StreamMsgCursorSet *>(msg.get() +
> sizeof(StreamDevHeader)));
> +
> +        memset(&cursor_msg, 0, sizeof(cursor_msg));
> +
> +        cursor_msg.type = SPICE_CURSOR_TYPE_ALPHA;
> +        cursor_msg.width = width;
> +        cursor_msg.height = height;
> +        cursor_msg.hot_spot_x = hotspot_x;
> +        cursor_msg.hot_spot_y = hotspot_y;
> +
> +        uint32_t *pixels = reinterpret_cast<uint32_t *>(cursor_msg.data);
> +        fill_cursor(pixels);
> +
> +        std::lock_guard<std::mutex> stream_guard(stream_mtx);
> +        write_all(streamfd, msg.get(), cursor_size);
> +    }
> +
> +    Display* display;
> +    int event_base = 0;
> +    int streamfd = -1;
> +    std::mutex &stream_mtx;
> +};
> +
> +} // namespace
>  
> -static int have_something_to_read(int *pfd, int timeout)
> +AgentRunner::AgentRunner(const std::string &stream_port,
> +                         const std::string &log_filename,
> +                         bool log_binary,
> +                         bool stdin_ok)
> +:
> +    stream_port(stream_port),
> +    log_binary(log_binary),
> +    stdin_ok(stdin_ok)
> +{
> +    agent.LoadPlugins(PLUGINSDIR);
> +
> +    // TODO proper cleanup on exceptions thrown here - wrap in classes?
> +    streamfd = open(stream_port.c_str(), O_RDWR);
> +    if (streamfd < 0)
> +        throw std::runtime_error("failed to open the streaming device (" +
> +                                 stream_port + "): " + strerror(errno));
> +
> +    if (!log_filename.empty()) {
> +        log_file = fopen(log_filename.c_str(), "wb");
> +        if (!log_file) {
> +            throw std::runtime_error("Failed to open log file '" +
> log_filename +
> +                                     "': " + strerror(errno) + "'");
> +        }
> +    }
> +}
> +
> +AgentRunner::~AgentRunner() {

minor: bracket on next line

> +    if (streamfd >= 0) {
> +        close(streamfd);
> +        streamfd = -1;
> +    }
> +
> +    if (log_file) {
> +        fclose(log_file);
> +        log_file = nullptr;
> +    }
> +}
> +
> +int AgentRunner::have_something_to_read(int *pfd, int timeout)
>  {
>      int nfds;
>      struct pollfd pollfds[2] = {
> @@ -82,7 +215,7 @@ static int have_something_to_read(int *pfd, int timeout)
>      return *pfd != -1;
>  }
>  
> -static int read_command_from_stdin(void)
> +int AgentRunner::read_command_from_stdin()
>  {
>      char buffer[64], *p, *save = NULL;
>  
> @@ -106,7 +239,7 @@ static int read_command_from_stdin(void)
>      return 1;
>  }
>  
> -static int read_command_from_device(void)
> +int AgentRunner::read_command_from_device()
>  {
>      StreamDevHeader hdr;
>      uint8_t msg[64];
> @@ -151,7 +284,7 @@ static int read_command_from_device(void)
>      return 1;
>  }
>  
> -static int read_command(bool blocking)
> +int AgentRunner::read_command(bool blocking)
>  {
>      int fd, n=1;
>      int timeout = blocking?-1:0;
> @@ -173,28 +306,8 @@ static int read_command(bool blocking)
>      return n;
>  }
>  
> -static size_t
> -write_all(int fd, const void *buf, const size_t len)
> -{
> -    size_t written = 0;
> -    while (written < len) {
> -        int l = write(fd, (const char *) buf + written, len - written);
> -        if (l < 0 && errno == EINTR) {
> -            continue;
> -        }
> -        if (l < 0) {
> -            syslog(LOG_ERR, "write failed - %m");
> -            return l;
> -        }
> -        written += l;
> -    }
> -    syslog(LOG_DEBUG, "write_all -- %u bytes written\n", (unsigned)written);
> -    return written;
> -}
> -
> -static int spice_stream_send_format(unsigned w, unsigned h, unsigned c)
> +int AgentRunner::spice_stream_send_format(unsigned w, unsigned h, unsigned
> c)
>  {
> -
>      SpiceStreamFormatMessage msg;
>      const size_t msgsize = sizeof(msg);
>      const size_t hdrsize  = sizeof(msg.hdr);
> @@ -213,7 +326,7 @@ static int spice_stream_send_format(unsigned w, unsigned
> h, unsigned c)
>      return EXIT_SUCCESS;
>  }
>  
> -static int spice_stream_send_frame(const void *buf, const unsigned size)
> +int AgentRunner::spice_stream_send_frame(const void *buf, const unsigned
> size)
>  {
>      SpiceStreamDataMessage msg;
>      const size_t msgsize = sizeof(msg);
> @@ -244,7 +357,7 @@ static int spice_stream_send_frame(const void *buf, const
> unsigned size)
>  }
>  
>  /* returns current time in micro-seconds */
> -static uint64_t get_time(void)
> +uint64_t AgentRunner::get_time(void)
>  {
>      struct timeval now;
>  
> @@ -254,116 +367,13 @@ static uint64_t get_time(void)
>  
>  }
>  
> -static void handle_interrupt(int intr)
> -{
> -    syslog(LOG_INFO, "Got signal %d, exiting", intr);
> -    quit = true;
> -}
> -
> -static void register_interrupts(void)
> -{
> -    struct sigaction sa;
> -
> -    memset(&sa, 0, sizeof(sa));
> -    sa.sa_handler = handle_interrupt;
> -    if ((sigaction(SIGINT, &sa, NULL) != 0) &&
> -        (sigaction(SIGTERM, &sa, NULL) != 0)) {
> -        syslog(LOG_WARNING, "failed to register signal handler %m");
> -    }
> -}
> -
> -static void usage(const char *progname)
> -{
> -    printf("usage: %s <options>\n", progname);
> -    printf("options are:\n");
> -    printf("\t-p portname  -- virtio-serial port to use\n");
> -    printf("\t-i accept commands from stdin\n");
> -    printf("\t-l file -- log frames to file\n");
> -    printf("\t--log-binary -- log binary frames (following -l)\n");
> -    printf("\t-d -- enable debug logs\n");
> -    printf("\t-c variable=value -- change settings\n");
> -    printf("\t\tframerate = 1-100 (check 10,20,30,40,50,60)\n");
> -    printf("\n");
> -    printf("\t-h or --help     -- print this help message\n");
> -
> -    exit(1);
> -}
> -
> -static void
> -send_cursor(unsigned width, unsigned height, int hotspot_x, int hotspot_y,
> -            std::function<void(uint32_t *)> fill_cursor)
> -{
> -    if (width >= STREAM_MSG_CURSOR_SET_MAX_WIDTH ||
> -        height >= STREAM_MSG_CURSOR_SET_MAX_HEIGHT)
> -        return;
> -
> -    size_t cursor_size =
> -        sizeof(StreamDevHeader) + sizeof(StreamMsgCursorSet) +
> -        width * height * sizeof(uint32_t);
> -    std::unique_ptr<uint8_t[]> msg(new uint8_t[cursor_size]);
> -
> -    StreamDevHeader
> &dev_hdr(*reinterpret_cast<StreamDevHeader*>(msg.get()));
> -    memset(&dev_hdr, 0, sizeof(dev_hdr));
> -    dev_hdr.protocol_version = STREAM_DEVICE_PROTOCOL;
> -    dev_hdr.type = STREAM_TYPE_CURSOR_SET;
> -    dev_hdr.size = cursor_size - sizeof(StreamDevHeader);
> -
> -    StreamMsgCursorSet &cursor_msg(*reinterpret_cast<StreamMsgCursorSet
> *>(msg.get() + sizeof(StreamDevHeader)));
> -    memset(&cursor_msg, 0, sizeof(cursor_msg));
> -
> -    cursor_msg.type = SPICE_CURSOR_TYPE_ALPHA;
> -    cursor_msg.width = width;
> -    cursor_msg.height = height;
> -    cursor_msg.hot_spot_x = hotspot_x;
> -    cursor_msg.hot_spot_y = hotspot_y;
> -
> -    uint32_t *pixels = reinterpret_cast<uint32_t *>(cursor_msg.data);
> -    fill_cursor(pixels);
> -
> -    std::lock_guard<std::mutex> stream_guard(stream_mtx);
> -    write_all(streamfd, msg.get(), cursor_size);
> -}
> -
> -static void cursor_changes(Display *display, int event_base)
> -{
> -    unsigned long last_serial = 0;
> -
> -    while (1) {
> -        XEvent event;
> -        XNextEvent(display, &event);
> -        if (event.type != event_base + 1)
> -            continue;
> -
> -        XFixesCursorImage *cursor = XFixesGetCursorImage(display);
> -        if (!cursor)
> -            continue;
> -
> -        if (cursor->cursor_serial == last_serial)
> -            continue;
> -
> -        last_serial = cursor->cursor_serial;
> -        auto fill_cursor = [&](uint32_t *pixels) {
> -            for (unsigned i = 0; i < cursor->width * cursor->height; ++i)
> -                pixels[i] = cursor->pixels[i];
> -        };
> -        send_cursor(cursor->width, cursor->height, cursor->xhot,
> cursor->yhot, fill_cursor);
> -    }
> -}
> -
> -static void
> -do_capture(const string &streamport, FILE *f_log)
> +void AgentRunner::do_capture()
>  {
> -    streamfd = open(streamport.c_str(), O_RDWR);
> -    if (streamfd < 0)
> -        throw std::runtime_error("failed to open the streaming device (" +
> -                                 streamport + "): " + strerror(errno));
> -
>      unsigned int frame_count = 0;
>      while (! quit) {
>          while (!quit && !streaming_requested) {
>              if (read_command(true) < 0) {
> -                syslog(LOG_ERR, "FAILED to read command\n");
> -                goto done;
> +                throw std::runtime_error("FAILED to read command");
>              }
>          }
>  
> @@ -406,12 +416,12 @@ do_capture(const string &streamport, FILE *f_log)
>                  if (spice_stream_send_format(width, height, codec) ==
>                  EXIT_FAILURE)
>                      throw std::runtime_error("FAILED to send format
>                      message");
>              }
> -            if (f_log) {
> +            if (log_file) {
>                  if (log_binary) {
> -                    fwrite(frame.buffer, frame.buffer_size, 1, f_log);
> +                    fwrite(frame.buffer, frame.buffer_size, 1, log_file);
>                  } else {
> -                    fprintf(f_log, "%lu: Frame of %zu bytes:\n", get_time(),
> frame.buffer_size);
> -                    hexdump(frame.buffer, frame.buffer_size, f_log);
> +                    fprintf(log_file, "%lu: Frame of %zu bytes:\n",
> get_time(), frame.buffer_size);
> +                    hexdump(frame.buffer, frame.buffer_size, log_file);
>                  }
>              }
>              if (spice_stream_send_frame(frame.buffer, frame.buffer_size) ==
>              EXIT_FAILURE) {
> @@ -420,118 +430,24 @@ do_capture(const string &streamport, FILE *f_log)
>              }
>              //usleep(1);
>              if (read_command(false) < 0) {
> -                syslog(LOG_ERR, "FAILED to read command\n");
> -                goto done;
> +                throw std::runtime_error("FAILED to read command");
>              }
>          }
>      }
> +}
>  
> -done:
> -    if (streamfd >= 0) {
> -        close(streamfd);
> -        streamfd = -1;
> +void AgentRunner::add_options(const std::vector<std::pair<std::string,
> std::string>> &options) {
> +    for (const auto& option : options) {
> +        agent.AddOption(option.first.c_str(), option.second.c_str());
>      }
>  }
>  
> -#define arg_error(...) syslog(LOG_ERR, ## __VA_ARGS__);
> -
> -int main(int argc, char* argv[])
> +void AgentRunner::run()
>  {
> -    string streamport = "/dev/virtio-ports/com.redhat.stream.0";
> -    char opt;
> -    const char *log_filename = NULL;
> -    int logmask = LOG_UPTO(LOG_WARNING);
> -    struct option long_options[] = {
> -        { "log-binary", no_argument, &log_binary, 1},
> -        { "help", no_argument, NULL, 'h'},
> -        { 0, 0, 0, 0}
> -    };
> -
> -    if (isatty(fileno(stderr)) && isatty(fileno(stdin))) {
> -        stdin_ok = true;
> -    }
> -
> -    openlog("spice-streaming-agent", stdin_ok? (LOG_PERROR|LOG_PID) :
> LOG_PID, LOG_USER);
> -    setlogmask(logmask);
> -
> -    while ((opt = getopt_long(argc, argv, "hip:c:l:d", long_options, NULL))
> != -1) {
> -        switch (opt) {
> -        case 0:
> -            /* Handle long options if needed */
> -            break;
> -        case 'i':
> -            stdin_ok = true;
> -            openlog("spice-streaming-agent", LOG_PERROR|LOG_PID, LOG_USER);
> -            break;
> -        case 'p':
> -            streamport = optarg;
> -            break;
> -        case 'c': {
> -            char *p = strchr(optarg, '=');
> -            if (p == NULL) {
> -                arg_error("wrong 'c' argument %s\n", optarg);
> -                usage(argv[0]);
> -            }
> -            *p++ = '\0';
> -            agent.AddOption(optarg, p);
> -            break;
> -        }
> -        case 'l':
> -            log_filename = optarg;
> -            break;
> -        case 'd':
> -            logmask = LOG_UPTO(LOG_DEBUG);
> -            setlogmask(logmask);
> -            break;
> -        case 'h':
> -            usage(argv[0]);
> -            break;
> -        }
> -    }
> -
> -    agent.LoadPlugins(PLUGINSDIR);
> -
> -    register_interrupts();
> -
> -    FILE *f_log = NULL;
> -    if (log_filename) {
> -        f_log = fopen(log_filename, "wb");
> -        if (!f_log) {
> -            syslog(LOG_ERR, "Failed to open log file '%s': %s\n",
> -                   log_filename, strerror(errno));
> -            return EXIT_FAILURE;
> -        }
> -    }
> -
> -    Display *display = XOpenDisplay(NULL);
> -    if (display == NULL) {
> -        syslog(LOG_ERR, "failed to open display\n");
> -        return EXIT_FAILURE;
> -    }
> -    int event_base, error_base;
> -    if (!XFixesQueryExtension(display, &event_base, &error_base)) {
> -        syslog(LOG_ERR, "XFixesQueryExtension failed\n");
> -        return EXIT_FAILURE;
> -    }
> -    Window rootwindow = DefaultRootWindow(display);
> -    XFixesSelectCursorInput(display, rootwindow,
> XFixesDisplayCursorNotifyMask);
> -
> -    std::thread cursor_th(cursor_changes, display, event_base);
> +    std::thread cursor_th(CursorThread(streamfd, stream_mtx));
>      cursor_th.detach();
>  
> -    int ret = EXIT_SUCCESS;
> -    try {
> -        do_capture(streamport, f_log);
> -    }
> -    catch (std::runtime_error &err) {
> -        syslog(LOG_ERR, "%s\n", err.what());
> -        ret = EXIT_FAILURE;
> -    }
> -
> -    if (f_log) {
> -        fclose(f_log);
> -        f_log = NULL;
> -    }
> -    closelog();
> -    return ret;
> +    do_capture();
>  }
> +
> +}} // namespace spice::streaming_agent
> diff --git a/src/spice-streaming-agent.hpp b/src/spice-streaming-agent.hpp
> new file mode 100644
> index 0000000..1740a02
> --- /dev/null
> +++ b/src/spice-streaming-agent.hpp
> @@ -0,0 +1,57 @@
> +/* An implementation of a SPICE streaming agent
> + *
> + * \copyright
> + * Copyright 2016-2018 Red Hat Inc. All rights reserved.
> + */
> +
> +#ifndef SPICE_STREAMING_AGENT_SPICE_STREAMING_AGENT_HPP
> +#define SPICE_STREAMING_AGENT_SPICE_STREAMING_AGENT_HPP
> +
> +#include "concrete-agent.hpp"
> +
> +#include <cstdint>
> +#include <mutex>
> +#include <set>
> +
> +
> +namespace spice {
> +namespace streaming_agent {
> +
> +class AgentRunner
> +{

bracket

> +public:
> +    AgentRunner(const std::string &stream_port,
> +                const std::string &log_filename,
> +                bool log_binary,
> +                bool stdin_ok);

It does not look so extensible, every option require an additional
argument.

> +    ~AgentRunner();
> +
> +    void do_capture();

Why this is public? What is responsible for this class?

> +    void add_options(const std::vector<std::pair<std::string, std::string>>
> &options);
> +    void run();
> +
> +    static bool quit;
> +
> +private:
> +    int have_something_to_read(int *pfd, int timeout);
> +    int read_command_from_stdin();
> +    int read_command_from_device();
> +    int read_command(bool blocking);
> +    int spice_stream_send_format(unsigned w, unsigned h, unsigned c);
> +    int spice_stream_send_frame(const void *buf, const unsigned size);
> +    uint64_t get_time(void);

why to get the time you need an AgentRunner?

> +
> +    std::string stream_port;
> +    FILE *log_file = nullptr;
> +    ConcreteAgent agent;
> +    int streaming_requested = 0;

OT: why not bool?

> +    std::set<SpiceVideoCodecType> client_codecs;
> +    int streamfd = -1;
> +    bool log_binary = false;
> +    bool stdin_ok = false;
> +    std::mutex stream_mtx;
> +};
> +
> +}} // namespace spice::streaming_agent
> +
> +#endif // SPICE_STREAMING_AGENT_SPICE_STREAMING_AGENT_HPP

Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Reply via email to