[imageio] [DAM] Support Exif flash tag value in addition to its exiv2 textual explanation

[victoryforce]

This allows us to correctly detect whether the flash has fired. The previous code did this by parsing the textual explanation of the tag value from exiv2. But it gave the wrong result when these strings were translated.

We also correctly distinguish between no flash firing and no flash data available.

[victoryforce]

@zisoft Could I trouble you to review this PR, please?

[zisoft]

sure :) will look

[zisoft]

First test.
The database upgrade creates the new column flash_tagvalue with a default value of -1.

Now all existing images are tagged with -1 which gives inconsistent image infomation:

I have then refreshed the exif information of that image, the flash_tagvalue column value is now 16, but the image thumbnail still shows no info

EDIT: Ok, after dt restart the thumbnail info shows the correct text no.

[victoryforce]

The database upgrade creates the new column flash_tagvalue with a default value of -1.

Yes, at the time of updating the database schema we do not yet have information from this tag, so it should be -1.

Now all existing images are tagged with -1 which gives inconsistent image infomation

The reason for the inconsistency is that the image information module shows text from another database field, where a text explanation of the tag value from exiv2 was previously inserted.

If this PR had also made a change to the image information module so that it would make strings from the flash tag value itself, then there would be no inconsistency. I just decided that for this PR it would be enough to focus on one task.

EDIT: Ok, after dt restart the thumbnail info shows the correct text no.

I still have no idea how to get rid of this shortcoming (it's not part of the darktable code I'm familiar with), so I'd appreciate some advice.

[victoryforce]

I found a PR (#4592) that did essentially the same thing while adding support for a different Exif tag, Exif.Photo.ExposureBiasValue. Adjusted for restructuring and renaming some functions since then, it made the same changes to the same functions and does not contain any additional code beyond what is in my PR. (EDIT: Well, more precisely, it contains, but that's because some code is already added, I'm just changing the source from which it takes data.)

It also had a comment stating that to get the value we have to to select image(s) and refresh Exif.

So it seems that this older PR had the same behavior as mine. Since refreshing Exif can reset star ratings, it is not something that should be done automatically. So it seems that it is unlikely to improve the user experience here (apart from the inconsistency of flash data in different places, which can be fixed in a future PR).

[ralfbrown]

There's a preference setting for whether to use the Exif star rating. If that's the only user-editable metadata affected by an Exif refresh, it should be safe (though slow on large libraries) to do an automatic refresh with the preference temporarily forced to "ignore".

[victoryforce]

should be safe (though slow on large libraries) to do an automatic refresh with the preference temporarily forced to "ignore".

Or extremely slow in case of large libraries on external / network mounted disk.

So I'll stop here. If anyone wants to implement this, feel free to take it over. I don't have a strong arguments against adding automatic refresh, but I don't feel this is a right thing either...

[TurboGit]

Works for me, thanks!

[TurboGit]

Need a release note entry. TIA.

[victoryforce]

Suggestion for release notes:

Fixed incorrect reporting of whether a flash was fired in expansion variables $(EXIF.FLASH.ICON) and $(EXIF.FLASH) under certain conditions.