labstreaminglayer
Update clirecorder.cpp
better CLI:
- accurate help message
- streams that can't be found on start are added to wachfor
- runs until exited, not until keypress
Thanks for the PR. How do you envision to stop recording then from the command line, instead? Keeping execution in a while(true) loop makes it hard to gracefully shut down recording.
As a note, some users use a python wrapper for scripting recording, and changing would be a breaking change in LabRecorderCLIs "API". Not saying this can't be addressed, only highlighting that this is a breaking change.
Well, we use kill (or taskkill in windows) - makes us less likely to accidentally stop recording by pressing a key in the wrong window.
If that is a breaking change that we want to avoid, it can be made optional, with the current behaviour as the default - or removed altogether, it's not the main change here.
Thanks.
About the change to add streams that are not found to watchfor (instead of returning 2), i feel it might be better if that becomes optional, too. Maybe an additional command line argument like --watch-missing-streams, that'd trigger the new behavior?
That could be also a solution for the wait-for-return versus kill/signal approach. Maybe add a command line argument like --to-background? At least on unix, it might be the case that adding & already does the trick...
Not a thorough review, but from a quick glance the "signal handler" approach is better and even works in the console, you'd just need to switch to Ctrl+C when running it manually.
The while (true) std::this_thread::yield(); on the other hand needlessly wastes CPU cycles. Blocking on user input (while (true) std::cin.get();) or even reading in commands would be better, imho.
