feat: add HDR to SDR tone mapping support#274
Conversation
salarkhan
changed the title
salarkhan
force-pushed
the
salar/tone-mapping
branch
from
2500fcc to
0f0379c
Compare
salarkhan
force-pushed
the
salar/tone-mapping
branch
from
c80c03f to
d851fcf
Compare
webp.hpp
Outdated
| int dispose, | ||
| int x_offset, | ||
| int y_offset); | ||
| size_t webp_encoder_write_with_tone_mapping(webp_encoder e, |
There was a problem hiding this comment.
architecturally it seems like tone mapping should be done independently of the format specific encoders?
There was a problem hiding this comment.
oh damn, i didnt think of that. that would def be cleaner, let me take a crack at it
| // Apply Reinhard tone mapping | ||
| std::unique_ptr<cv::Mat> dst_bgr = std::make_unique<cv::Mat>(src_for_transform->rows, src_for_transform->cols, CV_8UC3); | ||
|
|
||
| // Apply luminance-based tone mapping to preserve color relationships |
There was a problem hiding this comment.
It seems a bit odd to hardcode a tone mapping algorithm here ... i thought opencv had some functions for applying tone mapping or there could be other alternative libraries out there worth looking at?
There was a problem hiding this comment.
smh, i didnt realize opencv had a built in. yeah, lemme swap that in and see how it goes.
There was a problem hiding this comment.
okay, the built in one seems to do a pretty poor job of handling the super-brightness that we're running into. gonna keep fiddling with it to see what's up, but may just stick with my algo for now if i can't get it to play nice
There was a problem hiding this comment.
i would leave some comments in the code to that effect but yeah, makes sense overall. Some iteration to improve would help when we have the cycles.
- low level API: Users call fb.ApplyToneMapping(icc) explicitly before encoder.Encode() - hi level API: Users can still use ImageOptions.ForceSdr with Transform()
salarkhan
force-pushed
the
salar/tone-mapping
branch
from
e96f3c7 to
4b8add2
Compare
salarkhan
force-pushed
the
salar/tone-mapping
branch
from
7b671ba to
9467c75
Compare
| // Apply Reinhard tone mapping | ||
| std::unique_ptr<cv::Mat> dst_bgr = std::make_unique<cv::Mat>(src_for_transform->rows, src_for_transform->cols, CV_8UC3); | ||
|
|
||
| // Apply luminance-based tone mapping to preserve color relationships |
There was a problem hiding this comment.
i would leave some comments in the code to that effect but yeah, makes sense overall. Some iteration to improve would help when we have the cycles.
| } | ||
|
|
||
| // Load ICC profile | ||
| cmsHPROFILE src_profile = cmsOpenProfileFromMem(icc_data, icc_len); |
There was a problem hiding this comment.
it may not be reliable to use just the icc profile for detecting HDR. some formats contain flags and not profiles ... would recommend looking more into this. This possibly could get complex so it's probably fine to narrow the scope to what we've been seeing in the wild as long as we make a note of it.
There was a problem hiding this comment.
ah jesus. okay, i'm going to kick this can down the road a bit for now
salarkhan
force-pushed
the
salar/tone-mapping
branch
from
d47684c to
0c68cbb
Compare
salarkhan
changed the title
3 participants
summary
adds HDR to SDR tone mapping functionality for PNG and WebP encoding to handle high dynamic range images with PQ (Perceptual Quantizer) color profiles. this prevents overly bright/blown-out images when converting HDR content to SDR formats.
motivation
when encoding some images with HDR PQ profiles (such as Rec. 2100 or Rec. 2020 with PQ transfer function) without tone mapping, the resulting images appear extremely bright to the point of it being an accessibility issue.
changes
the implementation uses a luminance-based Reinhard tone mapping approach:
this approach prevents oversaturation by maintaining color relationships while reducing brightness.