[imageio] Modify the libraw loader so that it can work as a fallback loader

[victoryforce]

The libraw loader was originally added to handle only the cr3 format, which is not supported by rawspeed. But there are cases where rawspeed failed to handle certain files of supported formats, while libraw loaded them successfully. So, an option was added to allow the rawspeed loader to ignore the processing of certain formats and offload them to the libraw loader. However, all this time we restricted the libraw loader from trying to process files that rawspeed previously worked with. This PR improves the user experience by giving libraw a chance (by removing the self-restriction) to better handle a file on which rawspeed failed.

For testing, you can use the files from #14848. They were not opened in darktable by the rawspeed loader, but they are opened with this PR because libraw works as a fallback and opens them successfully.

[kmilos]

This is not without risk: when/if RawSpeed gets the same functionality, there is no guarantee the parameters (crop, black/white point, input colour matrix) will be identical, so LibRaw edits might not be future proof?

[victoryforce]

@kmilos Why would these parameters be different?

[kmilos]

At least crops are determined manually (and somewhat arbitrarily) separately by owners of both libraries. To make it worse, all prams are hard-coded in LibRaw, while tweakable in XML for RawSpeed.

[victoryforce]

At least crops are determined manually (and somewhat arbitrarily)

@kmilos OK, to be honest, the ideal solution would be to improve the rawspeed code so that it can handle the possibly somewhat, shall we say, somewhat malformed files that libraw successfully handles. Then there will be no need at all for a fallback loader or support for ignoring certain extensions by the rawspeed loader. Is it feasible?

If there is no movement in this direction, for risk management we can add to this PR a parameter in the preferences with a tooltip that clearly warns about the risk you mentioned. And the default value of which will be not to enable fallback.

[MStraeten]

This is not without risk: when/if RawSpeed gets the same functionality, there is no guarantee the parameters (crop, black/white point, input colour matrix) will be identical, so LibRaw edits might not be future proof?

they are as future proof as cr3 edits - and these aren’t just fallback cases…

[kmilos]

It's a good feature, but I also feel hiding it behind a warning/disclaimer the user has to explicitly accept would maybe be the best?

[mfg92]

Another solution would be to store which raw library was used for editing in the sidecar file:
When editing a file, the library used (libraw or rawspeed) is stored in the sidecar XML. Then, when a raw image is displayed with a sidecar file, the sidecar determines which raw library to use, not the procedure in the darktable code. That way a once opend raw would always use the same raw library even if the default raw library had changed since the last display of that image.

[TurboGit]

Another solution would be to store which raw library was used for editing in the sidecar file:
When editing a file, the library used (libraw or rawspeed) is stored in the sidecar XML. Then, when a raw image is displayed with a sidecar file, the sidecar determines which raw library to use, not the procedure in the darktable code. That way a once opend raw would always use the same raw library even if the default raw library had changed since the last display of that image.

Fully agreed, I was about to propose this. As for all other default changed in the IOP we do that only if the module is reset otherwise we just keep the stored default in XMP or DB.

@victoryforce : I think this is the way to go.

[kmilos]

That's a good option as well.

One more piece of the puzzle: we also rely on RawSpeed to normalize maker and model strings and handle model aliases. For CR3s only we keep a hard-coded table in this loader, but if it's to get wider use, perhaps one could also use the LibRaw built-in normalisation? Doesn't work exactly the same though - don't think we end with both the clean model and clean alias...

Edit: raw-identify -v from the system LibRaw package should print both detected and normalized strings, so these should be checked on some corner cases. For example, I don't particularly like changing the maker (Leica -> Panasonic etc.)...

Edit2: We could maybe consider adding all the known, not yet working cameras to the RawSpeed database anyway w/ supported="no" attribute, and still use just that central place for name normalization...

[LebedevRI]

(Note: i do not receive notifications for this repository, i've seen this accidentally.)

Much like with many of the recent darktable changes, i really don't understand what is going on,
i worry things are being "considered" on a case-by-case basis without any larger plan/consideration.

Concretely:

1. libraw was very painfully purged from dt codebase over the course of many years
2. at some point in time it was re-added as a stop-gap measure to support cr3
3. After this change, re-removing it will become, again, much more painful.
4. I worry that by things will be happening to work through non-rawspeed,
   and there won't be incentive to actually add anything to rawspeed,
   and it will die. It's already on lifesupport. If people care, they need to help.
5. I worry that the next proposal would be to drop rawspeed
   as a standalone loader in the first place, since, well, libraw tries rawspeed first :)
6. I'm completely flabbergasted that the (only) reasoning for this change is support for corrupted files.

This is not the project i've once contributed to.

[kmilos]

@LebedevRI At least from my point of view, this is still a "last resort" solution, and I agree w/ most of your points, except

I'm completely flabbergasted that the (only) reasoning for this change is support for corrupted files.

Many users are missing and asking for support for completely valid, non-corrupt, OOC raws from modern gear:

• CR3s (the PR is still open after many years; I counted 23 camera models as of today)
• Fujifilm lossy (available for a while as well and hasn't been ported yet, but it is of lower priority of course as there are other modes available; at least 12 models)
• Panasonic v8 (presumably will be public soon, but cameras have been out for a long while, we're at 4 now w/o any means of support)
• DNGs w/ non-Adobe compression scheme (perfectly valid still)

It started w/ CR3s only, but the list of codecs and camera models is relentlessly growing as time goes by unfortunately. So, while we desperately need/want to have RawSpeed contributors for those, dt can't really stand idly by either. Is there an alternative you can envisage or propose?

P.S. Note that the CR3 issue also made the vkdt project have two backends: RawSpeed and rawler.

[LebedevRI]

Well, i can't conjure up new contributors from thin air, so the options are limited :)

It was perhaps unfortunate how much i course-corrected after merging that rapid-fire of external PR's
[right after the library maintainership was just passed to darktable],
and before things were in a place where external contributions were within the realm of possibility,
but really i still haven't fixed all the design decisions taken in those changes...
I got really burned there, it was a loss-loss situation, and it cost a contributor :(

Finally, after many years, i'm looking into the second half of the puzzle, encoding,
that will finally make fuzzing better, allow to actually benchmark/test
e.g. huffman code decoders and LJpeg decoder, and will finally allow to
implement the rest of the LJpeg stuff.
(DNGs w/ non-Adobe compression scheme (perfectly valid still)).

I think, as the pr log shows, i do merge external PR's,
so as long as things are sufficiently self-contained,
and have little to no chance of hurting existing stuff,
i think they can be merged speedily.

[kmilos]

Well, i can't conjure up new contributors from thin air, so the options are limited :)

Very true! For starters, for the non-trivial PRs, maybe try to be more transparent in why it's not trivial and what needs to happen first, provide an update/timeline every once in a while? If the bar is too high or there is no feedback/interest, they will either not come or go away?

Maybe have an "up for grabs" label for the possibly low-impact stuff while you work on the architecture? We can perhaps then advertise these "up for grabs" bounties on discuss.pixls.us etc.

[LebedevRI]

Very true! For starters, for the non-trivial PRs, maybe try to be more transparent in why it's not trivial and what needs to happen first, provide an update/timeline every once in a while?

Yeah believe me, i know :( I have, ..., communication issues, let's put it that way.
I'm very much aware that i should be doing certain things,
i just can't come to bring myself to do them sometimes,
it's like an unscalable wall sometimes, that pushes back.

[kmilos]

I'm very much aware that i should be doing certain things,
i just can't come to bring myself to do them sometimes,
it's like an unscalable wall sometimes, that pushes back.

You are not alone, I think everyone is familiar with this feeling! I guess the trick is to recognize/remind oneself there are still more important/grander things in life so that wall becomes less significant ;)

[LebedevRI]

I think everyone is familiar with this feeling!

Uh, no, i don't think so.

[dtrtuser]

Well, i can't conjure up new contributors from thin air, so the options are limited :)

I would like to contribute (although not sure that i have the skills) but the lack of documentation would make it very difficult to know where to start.

[github-actions]

This pull request 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 verify it has no conflicts with the master branch and rebase if needed. Mention it now if you need help or give permission to other people to finish your work.

[TurboGit]

I like the idea but I see the possible issue with file being later supported by rawspeed. My proposal would be:

• Record the loader used the first time (already in the DB IINM)
• Always use it to render the image
• If a new loader is available let the user have the choice to use it (a button in the rawprepare module maybe?). Maybe we don't have a way to know that, no big deal, the button is to check for new loader.

This would give the us the best options for our users. Supporting more formats and have a safe move if needed, user will be aware that this change could be affecting the image rendering and so he will be prepared to adjust the developement.

[TurboGit]

@victoryforce : Can you comment about my last proposal?

[TurboGit]

@victoryforce : Is that still on your radar? Or do you want to close this?

[victoryforce]

In fact, the need for this PR (or the alternative proposed by @TurboGit) should not arise at all. Currently rawspeed does not support CR3 files and libraw works here. Also, there may be some compression schemes and codecs that are not yet supported in rawspeed for other raw file formats, but are supported in libraw. In such cases, users can use the option to switch certain raw extensions to work with libraw.

In the above-mentioned cases, there is no need to implement the use of the libraw fallback loader after the failure of the rawspeed loader. If something is not supported in rawspeed but is supported in libraw, you simply switch to using an alternative loader.

The problem that this PR tried to solve is the following. There are cases when the raw of a certain camera is supported by both rawspeed and libraw. But there are times when, due to rawspeed's stricter requirements for data correctness, it rejects certain files, while libraw loads them successfully. My guess is that the most likely cause of such problem files are bugs in certain versions of the cameras' microcode. I want to emphasize that such files are usually not visually damaged and there is no reason to punish the user for camera bugs by throwing such pictures in the trash.

I like the idea but I see the possible issue with file being later supported by rawspeed. My proposal would be:

Will a large number of users experience these issues? Surely not. So is it worth implementing this proposal for a better user experience? I'm not sure. I definitely won't do that.

P.S. In most of the programs, the code of which I reviewed, where there is reading of certain file formats, the decoders try to correct as much as possible even if the format contains certain errors or does not strictly comply with the standard. So, in my opinion, the best solution would be to change the rawspeed approach. It is fundamentally important to always adhere to the principle: the interpretation of the spec should be as strict as possible on the side of the data creator and as permissive as possible on the side of the data reader.