Skip to content

Commit

Permalink
Prevent ABI problems incompatibility on GCC + Arm (libjxl#4059)
Browse files Browse the repository at this point in the history
std::pair<float,float> is ABI incompatible across different C++ standards on ARM

Switch to std::array<float, 2>.
That will cause minor API change: in header-only part of jxl_cms
But google-search haven't found any public uses of that API
  • Loading branch information
eustas authored Jan 10, 2025
1 parent 1abebd0 commit 272f579
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 43 deletions.
10 changes: 5 additions & 5 deletions lib/extras/tone_mapping.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ namespace HWY_NAMESPACE {

static constexpr Vector3 rec2020_luminances{0.2627f, 0.6780f, 0.0593f};

Status ToneMapFrame(const std::pair<float, float> display_nits,
ImageBundle* const ib, ThreadPool* const pool) {
Status ToneMapFrame(const Range& display_nits, ImageBundle* const ib,
ThreadPool* const pool) {
// Perform tone mapping as described in Report ITU-R BT.2390-8, section 5.4
// (pp. 23-25).
// https://www.itu.int/pub/R-REP-BT.2390-8-2020
Expand Down Expand Up @@ -125,13 +125,13 @@ HWY_EXPORT(ToneMapFrame);
HWY_EXPORT(GamutMapFrame);
} // namespace

Status ToneMapTo(const std::pair<float, float> display_nits,
CodecInOut* const io, ThreadPool* const pool) {
Status ToneMapTo(const Range& display_nits, CodecInOut* const io,
ThreadPool* const pool) {
const auto tone_map_frame = HWY_DYNAMIC_DISPATCH(ToneMapFrame);
for (ImageBundle& ib : io->frames) {
JXL_RETURN_IF_ERROR(tone_map_frame(display_nits, &ib, pool));
}
io->metadata.m.SetIntensityTarget(display_nits.second);
io->metadata.m.SetIntensityTarget(display_nits[1]);
return true;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/extras/tone_mapping.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace jxl {
// Important: after calling this, the result will contain many out-of-gamut
// colors. It is very strongly recommended to call GamutMap afterwards to
// rectify this.
Status ToneMapTo(std::pair<float, float> display_nits, CodecInOut* io,
Status ToneMapTo(const Range& display_nits, CodecInOut* io,
ThreadPool* pool = nullptr);

// `preserve_saturation` indicates to what extent to favor saturation over
Expand Down
4 changes: 2 additions & 2 deletions lib/jxl/cms/tone_mapping-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class Rec2408ToneMapper : Rec2408ToneMapperBase {
using Rec2408ToneMapperBase::Rec2408ToneMapperBase;

void ToneMap(V* red, V* green, V* blue) const {
const V luminance = Mul(Set(df_, source_range_.second),
const V luminance = Mul(Set(df_, source_range_[1]),
(MulAdd(Set(df_, red_Y_), *red,
MulAdd(Set(df_, green_Y_), *green,
Mul(Set(df_, blue_Y_), *blue)))));
Expand All @@ -58,7 +58,7 @@ class Rec2408ToneMapper : Rec2408ToneMapperBase {
const V pq_mastering_range = Set(df_, pq_mastering_range_);
const V e4 = MulAdd(e3, pq_mastering_range, pq_mastering_min);
const V new_luminance =
Min(Set(df_, target_range_.second),
Min(Set(df_, target_range_[1]),
ZeroIfNegative(tf_pq_.DisplayFromEncoded(df_, e4)));
const V min_luminance = Set(df_, 1e-6f);
const auto use_cap = Le(luminance, min_luminance);
Expand Down
36 changes: 19 additions & 17 deletions lib/jxl/cms/tone_mapping.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
#define LIB_JXL_CMS_TONE_MAPPING_H_

#include <algorithm>
#include <array>
#include <cmath>
#include <cstddef>
#include <utility>

#include "lib/jxl/base/common.h"
#include "lib/jxl/base/compiler_specific.h"
Expand All @@ -18,21 +18,23 @@

namespace jxl {

typedef std::array<float, 2> Range;

class Rec2408ToneMapperBase {
public:
explicit Rec2408ToneMapperBase(std::pair<float, float> source_range,
std::pair<float, float> target_range,
explicit Rec2408ToneMapperBase(const Range& source_range,
const Range& target_range,
const Vector3& primaries_luminances)
: source_range_(std::move(source_range)),
target_range_(std::move(target_range)),
: source_range_(source_range),
target_range_(target_range),
red_Y_(primaries_luminances[0]),
green_Y_(primaries_luminances[1]),
blue_Y_(primaries_luminances[2]) {}

// TODO(eustas): test me
void ToneMap(Color& rgb) const {
const float luminance =
source_range_.second *
source_range_[1] *
(red_Y_ * rgb[0] + green_Y_ * rgb[1] + blue_Y_ * rgb[2]);
const float normalized_pq =
std::min(1.f, (InvEOTF(luminance) - pq_mastering_min_) *
Expand All @@ -45,7 +47,7 @@ class Rec2408ToneMapperBase {
const float e4 = e3 * pq_mastering_range_ + pq_mastering_min_;
const float d4 =
TF_PQ_Base::DisplayFromEncoded(/*display_intensity_target=*/1.0, e4);
const float new_luminance = Clamp1(d4, 0.f, target_range_.second);
const float new_luminance = Clamp1(d4, 0.f, target_range_[1]);
const float min_luminance = 1e-6f;
const bool use_cap = (luminance <= min_luminance);
const float ratio = new_luminance / std::max(luminance, min_luminance);
Expand All @@ -71,28 +73,28 @@ class Rec2408ToneMapperBase {
(-2 * t_b_3 + 3 * t_b_2) * max_lum_;
}

const std::pair<float, float> source_range_;
const std::pair<float, float> target_range_;
const Range source_range_;
const Range target_range_;
const float red_Y_;
const float green_Y_;
const float blue_Y_;

const float pq_mastering_min_ = InvEOTF(source_range_.first);
const float pq_mastering_max_ = InvEOTF(source_range_.second);
const float pq_mastering_min_ = InvEOTF(source_range_[0]);
const float pq_mastering_max_ = InvEOTF(source_range_[1]);
const float pq_mastering_range_ = pq_mastering_max_ - pq_mastering_min_;
const float inv_pq_mastering_range_ = 1.0f / pq_mastering_range_;
// TODO(eustas): divide instead of inverse-multiply?
const float min_lum_ = (InvEOTF(target_range_.first) - pq_mastering_min_) *
inv_pq_mastering_range_;
const float min_lum_ =
(InvEOTF(target_range_[0]) - pq_mastering_min_) * inv_pq_mastering_range_;
// TODO(eustas): divide instead of inverse-multiply?
const float max_lum_ = (InvEOTF(target_range_.second) - pq_mastering_min_) *
inv_pq_mastering_range_;
const float max_lum_ =
(InvEOTF(target_range_[1]) - pq_mastering_min_) * inv_pq_mastering_range_;
const float ks_ = 1.5f * max_lum_ - 0.5f;

const float inv_one_minus_ks_ = 1.0f / std::max(1e-6f, 1.0f - ks_);

const float normalizer_ = source_range_.second / target_range_.second;
const float inv_target_peak_ = 1.f / target_range_.second;
const float normalizer_ = source_range_[1] / target_range_[1];
const float inv_target_peak_ = 1.f / target_range_[1];
};

class HlgOOTF_Base {
Expand Down
7 changes: 3 additions & 4 deletions lib/jxl/render_pipeline/stage_tone_mapping.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,10 @@ class ToneMappingStage : public RenderPipelineStage {
if (orig_tf.IsPQ() && output_encoding_info_.desired_intensity_target <
output_encoding_info_.orig_intensity_target) {
tone_mapper_ = jxl::make_unique<ToneMapper>(
/*source_range=*/std::pair<float, float>(
0.0f, output_encoding_info_.orig_intensity_target),
/*source_range=*/Range{0.0f,
output_encoding_info_.orig_intensity_target},
/*target_range=*/
std::pair<float, float>(
0.0f, output_encoding_info_.desired_intensity_target),
Range{0.0f, output_encoding_info_.desired_intensity_target},
output_encoding_info_.luminances);
} else if (orig_tf.IsHLG() && !dest_tf.IsHLG()) {
hlg_ootf_ = jxl::make_unique<HlgOOTF>(
Expand Down
14 changes: 0 additions & 14 deletions tools/args.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,6 @@ static inline bool ParseOverride(const char* arg, jxl::Override* out) {
return JXL_FAILURE("Args");
}

static inline bool ParseFloatPair(const char* arg,
std::pair<float, float>* out) {
int parsed = sscanf(arg, "%f,%f", &out->first, &out->second);
if (parsed == 1) {
out->second = out->first;
} else if (parsed != 2) {
fprintf(stderr,
"Unable to interpret as float pair separated by a comma: %s.\n",
arg);
return JXL_FAILURE("Args");
}
return true;
}

template <typename Callback>
static inline bool ParseAndAppendKeyValue(const char* arg, Callback* cb) {
const char* eq = strchr(arg, '=');
Expand Down

0 comments on commit 272f579

Please sign in to comment.