Try to make client/server communications a little more robust.

6 files changed, 134 insertions, 77 deletions

diff --git a/src/lib/dcp_video_frame.cc b/src/lib/dcp_video_frame.cc
index e074b1cd1..bf29e6819 100644
--- a/src/lib/dcp_video_frame.cc
+++ b/src/lib/dcp_video_frame.cc
@@ -167,8 +167,8 @@ DCPVideoFrame::encode_locally ()
 			_subtitle->area(), _subtitle_offset, _subtitle_scale
 			);
 
-		shared_ptr<Image> im = _subtitle->image()->scale (Size (tx.w, tx.h), _scaler);
-		prepared->alpha_blend (im, Position (tx.x, tx.y));
+		shared_ptr<Image> im = _subtitle->image()->scale (tx.size(), _scaler);
+		prepared->alpha_blend (im, tx.position());
 	}
 
 	create_openjpeg_container ();
@@ -312,25 +312,31 @@ DCPVideoFrame::encode_remotely (ServerDescription const * serv)
 	socket->connect (*endpoint_iterator, 30);
 
 	stringstream s;
-	s << "encode "
-	  << _input->size().width << " " << _input->size().height << " "
-	  << _input->pixel_format() << " "
-	  << _out_size.width << " " << _out_size.height << " "
-	  << _padding << " "
-	  << _subtitle_offset << " "
-	  << _subtitle_scale << " "
-	  << _scaler->id () << " "
-	  << _frame << " "
-	  << _frames_per_second << " "
-	  << (_post_process.empty() ? "none" : _post_process) << " "
-	  << Config::instance()->colour_lut_index () << " "
-	  << Config::instance()->j2k_bandwidth () << " ";
+	s << "encode please\n"
+	  << "input_width " << _input->size().width << "\n"
+	  << "input_height " << _input->size().height << "\n"
+	  << "input_pixel_format " << _input->pixel_format() << "\n"
+	  << "output_width " << _out_size.width << "\n"
+	  << "output_height " << _out_size.height << "\n"
+	  << "padding " <<  _padding << "\n"
+	  << "subtitle_offset " << _subtitle_offset << "\n"
+	  << "subtitle_scale " << _subtitle_scale << "\n"
+	  << "scaler " << _scaler->id () << "\n"
+	  << "frame " << _frame << "\n"
+	  << "frames_per_second " << _frames_per_second << "\n";
+
+	if (!_post_process.empty()) {
+		s << "post_process " << _post_process << "\n";
+	}
+	
+	s << "colour_lut " << Config::instance()->colour_lut_index () << "\n"
+	  << "j2k_bandwidth " << Config::instance()->j2k_bandwidth () << "\n";
 
 	if (_subtitle) {
-		s << _subtitle->position().x << " " << _subtitle->position().y << " "
-		  << _subtitle->image()->size().width << " " << _subtitle->image()->size().height;
-	} else {
-		s << "-1 -1 0 0";
+		s << "subtitle_x " << _subtitle->position().x << "\n"
+		  << "subtitle_y " << _subtitle->position().y << "\n"
+		  << "subtitle_width " << _subtitle->image()->size().width << "\n"
+		  << "subtitle_height " << _subtitle->image()->size().height << "\n";
 	}
 
 	socket->write ((uint8_t *) s.str().c_str(), s.str().length() + 1, 30);
diff --git a/src/lib/server.cc b/src/lib/server.cc
index d40325db0..e8a9804ed 100644
--- a/src/lib/server.cc
+++ b/src/lib/server.cc
@@ -27,6 +27,7 @@
 #include <sstream>
 #include <iostream>
 #include <boost/algorithm/string.hpp>
+#include <boost/lexical_cast.hpp>
 #include "server.h"
 #include "util.h"
 #include "scaler.h"
@@ -73,60 +74,44 @@ Server::Server (Log* log)
 int
 Server::process (shared_ptr<Socket> socket)
 {
-	char buffer[256];
+	char buffer[512];
 	socket->read_indefinite ((uint8_t *) buffer, sizeof (buffer), 30);
 	socket->consume (strlen (buffer) + 1);
 	
 	stringstream s (buffer);
+	multimap<string, string> kv = read_key_value (s);
 
-	string command;
-	s >> command;
-	if (command != "encode") {
+	if (get_required_string (kv, "encode") != "please") {
 		return -1;
 	}
 
-	Size in_size;
-	int pixel_format_int;
-	Size out_size;
-	int padding;
-	int subtitle_offset;
-	int subtitle_scale;
-	string scaler_id;
-	int frame;
-	float frames_per_second;
-	string post_process;
-	int colour_lut_index;
-	int j2k_bandwidth;
-	Position subtitle_position;
-	Size subtitle_size;
-	
-	s >> in_size.width >> in_size.height
-	  >> pixel_format_int
-	  >> out_size.width >> out_size.height
-	  >> padding
-	  >> subtitle_offset
-	  >> subtitle_scale
-	  >> scaler_id
-	  >> frame
-	  >> frames_per_second
-	  >> post_process
-	  >> colour_lut_index
-	  >> j2k_bandwidth
-	  >> subtitle_position.x >> subtitle_position.y
-	  >> subtitle_size.width >> subtitle_size.height;
+	Size in_size (get_required_int (kv, "input_width"), get_required_int (kv, "input_height"));
+	int pixel_format_int = get_required_int (kv, "input_pixel_format");
+	Size out_size (get_required_int (kv, "output_width"), get_required_int (kv, "output_height"));
+	int padding = get_required_int (kv, "padding");
+	int subtitle_offset = get_required_int (kv, "subtitle_offset");
+	int subtitle_scale = get_required_int (kv, "subtitle_scale");
+	string scaler_id = get_required_string (kv, "scaler");
+	int frame = get_required_int (kv, "frame");
+	int frames_per_second = get_required_int (kv, "frames_per_second");
+	string post_process = get_optional_string (kv, "post_process");
+	int colour_lut_index = get_required_int (kv, "colour_lut");
+	int j2k_bandwidth = get_required_int (kv, "j2k_bandwidth");
+	Position subtitle_position (get_optional_int (kv, "subtitle_x"), get_optional_int (kv, "subtitle_y"));
+	Size subtitle_size (get_optional_int (kv, "subtitle_width"), get_optional_int (kv, "subtitle_height"));
+
+	/* This checks that colour_lut_index is within range */
+	colour_lut_index_to_name (colour_lut_index);
 
 	PixelFormat pixel_format = (PixelFormat) pixel_format_int;
 	Scaler const * scaler = Scaler::from_id (scaler_id);
-	if (post_process == "none") {
-		post_process = "";
-	}
 	
 	shared_ptr<Image> image (new AlignedImage (pixel_format, in_size));
 
 	image->read_from_socket (socket);
 
 	shared_ptr<Subtitle> sub;
-	if (subtitle_position.x != -1) {
+	if (subtitle_size.width && subtitle_size.height) {
 		shared_ptr<Image> subtitle_image (new AlignedImage (PIX_FMT_RGBA, subtitle_size));
 		subtitle_image->read_from_socket (socket);
 		sub.reset (new Subtitle (subtitle_position, subtitle_image));
diff --git a/src/lib/subtitle.cc b/src/lib/subtitle.cc
index 7e1ca2d2a..01e8cac13 100644
--- a/src/lib/subtitle.cc
+++ b/src/lib/subtitle.cc
@@ -93,8 +93,8 @@ subtitle_transformed_area (
 	/* We will scale the subtitle by the same amount as the video frame, and also by the additional
 	   subtitle_scale
 	*/
-	tx.w = sub_area.w * target_x_scale * subtitle_scale;
-	tx.h = sub_area.h * target_y_scale * subtitle_scale;
+	tx.width = sub_area.width * target_x_scale * subtitle_scale;
+	tx.height = sub_area.height * target_y_scale * subtitle_scale;
 
 	/* Then we need a corrective translation, consisting of two parts:
 	 *
@@ -108,8 +108,8 @@ subtitle_transformed_area (
 	 * Combining these two translations gives these expressions.
 	 */
 	
-	tx.x = target_x_scale * (sub_area.x + (sub_area.w * (1 - subtitle_scale) / 2));
-	tx.y = target_y_scale * (sub_area.y + (sub_area.h * (1 - subtitle_scale) / 2));
+	tx.x = target_x_scale * (sub_area.x + (sub_area.width * (1 - subtitle_scale) / 2));
+	tx.y = target_y_scale * (sub_area.y + (sub_area.height * (1 - subtitle_scale) / 2));
 
 	return tx;
 }
diff --git a/src/lib/util.cc b/src/lib/util.cc
index a14db810f..c2b24944d 100644
--- a/src/lib/util.cc
+++ b/src/lib/util.cc
@@ -35,6 +35,7 @@
 #include <boost/algorithm/string.hpp>
 #include <boost/bind.hpp>
 #include <boost/lambda/lambda.hpp>
+#include <boost/lexical_cast.hpp>
 #include <openjpeg.h>
 #include <openssl/md5.h>
 #include <magick/MagickCore.h>
@@ -609,8 +610,8 @@ Rectangle::intersection (Rectangle const & other) const
 	
 	return Rectangle (
 		tx, ty,
-		min (x + w, other.x + other.w) - tx,
-		min (y + h, other.y + other.h) - ty
+		min (x + width, other.x + other.width) - tx,
+		min (y + height, other.y + other.height) - ty
 		);
 }
 
@@ -650,3 +651,56 @@ read_key_value (istream &s)
 
 	return kv;
 }
+
+string
+get_required_string (multimap<string, string> const & kv, string k)
+{
+	if (kv.count (k) > 1) {
+		throw StringError ("unexpected multiple keys in key-value set");
+	}
+
+	multimap<string, string>::const_iterator i = kv.find (k);
+	
+	if (i == kv.end ()) {
+		throw StringError (String::compose ("missing key %1 in key-value set", k));
+	}
+
+	return i->second;
+}
+
+int
+get_required_int (multimap<string, string> const & kv, string k)
+{
+	string const v = get_required_string (kv, k);
+	return lexical_cast<int> (v);
+}
+
+string
+get_optional_string (multimap<string, string> const & kv, string k)
+{
+	if (kv.count (k) > 1) {
+		throw StringError ("unexpected multiple keys in key-value set");
+	}
+
+	multimap<string, string>::const_iterator i = kv.find (k);
+	if (i == kv.end ()) {
+		return "";
+	}
+
+	return i->second;
+}
+
+int
+get_optional_int (multimap<string, string> const & kv, string k)
+{
+	if (kv.count (k) > 1) {
+		throw StringError ("unexpected multiple keys in key-value set");
+	}
+
+	multimap<string, string>::const_iterator i = kv.find (k);
+	if (i == kv.end ()) {
+		return 0;
+	}
+
+	return lexical_cast<int> (i->second);
+}
diff --git a/src/lib/util.h b/src/lib/util.h
index 58528c775..5f2e9a7fa 100644
--- a/src/lib/util.h
+++ b/src/lib/util.h
@@ -127,21 +127,29 @@ struct Rectangle
 	Rectangle ()
 		: x (0)
 		, y (0)
-		, w (0)
-		, h (0)
+		, width (0)
+		, height (0)
 	{}
 
 	Rectangle (int x_, int y_, int w_, int h_)
 		: x (x_)
 		, y (y_)
-		, w (w_)
-		, h (h_)
+		, width (w_)
+		, height (h_)
 	{}
 
 	int x;
 	int y;
-	int w;
-	int h;
+	int width;
+	int height;
+
+	Position position () const {
+		return Position (x, y);
+	}
+
+	Size size () const {
+		return Size (width, height);
+	}
 
 	Rectangle intersection (Rectangle const & other) const;
 };
@@ -151,6 +159,10 @@ extern int dcp_audio_sample_rate (int);
 extern std::string colour_lut_index_to_name (int index);
 extern int round_up (int, int);
 extern std::multimap<std::string, std::string> read_key_value (std::istream& s);
+extern int get_required_int (std::multimap<std::string, std::string> const & kv, std::string k);
+extern std::string get_required_string (std::multimap<std::string, std::string> const & kv, std::string k);
+extern int get_optional_int (std::multimap<std::string, std::string> const & kv, std::string k);
+extern std::string get_optional_string (std::multimap<std::string, std::string> const & kv, std::string k);
 
 /** @class Socket
  *  @brief A class to wrap a boost::asio::ip::tcp::socket with some things
@@ -187,7 +199,7 @@ private:
 	boost::asio::deadline_timer _deadline;
 	boost::asio::ip::tcp::socket _socket;
 	/** a buffer for small reads */
-	uint8_t _buffer[512];
+	uint8_t _buffer[1024];
 	/** amount of valid data in the buffer */
 	int _buffer_data;
 };
diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc
index 34140d16c..c42ffaeeb 100644
--- a/src/wx/film_viewer.cc
+++ b/src/wx/film_viewer.cc
@@ -154,7 +154,7 @@ private:
 		/* Target ratio */
 		float const target = _film->format() ? _film->format()->ratio_as_float (_film) : 1.78;
 
-		_transformed_image = _image->GetSubImage (wxRect (cropped_area.x, cropped_area.y, cropped_area.w, cropped_area.h));
+		_transformed_image = _image->GetSubImage (wxRect (cropped_area.x, cropped_area.y, cropped_area.width, cropped_area.height));
 
 		float x_scale = 1;
 		float y_scale = 1;
@@ -162,13 +162,13 @@ private:
 		if ((float (vw) / vh) > target) {
 			/* view is longer (horizontally) than the ratio; fit height */
 			_transformed_image.Rescale (vh * target, vh, wxIMAGE_QUALITY_HIGH);
-			x_scale = vh * target / cropped_area.w;
-			y_scale = float (vh) / cropped_area.h;
+			x_scale = vh * target / cropped_area.width;
+			y_scale = float (vh) / cropped_area.height;
 		} else {
 			/* view is shorter (horizontally) than the ratio; fit width */
 			_transformed_image.Rescale (vw, vw / target, wxIMAGE_QUALITY_HIGH);
-			x_scale = float (vw) / cropped_area.w;
-			y_scale = (vw / target) / cropped_area.h;
+			x_scale = float (vw) / cropped_area.width;
+			y_scale = (vw / target) / cropped_area.height;
 		}
 
 		_bitmap.reset (new wxBitmap (_transformed_image));
@@ -180,7 +180,7 @@ private:
 				);
 
 			_subtitle->transformed_image = _subtitle->base_image;
-			_subtitle->transformed_image.Rescale (_subtitle->transformed_area.w, _subtitle->transformed_area.h, wxIMAGE_QUALITY_HIGH);
+			_subtitle->transformed_image.Rescale (_subtitle->transformed_area.width, _subtitle->transformed_area.height, wxIMAGE_QUALITY_HIGH);
 			_subtitle->transformed_area.x -= _film->crop().left;
 			_subtitle->transformed_area.y -= _film->crop().top;
 			_subtitle->bitmap.reset (new wxBitmap (_subtitle->transformed_image));
@@ -203,8 +203,8 @@ private:
 		{
 			base_area.x = p.x;
 			base_area.y = p.y;
-			base_area.w = base_image.GetWidth ();
-			base_area.h = base_image.GetHeight ();
+			base_area.width = base_image.GetWidth ();
+			base_area.height = base_image.GetHeight ();
 		}
 
 		Rectangle base_area;