enfuseAdvanced isn't working

[wpferguson]

enfuseAdvanced is crashing on linux. On windows it runs, but produces a solid black tif that darktable refuses to read.

[supertobi]

@BzKevin Could you please check this.
@wpferguson What is is you enfuse version?

[wpferguson]

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 ***@***.***> 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 <#186?email_source=notifications&email_token=AAMPES4KASDKDWOAD6TAQO3PZ62M7A5CNFSM4HWH6IEKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXNLJQA#issuecomment-500872384>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMPES6XFB5B3PGZHF4X2ZDPZ62M7ANCNFSM4HWH6IEA> .

[BzKevin]

This is with the latest version of the script, my refactored version?

[wpferguson]

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

[BzKevin]

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:

1. 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!

2. I thought I handled setting the default values via this method:

   1. read the preference value
   2. 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

3. 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.

[wpferguson]

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.

[wpferguson]

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

[wpferguson]

@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.

[wpferguson]

@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.

[BzKevin]

On Fri, Jun 14, 2019 at 12:47 PM wpferguson ***@***.***> 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 <#186?email_source=notifications&email_token=AIVSGIGKSR5IRN4MQDR3GX3P2PDSVA5CNFSM4HWH6IEKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXLNRA#issuecomment-502183620>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIVSGIHKSWVIH3LXW6CYI7TP2PDSVANCNFSM4HWH6IEA> .

[wpferguson]

@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...

[BzKevin]

Sorry ,been jam-packed busy!

1. 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.

2. 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.

3. 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).

[wpferguson]

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.

[wpferguson]

@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.