8000 FR: LUA API: add load_sicecar for dt_lua_image_t or make dt_lua_image_t.sidecar writeable · Issue #16599 · darktable-org/darktable · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
micharambou opened this issue Apr 10, 2024 · 10 comments · Fixed by #18792
Closed

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

micharambou opened this issue Apr 10, 2024 · 10 comments · Fixed by #18792
Assignees
@wpferguson
Labels
feature: new new features to add lua no-issue-activity

Comments

@micharambou
Copy link
micharambou commented Apr 10, 2024 •
edited
Loading

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

  • add load_sidecar method for type dt_lua_image_t
    or
  • make dt_lua_image_t.sidecar writeable

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
Copy link
Member

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Member

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

  • Play Raw: I see an edit I like and I wonder how they did it. I download the image and the sidecar, apply the sidecar, and inspect the history stack to learn how they did it. I see another edit I like so I download the sidecar, duplicate the original image and apply it, inspect it, and maybe learn something else.
  • A user has a problem with processing problem (artifacts, etc) with an image. We can use the sidecar and apply it to an image and see if the problem can be recreated.

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:

  • I apply a sidecar from 6 months ago. Should the modified timestamp from the image be now or 6 months ago? Let's say that I printed the image 2 months ago. Should that be the timestamp or should I use whatever the sidecar has?
  • The steps (off the top of my head) to applying a sidecar through the Lua API would be delete the current history stack, then apply the sidecar. If there is any corruption in the sidecar, then the apply fails and you're left with an unedited image.
  • Tags may be applied multiple times. Tagging is not part of the history stack, so it wouldn't be cleared and I don't know if the tagging routines check if the tag has been applied before applying it.
  • Let's say that a new version of darktable comes out. You use it for awhile and don't like it, so you restore the old version and the old database. They you remember some edits you did in the new version and you try and apply those sidecars. Unfortunately the IOP order has changed and there was also a new module used. So the current edit gets deleted and the new one fails because of an invalid IOP order or a non-existent module.

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
Copy link
Author
micharambou commented Apr 11, 2024 •
edited
Loading

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.

@wpferguson wpferguson self-assigned this Apr 12, 2024
@wpferguson wpferguson added feature: new new features to add lua labels Apr 12, 2024
@github-actions GitHub Actions
Copy link

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.

@wpferguson
Copy link
Member

Coded up image:apply_sidecar(). I don't think I'd try it in darkroom, but it works fine in lighttable.

Here's the updated src/lua/image.c

image.c.txt

@micharambou
Copy link
Author
2025-02-04.21-09-27.mp4

Integrated and made a few tests in my testing environment - works as expected.
Would be delighted to see this implemented in a future release

@github-actions GitHub Actions
Copy link
github-actions bot commented Apr 6, 2025

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.

@wpferguson wpferguson mentioned this issue May 11, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@wpferguson wpferguson

Labels
feature: new new features to add lua no-issue-activity
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

add apply_sidecar function to dt_lua_image_t wpferguson/darktable
3 participants
@wpferguson @micharambou @jenshannoschwalm
0