From 107482fc19404e20db544e989d880752377fde26 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Wed, 7 Jan 2015 10:55:47 +0000 Subject: [PATCH] Clamp out-of-range XYZ values in xyz_to_rgb() and pass notes about their existance. --- src/rgb_xyz.cc | 38 +++++++++++++++++++++++----- src/rgb_xyz.h | 9 ++++++- test/rgb_xyz_test.cc | 59 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 7 deletions(-) diff --git a/src/rgb_xyz.cc b/src/rgb_xyz.cc index a3f7b424..a839dfa0 100644 --- a/src/rgb_xyz.cc +++ b/src/rgb_xyz.cc @@ -25,11 +25,13 @@ #include "colour_conversion.h" #include "transfer_function.h" #include "dcp_assert.h" +#include "compose.hpp" #include using std::min; using std::max; using boost::shared_ptr; +using boost::optional; using namespace dcp; #define DCI_COEFFICIENT (48.0 / 52.37) @@ -119,7 +121,8 @@ void dcp::xyz_to_rgb ( boost::shared_ptr xyz_frame, ColourConversion const & conversion, - uint16_t* buffer + uint16_t* buffer, + optional note ) { struct { @@ -143,12 +146,35 @@ dcp::xyz_to_rgb ( uint16_t* buffer_line = buffer; for (int x = 0; x < xyz_frame->size().width; ++x) { - DCP_ASSERT (*xyz_x >= 0 && *xyz_y >= 0 && *xyz_z >= 0 && *xyz_x < 4096 && *xyz_y < 4096 && *xyz_z < 4096); - + int cx = *xyz_x++; + int cy = *xyz_y++; + int cz = *xyz_z++; + + if (cx < 0 || cx > 4095) { + if (note) { + note.get() (DCP_NOTE, String::compose ("XYZ value %1 out of range", cx)); + } + cx = max (min (cx, 4095), 0); + } + + if (cy < 0 || cy > 4095) { + if (note) { + note.get() (DCP_NOTE, String::compose ("XYZ value %1 out of range", cy)); + } + cy = max (min (cy, 4095), 0); + } + + if (cz < 0 || cz > 4095) { + if (note) { + note.get() (DCP_NOTE, String::compose ("XYZ value %1 out of range", cz)); + } + cz = max (min (cz, 4095), 0); + } + /* In gamma LUT */ - s.x = lut_in[*xyz_x++]; - s.y = lut_in[*xyz_y++]; - s.z = lut_in[*xyz_z++]; + s.x = lut_in[cx]; + s.y = lut_in[cy]; + s.z = lut_in[cz]; /* DCI companding */ s.x /= DCI_COEFFICIENT; diff --git a/src/rgb_xyz.h b/src/rgb_xyz.h index 4dd25b28..53568350 100644 --- a/src/rgb_xyz.h +++ b/src/rgb_xyz.h @@ -17,7 +17,9 @@ */ +#include "types.h" #include +#include #include namespace dcp { @@ -28,7 +30,12 @@ class Image; class ColourConversion; extern boost::shared_ptr xyz_to_rgba (boost::shared_ptr, ColourConversion const & conversion); -extern void xyz_to_rgb (boost::shared_ptr, ColourConversion const & conversion, uint16_t* buffer); +extern void xyz_to_rgb ( + boost::shared_ptr, + ColourConversion const & conversion, + uint16_t* buffer, + boost::optional note = boost::optional () + ); extern boost::shared_ptr rgb_to_xyz (boost::shared_ptr, ColourConversion const & conversion); extern boost::shared_ptr xyz_to_xyz (boost::shared_ptr); diff --git a/test/rgb_xyz_test.cc b/test/rgb_xyz_test.cc index 2c76bb33..75e0d8b9 100644 --- a/test/rgb_xyz_test.cc +++ b/test/rgb_xyz_test.cc @@ -22,9 +22,13 @@ #include "xyz_frame.h" #include "colour_conversion.h" #include +#include using std::max; +using std::list; +using std::string; using boost::shared_ptr; +using boost::optional; class SimpleImage : public dcp::Image { @@ -127,3 +131,58 @@ BOOST_AUTO_TEST_CASE (rgb_xyz_test) } } } + +static list notes; + +static void +note_handler (dcp::NoteType n, string s) +{ + BOOST_REQUIRE_EQUAL (n, dcp::DCP_NOTE); + notes.push_back (s); +} + +/** Check that xyz_to_rgb clamps XYZ values correctly */ +BOOST_AUTO_TEST_CASE (xyz_rgb_range_test) +{ + shared_ptr xyz (new dcp::XYZFrame (dcp::Size (2, 2))); + + xyz->data(0)[0] = -4; + xyz->data(0)[1] = 6901; + xyz->data(0)[2] = 0; + xyz->data(0)[3] = 4095; + xyz->data(1)[0] = -4; + xyz->data(1)[1] = 6901; + xyz->data(1)[2] = 0; + xyz->data(1)[3] = 4095; + xyz->data(2)[0] = -4; + xyz->data(2)[1] = 6901; + xyz->data(2)[2] = 0; + xyz->data(2)[3] = 4095; + + uint16_t buffer[12]; + + notes.clear (); + dcp::xyz_to_rgb (xyz, dcp::ColourConversion::xyz_to_srgb (), buffer, boost::optional (boost::bind (¬e_handler, _1, _2))); + + /* The 6 out-of-range samples should have been noted */ + BOOST_REQUIRE_EQUAL (notes.size(), 6); + list::const_iterator i = notes.begin (); + BOOST_REQUIRE_EQUAL (*i++, "XYZ value -4 out of range"); + BOOST_REQUIRE_EQUAL (*i++, "XYZ value -4 out of range"); + BOOST_REQUIRE_EQUAL (*i++, "XYZ value -4 out of range"); + BOOST_REQUIRE_EQUAL (*i++, "XYZ value 6901 out of range"); + BOOST_REQUIRE_EQUAL (*i++, "XYZ value 6901 out of range"); + BOOST_REQUIRE_EQUAL (*i++, "XYZ value 6901 out of range"); + + /* And those samples should have been clamped, so check that they give the same result + as inputs at the extremes (0 and 4095). + */ + + BOOST_REQUIRE_EQUAL (buffer[0 * 3 + 0], buffer[2 * 3 + 1]); + BOOST_REQUIRE_EQUAL (buffer[0 * 3 + 1], buffer[2 * 3 + 1]); + BOOST_REQUIRE_EQUAL (buffer[0 * 3 + 2], buffer[2 * 3 + 2]); + + BOOST_REQUIRE_EQUAL (buffer[1 * 3 + 0], buffer[3 * 3 + 0]); + BOOST_REQUIRE_EQUAL (buffer[1 * 3 + 1], buffer[3 * 3 + 1]); + BOOST_REQUIRE_EQUAL (buffer[1 * 3 + 2], buffer[3 * 3 + 2]); +} -- 2.30.2