Open dtorop opened 3 years ago
I don't know if this is related to that topic but it should not very far, ... I'm thinking about 3D LUT making the transform between log camera encoding to a given color space. Is there a way to consider this ? Is colorin the right place ?
I have exactly that in the pipe. Work is started.
I have exactly that in the pipe. Work is started.
@aurelienpierre Great! If any help, I roughed out some profile handling changes (or at least FIXME's) before/while writing this FR: ef132e9c72cf9c9a61a28d23fab244784ad0bcc6. As the commit message notes, I haven't even tried to compile them.
On thinking it over, it seems like a good path would be for commit_params to even earlier call dt_ioppr_set_pipe_input_profile_info, and that should do as much work as possible. Then failing the matrix route, commit_params (or iop_profile?) would set up the LCMS fallback.
It seems like the goal of all this work would be to make a PR which makes absolutely no functional change to dt, but ideally would result in more legible, less bugprone, and perhaps faster code.
(Though one functional change may be that colorin sees rec.709 as the fallback colorspace, and iop_profile sees rec.2020 as the fallback. For modern cameras, is rec.2020 the better choice? Would switching break extant histories?)
Then there's colorout -- it's way less verbose now, but perhaps it could benefit from a similar lookover?
I'm thinking about 3D LUT making the transform between log camera encoding to a given color space.
@phweyland I wish I had the expertise/experience here. Would this involve setting up an an alternate path -- rather than overloading colorin, allowing for the existing lut3d or lut3dgmic modules to be repurposed to do this?
Not sure if log encoding situation adds more wrinkles, e.g. to LUT resolution. My impression is LCMS CLUT as used by dt is significantly slower than matrix and per-channel LUT methods, but at a guess lut3d/lut3dgmic are good/fast...?
This issue did not get any activity in the past 30 days and will be closed in 365 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.
Rewrite of the processing code is largely completed, with the majority of the SSE code removed.
Any activity on integration with iop_profile?
This issue has been marked as stale due to inactivity for the last 60 days. It will be automatically closed in 300 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.
This issue has been marked as stale due to inactivity for the last 60 days. It will be automatically closed in 300 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.
Is your feature request related to a problem? Please describe.
The colorin code is fairly original to darktable, and could be integrated more deeply with more recent work.
integration with iop_profile
commit_params
in colorin does some of the same work asdt_ioppr_generate_profile_info
. Both cache profile info (matrices and LUTs, unbounded coeffs).lerp_lut()
vsextrapolate_lut()
).commit_params
callsdt_ioppr_set_pipe_input_profile_info()
which, by setting up adt_iop_order_iccprofile_info_t
, repeats some of the matrix/LUT/coeff work done incommit_params
.dt_ioppr_generate_profile_info
warns on XYZ matrices, though these are valid for colorin.dt_ioppr_generate_profile_info()
advertises that it returns non-zero on failure, but instead it always returns 0 but returns a NAN at the start of matrices if it's anything but a matrix profile. This can confuse an invalid profile with a CLUT or an XYZ profile.processing
process_sse2
path) in colorin. With modern compiler optimizations, is the SSE code any faster? It certainly adds complexity.Describe the solution you'd like
commit_params
use iop_profile code to generate/retrieve/cache matrices/LUTs/coeffs.dt_ioppr_generate_profile_info()
to be OK with XYZ profiles, e.g. built-inDT_COLORSPACE_XYZ
dt_ioppr_generate_profile_info()
(or related) to understand camera matrices. Havedt_ioppr_set_pipe_input_profile_info()
do the work of customizing camera matrix profile based on the current pixelpipe's camera.dt_ioppr_generate_profile_info()
, if failures can occur there.mat3mul()
, etc. in colorin.process()
vs.process_sse2()
paths, remove SSE path if possible.This work could all be done early in the v3.8 cycle, so there'd be adequate time to test for unexpected side-effects.
Alternatives
Leave code as is. Or selectively do some but not all of these things.
Additional context (risks)
The iop_profile code (
dt_ioppr_generate_profile_info()
) will only pull matrices from the profile if both the input & output directions are matrix only (not CLUT). For colorin, we only care about the input direction. Would there be any profiles which are matrix input both CLUT output, hence causing a regression if we switch to iop_profile code?