Rawtherapee: cli version to speedup start time / rt-cli branch
I've created rt-cli branch that build 2 executables : rawtherapee.exe and rawtherapee-cli.exe. The later one doesn't integrate the GUI code at all but includes all the command line options, while the first one now only support directory or files through the command line options. Use -h to see what is supported. You'll se a new -q option (for _Quick Start_ mode) that will not load the HaldCLUT files nore the color profiles. Users should use this flag if they start RT through command line for every new file to process (i.e. don't use wildcards).
To test the -q mode, just include film simulation, custom input profile and non standard output profile to the pp3. Works fine here.
Example of processing a 10MPix PEF file with lots of processing on Core-i7 / Windows7/64 :
- rawtherapee-cli.exe - w/o
-q: 9.08s - rawtherapee-cli.exe - w/
-q: 7.50s
The difference is (should be) the gain of the -qflag.
This will increase the install bundle by ~2Mb. Please tell me if it's worth it.
It's still possible to revert the usual rawtherapee.exe to its initial state (i.e. get back all the options flags) and reserve the cli version to people that wants performance and can build themselves...
Hombre57
All 91 comments
I've found one limitation : when using the -q mode, the (automatic icc) will not find any suitable intput profile even if it exist, just because we haven't loaded them all. It will still process the file but with a wrong result. How should we handle this problem ? I'd suggest to put a warning when rawtherapee-cli is started.
Hombre57
on 14 Feb 2017
suggest to put a warning when rawtherapee-cli is started.
No, this has to be solved, not warned about!
heckflosse
on 14 Feb 2017
Ok. I'll scan the input profile directory for a suitable profile if this parameter is set and only load it, not load all of them.
Hombre57
on 14 Feb 2017
@Hombre57 iirc I made some changes to that part of code to only load profiles on demand, not in advance...
heckflosse
on 15 Feb 2017
Yes, but not for the auto selection mechanism (still have to look at the code, but now going to bed ;) ).
Hombre57
on 15 Feb 2017
@Hombre57 Oh, my bad :(
heckflosse
on 15 Feb 2017
@Hombre57 how is progress with this, should we move it to 5.2?
Beep6581
on 6 Mar 2017
Almost ready. Working on it tomorrow, should be ok this w.e.. It's up to you to move it to 5.2 if you want, I don't mind.
Hombre57
on 6 Mar 2017
Branch rt-cli updated with commit 567ed36. @Floessie, it would be nice if you could have a look at the removed const from rtengine/iccstore.h. I had to remove them because the profiles can now be loaded with rawtherapee-cli and -q switch. Do we have to make the functions const again ?
Last thing to update is the InnoSetup script, but that's trivial, I'll do that before merge.
Hombre57
on 13 Mar 2017
@Hombre57 I see.
Do we have to make the functions const again ?
From my POV yes, and I'll take care of it, if that's okay with you. I understand, why you had to remove the consts. The best thing to solve it, is to PIMPL ICCStore (which is due anyway). BTW: Shouldn't loadProfile() and friends reside in an anonymous namespace instead of rtengine?
If you like, I'll roll ICCStore up. :smile:
Best,
Flössie
Floessie
on 13 Mar 2017
@Hombre57
I just made a build of rt-cli branch uploaded at https://drive.google.com/open?id=0B2q9OrgyDEfPS2FpdDAtMVI1RG8.
As far as I can see the build seems correct and contains as I should expect the 3 exe files (rawtherapee.exe, rawtherapee_debug.exe and rawtherapee-cli.exe). I briefly tested the 3 exe and they run ok.
What you intend to update in the innosetup script?
gaaned92
on 13 Mar 2017
@Floessie Please do whatever you want regarding code correctness, the functionality is ok. What does PIMPL means btw ?
@gaaned92 I haven't checked that part, hence my notice. There's nothing to do then, unless decision is taken to bundle rt-cli separately.
Could you test that it's working as you'd would expect ? Works fine for me. I'll wait few days before merging, unless you give me a go.
Hombre57
on 13 Mar 2017
@Hombre57 PIMPL is the "pointer to implementation" idiom. You basically declare only a class Implementation and a const std::unique_ptr<Implementation> implementation in the private section of the header and forward all calls to the implementation defined in the source file. There are some more sophisticated rules to that, I know, but I always keep it that basic.
So, I'll have a look at it on your branch, okay?
Floessie
on 13 Mar 2017
not sure that I understood what you said :/
Yeah, you can take your hand over it.
Hombre57
on 13 Mar 2017
@Hombre57
Tested here on W10, GTK3.22.9.
Works fine here.
I used the -q option and tested successively S, s, d and p option.
For me S option is the most interesting as it can replace the batch option.
gaaned92
on 15 Mar 2017
@Hombre57 I'll push on next weekend. There's no spare time for me until then.
Floessie
on 16 Mar 2017
No problem, there's no hurry on my side. Thanks in advance for your cleanup !
Hombre57
on 16 Mar 2017
@Hombre57 Here you are. More could be done, but I've spend two hours on this relatively fruitless work and now I've got to do some things in real life. :smile: I hope, I didn't break anything.
Floessie
on 18 Mar 2017
Thanks a lot @Floessie . I had a look at your changes, but there are too much changes in iccstore.cc so I can't review that one. However I have confidence in your skills, and my test says that it's all good, so I've merged rt-cli into dev and will delete the branch.
Hombre57
on 18 Mar 2017
@Hombre57
[ 38%] Building CXX object rtgui/CMakeFiles/rth-cli.dir/main-cli.cc.obj
H:/rt5master/rtgui/main-cli.cc: In function 'int processLineParams(int, char**)':
H:/rt5master/rtgui/main-cli.cc:669:69: error: 'loadDynamicProfile' was not declared in this scope
rawParams = loadDynamicProfile(ii->getMetaData());
^
H:/rt5master/rtgui/main-cli.cc:677:69: error: 'loadDynamicProfile' was not declared in this scope
imgParams = loadDynamicProfile(ii->getMetaData());
^
rtgui/CMakeFiles/rth-cli.dir/build.make:222: die Regel für Ziel „rtgui/CMakeFiles/rth-cli.dir/main-cli.cc.obj“ scheiterte
make[2]: *** [rtgui/CMakeFiles/rth-cli.dir/main-cli.cc.obj] Fehler 1
CMakeFiles/Makefile2:276: die Regel für Ziel „rtgui/CMakeFiles/rth-cli.dir/all“ scheiterte
make[1]: *** [rtgui/CMakeFiles/rth-cli.dir/all] Fehler 2
make[1]: *** Es wird auf noch nicht beendete Prozesse gewartet...
[ 96%] Built target rth
Makefile:127: die Regel für Ziel „all“ scheiterte
make: *** [all] Fehler 2
@heckflosse Ingo, I take this one! I'm about to commit a fix for #3763.
Floessie
on 18 Mar 2017
@heckflosse @Floessie The bugfix should be to add #include "dynamicprofile.h" into main-cli.cc. I should have built before committing the merge (I thought it was safe, sorry).
Hombre57
on 18 Mar 2017
Damn it! This adds dynamicprofile.cc and that in turn profilestore.cc to CLISOURCEFILES. I can't handle that...
Floessie
on 18 Mar 2017
@heckflosse @Hombre57 I'll commit my fix on dev now anyway. You'll have to rebase your solution then, but the should be no conflicts.
Good night,
Flössie
Floessie
on 18 Mar 2017
@Floessie Thank you :+1:
heckflosse
on 18 Mar 2017
@heckflosse I hope, you didn't get me wrong: I'm not working on this issue. The fix was for #3763. Resolving those new link dependencies is over my head, sorry.
Floessie
on 18 Mar 2017
@Floessie Ok, that explains why it still does not build...
heckflosse
on 18 Mar 2017
Working on it, commit to come in an hour max.
Hombre57
on 18 Mar 2017
@Hombre57 If you need more time we can also temporary disable the rth-cli target in rtgui\CMakeLists.txt
heckflosse
on 18 Mar 2017
@agriggio I hope you don't mind but I've moved dynamicprofile.xx as is in rtengine, because it's part of the engine and don't include anything about the GUI. It will be part of my next commit.
Hombre57
on 18 Mar 2017
@heckflosse You're right, it will require a little bit of work because of the profilestore requirement. I'm working on this.
Hombre57
on 18 Mar 2017
@Hombre57 Shall I temporary disable the rth-cli target in dev?
heckflosse
on 18 Mar 2017
@Hombre57
I disabled rth-cli target in dev branch to allow building again. Please enable it again when you made the real fix.
heckflosse
on 18 Mar 2017
@Hombre57 I reverted my fix for the broken build with the intention to break the build because there is a critical bug in the rt-cli merge (no output profiles) which shouldn't go live.
@Beep6581 will report that more precise
heckflosse
on 19 Mar 2017
Hello
I returned from Uganda and hoped to develop some photos, but found that my PP3s failed to load the correct output profile. Turns out that apart from "No ICM" entry, the output profile combobox is empty!
The combo is full in commit 567ed362 but empty in 97caf21b.
I would call this bug very high priority, as it's easy to miss and may lead to wrong colors.
I will look for further bugs tomorrow.
Beep6581
on 19 Mar 2017
@heckflosse @Beep6581 Understood. I'l dig in ICCStore.
Hombre57
on 19 Mar 2017
@Hombre57 Thank you :+1:
heckflosse
on 19 Mar 2017
@Hombre57 Let me know if I can help at that task
heckflosse
on 19 Mar 2017
@heckflosse You can have a look at iccstore.cc, if you can understand it :sweat: , I'm looking at it on my side too. There must be something in getStdProfiles and/or getProfile I guess.
Hombre57
on 19 Mar 2017
@Hombre57 Thanks for fixing the bug :+1:
heckflosse
on 19 Mar 2017
@Hombre57 no problem at all! as a rule, when my contributions get merged in the official RT, then they are all yours :-) what I mean is that you don't need to ask me if you want to change the code, but I will still provide support if requested
agriggio
on 19 Mar 2017
Hi,
There is something more broken around camera profiles with this feature, at least with jpeg from my X20 (I have not this problems with the RAF files from my X100s). Here are a sample, more explanatory than any words.
- with the current HEAD (99f0d723b5d1c3d0c05d56a994b6c5c9c4c68601), so with the rt-cli merged and with the last fix:
- Then I reverted to ace00a8c8d0c31fe83e0d40b7de391f13af83fc9, so just before the rt-cli merge:
- An for the record, here is the result I would get without using any camera profile, that looks bad, but not worse than the one with rt-cli merged:
I can provide jpeg + pp3 if needed.
casta
on 19 Mar 2017
@Beep6581 @Hombre57
The combo is full in commit 567ed362 but empty in 97caf21b.
Guys, sorry for this one. It was my fault when converting that part to the wonderful rtengine::getFileExtension(). :disappointed:
Floessie
on 19 Mar 2017
@heckflosse @Hombre57 I'm at the one by @casta...
Floessie
on 19 Mar 2017
Found it! :tada: Will push to dev soon!
Floessie
on 19 Mar 2017
@casta Guillaume, could you have a look?
Floessie
on 19 Mar 2017
Yes indeed, it looks ok now.
Thank’s, that was fast ! ;)
casta
on 19 Mar 2017
@casta I broke it so I had to fix it. :wink: Thanks for reporting and testing! :+1:
Floessie
on 19 Mar 2017
All good now, thank you!
Beep6581
on 19 Mar 2017
Just FYI, I'm still busy on integrating the DynamicProfileStore. I'll have to reorganize the code quite a lot to keep the optimization (partly moved from rtgui to rtengine).
Hombre57
on 20 Mar 2017
@Hombre57 I don't want to trade with you... :neutral_face:
Floessie
on 20 Mar 2017
@Floessie Which trade are you talking about ? All your changes will be kept, I'm speaking of the DynamicProfile feature only. I'll copy your way of handling the ICCStore through ICCStore::getInstance() to the the ProfileStore wich will now BE and DynamicProfileRules class (a new class that will handle the loading of the rules). Anyway, you'll see the change when ready. I'll create a PR this time :wink:
Hombre57
on 20 Mar 2017
@Hombre57 Maybe you misunderstood: This was only an expression of empathy, meaning: I regret you and feel your pain for shuffling all those code around. When I first said I'll take this one, I wasn't aware of all the dependencies, but we both only realized later. So I'm still emotionally with you and know, what a job you're doing right now. I guess, we just hit a language barrier.
which will now BE and DynamicProfileRules class (a new class that will handle the loading of the rules)
That's wonderful.
Best,
Flössie
Floessie
on 20 Mar 2017
Patch pushed in rt-cli2 branch. Please test with options.verbose = true. You should see profile loading and rules loading when necessary only with -q switch (rt-cli).
Hombre57
on 25 Apr 2017
@agriggio If you intend to update the Dynamic profile mechanism, please do it in rt-cli2 branch. Could you also have a look on whether it is still fully functional in GUI and CLI mode ?
Hombre57
on 25 Apr 2017
:+1: I'll take a look asap (hopefully tomorrow)
agriggio
on 25 Apr 2017
@Hombre57
1- RT-cli2 builds and install ok on W10. available at https://drive.google.com/open?id=0B2q9OrgyDEfPS2FpdDAtMVI1RG8
2- dynamic profiles are not accessible with the GUI.
- run rawtherapee dev branch, verify that you are in dynamic mode and verify that dynamic profiles rules are ok. close the app.
- run rawtherapee rt-cli2 branch. verify that you are in dynamic mode : ok. The dynamic profile rules are not displayed.
@agriggio could it be possible to display in history the successive profiles applied when opening an image?
gaaned92
on 25 Apr 2017
@gaaned92 for debugging, you can see them in the console in verbose mode.
if you want them in the GUI, it's a bit of work which I can't promise at
the moment (and I'm not yet sure if I like it or not... but that's not a
big problem). feel free to open a feature request, but don't hold your
breath...
agriggio
on 25 Apr 2017
@agriggio It was just a suggestion. even a static default profile is not displayed in history.
gaaned92
on 25 Apr 2017
@gaaned92 Dynamic profiles wasn't loaded in Preferences. Should work fine now, thought that I hope there can't be any race condition on loading rules.
Hombre57
on 26 Apr 2017
@Hombre57 In RT gui, dynamic profiles are loaded. processing ok
RT cli : except for the -help, the console window closes and I have no time to read what happened. Surely there is a crash.
my command line is
D:\RAWTHERAPEE\rtinstall\rt-cli2release64\rawtherapee-cli.exe -O D:\PHOTOS\test\cli -S -Y -j98 -c D:\PHOTOS\test
where test and test/cli are existing directories. test contains photos with pp3 files.
Questions : is it possible to specify a directory in the -c parameter? the help indicates only files. specifying a directory would be more useful.
I will try the debug rtcli build.
@agriggio with a dev build, in verbose mode, when loading a photo without pp3, I am not able to see what dynamic profiles are loaded! As there are a lot of text, perhaps I missed it.
gaaned92
on 26 Apr 2017
@gaaned92 the messages are not prominent, but they should be there. Look for something like this:
found matching profile ${U}/default
found matching profile ${U}/tone-boost
found matching profile ${U}/resize_1920
@Hombre57 rt-cli2 tries to open a file "2016_1221_143429_099.raw$" that is corrupted.
I think it should not because first extension is not listed and second there is no pp3 sidecar and this file should not be processed.
If you want a bt full, I can upload it.
With regular dev build, even if I change the extension to "raw" listed in preferences, the thumbnail is not displayed.
gaaned92
on 27 Apr 2017
@agriggio I cannot find the messages in verbose mode.
I deleted the cache, put a few files in a separate directory without .pp3 files, opened succesively the images.
Nevertheless it works, but I think it could be useful to have the list of applied profiles at least in verbose mode, failing to have it in history panel.
André
gaaned92
on 27 Apr 2017
@gaaned92 strange, it works for me... :-/
does anyone else not see the messages in verbose mode?
agriggio
on 27 Apr 2017
@agriggio Alberto, I made a dynamic profile which opens all D700 files black and white. Then I opened a D700 file which had no pp3 file. It opened black and white and I have this message in log:
found matching profile ${G}\Black-and-White\Black-and-White 1
Works here using dev on Win7/64 in msys console. Maybe it does not work correctly when not started in msys console? In this case we should fix that.
@gaaned92 can you test wether it works when you start rt in msys concole?
heckflosse
on 27 Apr 2017
@agriggio I just tested starting rt from explorer (without msys console). The message appears in log there too.
heckflosse
on 27 Apr 2017
@heckflosse thanks for testing, this is the expected behaviour -- I just
use printf so it should work everywhere...
agriggio
on 27 Apr 2017
@agriggio Yes its ok with dev build. I think that in my first test of dev build, there was a pp3 in the cache. Sorry for this false diagnostic.
now, with rt-cli, I cannot get the loaded profiles in the console window. I will try further and report. using a msys console now.
gaaned92
on 27 Apr 2017
I found a bug.
When I start rawtherapee-cli.exe with Verbose=false in options file it reports:
RawTherapee, version 5.0-r1-gtk3-467-gd3ab104, command line
Terminating without anything to do.
which is perfectly fine.
When I start rawtherapee-cli.exe with Verbose=true in options file it reports:
Add camera constants for "HASSELB
This application has requested the Runtime to terminate it in an unusual way.
Please contact the application's support team for more information.
LAD CF-39"
Add camera constants for "HASSELBLAD CF-39MS"
Add camera constants for "HASSELBLAD CFH-39"
Add camera constants for "HASSELBLAD CFV-39"
Add camera constants for "HASSELBLAD H3D-39"
Add camera constants for "HASSELBLAD H3DII-39"
Add camera constants for "HASSELBLAD H3DII-39MS"
Add camera constants for "HASSELBLAD CFV-50"
Add camera constants for "HASSELBLAD H3DII-50"
Add camera constants for "HASSELBLAD H3DII-50MS"
Add camera constants for "HASSELBLAD H4D-50"
Add camera constants for "HASSELBLAD H4D-50MS"
Add camera constants for "HASSELBLAD H4D-200MS"
Add camera constants for "HASSELBLAD H5D-50"
Add camera constants for "HASSELBLAD H5D-50MS"
Add camera constants for "HASSELBLAD H5D-200MS"
Add camera constants for "HASSELBLAD H4D-40"
Add camera constants for "HASSELBLAD H5D-40"
Add camera constants for "HASSELBLAD H4D-60"
Add camera constants for "HASSELBLAD H5D-60"
Add camera constants for "HASSELBLAD H5D-50C"
Add camera constants for "HASSELBLAD CFV-50C"
Add camera constants for "DUMMYMAKE DUMMYMODEL"
Loaded H:\dfxtrans\Fujifilm X-T1.badpixels: 2 pixels
Loaded H:\dfxtrans\Pentax K-1.badpixels: 1 pixels
Loaded H:\dfxtrans\Pentax K-3 II.badpixels: 1 pixels
terminate called after throwing an instance of 'std::logic_error'
what(): basic_string::_M_construct null not valid
@Hombre57 #3768 is not fixed in main-cli.cc
diff --git a/rtgui/main-cli.cc b/rtgui/main-cli.cc
index e59ebaf..18734e0 100644
--- a/rtgui/main-cli.cc
+++ b/rtgui/main-cli.cc
@@ -198,8 +198,7 @@ int main(int argc, char **argv)
break;
}
- if(Console) {
- AllocConsole();
+ if(Console && AllocConsole()) {
AttachConsole( GetCurrentProcessId() ) ;
// Don't allow CTRL-C in console to terminate RT
SetConsoleCtrlHandler( NULL, true );
@Hombre57 a few post above, I noticed a crash of rt-cli2.
I can reproduce that crash if I put a txt file inside the directory (or .exe, dll or whatever). it tries to open it and segfault.
It happens with -d or -S.
gaaned92
on 27 Apr 2017
@gaaned92 Do you use wildcards when specifying the image files (e.g. IMGP*) ?
Hombre57
on 27 Apr 2017
@Hombre57 In the -c parameter I just give the directory name as I supposed it used the parsed extensions list.
gaaned92
on 28 Apr 2017
@gaaned92 No, it scans everything actually (and is the case in Dev too). I'm looking at that...
Hombre57
on 28 Apr 2017
@Hombre57 thank you
@agriggio I think I found the bug that puzzled me
- set RT in single editor and with dynamic profiles
- open a dir with images with no pp3
- open an image in the editor : the dynamic profile is loaded
- come back to browser, suppress the profile
- go to editor, open same file (thus already displayed), no dynamic profile is loaded as it should.
Can you confirm?
gaaned92
on 28 Apr 2017
@gaaned92 I'm not sure I understand what you mean by:
- come back to browser, suppress the profile
can you elaborate? do you mean that you clear the profile with the context menu? if the file is already open, I think this is the correct behaviour... there's an explicit "reset to default" menu to rebuild the dynamic profile. Or maybe I misunderstood?
agriggio
on 28 Apr 2017
Yes I delete from context menu. The displayed and processed photo gets then the neutral profile thus I do'nt think it's a correct behaviour. That just puzzled me when I wanted to verify what profiles were loaded and you can let it as it is. Thanks for patience.
André
gaaned92
on 28 Apr 2017
"processing profile operations -> clear" means that the pp3 gets removed, and reset to neutral
"processing profile operations -> reset to default" means that the pp3 gets restored to the default one. If this is "(Dynamic)", then the dynamic profile rules are invoked again, but this works the same way even if it is another "static" default.
I think the current behaviour makes sense, but maybe I'm biased :-)
What do the others think?
agriggio
on 28 Apr 2017
@agriggio
"processing profile operations -> clear" means that the pp3 gets removed and there is no neutral profile applied. Then the embedded jpg is displayed as thumbnail.
When you go in editor and select an image to process, the already displayed image acquires the neutral profile, and the selected image acquires the default profile (static or dynamic).
gaaned92
on 29 Apr 2017
ah, now I understand that you meant. I agree that it is a bug then :+1:
agriggio
on 29 Apr 2017
@agriggio If you need to fix something about Dynamic profile, please do it in rt-cli2.
Hombre57
on 29 Apr 2017
@Hombre57 ok, though I don't think this is specific to dynamic profiles (but I have to dig deeper to be sure)
agriggio
on 29 Apr 2017
The reason is that I've moved files from one dir to another, so it'll be easier for me to merge (which will happen soon).
Hombre57
on 29 Apr 2017
I think I will need to work only on rtgui/thumbnail.cc (still investigating though)
agriggio
on 29 Apr 2017
I confirm this is unrelated to the dynamic profiles. It is also a bit tricky to fix. Given that this is really a corner case (it happens only if you clear the profile of the image that is open in the editor) and that a workaround already exists (just call "reset to default"), I'm giving this low priority.
agriggio
on 29 Apr 2017
@gaaned92 @heckflosse @Beep6581 @agriggio I think that it's complete now. So if no one objects, I'll merge in 2 days.
[Edit: sh.. ! I mistakenly sent a message to Adam instead of Ingo... Sorry]
Hombre57
on 2 May 2017
@Hombre57 could I ask you to merge today if you feel it's ready? The reason is that we're behind schedule and being in dev gets it more testing.
Beep6581
on 2 May 2017
@Beep6581 Done.
Hombre57
on 3 May 2017
@Hombre57 on W10 dev branch builds and run ok.
RawTherapee_dev_5.0-r1-gtk3-483-gedfea689_WinVista_64 uploaded at https://drive.google.com/open?id=0B2q9OrgyDEfPS2FpdDAtMVI1RG8
gaaned92
on 3 May 2017
No bug reported, closing.
Hombre57
on 12 May 2017
Related issues
Beep6581
·
4Comments
heckflosse
·
5Comments
Beep6581
·
3Comments
heckflosse
·
3Comments
JIPG2
·
3Comments