Move video level conversion for RGB from FFmpegImageProxy to Image.

Since FFmpeg does not do video level conversion for RGB sources
when we (sort of) ask it to in Image::crop_scale_window() it seems
to make more sense to compensate for that by calling
full_to_video_range() in the same place (rather than in
FFmpegImageProxy).

8 files changed, 22 insertions, 27 deletions

diff --git a/src/lib/ffmpeg_image_proxy.cc b/src/lib/ffmpeg_image_proxy.cc
index ab9b94b14..0b40b8f83 100644
--- a/src/lib/ffmpeg_image_proxy.cc
+++ b/src/lib/ffmpeg_image_proxy.cc
@@ -53,26 +53,23 @@ using std::dynamic_pointer_cast;
 using dcp::raw_convert;
 
 
-FFmpegImageProxy::FFmpegImageProxy (boost::filesystem::path path, VideoRange video_range)
+FFmpegImageProxy::FFmpegImageProxy (boost::filesystem::path path)
 	: _data (path)
-	, _video_range (video_range)
 	, _pos (0)
 	, _path (path)
 {
 
 }
 
-FFmpegImageProxy::FFmpegImageProxy (dcp::ArrayData data, VideoRange video_range)
+FFmpegImageProxy::FFmpegImageProxy (dcp::ArrayData data)
 	: _data (data)
-	, _video_range (video_range)
 	, _pos (0)
 {
 
 }
 
-FFmpegImageProxy::FFmpegImageProxy (shared_ptr<cxml::Node> node, shared_ptr<Socket> socket)
-	: _video_range (string_to_video_range(node->string_child("VideoRange")))
-	, _pos (0)
+FFmpegImageProxy::FFmpegImageProxy (shared_ptr<Socket> socket)
+	: _pos (0)
 {
 	uint32_t const size = socket->read_uint32 ();
 	_data = dcp::ArrayData (size);
@@ -208,16 +205,7 @@ FFmpegImageProxy::image (optional<dcp::Size>) const
 		throw DecodeError (N_("avcodec_receive_frame"), name_for_errors, r);
 	}
 
-	auto const pix_fmt = static_cast<AVPixelFormat>(frame->format);
-
 	_image = make_shared<Image>(frame);
-	if (_video_range == VideoRange::VIDEO && av_pix_fmt_desc_get(pix_fmt)->flags & AV_PIX_FMT_FLAG_RGB) {
-		/* Asking for the video range to be converted by libswscale (in Image) will not work for
-		 * RGB sources since that method only processes video range in YUV and greyscale.  So we have
-		 * to do it ourselves here.
-		 */
-		_image->video_range_to_full_range();
-	}
 
 	av_packet_unref (&packet);
 	av_frame_free (&frame);
@@ -234,7 +222,6 @@ void
 FFmpegImageProxy::add_metadata (xmlpp::Node* node) const
 {
 	node->add_child("Type")->add_child_text (N_("FFmpeg"));
-	node->add_child("VideoRange")->add_child_text(video_range_to_string(_video_range));
 }
 
 void
@@ -247,7 +234,7 @@ FFmpegImageProxy::write_to_socket (shared_ptr<Socket> socket) const
 bool
 FFmpegImageProxy::same (shared_ptr<const ImageProxy> other) const
 {
-	shared_ptr<const FFmpegImageProxy> mp = dynamic_pointer_cast<const FFmpegImageProxy> (other);
+	auto mp = dynamic_pointer_cast<const FFmpegImageProxy>(other);
 	if (!mp) {
 		return false;
 	}
diff --git a/src/lib/ffmpeg_image_proxy.h b/src/lib/ffmpeg_image_proxy.h
index 92689abe8..21109c9d6 100644
--- a/src/lib/ffmpeg_image_proxy.h
+++ b/src/lib/ffmpeg_image_proxy.h
@@ -27,9 +27,9 @@
 class FFmpegImageProxy : public ImageProxy
 {
 public:
-	explicit FFmpegImageProxy (boost::filesystem::path, VideoRange video_range);
-	explicit FFmpegImageProxy (dcp::ArrayData, VideoRange video_range);
-	FFmpegImageProxy (std::shared_ptr<cxml::Node> xml, std::shared_ptr<Socket> socket);
+	explicit FFmpegImageProxy (boost::filesystem::path);
+	explicit FFmpegImageProxy (dcp::ArrayData);
+	FFmpegImageProxy (std::shared_ptr<Socket> socket);
 
 	Result image (
 		boost::optional<dcp::Size> size = boost::optional<dcp::Size> ()
@@ -45,7 +45,6 @@ public:
 
 private:
 	dcp::ArrayData _data;
-	VideoRange _video_range;
 	mutable int64_t _pos;
 	/** Path of a file that this image came from, if applicable; stored so that
 	    failed-decode errors can give more detail.
diff --git a/src/lib/image.cc b/src/lib/image.cc
index 5b926d77c..8e6c5717b 100644
--- a/src/lib/image.cc
+++ b/src/lib/image.cc
@@ -297,6 +297,15 @@ Image::crop_scale_window (
 		out->make_part_black (corner.x + cropped_size.width, out_size.width - cropped_size.width);
 	}
 
+	if (
+		video_range == VideoRange::VIDEO &&
+		out_video_range == VideoRange::FULL &&
+		av_pix_fmt_desc_get(_pixel_format)->flags & AV_PIX_FMT_FLAG_RGB
+	   ) {
+		/* libswscale will not convert video range for RGB sources, so we have to do it ourselves */
+		out->video_range_to_full_range ();
+	}
+
 	return out;
 }
 
diff --git a/src/lib/image.h b/src/lib/image.h
index 2ef7d0797..cb8f11ffc 100644
--- a/src/lib/image.h
+++ b/src/lib/image.h
@@ -80,7 +80,6 @@ public:
 	void alpha_blend (std::shared_ptr<const Image> image, Position<int> pos);
 	void copy (std::shared_ptr<const Image> image, Position<int> pos);
 	void fade (float);
-	void video_range_to_full_range ();
 
 	void read_from_socket (std::shared_ptr<Socket>);
 	void write_to_socket (std::shared_ptr<Socket>) const;
@@ -106,6 +105,7 @@ private:
 	void make_part_black (int x, int w);
 	void yuv_16_black (uint16_t, bool);
 	static uint16_t swap_16 (uint16_t);
+	void video_range_to_full_range ();
 
 	dcp::Size _size;
 	AVPixelFormat _pixel_format; ///< FFmpeg's way of describing the pixel format of this Image
diff --git a/src/lib/image_decoder.cc b/src/lib/image_decoder.cc
index 2e0a98092..e1106f86d 100644
--- a/src/lib/image_decoder.cc
+++ b/src/lib/image_decoder.cc
@@ -74,7 +74,7 @@ ImageDecoder::pass ()
 			*/
 			_image = make_shared<J2KImageProxy>(path, _image_content->video->size(), pf);
 		} else {
-			_image = make_shared<FFmpegImageProxy>(path, _image_content->video->range());
+			_image = make_shared<FFmpegImageProxy>(path);
 		}
 	}
 
diff --git a/src/lib/image_examiner.cc b/src/lib/image_examiner.cc
index acbf55696..89d1517ce 100644
--- a/src/lib/image_examiner.cc
+++ b/src/lib/image_examiner.cc
@@ -66,7 +66,7 @@ ImageExaminer::ImageExaminer (shared_ptr<const Film> film, shared_ptr<const Imag
 		}
 		delete[] buffer;
 	} else {
-		FFmpegImageProxy proxy(content->path(0), content->video->range());
+		FFmpegImageProxy proxy(content->path(0));
 		_video_size = proxy.image().image->size();
 	}
 
diff --git a/src/lib/image_proxy.cc b/src/lib/image_proxy.cc
index d81a3ffef..9e456c941 100644
--- a/src/lib/image_proxy.cc
+++ b/src/lib/image_proxy.cc
@@ -45,7 +45,7 @@ image_proxy_factory (shared_ptr<cxml::Node> xml, shared_ptr<Socket> socket)
 	if (xml->string_child("Type") == N_("Raw")) {
 		return make_shared<RawImageProxy>(xml, socket);
 	} else if (xml->string_child("Type") == N_("FFmpeg")) {
-		return shared_ptr<FFmpegImageProxy> (new FFmpegImageProxy(xml, socket));
+		return make_shared<FFmpegImageProxy>(socket);
 	} else if (xml->string_child("Type") == N_("J2K")) {
 		return make_shared<J2KImageProxy>(xml, socket);
 	}
diff --git a/src/lib/util.cc b/src/lib/util.cc
index 6286d1a65..a3c8c5082 100644
--- a/src/lib/util.cc
+++ b/src/lib/util.cc
@@ -951,7 +951,7 @@ void
 emit_subtitle_image (ContentTimePeriod period, dcp::SubtitleImage sub, dcp::Size size, shared_ptr<TextDecoder> decoder)
 {
 	/* XXX: this is rather inefficient; decoding the image just to get its size */
-	FFmpegImageProxy proxy (sub.png_image(), VideoRange::FULL);
+	FFmpegImageProxy proxy (sub.png_image());
 	auto image = proxy.image().image;
 	/* set up rect with height and width */
 	dcpomatic::Rect<double> rect(0, 0, image->size().width / double(size.width), image->size().height / double(size.height));