darktable-org / darktable

darktable is an open source photography workflow application and raw developer
https://www.darktable.org
GNU General Public License v3.0
9.69k stars 1.14k forks source link

unsupported output profile reported #6774

Closed TurboGit closed 3 years ago

TurboGit commented 3 years ago

Describe the bug

unsupported output profile reported

To Reproduce

Just start current dt, and see on console:

[dt_ioppr_set_pipe_output_profile_info] unsupported output profile 2 , it will be replaced with sRGB
[dt_ioppr_set_pipe_output_profile_info] unsupported output profile 2 , it will be replaced with sRGB
[dt_ioppr_set_pipe_output_profile_info] unsupported output profile 2 , it will be replaced with sRGB
[dt_ioppr_set_pipe_output_profile_info] unsupported output profile 2 , it will be replaced with sRGB
[dt_ioppr_set_pipe_output_profile_info] unsupported output profile 8 , it will be replaced with sRGB
[dt_ioppr_set_pipe_output_profile_info] unsupported output profile 8 , it will be replaced with sRGB
[dt_ioppr_set_pipe_output_profile_info] unsupported output profile 8 , it will be replaced with sRGB
[dt_ioppr_set_pipe_output_profile_info] unsupported output profile 8 , it will be replaced with sRGB
[dt_ioppr_set_pipe_output_profile_info] unsupported output profile 8 , it will be replaced with sRGB
[dt_ioppr_set_pipe_output_profile_info] unsupported output profile 8 , it will be replaced with sRGB
[dt_ioppr_set_pipe_output_profile_info] unsupported output profile 8 , it will be replaced with sRGB
[dt_ioppr_set_pipe_output_profile_info] unsupported output profile 8 , it will be replaced with sRGB

Expected behavior

no error on console

Screenshots

Platform (please complete the following information):

Additional context

Very recent regression.

aurelienpierre commented 3 years ago

Culprit is dt_ioppr_set_pipe_output_profile_info in iop_profile.c, called from commit_params() in colorout.

It fetches the color profile matrix from what the colorout module produces. When this message appears, it means that the first element of the matrix is NAN. This is not supposed to happen.

aurelienpierre commented 3 years ago

I don't reproduce @TurboGit. Do you have some weird display profile (like a LUT-based profile) ?

TurboGit commented 3 years ago

I may have some in my output profile dir for printing.

But my display profile is set to system which is a profile created by Displaycal. Using a display profile as Adbobe RGB still produce the issue.

I have also some: [dt_ioppr_set_pipe_work_profile_info] unsupported working profile 4 , it will be replaced with linear rec2020

TurboGit commented 3 years ago

Whatever profile I set in colorout I have the error. Even when setting the profile to sRGB or AdobeRGB.

aurelienpierre commented 3 years ago

dt_ioppr_set_pipe_work_profile_info has not been changed, only the call to the function has been moved from process() to commit_params in colorin. The profile type 4 is defined as DT_COLORSPACE_LIN_REC2020 = 4 anyway, so that's super weird reading thet Rec2020 has not been found, let's replace it with Rec2020.

aurelienpierre commented 3 years ago

Ok, for the output color profile, I can reproduce if LittleCMS2 is always on.

Still not reproduced for work profile.

TurboGit commented 3 years ago

Strange, I have also this issue with LittleCMS2 off.

aurelienpierre commented 3 years ago

@TurboGit in iop_profiles.c, could you add, after line 746 :

  if(profile_info)
  {
    fprintf(stdout, "matrix_in : %f, matrix_out : %f \n", profile_info->matrix_in[0], profile_info->matrix_out[0]);
  }

and see what it says when you get [dt_ioppr_set_pipe_work_profile_info] unsupported working profile 4 , it will be replaced with linear rec2020 ?

Because it's either that values are NAN, which is weird since the profile is generated internally, or the profile_info pointer is NULL, which means the profile is not properly initialised.

TurboGit commented 3 years ago
matrix_in : nan, matrix_out : nan 
[dt_ioppr_set_pipe_output_profile_info] unsupported output profile 8 , it will be replaced with sRGB

I have put this message for output and work profile info, in both cases I have nan.

TurboGit commented 3 years ago

Ok this happens when LittleCMS2 is on or if the display profile is set to "system" and a profile is set in colord.

TurboGit commented 3 years ago

@aurelienpierre : I must reopen this one. I still have the following output:

[dt_ioppr_set_pipe_output_profile_info] unsupported output profile 8 , it will be replaced with sRGB

This happens only when a display profile is set in colord.

TurboGit commented 3 years ago

This only happens when the pipe is run for display (type = 8 => DT_COLORSPACE_DISPLAY). It only happens if colord is set with an .icc (or for default setting or deactivated).

In this case the out_filename is NULL and we get nan in both matrix matrix_in & matrix_out.

Not sure what should be done in this case. @aurelienpierre : hope you'll shine on this one...

TurboGit commented 3 years ago

Some more investigation. First it looks like this is a pathological case and actually does not break anything.

The pipe output profile is set in colorout() calling dt_ioppr_set_pipe_output_profile_info().

The pipe output profile is read (calling dt_ioppr_get_pipe_output_profile_info()) at a single place in dt_ioppr_get_pipe_current_profile_info() and returned for modules AFTER colorout.

And dt_ioppr_get_pipe_current_profile_info() is currently not called, so at the end all this does nothing at the moment.

Now, the console error output will make people anxious and will raise many questions.

@aurelienpierre : What can we do for the release ? my proposal if we cannot get this right (whatever it means) for 3.4 I would suggest to kill the warning for type == 8 (DT_COLORSPACE_DISPLAY). Or do you have any other proposals / ideas ?

aurelienpierre commented 3 years ago

dt_ioppr_get_pipe_current_profile_info() is now used in channelmixerrgb.c and is planned for use in blending.

This error is indeed only mildly annoying.

So far, the pipeline working profile is fetched by RGB modules that need it, but it is available only in the pipe between colorin and colorout. This profile is used mostly to retrieve the Y luminance or the XYZ vector, in channelmixerrgb, filmicrgb, basicadjustments, etc. If these modules get moved out of the colorin/colorout, the working profile is unavailable and the code falls-back to something very wrong, color-wise, that will still let the program output a result with no crash, but produces garbage XYZ space.

That garbage XYZ space makes the JzAzBz color space fail badly, because it will output a chroma for white pixel and make the hue wrong. For example, mask an exposure module on red-ish hues, then move it between colorin and colorout, and now the mask has changed. That's all because of bad XYZ.

The purpose of dt_ioppr_get_pipe_current_profile_info() is to do our best effort to try to retrieve the current RGB profile wherever we are in the pipe, and at least solve some of the cases where work_profile can't be fetched. It is already much better than what is done now. As a result, moving an exposure module in the pipe will preserve its hue-based parametric mask. That improves a lot usability.

That said, if weird profiles are getting used, the code falls back to sRGB for output, which is already an improvement over the hard fail that we have now. But there is not much use for parametric blending in modules past colorout anyway.

So, if the log warning is a problem, it's only a matter of removing the fprintf. I have kept it only for consistency with the rest of the lib.