From 11efa93e15b694b8ea6f0a2bc68c87503cc570bb Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Thu, 25 Apr 2019 16:19:25 +0100 Subject: [PATCH] Fix or remove several broken pixel formats in Image::fade and add unit tests for the remainder. Fixes #1532. --- src/lib/image.cc | 118 +++++++++++++++++++++++---------------------- test/data | 2 +- test/image_test.cc | 49 +++++++++++++++++++ 3 files changed, 111 insertions(+), 58 deletions(-) diff --git a/src/lib/image.cc b/src/lib/image.cc index 95cb93b65..959eec5a5 100644 --- 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 (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 (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 aa4953ed0..e5257e572 160000 --- 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 2bbe9d14b..8a7feb126 100644 --- 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 proxy(new FFmpegImageProxy("test/data/flat_red.png")); + shared_ptr 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"); +} -- 2.30.2