search

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.75k stars 1.14k forks source link

colorin and iop_order issue #2258

Closed TurboGit closed 5 years ago

TurboGit commented 5 years ago

Describe the bug

dt crash when I open many old pictures with a colorin version 1.

To Reproduce

Difficult has you need a database with a specific history entry.

Here is my finding.

I have found that the picture on which I was working has no iop_order defined:

sqlite> select * from history where imgid in (31935) and operation="colorin";
31935|3|1|colorin|cmatrix|1||4|||

As you can see, last value is empty and it is iop_order.

I have seen that other iop_order for colorin are not null and often 27:

sqlite> select distinct iop_order, count(*) from history where operation="colorin" group by iop_order;
|23814
8.5|4
27.0009994506836|4
27.0009994506836|1
27.001|5328

But as you can see, I have 23814 empty iop_order!

Setting iop_order to 0 still crash dt.

Setting iop_order to 8 seems to make dt happy and I do not have anymore error messages like those:

[dt_ioppr_transform_image_colorspace_cl] module basecurve must be between input color profile and output color profile [dt_ioppr_transform_image_colorspace] module basecurve must be between input color profile and output color profile

Expected behavior

dt should not crash.

Platform (please complete the following information):

My questions:

edgardoh commented 5 years ago

What does it mean "buggy" temp dt version?

If you import an xmp of one of the offended images you can reproduce the error?

When migrating did you see an error, it is tested for iop_order not set.

To "fix" your db probably all that is needed is to set all the NULL / zero iop_order of colorin to the right value. The right value depend on the iop_order version on main.images. Check if you have an image with colorin->iop_order > 0 with the same iop_order_version than the ones that are NULL.

El mié., 20 mar. 2019 a las 18:05, Pascal Obry (notifications@github.com) escribió:

Describe the bug

dt crash when I open many old pictures with a colorin version 1.

To Reproduce

Difficult has you need a database with a specific history entry.

Here is my finding.

I have found that the picture on which I was working has no iop_order defined:

sqlite> select * from history where imgid in (31935) and operation="colorin"; 31935|3|1|colorin|cmatrix|1||4|||

As you can see, last value is empty and it is iop_order.

I have seen that other iop_order for colorin are not null and often 27:

sqlite> select distinct iop_order, count(*) from history where operation="colorin" group by iop_order; |23814 8.5|4 27.0009994506836|4 27.0009994506836|1 27.001|5328

But as you can see, I have 23814 empty iop_order!

Setting iop_order to 0 still crash dt.

Setting iop_order to 8 seems to make dt happy and I do not have anymore error messages like those:

[dt_ioppr_transform_image_colorspace_cl] module basecurve must be between input color profile and output color profile [dt_ioppr_transform_image_colorspace] module basecurve must be between input color profile and output color profile

Expected behavior

dt should not crash.

Platform (please complete the following information):

  • OS: Linux
  • Version current master

My questions:

  • Is that because I have used a temp dt version which was buggy?
  • What is the best way to fix that?
  • Is there a safe iop_order I can set to all colorin?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/darktable-org/darktable/issues/2258, or mute the thread https://github.com/notifications/unsubscribe-auth/APq1pTkt5StR3d_EplCOJ0SogR4YbpBMks5vYqKdgaJpZM4cAREH .

TurboGit commented 5 years ago

What does it mean "buggy" temp dt version?

A dt version I have compiled which had a iop order issue?

If you import an xmp of one of the offended images you can reproduce the error?

No, all is fine in this case. And the iop_order is set to : 27.0009994506836

TurboGit commented 5 years ago

Looks like all iop_order (version 5) of colorin has a value of 27.0009994506836 or 27.001.

So should I set all NULL value to 27.001? Should work?

Any idea to what could have happened? And no I don't remember having seen error, but as you know I have tested your corresponding PR many many times... With possibly some intermediate version not correct.

edgardoh commented 5 years ago

What does it mean "buggy" temp dt version?

A dt version I have compiled which had a iop order issue?

I haven't modified this in a while, so if there were a bug when you migrated probably is still there, will be great if you can duplicate it.

If you import an xmp of one of the offended images you can reproduce the error?

No, all is fine in this case. And the iop_order is set to : 27.0009994506836

Then this is probably the right iop_order, to be sure check against some other images.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/darktable-org/darktable/issues/2258#issuecomment-475035040, or mute the thread https://github.com/notifications/unsubscribe-auth/APq1pbfB3HJiX35Bx72uaR8RRLIGV-Iwks5vYqeMgaJpZM4cAREH .

edgardoh commented 5 years ago

Looks like all iop_order (version 5) of colorin has a value of 27.0009994506836 or 27.001.

27.0009994506836 is what I have for every single image that was not modified after the migration, in that case iop_order_version is 1.

So should I set all NULL value to 27.001? Should work?

Check the iop_order_version for each image, images with iop_order_version=1 should have 27.0009994506836

Any idea to what could have happened? And no I don't remember having seen error, but as you know I have tested your corresponding PR many many times... With possibly some intermediate version not correct.

No idea, I don't have a single image with this problem, and right now we can't be sure if it was the migration or some bug while editing. Only way to be sure is re-migrate it and check the values.

edgardoh commented 5 years ago

My bad, I wasn't checking for NULL, I just entered #2260, we can keep it open while working on this issue.

TurboGit commented 5 years ago

So, this means that not only colorin is affected but potentially all modules, right?

edgardoh commented 5 years ago

No idea, we'll need someone with this same issue that can migrate a backup to be sure.

I'll add a check at dt start up for iop_order==NULL, it must be done after the db migration, do you know where that is done?

TurboGit commented 5 years ago

I suppose this could be added in src/common/database.c

edgardoh commented 5 years ago

I just added it to dt startup, seems cleaner. It won't make it to the release anyway, is just to be sure that everything is OK.

TurboGit commented 5 years ago

Just seen that, even if it do not go into the release some devs/users building from master do not know how to handle the database, so I suggested to add some information that could be cut&paste here for example to better understand the issue.

TurboGit commented 5 years ago

Closing at this stage. Thanks a lot Edgardoh for helping me on this one.