Fix or remove several broken pixel formats in Image::fade and add

unit tests for the remainder.  Fixes #1532.

diff --git a/src/lib/image.cc b/src/lib/image.cc
index 95cb93b65ff42c124e088385d3ba0c272d0f649b..959eec5a549732443df4f6037d3dc795ac008803 100644 (file)
--- a/src/lib/image.cc
+++ b/src/lib/image.cc
@@ -1058,49 +1058,61 @@ operator== (Image const & a, Image const & b)
 void
 Image::fade (float f)
 {
+       /* U/V black value for 8-bit colour */
+       static int const eight_bit_uv =    (1 << 7) - 1;
+       /* U/V black value for 10-bit colour */
+       static uint16_t const ten_bit_uv = (1 << 9) - 1;
+
        switch (_pixel_format) {
        case AV_PIX_FMT_YUV420P:
-       case AV_PIX_FMT_YUV422P:
-       case AV_PIX_FMT_YUV444P:
-       case AV_PIX_FMT_YUV411P:
-       case AV_PIX_FMT_YUVJ420P:
-       case AV_PIX_FMT_YUVJ422P:
-       case AV_PIX_FMT_YUVJ444P:
-       case AV_PIX_FMT_RGB24:
-       case AV_PIX_FMT_ARGB:
-       case AV_PIX_FMT_RGBA:
-       case AV_PIX_FMT_ABGR:
-       case AV_PIX_FMT_BGRA:
-       case AV_PIX_FMT_RGB555LE:
-               /* 8-bit */
-               for (int c = 0; c < 3; ++c) {
+       {
+               /* Y */
+               uint8_t* p = data()[0];
+               int const lines = sample_size(0).height;
+               for (int y = 0; y < lines; ++y) {
+                       uint8_t* q = p;
+                       for (int x = 0; x < line_size()[0]; ++x) {
+                               *q = int(float(*q) * f);
+                               ++q;
+                       }
+                       p += stride()[0];
+               }
+
+               /* U, V */
+               for (int c = 1; c < 3; ++c) {
                        uint8_t* p = data()[c];
                        int const lines = sample_size(c).height;
                        for (int y = 0; y < lines; ++y) {
                                uint8_t* q = p;
                                for (int x = 0; x < line_size()[c]; ++x) {
-                                       *q = int (float (*q) * f);
+                                       *q = eight_bit_uv + int((int(*q) - eight_bit_uv) * f);
                                        ++q;
                                }
                                p += stride()[c];
                        }
                }
+
                break;
+       }
+
+       case AV_PIX_FMT_RGB24:
+       {
+               /* 8-bit */
+               uint8_t* p = data()[0];
+               int const lines = sample_size(0).height;
+               for (int y = 0; y < lines; ++y) {
+                       uint8_t* q = p;
+                       for (int x = 0; x < line_size()[0]; ++x) {
+                               *q = int (float (*q) * f);
+                               ++q;
+                       }
+                       p += stride()[0];
+               }
+               break;
+       }
 
-       case AV_PIX_FMT_YUV422P9LE:
-       case AV_PIX_FMT_YUV444P9LE:
-       case AV_PIX_FMT_YUV422P10LE:
-       case AV_PIX_FMT_YUV444P10LE:
-       case AV_PIX_FMT_YUV422P16LE:
-       case AV_PIX_FMT_YUV444P16LE:
-       case AV_PIX_FMT_YUVA420P9LE:
-       case AV_PIX_FMT_YUVA422P9LE:
-       case AV_PIX_FMT_YUVA444P9LE:
-       case AV_PIX_FMT_YUVA420P10LE:
-       case AV_PIX_FMT_YUVA422P10LE:
-       case AV_PIX_FMT_YUVA444P10LE:
-       case AV_PIX_FMT_RGB48LE:
        case AV_PIX_FMT_XYZ12LE:
+       case AV_PIX_FMT_RGB48LE:
                /* 16-bit little-endian */
                for (int c = 0; c < 3; ++c) {
                        int const stride_pixels = stride()[c] / 2;
@@ -1118,22 +1130,26 @@ Image::fade (float f)
                }
                break;
 
-       case AV_PIX_FMT_YUV422P9BE:
-       case AV_PIX_FMT_YUV444P9BE:
-       case AV_PIX_FMT_YUV444P10BE:
-       case AV_PIX_FMT_YUV422P10BE:
-       case AV_PIX_FMT_YUVA420P9BE:
-       case AV_PIX_FMT_YUVA422P9BE:
-       case AV_PIX_FMT_YUVA444P9BE:
-       case AV_PIX_FMT_YUVA420P10BE:
-       case AV_PIX_FMT_YUVA422P10BE:
-       case AV_PIX_FMT_YUVA444P10BE:
-       case AV_PIX_FMT_YUVA420P16BE:
-       case AV_PIX_FMT_YUVA422P16BE:
-       case AV_PIX_FMT_YUVA444P16BE:
-       case AV_PIX_FMT_RGB48BE:
-               /* 16-bit big-endian */
-               for (int c = 0; c < 3; ++c) {
+       case AV_PIX_FMT_YUV422P10LE:
+       {
+               /* Y */
+               {
+                       int const stride_pixels = stride()[0] / 2;
+                       int const line_size_pixels = line_size()[0] / 2;
+                       uint16_t* p = reinterpret_cast<uint16_t*> (data()[0]);
+                       int const lines = sample_size(0).height;
+                       for (int y = 0; y < lines; ++y) {
+                               uint16_t* q = p;
+                               for (int x = 0; x < line_size_pixels; ++x) {
+                                       *q = int(float(*q) * f);
+                                       ++q;
+                               }
+                               p += stride_pixels;
+                       }
+               }
+
+               /* U, V */
+               for (int c = 1; c < 3; ++c) {
                        int const stride_pixels = stride()[c] / 2;
                        int const line_size_pixels = line_size()[c] / 2;
                        uint16_t* p = reinterpret_cast<uint16_t*> (data()[c]);
@@ -1141,7 +1157,7 @@ Image::fade (float f)
                        for (int y = 0; y < lines; ++y) {
                                uint16_t* q = p;
                                for (int x = 0; x < line_size_pixels; ++x) {
-                                       *q = swap_16 (int (float (swap_16 (*q)) * f));
+                                       *q = ten_bit_uv + int((int(*q) - ten_bit_uv) * f);
                                        ++q;
                                }
                                p += stride_pixels;
@@ -1149,18 +1165,6 @@ Image::fade (float f)
                }
                break;
 
-       case AV_PIX_FMT_UYVY422:
-       {
-               int const Y = sample_size(0).height;
-               int const X = line_size()[0];
-               uint8_t* p = data()[0];
-               for (int y = 0; y < Y; ++y) {
-                       for (int x = 0; x < X; ++x) {
-                               *p = int (float (*p) * f);
-                               ++p;
-                       }
-               }
-               break;
        }
 
        default:

diff --git a/test/data b/test/data
index aa4953ed0621b3867f490b6179662cd665940a6f..e5257e5721bde4d182f8317373030a1d99fbff93 160000 (submodule)
--- a/test/data
+++ b/test/data
@@ -1 +1 @@
-Subproject commit aa4953ed0621b3867f490b6179662cd665940a6f
+Subproject commit e5257e5721bde4d182f8317373030a1d99fbff93

diff --git a/test/image_test.cc b/test/image_test.cc
index 2bbe9d14b1e5c789031a8c52a69758287891bc3c..8a7feb1267fa0761e46457e188ea9cdd68de2e62 100644 (file)
--- a/test/image_test.cc
+++ b/test/image_test.cc
@@ -278,3 +278,52 @@ BOOST_AUTO_TEST_CASE (as_png_test)
        check_image ("test/data/3d_test/000001.png", "build/test/as_png_rgb.png");
        check_image ("test/data/3d_test/000001.png", "build/test/as_png_bgr.png");
 }
+
+/* Very dumb test to fade black to make sure it stays black */
+static void
+fade_test_format_black (AVPixelFormat f, string name)
+{
+       Image yuv (f, dcp::Size(640, 480), true);
+       yuv.make_black ();
+       yuv.fade (0);
+       string const filename = "fade_test_black_" + name + ".png";
+       yuv.convert_pixel_format(dcp::YUV_TO_RGB_REC709, AV_PIX_FMT_RGBA, true, false)->as_png().write("build/test/" + filename);
+       check_image ("test/data/" + filename, "build/test/" + filename);
+}
+
+/* Fade red to make sure it stays red */
+static void
+fade_test_format_red (AVPixelFormat f, float amount, string name)
+{
+       shared_ptr<FFmpegImageProxy> proxy(new FFmpegImageProxy("test/data/flat_red.png"));
+       shared_ptr<Image> red = proxy->image().first->convert_pixel_format(dcp::YUV_TO_RGB_REC709, f, true, false);
+       red->fade (amount);
+       string const filename = "fade_test_red_" + name + ".png";
+       red->convert_pixel_format(dcp::YUV_TO_RGB_REC709, AV_PIX_FMT_RGBA, true, false)->as_png().write("build/test/" + filename);
+       check_image ("test/data/" + filename, "build/test/" + filename);
+}
+
+BOOST_AUTO_TEST_CASE (fade_test)
+{
+       fade_test_format_black (AV_PIX_FMT_YUV420P,   "yuv420p");
+       fade_test_format_black (AV_PIX_FMT_YUV422P10, "yuv422p10");
+       fade_test_format_black (AV_PIX_FMT_RGB24,     "rgb24");
+       fade_test_format_black (AV_PIX_FMT_XYZ12LE,   "xyz12le");
+       fade_test_format_black (AV_PIX_FMT_RGB48LE,   "rgb48le");
+
+       fade_test_format_red   (AV_PIX_FMT_YUV420P,   0,   "yuv420p_0");
+       fade_test_format_red   (AV_PIX_FMT_YUV420P,   0.5, "yuv420p_50");
+       fade_test_format_red   (AV_PIX_FMT_YUV420P,   1,   "yuv420p_100");
+       fade_test_format_red   (AV_PIX_FMT_YUV422P10, 0,   "yuv422p10_0");
+       fade_test_format_red   (AV_PIX_FMT_YUV422P10, 0.5, "yuv422p10_50");
+       fade_test_format_red   (AV_PIX_FMT_YUV422P10, 1,   "yuv422p10_100");
+       fade_test_format_red   (AV_PIX_FMT_RGB24,     0,   "rgb24_0");
+       fade_test_format_red   (AV_PIX_FMT_RGB24,     0.5, "rgb24_50");
+       fade_test_format_red   (AV_PIX_FMT_RGB24,     1,   "rgb24_100");
+       fade_test_format_red   (AV_PIX_FMT_XYZ12LE,   0,   "xyz12le_0");
+       fade_test_format_red   (AV_PIX_FMT_XYZ12LE,   0.5, "xyz12le_50");
+       fade_test_format_red   (AV_PIX_FMT_XYZ12LE,   1,   "xyz12le_100");
+       fade_test_format_red   (AV_PIX_FMT_RGB48LE,   0,   "rgb48le_0");
+       fade_test_format_red   (AV_PIX_FMT_RGB48LE,   0.5, "rgb48le_50");
+       fade_test_format_red   (AV_PIX_FMT_RGB48LE,   1,   "rgb48le_100");
+}