Replace crop() with actual_crop and requested_crop()yuv-cropping

This means that the crop rounding we do is reported properly,
not done behind the user's back.

6 files changed, 46 insertions, 28 deletions

diff --git a/src/lib/player.cc b/src/lib/player.cc
index 5de089ba9..a24706ff0 100644
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -922,7 +922,7 @@ Player::video (weak_ptr<Piece> wp, ContentVideo video)
 
 	_last_video[wp] = std::make_shared<PlayerVideo>(
 		video.image,
-		piece->content->video->crop (),
+		piece->content->video->actual_crop(),
 		piece->content->video->fade (_film, video.frame),
 		scale_for_display(piece->content->video->scaled_size(_film->frame_size()), _video_container_size, _film->frame_size()),
 		_video_container_size,
diff --git a/src/lib/player_video.cc b/src/lib/player_video.cc
index b0e75972c..4285bba7c 100644
--- a/src/lib/player_video.cc
+++ b/src/lib/player_video.cc
@@ -336,7 +336,7 @@ PlayerVideo::reset_metadata (shared_ptr<const Film> film, dcp::Size player_video
 		return false;
 	}
 
-	_crop = content->video->crop();
+	_crop = content->video->actual_crop();
 	_fade = content->video->fade(film, _video_frame.get());
 	_inter_size = scale_for_display(content->video->scaled_size(film->frame_size()), player_video_container_size, film->frame_size());
 	_out_size = player_video_container_size;
diff --git a/src/lib/video_content.cc b/src/lib/video_content.cc
index c3e28fe09..c7764953c 100644
--- a/src/lib/video_content.cc
+++ b/src/lib/video_content.cc
@@ -222,7 +222,7 @@ VideoContent::VideoContent (Content* parent, vector<shared_ptr<Content>> c)
 			throw JoinError (_("Content to be joined must have the same video frame type."));
 		}
 
-		if (c[i]->video->crop() != ref->crop()) {
+		if (c[i]->video->requested_crop() != ref->requested_crop()) {
 			throw JoinError (_("Content to be joined must have the same crop."));
 		}
 
@@ -259,7 +259,7 @@ VideoContent::VideoContent (Content* parent, vector<shared_ptr<Content>> c)
 	_use = ref->use ();
 	_size = ref->size ();
 	_frame_type = ref->frame_type ();
-	_crop = ref->crop ();
+	_crop = ref->requested_crop ();
 	_custom_ratio = ref->custom_ratio ();
 	_colour_conversion = ref->colour_conversion ();
 	_fade_in = ref->fade_in ();
@@ -345,10 +345,10 @@ VideoContent::identifier () const
 	snprintf (
 		buffer, sizeof(buffer), "%d_%d_%d_%d_%d_%f_%d_%d%" PRId64 "_%" PRId64 "_%d",
 		(_use ? 1 : 0),
-		crop().left,
-		crop().right,
-		crop().top,
-		crop().bottom,
+		actual_crop().left,
+		actual_crop().right,
+		actual_crop().top,
+		actual_crop().bottom,
 		_custom_ratio.get_value_or(0),
 		_custom_size ? _custom_size->width : 0,
 		_custom_size ? _custom_size->height : 0,
@@ -407,7 +407,7 @@ VideoContent::size_after_3d_split () const
 dcp::Size
 VideoContent::size_after_crop () const
 {
-	return crop().apply (size_after_3d_split ());
+	return actual_crop().apply(size_after_3d_split());
 }
 
 
@@ -460,8 +460,10 @@ VideoContent::processing_description (shared_ptr<const Film> film)
 		d += buffer;
 	}
 
-	if ((crop().left || crop().right || crop().top || crop().bottom) && size() != dcp::Size (0, 0)) {
-		dcp::Size cropped = size_after_crop ();
+	auto const crop = actual_crop();
+
+	if ((crop.left || crop.right || crop.top || crop.bottom) && size() != dcp::Size(0, 0)) {
+		auto const cropped = size_after_crop();
 		d += String::compose (
 			_("\nCropped to %1x%2"),
 			cropped.width, cropped.height
@@ -676,3 +678,16 @@ VideoContent::set_custom_size (optional<dcp::Size> size)
 {
 	maybe_set (_custom_size, size, VideoContentProperty::CUSTOM_SIZE);
 }
+
+
+Crop
+VideoContent::actual_crop () const
+{
+	return Crop(
+		round_to_log_2(_crop.left, _log2_chroma_width),
+		round_to_log_2(_crop.right, _log2_chroma_width),
+		round_to_log_2(_crop.top, _log2_chroma_height),
+		round_to_log_2(_crop.bottom, _log2_chroma_height)
+		);
+}
+
diff --git a/src/lib/video_content.h b/src/lib/video_content.h
index 31346ffcd..d670f72c8 100644
--- a/src/lib/video_content.h
+++ b/src/lib/video_content.h
@@ -112,27 +112,29 @@ public:
 		return _frame_type;
 	}
 
-	Crop crop () const {
+	Crop actual_crop () const;
+
+	Crop requested_crop () const {
 		boost::mutex::scoped_lock lm (_mutex);
 		return _crop;
 	}
 
-	int left_crop () const {
+	int requested_left_crop () const {
 		boost::mutex::scoped_lock lm (_mutex);
 		return _crop.left;
 	}
 
-	int right_crop () const {
+	int requested_right_crop () const {
 		boost::mutex::scoped_lock lm (_mutex);
 		return _crop.right;
 	}
 
-	int top_crop () const {
+	int requested_top_crop () const {
 		boost::mutex::scoped_lock lm (_mutex);
 		return _crop.top;
 	}
 
-	int bottom_crop () const {
+	int requested_bottom_crop () const {
 		boost::mutex::scoped_lock lm (_mutex);
 		return _crop.bottom;
 	}
@@ -227,6 +229,7 @@ private:
 	boost::optional<ColourConversion> _colour_conversion;
 	dcp::Size _size;
 	VideoFrameType _frame_type;
+	/** crop that the user has asked for; it may be rounded if we are cropping a subsampled source */
 	Crop _crop;
 	/** ratio to scale cropped image to (or none to guess); i.e. if set, scale to _custom_ratio:1 */
 	boost::optional<float> _custom_ratio;
diff --git a/src/tools/dcpomatic_cli.cc b/src/tools/dcpomatic_cli.cc
index 59d85d4c4..75f57fa89 100644
--- a/src/tools/dcpomatic_cli.cc
+++ b/src/tools/dcpomatic_cli.cc
@@ -92,10 +92,10 @@ print_dump (shared_ptr<Film> film)
 		if (c->video) {
 			cout << "\t" << c->video->size().width << "x" << c->video->size().height << "\n"
 			     << "\t" << c->active_video_frame_rate(film) << "fps\n"
-			     << "\tcrop left " << c->video->left_crop()
-			     << " right " << c->video->right_crop()
-			     << " top " << c->video->top_crop()
-			     << " bottom " << c->video->bottom_crop() << "\n";
+			     << "\tcrop left " << c->video->requested_left_crop()
+			     << " right " << c->video->requested_right_crop()
+			     << " top " << c->video->requested_top_crop()
+			     << " bottom " << c->video->requested_bottom_crop() << "\n";
 			if (c->video->custom_ratio()) {
 				cout << "\tscale to custom ratio " << *c->video->custom_ratio() << ":1\n";
 			}
diff --git a/src/wx/video_panel.cc b/src/wx/video_panel.cc
index 33961ea1d..f81b63aac 100644
--- a/src/wx/video_panel.cc
+++ b/src/wx/video_panel.cc
@@ -118,7 +118,7 @@ VideoPanel::create ()
 		new wxSpinCtrl (this, wxID_ANY, wxEmptyString, wxDefaultPosition, wxSize(crop_width, -1)),
 		VideoContentProperty::CROP,
 		&Content::video,
-		boost::mem_fn (&VideoContent::left_crop),
+		boost::mem_fn (&VideoContent::requested_left_crop),
 		boost::mem_fn (&VideoContent::set_left_crop),
 		boost::bind (&VideoPanel::left_crop_changed, this)
 		);
@@ -132,7 +132,7 @@ VideoPanel::create ()
 		new wxSpinCtrl (this, wxID_ANY, wxEmptyString, wxDefaultPosition, wxSize(crop_width, -1)),
 		VideoContentProperty::CROP,
 		&Content::video,
-		boost::mem_fn (&VideoContent::right_crop),
+		boost::mem_fn (&VideoContent::requested_right_crop),
 		boost::mem_fn (&VideoContent::set_right_crop),
 		boost::bind (&VideoPanel::right_crop_changed, this)
 		);
@@ -143,7 +143,7 @@ VideoPanel::create ()
 		new wxSpinCtrl (this, wxID_ANY, wxEmptyString, wxDefaultPosition, wxSize(crop_width, -1)),
 		VideoContentProperty::CROP,
 		&Content::video,
-		boost::mem_fn (&VideoContent::top_crop),
+		boost::mem_fn (&VideoContent::requested_top_crop),
 		boost::mem_fn (&VideoContent::set_top_crop),
 		boost::bind (&VideoPanel::top_crop_changed, this)
 		);
@@ -157,7 +157,7 @@ VideoPanel::create ()
 		new wxSpinCtrl (this, wxID_ANY, wxEmptyString, wxDefaultPosition, wxSize(crop_width, -1)),
 		VideoContentProperty::CROP,
 		&Content::video,
-		boost::mem_fn (&VideoContent::bottom_crop),
+		boost::mem_fn (&VideoContent::requested_bottom_crop),
 		boost::mem_fn (&VideoContent::set_bottom_crop),
 		boost::bind (&VideoPanel::bottom_crop_changed, this)
 		);
@@ -759,7 +759,7 @@ VideoPanel::left_crop_changed ()
 	_left_changed_last = true;
 	if (_left_right_link->GetValue()) {
 		for (auto i: _parent->selected_video()) {
-			i->video->set_right_crop (i->video->left_crop());
+			i->video->set_right_crop (i->video->requested_left_crop());
 		}
 	}
 }
@@ -771,7 +771,7 @@ VideoPanel::right_crop_changed ()
 	_left_changed_last = false;
 	if (_left_right_link->GetValue()) {
 		for (auto i: _parent->selected_video()) {
-			i->video->set_left_crop (i->video->right_crop());
+			i->video->set_left_crop (i->video->requested_right_crop());
 		}
 	}
 }
@@ -783,7 +783,7 @@ VideoPanel::top_crop_changed ()
 	_top_changed_last = true;
 	if (_top_bottom_link->GetValue()) {
 		for (auto i: _parent->selected_video()) {
-			i->video->set_bottom_crop (i->video->top_crop());
+			i->video->set_bottom_crop (i->video->requested_top_crop());
 		}
 	}
 }
@@ -795,7 +795,7 @@ VideoPanel::bottom_crop_changed ()
 	_top_changed_last = false;
 	if (_top_bottom_link->GetValue()) {
 		for (auto i: _parent->selected_video()) {
-			i->video->set_top_crop (i->video->bottom_crop());
+			i->video->set_top_crop (i->video->requested_bottom_crop());
 		}
 	}
 }