Clamp out-of-range XYZ values in xyz_to_rgb() and pass notes about their existance.

diff --git a/src/rgb_xyz.cc b/src/rgb_xyz.cc
index a3f7b424d49e7c5a7300581c8a646733febc8fd3..a839dfa00a8b32f8591a726412dc9d7cf5325fae 100644 (file)
--- 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 <cmath>
 
 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<const XYZFrame> xyz_frame,
        ColourConversion const & conversion,
-       uint16_t* buffer
+       uint16_t* buffer,
+       optional<NoteHandler> 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 4dd25b282f3f43c1d17930f4f5efc801059ed23e..53568350973a201984ac674b9c99c2c57968f4e1 100644 (file)
--- a/src/rgb_xyz.h
+++ b/src/rgb_xyz.h
@@ -17,7 +17,9 @@
 
 */
 
+#include "types.h"
 #include <boost/shared_ptr.hpp>
+#include <boost/optional.hpp>
 #include <stdint.h>
 
 namespace dcp {
@@ -28,7 +30,12 @@ class Image;
 class ColourConversion;
        
 extern boost::shared_ptr<ARGBFrame> xyz_to_rgba (boost::shared_ptr<const XYZFrame>, ColourConversion const & conversion);
-extern void xyz_to_rgb (boost::shared_ptr<const XYZFrame>, ColourConversion const & conversion, uint16_t* buffer);
+extern void xyz_to_rgb (
+       boost::shared_ptr<const XYZFrame>,
+       ColourConversion const & conversion,
+       uint16_t* buffer,
+       boost::optional<NoteHandler> note = boost::optional<NoteHandler> ()
+       );
 extern boost::shared_ptr<XYZFrame> rgb_to_xyz (boost::shared_ptr<const Image>, ColourConversion const & conversion);
 extern boost::shared_ptr<XYZFrame> xyz_to_xyz (boost::shared_ptr<const Image>);
        

diff --git a/test/rgb_xyz_test.cc b/test/rgb_xyz_test.cc
index 2c76bb332c448ca3e727846a789a78ea7929258a..75e0d8b929f3f2fea507a46b6e17ec17f77ce3da 100644 (file)
--- 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 <boost/test/unit_test.hpp>
+#include <boost/bind.hpp>
 
 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<string> 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<dcp::XYZFrame> 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<dcp::NoteHandler> (boost::bind (&note_handler, _1, _2)));
+
+       /* The 6 out-of-range samples should have been noted */
+       BOOST_REQUIRE_EQUAL (notes.size(), 6);
+       list<string>::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]);
+}