[Spice-devel] [RFC PATCH spice-streaming-agent v2] separate and encapsulate the agent business code
Frediano Ziglio
fziglio at redhat.com
Mon Feb 12 08:53:03 UTC 2018
>
> 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ý <lhrazky at 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
More information about the Spice-devel
mailing list