dterrahe / darktable

darktable is an open source photography workflow application and raw developer
https://www.darktable.org
GNU General Public License v3.0
2 stars 0 forks source link

implement params overrides from file #28

Closed dterrahe closed 5 months ago

dterrahe commented 5 months ago

Allow overriding individual values within the xmp params blob by specifying them in human readable form in a companion sidecar file called <image_file>.override.

For example, when loading D71_5191.NEF, if a file D71_5191.NEF.override exists, any lines it contains are interpreted as <module>[,<instance name>]/<field>=<value> so one could force a non-default exposure setting with: exposure/exposure=2.0

The module and field codes are the internal introspection ones, so may need to be looked up in the xmp or by switching to shortcut mapping mode and hovering over the widget associated with the field and noting the lua command details in the tooltip. Still, the field name might not work, especially in case of sections. colorequal/saturation/red is actually internally called sat_red. To find the real introspection names, one could read the source, but if a line in the .override file contains just a module name, all its manipulable fields are printed to the terminal (together with the valid range or available dropdown values).

Arrays (curves and for example the multiple tabs in colormixerrgb, but also strings), are not supported. You would see something like red[0] (so in theory you could override the first element, which is probably not that useful).

If multiple instances of the same module exist, the instance name should be specified, for example:

exposure,first/exposure=1.4
exposure,second/exposure=3.0

You cannot activate (or create a new instance of) a module this way; it already needs to be in the history (or in the default or an automatically applied style/preset). No range validation takes place, so it is probably very easy to trigger a crash. At least with -d params (or --core -d params) an out-of-bounds will be flagged.

EDIT: The file extension is changed from .override to .params (since it can also be used just to query params fields, individually, for a whole module or all). To dump all the params in history to output, put a line "all" in the file.

@AxelG-DE

AxelG-DE commented 5 months ago

That looks promising. What would be needed (and I assume that is simple) is a function to write xmp 'as is' into xmp.override (kinda export), in order to have the keyframes (as LRT interpolates between keyframes) and one has a starting point, what is in the history already

AxelG-DE commented 5 months ago

is there a reason, you don't add it as PR to the darktable-repo? Maybe just having the chance to silently kill it again? :-D

I am not a pro in terms of github, but I will try to inject that code into my darktable and see, what happens, respectively whether or not I am able to do handle it...

AxelG-DE commented 5 months ago

For example, when loading D71_5191.NEF, if a file D71_5191.NEF.override exists, any lines it contains are interpreted as <module>[,<instance name>]/<field>=<value> so one could force a non-default exposure setting with: exposure/exposure=2.0

I assume you meant D71_5191.NEF.xmp and D71_5191.NEF.xmp.override ??

EDIT: I got it, it is actually the NEF (in my todays test ORF) wich will be .override --> works in the first place

AxelG-DE commented 5 months ago

You cannot activate (or create a new instance of) a module this way

this will be the ultimate status, no matter what? Why I ask, LRT actually needs to create modules and even would like to hide them, to avoid users fiddle on those values... (but this is a far away thingie, maybe)

AxelG-DE commented 5 months ago

It works nicely!

BTW. I see a red frame around my picture, when the machine is "...working...". Once it ran crazy. Moving my mouse caused the filmstrip to slide and all pics in the filmstrip got that red/purple boarder...

I would be happy you add this as WIP into the official repo as it would my life easier to inject it (I had to add your repo to the git remote and I am not sure how to handle it, so that I can still add things from the official repo. Sorry I am quite boon here)

dterrahe commented 5 months ago

What would be needed (and I assume that is simple) is a function to write xmp 'as is' into xmp.override

Presumably this could be "simply" implemented in lua and run in the gui while you are editing the key frames. There's no need to hardcode this in the core functionality of darktable, is there?

LRT actually needs to create modules

You can copy whole xmp's (say, from the key frames) if you want them to all have identical modules or sections within the xmp's with the unreadable params blobs. Or even use auto applied presets. The additional functionality needed, if you want to run outside the gui so that dt.gui.action can't be used, and which this PR provides, is to override individual fields within those modules.

I see a red frame around my picture

That's because this is built on top of other changes in my personal fork (https://github.com/dterrahe/darktable/commits/ancil/ for "ancillary") that are already rejected upstream or never submitted because they are just for my own benefit or amusement. To include only this PR in upstream someone would simply cherry-pick the single commit it contains.

AxelG-DE commented 5 months ago

Presumably this could be "simply" implemented in lua and run in the gui while you are editing the key frames. There's no need to hardcode this in the core functionality of darktable, is there?

technically from a coder's perspective maybe not. Just e.g. me cannot code LUA and also I experienced, Lua is sometimes stopped working or did other crazy stuffs. From that users perspective, I actually was dreaming of a helper's softare, which could read the blob and write it into such human legible values either in the xmp or into another file like .xmp.int (int for introspection) and the same helper program reads those values back and write them into the blob (the latter functinoaliy, I understand is somewhat already existing now, which makes me happy already).

You can copy whole xmp's (say, from the key frames) if you want

I think here we are not on the same page yet. Also we are atm still on a basic level

To include only this PR in upstream someone would simply cherry-pick the single commit it contains.

unfortunately this is above my head. Would be nice to hook on here and learn coding, if life wouldn't be in my way (since 2021 everything got upside down here, not due to Covid but that didn't make it easier as for everyone)

dterrahe commented 5 months ago

The file extension is changed from .override to .params (since it can also be used just to query params fields, individually, for a whole module or all). To dump all the params in history to output, put a line "all" in the file.

for example

exposure/exposure=-3.5
channelmixerrgb

would output

exposure/exposure=1.444544 [-18.000000..18.000000] -> -3.5
channelmixerrgb/red[0]=1.000000 [-2.000000..2.000000]
channelmixerrgb/green[0]=0.000000 [-2.000000..2.000000]
channelmixerrgb/blue[0]=0.000000 [-2.000000..2.000000]
channelmixerrgb/saturation[0]=0.000000 [-2.000000..2.000000]
channelmixerrgb/lightness[0]=0.000000 [-2.000000..2.000000]
channelmixerrgb/grey[0]=0.000000 [-2.000000..2.000000]
channelmixerrgb/normalize_R=FALSE [FALSE/TRUE]
channelmixerrgb/normalize_G=FALSE [FALSE/TRUE]
channelmixerrgb/normalize_B=FALSE [FALSE/TRUE]
channelmixerrgb/normalize_sat=FALSE [FALSE/TRUE]
channelmixerrgb/normalize_light=FALSE [FALSE/TRUE]
channelmixerrgb/normalize_grey=FALSE [FALSE/TRUE]
channelmixerrgb/illuminant=DT_ILLUMINANT_BB [DT_ILLUMINANT_PIPE, DT_ILLUMINANT_A, DT_ILLUMINANT_D, DT_ILLUMINANT_E, DT_ILLUMINANT_F, DT_ILLUMINANT_LED, DT_ILLUMINANT_BB, DT_ILLUMINANT_CUSTOM, DT_ILLUMINANT_DETECT_SURFACES, DT_ILLUMINANT_DETECT_EDGES, DT_ILLUMINANT_CAMERA, DT_ILLUMINANT_LAST]
etc.

This can then be saved/redirected to the .params file, edited and used as input for the next run. Anything after the "[" at the end (specifying the allowed values) will be ignored during processing.

AxelG-DE commented 5 months ago

The file extension is changed from .override to .params (since it can also be used just to query params fields, individually, for a whole module or all). To dump all the params in history to output, put a line "all" in the file.

for example

exposure/exposure=-3.5
channelmixerrgb

I am a little bit confused you say put "all" in that file and all params will be exported. And then your "example" does not show that "all" in the particular place or did my limited language skills again trick me?

dterrahe commented 5 months ago

An "example" doesn't show every possible option. The output of "all" is large (have you maybe just tried it?) and would drown out the output of other possible formats in the example. It is also possibly not the option that would be most useful in everyday use.

AxelG-DE commented 5 months ago

I haven't tried yet. I understood your first part of the example as the initiation and I got confused already.

For everyday use, it would be way more practical to initiate the export from within the GUI.

AxelG-DE commented 5 months ago

What does it mean, this PR is closed?

dterrahe commented 4 months ago

Just saw https://www.mail-archive.com/darktable-user@lists.darktable.org/msg13175.html and following discussion. This is meant to help implement exactly something like that.

@MStraeten @wpferguson

wpferguson commented 4 months ago

I think the real answer to this is houz's comment on https://github.com/darktable-org/lua-scripts/issues/195 where he said implement introspection into the Lua API. But, first I'd have to understand it :-), though I've been looking at it.

dterrahe commented 4 months ago

implement introspection into the Lua API

That is basically one of the functionalities of dt.gui.action, so I believe the lua case is covered.

This PR was intended to enable an external tool to use the command line to generate output. This requires any parameter changes to be made in a file, either the xmp or another separate sidecar. I decided here the latter approach seemed to be more convenient. What this specifically adds is that the changes/overrides can be made in human readable form and are independent of the version of the module impacted (as long as the field is still present).

wpferguson commented 4 months ago

I know there are several people trying to implement the external (xmp modification) approach and this would really lower the bar for implementing it.

dterrahe commented 4 months ago

Please @ any that are on GitHub. I've send emails to some that don't seem to be. The mailinglist gave me problems in the past so I'm not subscribed.

homer3018 commented 4 months ago

Very interesting indeed, thanks for sending me the link ! I'll have a deeper look as soon as I can.

ralfbrown commented 4 months ago

At a quick glance, this looks like it could form the core for switching the sidecars from using blobs to actual parameter/value lists as already discussed many years ago. Advantages of the latter approach, as I'm sure was already mentioned back then, is that it's not only human-readable, but easier to handle newly-added parameters without making things version-specific and easy to make presets/styles which only change some of the module's parameters.

dterrahe commented 4 months ago

this looks like it could form the core for switching the sidecars from using blobs to actual parameter/value lists

Definitely not where this is intended to go. Both the data format and the loading mechanism are too inefficient for that purpose and really meant to just override a few individual parameters. And doesn't support arrays (yet). Nor blending/masks.

The "real" problem with versioning is that validation is currently only done in the ui. There's no data/view separation. If the versioned blobs are replaced with something more freestyle, then the legacy_params mechanism needs to be replaced/augmented with something in the loading process (which is currently module agnostic; directly to database) that translates old fields into new ones and sets added fields to (conditioned) defaults. Sure, possible. Work. And needs more "design" than what this provides at its "core".

So either wait (some more) until somebody picks up that large project, or use a simple special purpose mechanism, like this, in the mean time.

AxelG-DE commented 4 months ago

one issue besides the technical, is the discussion place. For example I am not listed on the mailinglist anymore (too much noise) and Sebastien is potentially not member here or on pixls and .....

BTW: the automatic mode of exposure is not suitable at all, as it normalizes everything. So a sunset and subsequent night will be the same bright, which is bollox

For Timelapses with changing light conditions, we use qdslrdashboard to automatically alter ISO, aperture and exposure. Those steps (usually 1/3EV) need to be harmonised, which is a very difficult job, I assume.

Yes WB ramping (or colorCAL) is essential for the so called holy-grail timelapse (transition form day-night and vice versa)