Skip to content

Commit

Permalink
Fix assert in CMS stage (#3379)
Browse files Browse the repository at this point in the history 
Also transform only as many pixels as needed.

Fixes #3348.
  • Loading branch information
@sboukortt
sboukortt committed Mar 5, 2024
1 parent 4255902 commit 05fb8f0
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 19 deletions.
2 changes: 1 addition & 1 deletion lib/extras/enc/jpegli.cc
View file
Expand Up @@ -480,7 +480,7 @@ Status EncodeJpeg(const PackedPixelFile& ppf, const JpegSettings& jpeg_settings,
ToFloatRow(&pixels[y * image.stride], image.format, 3 * image.xsize,
src_buf);
// convert to linear srgb
if (!c_transform.Run(0, src_buf, dst_buf)) {
if (!c_transform.Run(0, src_buf, dst_buf, image.xsize)) {
return false;
}
// deinterleave channels
Expand Down
7 changes: 3 additions & 4 deletions lib/jxl/color_encoding_internal.h
View file
Expand Up @@ -314,7 +314,6 @@ class ColorSpaceTransform {

Status Init(const ColorEncoding& c_src, const ColorEncoding& c_dst,
float intensity_target, size_t xsize, size_t num_threads) {
xsize_ = xsize;
JxlColorProfile input_profile;
icc_src_ = c_src.ICC();
input_profile.icc.data = icc_src_.data();
Expand Down Expand Up @@ -343,9 +342,10 @@ class ColorSpaceTransform {
return cms_.get_dst_buf(cms_data_, thread);
}

Status Run(const size_t thread, const float* buf_src, float* buf_dst) {
Status Run(const size_t thread, const float* buf_src, float* buf_dst,
size_t xsize) {
// TODO(eustas): convert false to Status?
return FROM_JXL_BOOL(cms_.run(cms_data_, thread, buf_src, buf_dst, xsize_));
return FROM_JXL_BOOL(cms_.run(cms_data_, thread, buf_src, buf_dst, xsize));
}

private:
Expand All @@ -354,7 +354,6 @@ class ColorSpaceTransform {
// The interface may retain pointers into these.
IccBytes icc_src_;
IccBytes icc_dst_;
size_t xsize_;
};

} // namespace jxl
Expand Down
22 changes: 12 additions & 10 deletions lib/jxl/color_management_test.cc
View file
Expand Up @@ -182,8 +182,10 @@ class ColorManagementTest
const size_t thread = 0;
const ImageF& in = c.IsGray() ? g->in_gray : g->in_color;
ImageF* JXL_RESTRICT out = c.IsGray() ? &g->out_gray : &g->out_color;
ASSERT_TRUE(xform_fwd.Run(thread, in.Row(0), xform_fwd.BufDst(thread)));
ASSERT_TRUE(xform_rev.Run(thread, xform_fwd.BufDst(thread), out->Row(0)));
ASSERT_TRUE(
xform_fwd.Run(thread, in.Row(0), xform_fwd.BufDst(thread), kWidth));
ASSERT_TRUE(
xform_rev.Run(thread, xform_fwd.BufDst(thread), out->Row(0), kWidth));

// With lcms2, this value is lower: 5E-5
double max_l1 = 7E-4;
Expand Down Expand Up @@ -267,7 +269,7 @@ TEST_F(ColorManagementTest, D2700ToSRGB) {
kDefaultIntensityTarget, 1, 1));
const float sRGB_D2700_values[3] = {0.863, 0.737, 0.490};
float sRGB_values[3];
ASSERT_TRUE(transform.Run(0, sRGB_D2700_values, sRGB_values));
ASSERT_TRUE(transform.Run(0, sRGB_D2700_values, sRGB_values, 1));
EXPECT_THAT(sRGB_values,
ElementsAre(FloatNear(0.914, 1e-3), FloatNear(0.745, 1e-3),
FloatNear(0.601, 1e-3)));
Expand All @@ -289,7 +291,7 @@ TEST_F(ColorManagementTest, P3HlgTo2020Hlg) {
ASSERT_TRUE(transform.Init(p3_hlg, rec2020_hlg, 1000, 1, 1));
const float p3_hlg_values[3] = {0., 0.75, 0.};
float rec2020_hlg_values[3];
ASSERT_TRUE(transform.Run(0, p3_hlg_values, rec2020_hlg_values));
ASSERT_TRUE(transform.Run(0, p3_hlg_values, rec2020_hlg_values, 1));
EXPECT_THAT(rec2020_hlg_values,
ElementsAre(FloatNear(0.3973, 1e-4), FloatNear(0.7382, 1e-4),
FloatNear(0.1183, 1e-4)));
Expand All @@ -309,7 +311,7 @@ TEST_F(ColorManagementTest, HlgOotf) {
// HDR reference white: https://www.itu.int/pub/R-REP-BT.2408-4-2021
float p3_hlg_values[3] = {0.75, 0.75, 0.75};
float linear_srgb_values[3];
ASSERT_TRUE(transform_to_1000.Run(0, p3_hlg_values, linear_srgb_values));
ASSERT_TRUE(transform_to_1000.Run(0, p3_hlg_values, linear_srgb_values, 1));
// On a 1000-nit display, HDR reference white should be 203 cd/m² which is
// 0.203 times the maximum.
EXPECT_THAT(linear_srgb_values,
Expand All @@ -319,14 +321,14 @@ TEST_F(ColorManagementTest, HlgOotf) {
ColorSpaceTransform transform_to_400(*JxlGetDefaultCms());
ASSERT_TRUE(
transform_to_400.Init(p3_hlg, ColorEncoding::LinearSRGB(), 400, 1, 1));
ASSERT_TRUE(transform_to_400.Run(0, p3_hlg_values, linear_srgb_values));
ASSERT_TRUE(transform_to_400.Run(0, p3_hlg_values, linear_srgb_values, 1));
// On a 400-nit display, it should be 100 cd/m².
EXPECT_THAT(linear_srgb_values,
ElementsAre(FloatNear(0.250, 1e-3), FloatNear(0.250, 1e-3),
FloatNear(0.250, 1e-3)));

p3_hlg_values[2] = 0.50;
ASSERT_TRUE(transform_to_1000.Run(0, p3_hlg_values, linear_srgb_values));
ASSERT_TRUE(transform_to_1000.Run(0, p3_hlg_values, linear_srgb_values, 1));
EXPECT_THAT(linear_srgb_values,
ElementsAre(FloatNear(0.201, 1e-3), FloatNear(0.201, 1e-3),
FloatNear(0.050, 1e-3)));
Expand All @@ -335,7 +337,7 @@ TEST_F(ColorManagementTest, HlgOotf) {
ASSERT_TRUE(
transform_from_400.Init(ColorEncoding::LinearSRGB(), p3_hlg, 400, 1, 1));
linear_srgb_values[0] = linear_srgb_values[1] = linear_srgb_values[2] = 0.250;
ASSERT_TRUE(transform_from_400.Run(0, linear_srgb_values, p3_hlg_values));
ASSERT_TRUE(transform_from_400.Run(0, linear_srgb_values, p3_hlg_values, 1));
EXPECT_THAT(p3_hlg_values,
ElementsAre(FloatNear(0.75, 1e-3), FloatNear(0.75, 1e-3),
FloatNear(0.75, 1e-3)));
Expand All @@ -352,7 +354,7 @@ TEST_F(ColorManagementTest, HlgOotf) {
const float grayscale_hlg_value = 0.75;
float linear_grayscale_value;
ASSERT_TRUE(grayscale_transform.Run(0, &grayscale_hlg_value,
&linear_grayscale_value));
&linear_grayscale_value, 1));
EXPECT_THAT(linear_grayscale_value, FloatNear(0.203, 1e-3));
}

Expand Down Expand Up @@ -401,7 +403,7 @@ TEST_F(ColorManagementTest, XYBProfile) {
}

float* dst = xform.BufDst(0);
ASSERT_TRUE(xform.Run(0, src, dst));
ASSERT_TRUE(xform.Run(0, src, dst, kNumColors));

JXL_ASSIGN_OR_DIE(Image3F out, Image3F::Create(kNumColors, 1));
for (size_t i = 0; i < kNumColors; ++i) {
Expand Down
2 changes: 1 addition & 1 deletion lib/jxl/enc_image_bundle.cc
View file
Expand Up @@ -72,7 +72,7 @@ Status ApplyColorTransform(const ColorEncoding& c_current,
}
}
float* JXL_RESTRICT dst_buf = c_transform.BufDst(thread);
if (!c_transform.Run(thread, src_buf, dst_buf)) {
if (!c_transform.Run(thread, src_buf, dst_buf, rect.xsize())) {
has_error = true;
return;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/jxl/enc_xyb.cc
View file
Expand Up @@ -245,7 +245,7 @@ StatusOr<Image3F> TransformToLinearRGB(const Image3F& in,
}
}
float* JXL_RESTRICT dst_buf = c_transform.BufDst(thread);
if (!c_transform.Run(thread, src_buf, dst_buf)) {
if (!c_transform.Run(thread, src_buf, dst_buf, in.xsize())) {
has_error = true;
return;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/jxl/render_pipeline/stage_cms.cc
View file
Expand Up @@ -44,7 +44,7 @@ class CmsStage : public RenderPipelineStage {
Status ProcessRow(const RowInfo& input_rows, const RowInfo& output_rows,
size_t xextra, size_t xsize, size_t xpos, size_t ypos,
size_t thread_id) const final {
JXL_ASSERT(xsize == xsize_);
JXL_ASSERT(xsize <= xsize_);
// TODO(firsching): handle grey case seperately
// interleave
float* JXL_RESTRICT row0 = GetInputRow(input_rows, 0, 0);
Expand All @@ -60,7 +60,7 @@ class CmsStage : public RenderPipelineStage {
const float* buf_src = mutable_buf_src;
float* JXL_RESTRICT buf_dst = color_space_transform->BufDst(thread_id);
JXL_RETURN_IF_ERROR(
color_space_transform->Run(thread_id, buf_src, buf_dst));
color_space_transform->Run(thread_id, buf_src, buf_dst, xsize));
// de-interleave
for (size_t x = 0; x < xsize; x++) {
row0[x] = buf_dst[3 * x + 0];
Expand Down

0 comments on commit 05fb8f0

Please sign in to comment.