Fix cropping of YUV images by amounts that are finer than the U/V resolution (and add a test for that).

2 files changed, 35 insertions, 1 deletions

diff --git a/src/lib/image.cc b/src/lib/image.cc
index 0eabbe84d..ac30f4ff0 100644
--- a/src/lib/image.cc
+++ b/src/lib/image.cc
@@ -214,7 +214,11 @@ Image::crop (Crop crop, bool aligned) const
 
 	for (int c = 0; c < components(); ++c) {
 		int const crop_left_in_bytes = bytes_per_pixel(c) * crop.left;
-		int const cropped_width_in_bytes = bytes_per_pixel(c) * cropped_size.width;
+		/* bytes_per_pixel() could be a fraction; in this case the stride will be rounded
+		   up, and we need to make sure that we copy over the width (up to the stride)
+		   rather than short of the width; hence the ceil() here.
+		*/
+		int const cropped_width_in_bytes = ceil (bytes_per_pixel(c) * cropped_size.width);
 
 		/* Start of the source line, cropped from the top but not the left */
 		uint8_t* in_p = data()[c] + (crop.top / out->line_factor(c)) * stride()[c];
diff --git a/test/image_test.cc b/test/image_test.cc
index 9dd3a1ba4..b74531c46 100644
--- a/test/image_test.cc
+++ b/test/image_test.cc
@@ -107,3 +107,33 @@ BOOST_AUTO_TEST_CASE (crop_image_test)
 	crop.top = 3;
 	image->crop (crop, false);
 }
+
+/* Test cropping of a YUV 4:2:0 image by 1 pixel, which used to fail because
+   the U/V copying was not rounded up to the next sample.
+*/
+BOOST_AUTO_TEST_CASE (crop_image_test2)
+{
+	/* Here's a 1998 x 1080 image which is black */
+	shared_ptr<Image> image (new SimpleImage (PIX_FMT_YUV420P, libdcp::Size (1998, 1080), true));
+	image->make_black ();
+
+	/* Crop it by 1 pixel */
+	Crop crop;
+	crop.left = 1;
+	image = image->crop (crop, true);
+
+	/* Convert it back to RGB to make comparison to black easier */
+	image = image->scale_and_convert_to_rgb (image->size(), Scaler::from_id ("bicubic"), true);
+
+	/* Check that its still black after the crop */
+	uint8_t* p = image->data()[0];
+	for (int y = 0; y < image->size().height; ++y) {
+		uint8_t* q = p;
+		for (int x = 0; x < image->size().width; ++x) {
+			BOOST_CHECK_EQUAL (*q++, 0);
+			BOOST_CHECK_EQUAL (*q++, 0);
+			BOOST_CHECK_EQUAL (*q++, 0);
+		}
+		p += image->stride()[0];
+	}
+}