<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""> <div><blockquote type="cite" class=""><div class="">On 10 Nov 2017, at 14:15, Frediano Ziglio <<a href="mailto:fziglio@redhat.com" class="">fziglio@redhat.com</a>> wrote:</div> <div class=""><div style="" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""> From: Christophe de Dinechin <<a href="mailto:dinechin@redhat.com" class="">dinechin@redhat.com</a>> This incidentally fixes a race condition processing X events, where we could possibly start sending cursor events to the stream before it was actually open. Signed-off-by: Christophe de Dinechin <<a href="mailto:dinechin@redhat.com" class="">dinechin@redhat.com</a>> --- src/spice-streaming-agent.cpp | 68 +++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp index 8300cf2..33e0345 100644 --- a/src/spice-streaming-agent.cpp +++ b/src/spice-streaming-agent.cpp @@ -51,31 +51,39 @@ struct SpiceStreamDataMessage StreamMsgData msg; }; -struct Stream +struct SpiceStream { </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Would promote to a class.<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote>OK</div><div> <blockquote type="cite" class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">- Stream(const char *name, int &fd): fd(fd) + SpiceStream(const char *name): streamfd(open(name, O_RDWR)) { - fd = open(name, O_RDWR); - if (fd < 0) + if (streamfd < 0) throw std::runtime_error("failed to open streaming device"); } - ~Stream() + ~SpiceStream() { - if (fd >= 0) - close(fd); - fd = -1; + if (streamfd >= 0) + close(streamfd); + streamfd = -1; </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">is the if still needed?<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div> </div>No</div><div> <blockquote type="cite" class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""> } - int &fd; + + int have_something_to_read(int *pfd, int timeout); + int read_command_from_stdin(void); + int read_command_from_device(void); + int read_command(int blocking); </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Some are not strictly related to the stream.<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div> </div><div>But right now, things are a bit too interconnected because of the way read_command is structured.</div><div>Id’ like to split into another class, but I was planning to do that next.</div> <blockquote type="cite" class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">+ size_t write_all(int fd, const void *buf, const size_t len); </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">this has nothing to specifically do with the structure,<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">mostly with some API (write) behaviour like partial write and<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">signals.<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div> </div><div>Made it private. Also removed fd, it’s always streamfd.</div> <blockquote type="cite" class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">+ int send_format(unsigned w, unsigned h, unsigned c); + int send_frame(const void *buf, const unsigned size); + void send_cursor(const XFixesCursorImage &image); +private: + int streamfd; }; static int streaming_requested; static bool quit; -static int streamfd = -1; static bool stdin_ok; static int log_binary = 0; static std::mutex stream_mtx; -static int have_something_to_read(int *pfd, int timeout) +int SpiceStream::have_something_to_read(int *pfd, int timeout) { int nfds; struct pollfd pollfds[2] = { @@ -97,7 +105,7 @@ static int have_something_to_read(int *pfd, int timeout) return *pfd != -1; } -static int read_command_from_stdin(void) +int SpiceStream::read_command_from_stdin(void) { char buffer[64], *p, *save = NULL; @@ -121,7 +129,7 @@ static int read_command_from_stdin(void) return 1; } -static int read_command_from_device(void) +int SpiceStream::read_command_from_device(void) { StreamDevHeader hdr; uint8_t msg[64]; @@ -163,7 +171,7 @@ static int read_command_from_device(void) return 1; } -static int read_command(int blocking) +int SpiceStream::read_command(int blocking) </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">OT: maybe should be bool blocking ?<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div> </div>OK</div><div> <blockquote type="cite" class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">{ int fd, n=1; int timeout = blocking?-1:0; @@ -185,8 +193,7 @@ static int read_command(int blocking) return n; } -static size_t -write_all(int fd, const void *buf, const size_t len) +size_t SpiceStream::write_all(int fd, const void *buf, const size_t len) { size_t written = 0; while (written < len) { @@ -204,7 +211,7 @@ write_all(int fd, const void *buf, const size_t len) return written; } -static int spice_stream_send_format(unsigned w, unsigned h, unsigned c) +int SpiceStream::send_format(unsigned w, unsigned h, unsigned c) { SpiceStreamFormatMessage msg; @@ -225,7 +232,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 SpiceStream::send_frame(const void *buf, const unsigned size) { SpiceStreamDataMessage msg; const size_t msgsize = sizeof(msg); @@ -304,7 +311,7 @@ static void usage(const char *progname) exit(1); } -static void send_cursor(const XFixesCursorImage &image) +void SpiceStream::send_cursor(const XFixesCursorImage &image) { if (image.width >= 1024 || image.height >= 1024) return; @@ -337,7 +344,7 @@ static void send_cursor(const XFixesCursorImage &image) write_all(streamfd, msg.get(), cursor_size); } -static void cursor_changes(Display *display, int event_base) +static void cursor_changes(Display *display, int event_base, SpiceStream *stream) { unsigned long last_serial = 0; @@ -355,23 +362,25 @@ static void cursor_changes(Display *display, int event_base) continue; last_serial = cursor->cursor_serial; - send_cursor(*cursor); + stream->send_cursor(*cursor); } } static void -do_capture(const char *streamport, FILE *f_log) +do_capture(const char *streamport, FILE *f_log, Display *display, int event_base) { std::unique_ptr<FrameCapture> capture(agent.GetBestFrameCapture()); if (!capture) throw std::runtime_error("cannot find a suitable capture system"); - Stream stream(streamport, streamfd); + SpiceStream stream(streamport); + std::thread cursor_th(cursor_changes, display, event_base, &stream); + cursor_th.detach(); unsigned int frame_count = 0; while (! quit) { while (!quit && !streaming_requested) { - if (read_command(1) < 0) { + if (stream.read_command(1) < 0) { syslog(LOG_ERR, "FAILED to read command\n"); return; } @@ -406,7 +415,7 @@ do_capture(const char *streamport, FILE *f_log) syslog(LOG_DEBUG, "wXh %uX%u codec=%u\n", width, height, codec); - if (spice_stream_send_format(width, height, codec) == EXIT_FAILURE) + if (stream.send_format(width, height, codec) == EXIT_FAILURE) throw std::runtime_error("FAILED to send format message"); } if (f_log) { @@ -417,12 +426,12 @@ do_capture(const char *streamport, FILE *f_log) hexdump(frame.buffer, frame.buffer_size, f_log); } } - if (spice_stream_send_frame(frame.buffer, frame.buffer_size) == EXIT_FAILURE) { + if (stream.send_frame(frame.buffer, frame.buffer_size) == EXIT_FAILURE) { syslog(LOG_ERR, "FAILED to send a frame\n"); break; } //usleep(1); - if (read_command(0) < 0) { + if (stream.read_command(0) < 0) { syslog(LOG_ERR, "FAILED to read command\n"); return; } @@ -516,12 +525,9 @@ int main(int argc, char* argv[]) Window rootwindow = DefaultRootWindow(display); XFixesSelectCursorInput(display, rootwindow, XFixesDisplayCursorNotifyMask); - std::thread cursor_th(cursor_changes, display, event_base); - cursor_th.detach(); - int ret = EXIT_SUCCESS; try { - do_capture(streamport, f_log); + do_capture(streamport, f_log, display, event_base); } catch (std::runtime_error &err) { syslog(LOG_ERR, "%s\n", err.what()); </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Frediano</div></div></blockquote></div> </body></html>