darktable-org
enfuseAdvanced isn't working
enfuseAdvanced is crashing on linux. On windows it runs, but produces a solid black tif that darktable refuses to read.
@BzKevin Could you please check this. @wpferguson What is is you enfuse version?
I'm running enfuse 4.2. I've been looking at the code. Part of the problem is the first run scenario. If you check for preferences, they always return null or false because they don't exist. If you make decisions off of the results, then they may not necessarily be valid. Another issue is the default values don't yield a usable image, at least on windows. There is also an issue which causes a crash on both windows and linux, but I haven't been able to reliably recreate it. And finally, the executable paths on linux don't get set correctly. Actually, they shouldn't be set at all and the operating system will just find the executables.
I think I've identified all the problems, I'm just looking at the code trying to figure out the cleanest way to fix them (and I still need to recreate the crash problem).
Bill
On Tue, Jun 11, 2019 at 10:40 AM supertobi [email protected] wrote:
@BzKevin https://github.com/BzKevin Could you please check this. @wpferguson https://github.com/wpferguson What is is you enfuse version?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/darktable-org/lua-scripts/issues/186?email_source=notifications&email_token=AAMPES4KASDKDWOAD6TAQO3PZ62M7A5CNFSM4HWH6IEKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXNLJQA#issuecomment-500872384, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMPES6XFB5B3PGZHF4X2ZDPZ62M7ANCNFSM4HWH6IEA .
Yes. When I tested, I must not have removed all traces and then ran it for the first time. It ran ok the first time on windows, but didn't produce a readable image. I fiddled with the settings to try and get a better image, then it crashed with an error about _( wasn't defined (sorry, I don't remember the exact error message). When ran on linux for the first time I got the _( error message also and a crash.
I think part of the problem is the first run scenario, where you query a boolean preference and it always returns false, because it doesn't exist, so you don't know if it's false because it's supposed to be or false because it doesn't exist. I get around that be setting a initialized preference to true after the first run so that I know the preferences are good.
Another issue is the first run doesn't set reasonable default values. I looked up the program defaults and just added those to the program structures. I also added a structure for the target, with some default values. I haven't modified the code to use them yet.
Another thing I ran into on linux is that it sends you to the update executables on the first run even if it finds all of them. I need to add some log statements to better understand the behavior. If it found all of the executables, then why should it need to update them?
I haven't worked on recreating the crash error yet, but I can do that today.
@BzKevin if you want to work on it, I can send you what I've got so far, or if you don't have time I can do it. Your call
I'm a bit tied up for the next week or two, so if you're making progress and don't mind doing the work then feel free to continue. Otherwise I am happy to dig in on these and make updates, it'll just be a bit of time before I can do so.
A few notes:
-
I also had errors with the translate function claiming to be undefined. I could not for the life of me figure out what the heck was causing it. In the end I removed the function call in a few places in the code which cause the error to stop appearing, so I had to leave a few strings untranslated because of this. I am stumped on this one!
-
I thought I handled setting the default values via this method:
- read the preference value
- check if the read in value is within a valid range via InRange() 3.a) value is valid then assign that to the associated GUI element 3.b) value is NOT valid (which would be the case if the preference doesn't yet exist) then assign a hard-coded default number to the associated GUI element
-
On the very first run after installing the script the update binaries window WILL appear because the "bin_exists" preference hasn't been set to true. Once the "update" button is pressed then that window should no longer appear, ever. The reason for this behavior is that at the time of GUI initialization I have NOT attempted to open each program to see if it exists (I wait and only do that once the user has actually tried to utilize the plug-in) so I don't know if they are valid or not. The reason I took this approach is because I was running in to a scenario on my machine where I would have numerous CMD windows blinking open and closed whenever I started dt; because all my plugins were attempting to start instances of their dependent programs on dt startup. I felt it preferable behavior for the plug-in not to search for the dependent resources until the user actually needs to use them. (Thus, I can open dt and the enfuse advanced plugin can load up, but won't search for enfuse, AIS, or exiftool until I actually try and run the plug-in on some images)
I hope what I am saying here makes sense. I also hope it provides insight as to how I INTENDED for the program to function. Clearly if you're having these issues something isn't functioning as expected. For what it's worth, I haven't seen any of these issues on my PC, so I'm surprised to hear you're having such major failures. (aside from the _( not defined error, I saw that some while writing the code, but thought by removing the problematic instances the issue was no longer present)
Like I say, I probably don't have the time immediately to dive in to these, but if you continue working on it then I will do my best to come back here and help collaborate the best I can.
I just installed 2.7 to chase the gimp bug, so I thought I'd try enfuseAdvanced. It popped up the update executables window, but it doesn't say which executable goes to which file selector. I assigned them based on what I remembered from the code, but I might have enfuse doing image alignment, exiftool doing exposure blending and align image stack copying tags :-D I'll take care of the too.
I checked the darktablerc file to see how I did with my guesses and none of the selections got saved.
Note to self: In the future, wipe out all knowledge of the software under test and do a first run.
It would seem that I didn't do a very good job of testing. :-(
@BzKevin I'll go ahead and work on this, but I'll pick your brain if I get stuck or can't figure something out
@BzKevin I figured out the initialization problem. It was mostly the sliders for enfuse. When you read the uninitialized preference, it returned 0, which was a valid value for InRange, so it set all the sliders to 0, instead of reasonable values.
@BzKevin I have a question. The only thing you were using InRange for was to check the values and set the default? It appears that you only do the check when creating the widget, and since the only way to set the values is with the widget, then there should never be out of range values.
I managed to create the crash scenario. When the update executables widget popped up, I just hit update without actually setting values so ExeUpdate set the prog.bin to my home directory. Needless to say, when I tried to actually run it everything came to a crashing halt. I still don't know why that specific error message, but at least I know how to create it again. This also exposes a weakness in check_if_bin_exists() which checks to see if something exists, but doesn't check to see if it's a file. I'm thinking about how to fix that. i could add check_if_dir_exists() and make check_if_file_exists() make sure it's a file, but then I'd have to find every instance of it and change it to check_if_dir_exists() where they are actually checking for an existing directory.
I've pretty much got it running fine on linux. I just need to clean up a few things. I'll start testing on windows today. It's funny, I never noticed the shell windows flashing up and disappearing until you talked about it and I actually watched for it. Those are happening because of check_if_bin_exists() checking to make sure the file actually exists. It doesn't start the program, just executes an "if exist..." windows shell command. I was reading an article about upcoming windows shell improvements. Maybe they'll make it possible to execute a shell command without having to open a window.
On Fri, Jun 14, 2019 at 12:47 PM wpferguson [email protected] wrote:
@BzKevin https://github.com/BzKevin I have a question. The only thing you were using InRange for was to check the values and set the default? It appears that you only do the check when creating the widget, and since the only way to set the values is with the widget, then there should never be out of range values.
thats correct. I believe my logic was this would catch the scenario where the stored value hasn’t been initialized I.e. first run. You’ve proven that false. So I suppose we could just make sure the read-in value isn’t zero. If it’s zero use default, otherwise use the read-in value?
I managed to create the crash scenario. When the update executables widget popped up, I just hit update without actually setting values so ExeUpdate set the prog.bin to my home directory. Needless to say, when I tried to actually run it everything came to a crashing halt. I still don't know why that specific error message, but at least I know how to create it again. This also exposes a weakness in check_if_bin_exists() which checks to see if something exists, but doesn't check to see if it's a file. I'm thinking about how to fix that. i could add check_if_dir_exists() and make check_if_file_exists() make sure it's a file, but then I'd have to find every instance of it and change it to check_if_dir_exists() where they are actually checking for an existing directory.
ahh, good catch!
I've pretty much got it running fine on linux. I just need to clean up a few things. I'll start testing on windows today. It's funny, I never noticed the shell windows flashing up and disappearing until you talked about it and I actually watched for it. Those are happening because of check_if_bin_exists() checking to make sure the file actually exists. It doesn't start the program, just executes an "if exist..." windows shell command. I was reading an article about upcoming windows shell improvements. Maybe they'll make it possible to execute a shell command without having to open a window.
Glad it’s working on Linux! If they did a windows update that would be nice, the pop-ups really bother me...
—
You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/darktable-org/lua-scripts/issues/186?email_source=notifications&email_token=AIVSGIGKSR5IRN4MQDR3GX3P2PDSVA5CNFSM4HWH6IEKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXLNRA#issuecomment-502183620, or mute the thread https://github.com/notifications/unsubscribe-auth/AIVSGIHKSWVIH3LXW6CYI7TP2PDSVANCNFSM4HWH6IEA .
@BzKevin more questions. But first a discovery. If you want to recreate the messed up print message, don't create any variants then run it and tell it to create image variants. That gives you the attempt to call a number value (local ''), error when it tries the dt.print(ENF.name..(" failed")). I'm about to debug that and see what's really going on.
Now to the questions. Should we create the dri_1,2,3 variants, even if they are just copies of the default values? That way worst case is someone gets 3 images that look the same.
Do you have an idea of what good default values are for dff? I don't have an image set to test it with. Maybe I can create one. Same question about creating dff presets.
Should AIS and Target settings be part of the presets?
I think that's it for now...
Sorry ,been jam-packed busy!
-
I would say that if any variant is identical to another variant it should be omitted, however, that seemed like a somewhat intensive use of memory and code, so I just forwent this and decided to let the user do what they want to do for simplicity.
-
I have read some articles about this. I believe the values I used in the if statement after the InRange( ) calls are what were suggested. I am in the process of getting some image sets now actually, so I can provide if you like. I'll see if I can find the article explaining recommended DFF settings.
-
I do not think so, because AIS does not re-run for each dri/dff. My logic was: Do I need this value to run the ENFUSE part of this script? Yes - include in preset No - do not include in preset.
To be honest every time I work on this script I contemplate pulling out all the preset utility because it adds a lot of bloat and complications for (in my opinion) little return. I believe this feature is minimally utilized, but really I can't know for sure. Because I'm not certain how much it gets used I've always opted to just leave it in since it's already there and working (or so I thought).
I was hoping you'd be busy :-). I bought a Canon Pixma PRO-100 awhile ago and just got around to hooking it up a week or so ago. So, now I'm exploring the deep, dark, world of printing. And, my new camera arrived yesterday. And, on top of all that I've been running like a chicken with my head cut off, so I understand.
I think with the variants, I'll just add a check to see if there are any before I try and do something with them. If there aren't, I'll just pop and error and uncheck the box.
A while ago (pre enfuse 4.2) I rewrote the enfuse module and made it an exporter. I separated blending from focus stack and made two storages in the same file, because they shared a common core. From my memory, the arguments to the two were somewhat mutually exclusive. I read the 4.2 manual and it seems to confirm that. DRI uses exposure_weight and saturation_weight, but not contrast_weight, contrast_edge_scale and contrast_min_curvature. DFF is the opposite, exposure and saturation weights are set to 0 and contrast_weight to 1. DFF also needs hard_mask on to prevent softness.
Okay, I understand the preset logic.
Maybe you should ask the question in the mailing list and on pixls.us to see if the variant/preset stuff really get used. I agree there's a lot of complexity and bloat. I've been playing trying to add more fields to the ENF.args, etc. so that I can introduce more loops by using an extra argument or 2. On the plus side, it's exposed me to presets. I've written a post export sharpener and I wanted to have presets for web sharpening, print sharpening, etc. The script works fine, but I only have placeholders for the preset stuff. Working with this has made me think about how to do it and given me some ideas.
So, I guess I'll put my toys aside :-), and get back to work on this. I think I have a fairly good idea of what needs done now, so hopefully I'll have it by the first of next week.
@BzKevin I think I've solved the popping windows problem when enfuseAdvanced starts. While working on unit testing for the file lib I looked at check_if_file_exists(). If you run the if exist using popen and change the results to echo 'yes' and echo 'no', then test for it you can do it without any windows popping up.
Hello there, Sorry to bump this up, but since this is still open and describe well the issue I have... So I'm running DT 3.2.1, and I just cloned the Lua scripts repo at the right location on my Mac. At first it seems that I had issue with the API version which needed to be >6.0.0, but that seems to be gone now.
I cloned the repo because I was running an older version which was not working anymore with DT 3.2.1, and it was not git managed.
When I set my luarc file as require "contrib/enfuseAdvanced", it fails like so
enfuse: trailing garbage ",0" in "0,0"
49,304858 LUA ERROR : /Users/seb/.config/darktable/lua/contrib/enfuseAdvanced.lua:466: attempt to call a number value (local '_')
stack traceback:
[C]: in local '_'
/Users/seb/.config/darktable/lua/contrib/enfuseAdvanced.lua:466: in function </Users/seb/.config/darktable/lua/contrib/enfuseAdvanced.lua:420>
This occurs when all the pics are exported, and in DT it should be in the blending stage.
When I set luarc with --require "tools/script_manager", then
[...GTK messages...]
3,672560 LUA LUA_DIR is /Users/seb/.config/darktable/lua
3,672643 LUA found lua scripts directory
3,672647 LUA scripts managed by git
3,672675 LUA git not found
4,480220 LUA processing .git/logs/refs/remotes/homer3018/wordpress_export
4,480259 LUA category is .git
4,480262 LUA name is wordpress_export
4,480265 LUA category is .git
4,480266 LUA name is wordpress_export
4,480277 LUA processing .git/refs/remotes/homer3018/wordpress_export
4,480296 LUA category is .git
4,480298 LUA name is wordpress_export
4,480301 LUA category is .git
4,480303 LUA name is wordpress_export
4,480311 LUA processing contrib/AutoGrouper
4,480314 LUA category is contrib
4,480315 LUA name is AutoGrouper
4,480319 LUA category is contrib
4,480320 LUA name is AutoGrouper
[...]
4,481428 LUA name is get_libdoc
4,481430 LUA category is tools
4,481432 LUA name is get_libdoc
4,481440 LUA processing downloads/
4,481442 LUA category is downloads
4,481443 LUA name is
4,481447 LUA category is downloads
4,481448 LUA name is
4,481593 LUA ERROR Cant read from .git/wordpress_export
It still fires up DT but without any scripts.
I'm not too sure what's going on there, and I would appreciate some guidance. My apologies if this is actually not an error but a mistake on my part. My main issue at the moment is being able to run EnfuseAdvanced.
Thanks ahead of time for your support.
I just merged a fix for the error on line 466 in enfuseAdvanced. Go to your lua scripts directory and do a git pull to get the update.
If you want to use script_manager in the luarc file the line you need is
require tools/script_manager
with the -- which makes it a comment. Once script_manager starts, you can select which scripts to enable.
Hi,
Unfortunately I have the exact same error :
enfuse: trailing garbage ",0" in "0,0"
40,680477 LUA ERROR : /Users/seb/.config/darktable/lua/contrib/enfuseAdvanced.lua:466: attempt to call a nil value (upvalue '_')
stack traceback:
[C]: in upvalue '_'
/Users/seb/.config/darktable/lua/contrib/enfuseAdvanced.lua:466: in function </Users/seb/.config/darktable/lua/contrib/enfuseAdvanced.lua:420>
Is there another for loops that we may have forgotten ? The only one I've spotted is on line 484 for the main function. I've tested the same change you made, without success.
The script manager does not work any better which was expected since you made changes just in enfuseAdvanced.lua
The change I made at line 458 should have prevented this from happening. I'll find some photos I can test it with and see if I can create the error.
I managed to recreate the error by specifying an invalid enfuse executable. To work around this error you can start tools/executable_manager from script_manager or just require it in your luarc. Select enfuse as the executable and then locate the executable and select it so that enfuseAdvanced knows where to find it. That should get enfuseAdvanced working.
Now for the error. I can recreate it, but I don't understand it at this point so I'll have to play with it some and figure out what is happening.
Found the error. _ was declared globally in a function, so once the function was called any attempt at translation wouldn't work.
Well, more and more interesting :
enfuse: trailing garbage ",0" in "0,0"
45,502500 LUA ERROR enfuse failed
This occurs at the same time compared to before, ie ~45sec after I start the export which is the time required to export these 5 pics. It does not feel like it starts blending at all. How can I check which enfuse version I'm running ? Maybe I should get these updated as well ?
Anyhow, thanks for the help, very much appreciated !
@home3018 thanks for prompting. I'd forgotten about this. Sorry.
The good news is when you don't look at something for awhile and then you see it again and you sometimes immediately recognize it.
You live in a part of the world where the decimal separator is a ',' versus a '.'. So when enfuseAdvanced gets the slider values they come back as digit,digit instead of digit.digit and when you pass them as arguments to programs commas do funny things to the arguments and you end up with enfuse: trailing garbage ",0" in "0,0".
This is a bug with the slider widget implementation because it returns the printed value with localization instead of the float value. It's on my list of things to fix, but I don't think I'll get to it anytime soon. In the meantime, there is a workaround so I'll try and fix enfuseAdvanced in the next day or two
I was wrong about the bug with the slider implementation. I went back and looked at the code and it's fine. The problem happens when arguments are formatted for the command line and numbers are formatted in a locale specific way, i.e. using a comma as the decimal separator.
Fixed by #294
Hey, thank you for being here once more :) True that I'm having various issues with locale settings in DT... Not sure why this happens though but now that you mentioned it, it makes perfect sense.
I'll test #294 and report back !
Thanks again for the assistance.
I'm sorry to report that I'm experiencing the very same outcome upon fast-forwarding my master... I've checked several times with different combination of files, settings for enfuse, but it always end up the same with that trailing garbage.
And actually if I tell it to align images in the Align_Image_Stack section of the settings (which I do seldomly because I'd usually use my tripod), it fails earlier on aligning :
align_image_stack: Invalid parameter: correlation should be between 0 and 1
where by default this setting sits at 0.9. But this one is not treated as a regular slider, we only have a dropdown with integers to choose from (interestingly enough with a dot as the decimal separator)
If I choose 1.0 it aligns images but then fails with the trailing garbage.
Would it be that we need to replace the commas at several locations and not only one ?
There's actually 3 command lines and I sanitized the wrong one.
This time a added a sanitize_decimals() function and used it on all of the command lines. Go ahead and update your scripts and try it again.
Well done sir, it now works perfectly (and of course also with the alignment option on). So it was indeed the decimal separator.
Thanks for all the hard work !