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.48k stars 1.12k forks source link

FR: LUA API: add load_sicecar for dt_lua_image_t or make dt_lua_image_t.sidecar writeable #16599

Open micharambou opened 4 months ago

micharambou commented 4 months ago

Is your feature request related to a problem? Please describe. I am currently working on a sidecar / XMP history feature within darktable-git-annex (https://github.com/micharambou/darktable-git-annex). The idea is, that the user is able to go back and forth between previous versions of git-tracked sidecar files of an image and the resulting visual changes shall become visible within lighttable view immediately. However, in it's current state there is no possibility to trigger a reload of sidecar from within DT LUA API the same way as the user is able to do so from within history stack module.

Describe the solution you'd like

Alternatives no known alternatives or workarounds that do not require user interaction

Additional context ~~checkout sidecar_history branch in git https://github.com/micharambou/darktable-git-annex/tree/sidecar_history~~ merged as experimental opt-in feature in main (enable in preferences)

wpferguson commented 4 months ago

I'm a little concerned about the safety of doing this. Applying a sidecar to an undeveloped image in lighttable is one thing, but going back and forth between different sidecars for the same image seems to be a recipe for database corruption.

@jenshannoschwalm , @TurboGit thoughts?

jenshannoschwalm commented 4 months ago

I agree @wpferguson , the idea of reloading full history stack might be appealing but ... 1) ensuring database vs. sidecar identity will depend on more things than now ... 2) how do we handle the timestamps? 3) how can we maintain and test correctness ? Or how can we ever track down issues?

micharambou commented 4 months ago

Question for my personal understanding regarding your concerns about db integrity, which I higly appreciate: How does automated reloading of history stack from a git-reverted sidecar file differ from loading a sidecar file via lighttable -> history stack module (like one that was backed up in the past or downloaded from public resources like pixls.us). I mean: people share sidecar files for one particular raw all the time. I never read about db problems in this context.

jenshannoschwalm commented 4 months ago

How does automated reloading of history stack from a git-reverted sidecar file differ from loading a sidecar file via lighttable -> history stack module

I agree that is should be the same, my concern would be: How can we ensure integrity? We have database backups for safety, are we sure that by roll-back to an old version we have the corresponding sidecars? If writing to git fails for some reason, are we sure that available sidecar is valid and represents database? And how could we test that? Or - if dt database has a problem, can we make git aware of that (or don't we need to)

wpferguson commented 4 months ago

I believe that the apply sidecar function's intent is to provide an investigative tool:

This FR is wanting to use the sidecar files as a form of "snapshot" and maybe apply sidecars multiple times. Some of the questions/problems that I can think of:

The designed data flow in darktable is image -> edit -> database -> sidecar with an alternate data flow of image <- database <- sidecar but it wasn't designed for image<->[edit]<->database<->sidecar

I think this issue is much larger than just an API change. I think there is a HUGE amount of database interaction involved in multiple applications of sidecars to edited images that is only going to be understood with a LOT of testing. And, no matter how much we test, somebody will do something we never that of and it will be catastrophic.

micharambou commented 4 months ago

Thanks for your input so far but I believe, your certainly legitimate concerns regarding data integrity are out of the scope of this feature request and should be discussed in a separate stream. If there are no exisiting data integrity mechanism yet for an already existing feature (load sidecard file from within lighttable history stack module) than why even worrying about a API change, that most probably calls the same function/object method? As a side note: I once was thinking about, that I implement the feature following the following workflow:

  1. on image selection: get filename of image sidecar (image.sidecar)
  2. call and parse output of git log /path/to/sidecar and provide list of commits and timestamps for selection
  3. on selection of one commit: call git checkout %commit% /path/to/sidecar
  4. (from here in contrary to current implementation): create (virtual) copy of selected image and apply copy of sidecar
  5. if the user is happy and hits apply/commit button: git checkout sidecar back to commit of initially selected image resulting in the initial selected image unchanged and a new virtual copy of the image with rollbacked sidecar

Anyhow: the integrity issue remains the same and as mentioned - imho - should not be part of this feature request.

github-actions[bot] commented 2 months ago

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.