Note and indicate servers with bad link version (#982).

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  <cth@carlh.net>
+
+	* Add servers with bad server-link versions in the list (#982).
+
 2018-04-12  Carl Hetherington  <cth@carlh.net>
 
 	* 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_ptr<Sock
 	xml->read_string (s);
 
 	string const ip = socket->socket().remote_endpoint().address().to_string ();
-	if (!server_found (ip) && xml->optional_number_child<int>("Version").get_value_or (0) == SERVER_LINK_VERSION) {
-		EncodeServerDescription sd (ip, xml->number_child<int> ("Threads"));
-		{
+	if (!server_found (ip)) {
+		EncodeServerDescription sd (ip, xml->number_child<int>("Threads"), xml->optional_number_child<int>("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<EncodeServerDescription>::const_iterator i = _servers.begin();
-	while (i != _servers.end() && i->host_name() != ip) {
+	list<EncodeServerDescription>::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<EncodeServerDescription>
-EncodeServerFinder::servers () const
+EncodeServerFinder::good_servers () const
+{
+	boost::mutex::scoped_lock lm (_servers_mutex);
+	return _good_servers;
+}
+
+list<EncodeServerDescription>
+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<EncodeServerDescription> servers () const;
+	std::list<EncodeServerDescription> good_servers () const;
+	std::list<EncodeServerDescription> bad_servers () const;
 
 	/** Emitted whenever the list of servers changes */
 	boost::signals2::signal<void ()> ServersListChanged;
@@ -73,7 +74,11 @@ private:
 	/** Thread to listen to the responses from servers */
 	boost::thread* _listen_thread;
 
-	std::list<EncodeServerDescription> _servers;
+	/** Available servers with the correct link version */
+	std::list<EncodeServerDescription> _good_servers;
+	/** Available servers with an incorrect link version */
+	std::list<EncodeServerDescription> _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<EncodeServerDescription> servers = EncodeServerFinder::instance()->servers ();
+		list<EncodeServerDescription> 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<thread*> 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<thread*> 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<thread*> threads;
 	for (int i = 0; i < 8; ++i) {