Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix assert in CMS stage #3379

Merged
merged 1 commit into from Mar 5, 2024
Merged

Fix assert in CMS stage #3379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/extras/enc/jpegli.cc
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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