From 9f521cd010d95bebc4e0406aad3b83113254fdc5 Mon Sep 17 00:00:00 2001 From: Raphael Dumusc Date: Wed, 30 Aug 2017 10:59:15 +0200 Subject: [PATCH 1/2] DesktopStreamer: fix uncaught exception after #177 Also report stream connection errors in the GUI instead of cerr. --- apps/DesktopStreamer/MainWindow.cpp | 37 ++++++++++++++++------------- apps/DesktopStreamer/MainWindow.h | 2 +- deflect/Socket.cpp | 29 ++++++++++------------ deflect/Socket.h | 3 ++- deflect/Stream.h | 2 +- deflect/StreamPrivate.cpp | 4 ---- tests/cpp/SocketTests.cpp | 16 +++++++++---- 7 files changed, 47 insertions(+), 46 deletions(-) diff --git a/apps/DesktopStreamer/MainWindow.cpp b/apps/DesktopStreamer/MainWindow.cpp index f6f70d4..3457e12 100644 --- a/apps/DesktopStreamer/MainWindow.cpp +++ b/apps/DesktopStreamer/MainWindow.cpp @@ -325,28 +325,31 @@ void MainWindow::_updateSingleStream() return; } - if (_streams.empty()) + if (!_streams.empty()) + return; + + try { - const QPersistentModelIndex index; // default == use desktop - StreamPtr stream(new Stream(*this, index, - _streamIdLineEdit->text().toStdString(), - _getStreamHost())); - if (stream->isConnected()) - { - if (_remoteControlCheckBox->isChecked()) - stream->registerForEvents(); - _streams[index] = stream; - _startStreaming(); - } - else - _showConnectionErrorStatus(); + const auto index = QPersistentModelIndex(); // default == use desktop + const auto id = _streamIdLineEdit->text().toStdString(); + const auto host = _getStreamHost(); + + _streams[index] = std::make_shared(*this, index, id, host); + + if (_remoteControlCheckBox->isChecked()) + _streams[index]->registerForEvents(); + + _startStreaming(); + } + catch (const std::runtime_error& e) + { + _showConnectionErrorStatus(e.what()); } } -void MainWindow::_showConnectionErrorStatus() +void MainWindow::_showConnectionErrorStatus(const QString& message) { - _statusbar->showMessage( - QString("Cannot connect to host: '%1'").arg(_getStreamHost().c_str())); + _statusbar->showMessage(message); } void MainWindow::_processStreamEvents() diff --git a/apps/DesktopStreamer/MainWindow.h b/apps/DesktopStreamer/MainWindow.h index d306951..0fbe5e0 100644 --- a/apps/DesktopStreamer/MainWindow.h +++ b/apps/DesktopStreamer/MainWindow.h @@ -104,7 +104,7 @@ private slots: void _updateStreams(); void _updateMultipleStreams(); void _updateSingleStream(); - void _showConnectionErrorStatus(); + void _showConnectionErrorStatus(const QString& message); void _deselect(ConstStreamPtr stream); void _processStreamEvents(); diff --git a/deflect/Socket.cpp b/deflect/Socket.cpp index ad139a0..5be127e 100644 --- a/deflect/Socket.cpp +++ b/deflect/Socket.cpp @@ -47,7 +47,8 @@ #include #include #include -#include + +#include namespace { @@ -67,10 +68,7 @@ Socket::Socket(const std::string& host, const unsigned short port) // _connect(): QObject::connect: Cannot connect (null)::destroyed() to // QHostInfoLookupManager::waitForThreadPoolDone() if (!qApp) - { - QLoggingCategory* log = QLoggingCategory::defaultCategory(); - log->setEnabled(QtWarningMsg, false); - } + QLoggingCategory::defaultCategory()->setEnabled(QtWarningMsg, false); _connect(host, port); @@ -178,33 +176,30 @@ bool Socket::_receiveHeader(MessageHeader& messageHeader) return stream.status() == QDataStream::Ok; } -bool Socket::_connect(const std::string& host, const unsigned short port) +void Socket::_connect(const std::string& host, const unsigned short port) { _socket->connectToHost(host.c_str(), port); if (!_socket->waitForConnected(RECEIVE_TIMEOUT_MS)) { - std::cerr << "could not connect to " << host << ":" << port - << std::endl; - return false; + std::stringstream ss; + ss << "could not connect to " << host << ":" << port; + throw std::runtime_error(ss.str()); } if (!_receiveProtocolVersion()) { - std::cerr << "server protocol version was not received" << std::endl; _socket->disconnectFromHost(); - return false; + throw std::runtime_error("server protocol version was not received"); } if (_serverProtocolVersion < NETWORK_PROTOCOL_VERSION) { - std::cerr << "server uses unsupported protocol: " - << _serverProtocolVersion << " < " << NETWORK_PROTOCOL_VERSION - << std::endl; _socket->disconnectFromHost(); - return false; + std::stringstream ss; + ss << "server uses unsupported protocol: " << _serverProtocolVersion + << " < " << NETWORK_PROTOCOL_VERSION; + throw std::runtime_error(ss.str()); } - - return true; } bool Socket::_receiveProtocolVersion() diff --git a/deflect/Socket.h b/deflect/Socket.h index e0b124b..24a9e49 100644 --- a/deflect/Socket.h +++ b/deflect/Socket.h @@ -70,6 +70,7 @@ class Socket : public QObject * Construct a Socket and connect to host. * @param host The target host (IP address or hostname) * @param port The target port + * @throw std::runtime_error if the socket could not connect */ DEFLECT_API Socket(const std::string& host, unsigned short port); @@ -128,7 +129,7 @@ class Socket : public QObject int32_t _serverProtocolVersion; bool _receiveHeader(MessageHeader& messageHeader); - bool _connect(const std::string& host, const unsigned short port); + void _connect(const std::string& host, const unsigned short port); bool _receiveProtocolVersion(); bool _write(const QByteArray& data); }; diff --git a/deflect/Stream.h b/deflect/Stream.h index bd23fcb..f42f90c 100644 --- a/deflect/Stream.h +++ b/deflect/Stream.h @@ -114,7 +114,7 @@ class Stream : public Observer * @param image The image to send. Note that the image is not copied, so the * referenced must remain valid until the send is finished. * @return true if the image data could be sent, false otherwise - * @throw std::invalid_argument if RGBA and uncompressed + * @throw std::invalid_argument if not RGBA and uncompressed * @throw std::invalid_argument if invalid JPEG compression arguments * @throw std::runtime_error if pending finishFrame() has not been completed * @throw std::runtime_error if JPEG compression failed diff --git a/deflect/StreamPrivate.cpp b/deflect/StreamPrivate.cpp index 819b754..84142dd 100644 --- a/deflect/StreamPrivate.cpp +++ b/deflect/StreamPrivate.cpp @@ -85,10 +85,6 @@ StreamPrivate::StreamPrivate(const std::string& id_, const std::string& host, , socket{_getStreamHost(host), port} , sendWorker{socket, id} { - if (!socket.isConnected()) - throw std::runtime_error( - "Connection to deflect server could not be established"); - socket.connect(&socket, &Socket::disconnected, [this]() { if (disconnectedCallback) disconnectedCallback(); diff --git a/tests/cpp/SocketTests.cpp b/tests/cpp/SocketTests.cpp index 5051b3f..de7b141 100644 --- a/tests/cpp/SocketTests.cpp +++ b/tests/cpp/SocketTests.cpp @@ -1,6 +1,6 @@ /*********************************************************************/ -/* Copyright (c) 2013, EPFL/Blue Brain Project */ -/* Raphael Dumusc */ +/* Copyright (c) 2013-2017, EPFL/Blue Brain Project */ +/* Raphael Dumusc */ /* All rights reserved. */ /* */ /* Redistribution and use in source and binary forms, with or */ @@ -52,9 +52,15 @@ void testSocketConnect(const int32_t versionOffset) { MinimalDeflectServer server(versionOffset); - deflect::Socket socket("localhost", server.serverPort()); - - BOOST_CHECK(socket.isConnected() == (versionOffset >= 0)); + try + { + deflect::Socket socket("localhost", server.serverPort()); + BOOST_CHECK(socket.isConnected()); + } + catch (const std::runtime_error&) + { + BOOST_CHECK(versionOffset < 0); + } } BOOST_AUTO_TEST_CASE( From 486ab2dfd8d66c441a51bd4a84ded531ce610e23 Mon Sep 17 00:00:00 2001 From: Raphael Dumusc Date: Wed, 30 Aug 2017 12:54:14 +0200 Subject: [PATCH 2/2] Fix MultiWindowMode for OSX build, cleanups --- apps/DesktopStreamer/MainWindow.cpp | 115 +++++++++++++++++----------- apps/DesktopStreamer/MainWindow.h | 8 ++ 2 files changed, 79 insertions(+), 44 deletions(-) diff --git a/apps/DesktopStreamer/MainWindow.cpp b/apps/DesktopStreamer/MainWindow.cpp index 3457e12..24adcd1 100644 --- a/apps/DesktopStreamer/MainWindow.cpp +++ b/apps/DesktopStreamer/MainWindow.cpp @@ -214,16 +214,15 @@ void MainWindow::_showMultiWindowMode() _listView->setVisible(true); - // select 'Desktop' item as initial default stream item - _listView->setCurrentIndex(_listView->model()->index(0, 0)); + const auto desktopIndex = _listView->model()->index(0, 0); + _listView->setCurrentIndex(desktopIndex); _streamButton->setText(streamSelected); + const auto itemsCount = _listView->model()->rowCount(); const int itemsHorizontal = - std::min(3.f, - std::ceil(std::sqrt(float(_listView->model()->rowCount())))); + std::min(3.f, std::ceil(std::sqrt(float(itemsCount)))); const int itemsVertical = - std::min(3.f, std::ceil(float(_listView->model()->rowCount()) / - itemsHorizontal)); + std::min(3.f, std::ceil(float(itemsCount) / itemsHorizontal)); layout()->setSizeConstraint(QLayout::SetDefaultConstraint); setFixedSize(QWIDGETSIZE_MAX, QWIDGETSIZE_MAX); @@ -267,11 +266,10 @@ void MainWindow::_updateStreams() void MainWindow::_updateMultipleStreams() { #ifdef DEFLECT_USE_QT5MACEXTRAS - const QModelIndexList windowIndices = - _listView->selectionModel()->selectedIndexes(); + const auto windowIndices = _listView->selectionModel()->selectedIndexes(); StreamMap streams; - for (const QPersistentModelIndex& index : windowIndices) + for (const auto& index : windowIndices) { if (_streams.count(index)) { @@ -279,33 +277,19 @@ void MainWindow::_updateMultipleStreams() continue; } - const std::string appName = index.isValid() - ? _listView->model() - ->data(index, Qt::DisplayRole) - .toString() - .toStdString() - : std::string(); - const std::string streamId = std::to_string(++_streamID) + " " + - appName + " - " + - _streamIdLineEdit->text().toStdString(); - const int pid = index.isValid() - ? _listView->model() - ->data(index, DesktopWindowsModel::ROLE_PID) - .toInt() - : 0; - const std::string host = _getStreamHost(); - StreamPtr stream(new Stream(*this, index, streamId, host, pid)); - - if (!stream->isConnected()) + const auto appName = _getAppName(index); + const auto streamId = _getFormattedStreamId(++_streamID, appName); + const auto host = _getStreamHost(); + const auto pid = _getAppPid(index); + + try { - _showConnectionErrorStatus(); - continue; + streams.emplace(index, _makeStream(index, streamId, host, pid)); + } + catch (const std::runtime_error& e) + { + _showConnectionErrorStatus(e.what()); } - - if (_remoteControlCheckBox->isChecked()) - stream->registerForEvents(); - - streams[index] = stream; } _streams.swap(streams); @@ -328,17 +312,14 @@ void MainWindow::_updateSingleStream() if (!_streams.empty()) return; + const auto index = QPersistentModelIndex(); // default == use desktop + const auto streamId = _getStreamId(); + const auto host = _getStreamHost(); + const auto pid = 0; + try { - const auto index = QPersistentModelIndex(); // default == use desktop - const auto id = _streamIdLineEdit->text().toStdString(); - const auto host = _getStreamHost(); - - _streams[index] = std::make_shared(*this, index, id, host); - - if (_remoteControlCheckBox->isChecked()) - _streams[index]->registerForEvents(); - + _streams.emplace(index, _makeStream(index, streamId, host, pid)); _startStreaming(); } catch (const std::runtime_error& e) @@ -347,6 +328,19 @@ void MainWindow::_updateSingleStream() } } +MainWindow::StreamPtr MainWindow::_makeStream(const QPersistentModelIndex index, + const std::string& id, + const std::string& host, + const int pid) const +{ + auto stream = std::make_shared(*this, index, id, host, pid); + + if (_remoteControlCheckBox->isChecked()) + stream->registerForEvents(); + + return stream; +} + void MainWindow::_showConnectionErrorStatus(const QString& message) { _statusbar->showMessage(message); @@ -368,7 +362,7 @@ void MainWindow::_processStreamEvents() } } - for (ConstStreamPtr stream : closedStreams) + for (auto stream : closedStreams) _deselect(stream); } @@ -431,6 +425,39 @@ void MainWindow::_regulateFrameRate() } } +std::string MainWindow::_getAppName(const QModelIndex& appIndex) const +{ + if (!appIndex.isValid()) + return std::string(); + + const auto name = _listView->model()->data(appIndex, Qt::DisplayRole); + return name.toString().toStdString(); +} + +int MainWindow::_getAppPid(const QModelIndex& appIndex) const +{ + if (!appIndex.isValid()) + return 0; + +#ifdef DEFLECT_USE_QT5MACEXTRAS + const auto pidRole = DesktopWindowsModel::ROLE_PID; + return _listView->model()->data(appIndex, pidRole).toInt(); +#else + return 0; +#endif +} + +std::string MainWindow::_getFormattedStreamId(const uint32_t id, + const std::string& appName) const +{ + return std::to_string(id) + " " + appName + " - " + _getStreamId(); +} + +std::string MainWindow::_getStreamId() const +{ + return _streamIdLineEdit->text().toStdString(); +} + std::string MainWindow::_getStreamHost() const { if (_hostComboBox->findText(_hostComboBox->currentText()) != -1 && diff --git a/apps/DesktopStreamer/MainWindow.h b/apps/DesktopStreamer/MainWindow.h index 0fbe5e0..18a9ada 100644 --- a/apps/DesktopStreamer/MainWindow.h +++ b/apps/DesktopStreamer/MainWindow.h @@ -105,11 +105,19 @@ private slots: void _updateMultipleStreams(); void _updateSingleStream(); void _showConnectionErrorStatus(const QString& message); + StreamPtr _makeStream(QPersistentModelIndex index, const std::string& id, + const std::string& host, int pid) const; void _deselect(ConstStreamPtr stream); void _processStreamEvents(); void _shareDesktopUpdate(); void _regulateFrameRate(); + + std::string _getAppName(const QModelIndex& appIndex) const; + int _getAppPid(const QModelIndex& appIndex) const; + std::string _getFormattedStreamId(uint32_t id, + const std::string& appName) const; + std::string _getStreamId() const; std::string _getStreamHost() const; deflect::ChromaSubsampling _getSubsampling() const; };