From ed0b3ee0c5a0ba11d3a1a1dfee8e71238bcab4bd Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Fri, 13 Apr 2018 00:12:50 +0100 Subject: [PATCH] Note and indicate servers with bad link version (#982). --- ChangeLog | 4 +++ src/lib/encode_server_description.h | 11 +++++++- src/lib/encode_server_finder.cc | 43 +++++++++++++++++++++-------- src/lib/encode_server_finder.h | 9 ++++-- src/lib/j2k_encoder.cc | 2 +- src/tools/dcpomatic_cli.cc | 7 ++++- src/tools/server_test.cc | 2 +- src/wx/servers_list_dialog.cc | 18 ++++++++++-- test/client_server_test.cc | 6 ++-- 9 files changed, 79 insertions(+), 23 deletions(-) diff --git a/ChangeLog b/ChangeLog index 167bf0a15..5201319d6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2018-04-13 Carl Hetherington + + * Add servers with bad server-link versions in the list (#982). + 2018-04-12 Carl Hetherington * Prevent error when starting export without specifying a filename (#1260). diff --git a/src/lib/encode_server_description.h b/src/lib/encode_server_description.h index 60fb0a29d..864b0fdc1 100644 --- a/src/lib/encode_server_description.h +++ b/src/lib/encode_server_description.h @@ -30,14 +30,17 @@ public: EncodeServerDescription () : _host_name ("") , _threads (1) + , _link_version (0) {} /** @param h Server host name or IP address in string form. * @param t Number of threads to use on the server. + * @param l Server link version number of the server. */ - EncodeServerDescription (std::string h, int t) + EncodeServerDescription (std::string h, int t, int l) : _host_name (h) , _threads (t) + , _link_version (l) {} /* Default copy constructor is fine */ @@ -52,6 +55,10 @@ public: return _threads; } + int link_version () const { + return _link_version; + } + void set_host_name (std::string n) { _host_name = n; } @@ -65,6 +72,8 @@ private: std::string _host_name; /** number of threads to use on the server */ int _threads; + /** server link (i.e. protocol) version number */ + int _link_version; }; #endif diff --git a/src/lib/encode_server_finder.cc b/src/lib/encode_server_finder.cc index e87c55b71..1234dcd52 100644 --- a/src/lib/encode_server_finder.cc +++ b/src/lib/encode_server_finder.cc @@ -98,7 +98,8 @@ EncodeServerFinder::stop () _listen_thread = 0; boost::mutex::scoped_lock lm (_servers_mutex); - _servers.clear (); + _good_servers.clear (); + _bad_servers.clear (); } void @@ -206,11 +207,14 @@ EncodeServerFinder::handle_accept (boost::system::error_code ec, shared_ptrread_string (s); string const ip = socket->socket().remote_endpoint().address().to_string (); - if (!server_found (ip) && xml->optional_number_child("Version").get_value_or (0) == SERVER_LINK_VERSION) { - EncodeServerDescription sd (ip, xml->number_child ("Threads")); - { + if (!server_found (ip)) { + EncodeServerDescription sd (ip, xml->number_child("Threads"), xml->optional_number_child("Version").get_value_or(0)); + if (sd.link_version() == SERVER_LINK_VERSION) { + boost::mutex::scoped_lock lm (_servers_mutex); + _good_servers.push_back (sd); + } else { boost::mutex::scoped_lock lm (_servers_mutex); - _servers.push_back (sd); + _bad_servers.push_back (sd); } emit (boost::bind (boost::ref (ServersListChanged))); } @@ -222,12 +226,21 @@ bool EncodeServerFinder::server_found (string ip) const { boost::mutex::scoped_lock lm (_servers_mutex); - list::const_iterator i = _servers.begin(); - while (i != _servers.end() && i->host_name() != ip) { + list::const_iterator i = _good_servers.begin(); + while (i != _good_servers.end() && i->host_name() != ip) { + ++i; + } + + if (i != _good_servers.end()) { + return true; + } + + i = _bad_servers.begin(); + while (i != _bad_servers.end() && i->host_name() != ip) { ++i; } - return i != _servers.end (); + return i != _bad_servers.end (); } EncodeServerFinder* @@ -249,10 +262,17 @@ EncodeServerFinder::drop () } list -EncodeServerFinder::servers () const +EncodeServerFinder::good_servers () const +{ + boost::mutex::scoped_lock lm (_servers_mutex); + return _good_servers; +} + +list +EncodeServerFinder::bad_servers () const { boost::mutex::scoped_lock lm (_servers_mutex); - return _servers; + return _bad_servers; } void @@ -261,7 +281,8 @@ EncodeServerFinder::config_changed (Config::Property what) if (what == Config::USE_ANY_SERVERS || what == Config::SERVERS) { { boost::mutex::scoped_lock lm (_servers_mutex); - _servers.clear (); + _good_servers.clear (); + _bad_servers.clear (); } ServersListChanged (); _search_condition.notify_all (); diff --git a/src/lib/encode_server_finder.h b/src/lib/encode_server_finder.h index 46d66b191..90031d27b 100644 --- a/src/lib/encode_server_finder.h +++ b/src/lib/encode_server_finder.h @@ -48,7 +48,8 @@ public: void stop (); - std::list servers () const; + std::list good_servers () const; + std::list bad_servers () const; /** Emitted whenever the list of servers changes */ boost::signals2::signal ServersListChanged; @@ -73,7 +74,11 @@ private: /** Thread to listen to the responses from servers */ boost::thread* _listen_thread; - std::list _servers; + /** Available servers with the correct link version */ + std::list _good_servers; + /** Available servers with an incorrect link version */ + std::list _bad_servers; + /** Mutex for _good_servers and _bad_servers */ mutable boost::mutex _servers_mutex; boost::asio::io_service _listen_io_service; diff --git a/src/lib/j2k_encoder.cc b/src/lib/j2k_encoder.cc index 3d1df688c..bbd602dd0 100644 --- a/src/lib/j2k_encoder.cc +++ b/src/lib/j2k_encoder.cc @@ -412,7 +412,7 @@ J2KEncoder::servers_list_changed () } } - BOOST_FOREACH (EncodeServerDescription i, EncodeServerFinder::instance()->servers ()) { + BOOST_FOREACH (EncodeServerDescription i, EncodeServerFinder::instance()->good_servers ()) { LOG_GENERAL (N_("Adding %1 worker threads for remote %2"), i.threads(), i.host_name ()); for (int j = 0; j < i.threads(); ++j) { _threads.push_back (new boost::thread (boost::bind (&J2KEncoder::encoder_thread, this, i))); diff --git a/src/tools/dcpomatic_cli.cc b/src/tools/dcpomatic_cli.cc index 31681a7fc..52a113cbb 100644 --- a/src/tools/dcpomatic_cli.cc +++ b/src/tools/dcpomatic_cli.cc @@ -121,7 +121,7 @@ list_servers () { while (true) { int N = 0; - list servers = EncodeServerFinder::instance()->servers (); + list servers = EncodeServerFinder::instance()->good_servers (); /* This is a bit fiddly because we want to list configured servers that are down as well as all those (configured and found by broadcast) that are up. @@ -167,6 +167,11 @@ list_servers () cout << std::left << setw(24) << i.host_name() << " UP " << i.threads() << "\n"; ++N; } + + /* And those that have a bad version */ + BOOST_FOREACH (EncodeServerDescription i, EncodeServerFinder::instance()->good_servers()) { + cout << std::left << setw(24) << i.host_name() << " bad version\n"; + } } dcpomatic_sleep (1); diff --git a/src/tools/server_test.cc b/src/tools/server_test.cc index 3fa7ebb60..3d16038c1 100644 --- a/src/tools/server_test.cc +++ b/src/tools/server_test.cc @@ -142,7 +142,7 @@ main (int argc, char* argv[]) dcpomatic_setup (); try { - server = new EncodeServerDescription (server_host, 1); + server = new EncodeServerDescription (server_host, 1, SERVER_LINK_VERSION); film.reset (new Film (film_dir)); film->read_metadata (); diff --git a/src/wx/servers_list_dialog.cc b/src/wx/servers_list_dialog.cc index a83e42214..e4839916e 100644 --- a/src/wx/servers_list_dialog.cc +++ b/src/wx/servers_list_dialog.cc @@ -35,7 +35,7 @@ ServersListDialog::ServersListDialog (wxWindow* parent) wxBoxSizer* s = new wxBoxSizer (wxVERTICAL); SetSizer (s); - _list = new wxListCtrl (this, wxID_ANY, wxDefaultPosition, wxSize (400, 200), wxLC_REPORT | wxLC_SINGLE_SEL); + _list = new wxListCtrl (this, wxID_ANY, wxDefaultPosition, wxSize (500, 200), wxLC_REPORT | wxLC_SINGLE_SEL); { wxListItem ip; @@ -49,7 +49,7 @@ ServersListDialog::ServersListDialog (wxWindow* parent) wxListItem ip; ip.SetId (1); ip.SetText (_("Threads")); - ip.SetWidth (100); + ip.SetWidth (150); _list->InsertColumn (1, ip); } @@ -76,7 +76,8 @@ ServersListDialog::servers_list_changed () _list->DeleteAllItems (); int n = 0; - BOOST_FOREACH (EncodeServerDescription i, EncodeServerFinder::instance()->servers ()) { + + BOOST_FOREACH (EncodeServerDescription i, EncodeServerFinder::instance()->good_servers()) { wxListItem list_item; list_item.SetId (n); _list->InsertItem (list_item); @@ -86,4 +87,15 @@ ServersListDialog::servers_list_changed () ++n; } + + BOOST_FOREACH (EncodeServerDescription i, EncodeServerFinder::instance()->bad_servers()) { + wxListItem list_item; + list_item.SetId (n); + _list->InsertItem (list_item); + + _list->SetItem (n, 0, std_to_wx (i.host_name ())); + _list->SetItem (n, 1, _("Incorrect version")); + + ++n; + } } diff --git a/test/client_server_test.cc b/test/client_server_test.cc index b3d57e813..d77ed4c15 100644 --- a/test/client_server_test.cc +++ b/test/client_server_test.cc @@ -124,7 +124,7 @@ BOOST_AUTO_TEST_CASE (client_server_test_rgb) dcpomatic_sleep (1); /* "localhost" rather than "127.0.0.1" here fails on docker; go figure */ - EncodeServerDescription description ("127.0.0.1", 1); + EncodeServerDescription description ("127.0.0.1", 1, SERVER_LINK_VERSION); list threads; for (int i = 0; i < 8; ++i) { @@ -209,7 +209,7 @@ BOOST_AUTO_TEST_CASE (client_server_test_yuv) dcpomatic_sleep (1); /* "localhost" rather than "127.0.0.1" here fails on docker; go figure */ - EncodeServerDescription description ("127.0.0.1", 2); + EncodeServerDescription description ("127.0.0.1", 2, SERVER_LINK_VERSION); list threads; for (int i = 0; i < 8; ++i) { @@ -307,7 +307,7 @@ BOOST_AUTO_TEST_CASE (client_server_test_j2k) dcpomatic_sleep (1); /* "localhost" rather than "127.0.0.1" here fails on docker; go figure */ - EncodeServerDescription description ("127.0.0.1", 2); + EncodeServerDescription description ("127.0.0.1", 2, SERVER_LINK_VERSION); list threads; for (int i = 0; i < 8; ++i) { -- 2.30.2