Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run without background process when -r or -p is passed (also allows not using dbus) #1480

Closed
deliciouslytyped opened this issue Mar 16, 2021 · 15 comments · Fixed by #2003
Closed
Labels
CLI CLI specific Enhancement Feature requests and code enhancements

Comments

@deliciouslytyped
Copy link

deliciouslytyped commented Mar 16, 2021

Flameshot currently requires starting a separate background process, with the rationale that this is required to handle the clipboard.
Currently flameshot is the only screenshot application that I was able to find, that works on gnome3 with wayland. gnome-screenshot doesn't do what I want, so I need to use flameshot.

I want to output a screenshot on stdout, and this is possible with the -r option, however, needing to run a background process for flameshot gui to work, is awkward.

flameshot gui starting a proper flameshot process, taking a screenshot, and then returning the output, is the desired behaviour.
Having this all done in one process context would also allow flameshot to work properly without dbus.

My current solution is this awkward mess:

set +m; mkfifo wat; { ( flameshot & flame_pid="$!" ; cat wat; kill -9 "$flame_pid" ) & sleep 5 } 2>/dev/null; flameshot gui -r | display `#do stuff here`; echo > wat; rm wat; set -m;

which could be reduced to:

flameshot gui -r | display `#do stuff here`

#1468 (comment) is related to making this process less awkward as well.

@deliciouslytyped
Copy link
Author

deliciouslytyped commented Mar 16, 2021

Here is a oneliner for OCR-ing screenshots:

This is what it would look like if it supported what I'm asking for:

nix-shell -p flameshot tesseract imagemagick --run \
  "flameshot gui -r | convert -modulate 100,0 -resize 400% - - | tesseract - - 2>/dev/null;"

This is what it currently looks like:

nix-shell -p flameshot tesseract imagemagick --run "set +m; mkfifo wat; { ( flameshot & flame_pid="$!" ; cat wat; kill -9 "$flame_pid" ) & sleep 5; } 2>/dev/null; flameshot gui -r | convert -modulate 100,0 -resize 400% - - | tesseract - - 2>/dev/null; echo > wat; rm wat; set -m;"

Breakdown:

nix-shell -p flameshot tesseract imagemagick --run `# on NixOS we can get a shell with our dependencies.` " \
  set +m; `# Temporarily disable some job management messages.` \
  mkfifo wat; `# we wait on a named pipe to know when to kill the background process...` \
  { ( flameshot & flame_pid="$!" ; cat wat; kill -9 "$flame_pid" ) & sleep 5; } 2>/dev/null; `# The wrapping is to suppress the new PID message.` \
  flameshot gui -r | convert -modulate 100,0 -resize 400% - - | tesseract - - 2>/dev/null; \
  echo > wat; rm wat; set -m; `# cleanup` \
  "

@mmahmoudian
Copy link
Member

mmahmoudian commented Mar 16, 2021

@deliciouslytyped I don't this this dbus is going away anytime soon, but your "feature request" imho has nothing to do with the dbus but rather having an override on saving to the file when -r is specified.

Perhaps the more complete solution is that Flameshot respects all flags (-c, -r, and -p) if the user has specifically specified them. Would that satisfy your needs?

@deliciouslytyped
Copy link
Author

The dbus stuff was a side-issue.
The key thing is to not need to start a background process.
Using -r and -p are just the general contexts where this makes sense.
You can keep the dbus stuff even if you aren't running a separate background process, it just isn't needed anymore in that specific case, because the output target is not the X11 clipboard.

@mmahmoudian
Copy link
Member

mmahmoudian commented Mar 16, 2021

@borgmanJeremy @ZetaoYang @Martin-Eckleben @lupoDharkael
What do you think?

I personally think that:

  1. there is no necessity for Flameshot to do one single action and it can send the output in different routes sequentially (e.g flameshot screen -r -c -p img.png).

  2. Regarding the dbus, I don't have proper understanding on what are the use cases of it, so the only comment I can give is that it make sense to not have the dbus whenuser has specifically asked for -p and -r iff its only purpose of dbus is to handle the clipboard.

@mmahmoudian mmahmoudian added Enhancement Feature requests and code enhancements CLI CLI specific labels Mar 16, 2021
@migzamudior3
Copy link

migzamudior3 commented Mar 16, 2021 via email

@holazt
Copy link
Collaborator

holazt commented Mar 16, 2021

@borgmanJeremy @ZetaoYang @Martin-Eckleben @lupoDharkael
What do you think?

I personally think that:

  1. there is no necessity for Flameshot to do one single action and it can send the output in different routes sequentially (e.g flameshot screen -r -c -p img.png).
  2. Regarding the dbus, I don't have proper understanding on what are the use cases of it, so the only comment I can give is that it make sense to not have the dbus whenuser has specifically asked for -p and -r iff its only purpose of dbus is to handle the clipboard.

@deliciouslytyped

The following should work.

flameshot full -r | display `#do stuff here` 

or

flameshot screen -n 1 -r | display `#do stuff here` 

or
Via flameshot gui
image

image

@mmahmoudian
Copy link
Member

@ZetaoYang

This works (raw to stdout and copied to clipboard):

flameshot screen -n 2 -r -c | display

This does not work:

flameshot screen -n 2 -r -c -p ~/test.png | display

does not save the screenshot to \\test.ong, does not copy to clipboard and also produces:

display: no decode delegate for this image format `' @ error/constitute.c/ReadImage/572.

@deliciouslytyped
Copy link
Author

@mmahmoudian @ZetaoYang: None of these work without the background flameshot process running. Please check that you don't have any flameshot processes running in the background when you test these, or you can probaby kill it with echo Killed: $(kill -9 $(pgrep -f '/flameshot$')).

@mmahmoudian
Copy link
Member

mmahmoudian commented Mar 17, 2021

@deliciouslytyped

None of these work without the background flameshot process running. Please check that you don't have any flameshot processes running in the background when you test these

I changes the background of my monitors to be different make it easier to see what is screenshot and what is not. I'm recording from my 3rd monitor (with Manjaro background) and I took screenshot from my 2nd monitor (with dark camouflage-patterned background).

Peek 2021-03-17 11-16

@deliciouslytyped
Copy link
Author

deliciouslytyped commented Mar 17, 2021

Hm. Ok, I'm not sure what's going on.
I'll try to take a closer look later.

@l29ah
Copy link

l29ah commented Apr 28, 2021

Do i get it right that i need to explicitly killall flameshotafter every time i run flameshot to avoid the process running in the background indefinitely and eating my battery while listening to my key presses?

@mmahmoudian
Copy link
Member

mmahmoudian commented Apr 28, 2021

@l29ah You got the concept awfully wrong. Flameshot does not listen to your keys. To put it in simple terms, Flameshot has 2 parts:

  1. The main process which listens to org.flameshot.Flameshot dbus. your system already has tons of dbus which you can list using qdbus command. To count the number of dbus running on your computer you can do qdbus | wc -l (I have 131 on my computer).
  2. the flameshot command (e.g flameshot gui) which sends the command to the main process and asks it to do a specific thing (in case of my example, the gui command).

Flameshot main process does NOT listen to your key presses.

On my computer it uses 145KB of memory. So imho no one should be worried about saving that much memory:

image

if you want to try it yourself:

ps --pid $(pgrep flameshot) -o pid,%cpu,%mem,vsz,rss,command | numfmt --header --to=iec --field 4-5

The numfmt command is number formatter and it converts the numbers in column 4 and 5 from Bytes to Kilobyte or Megabyte to make it more human readable.

For more information about dbus read the following:

@l29ah
Copy link

l29ah commented Apr 28, 2021

Flameshot does not listen to your keys.

strace says it does, as it wakes up and syscalls every time i hit a key in my X session.

your system already has tons of dbus

‰ qdbus | wc -l
qdbus: could not find a Qt installation of ''
0

The numfmt command is number formatter and it converts the numbers in column 4 and 5 from Bytes to Kilobyte or Megabyte to make it more human readable.

No, your script (or in fact your interpretation of its output) is incorrect, and in your example it consumes 145MB of RAM (and being 0.4% of your 32GB installed), as ps outputs sizes in kilobytes.

@mmahmoudian
Copy link
Member

@l29ah you are right about the ps. For other who are on the same wrong wagon as I was, I just checked the documentation again and:
image

Regarding the qdbus, i think you need to have qt-dev installed. but according to a post I found on How to identify what processes are consuming dbus sockets the following should work to some extent:

sudo lsof -u dbus | grep -i socket | wc -l

About the listening to keystokes, perhaps @borgmanJeremy or @ThePurple can provide better answer.

@veracioux
Copy link
Contributor

veracioux commented Oct 26, 2021

#2003 is the solution to all our problems :) If you are up for it, you can help us with testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI CLI specific Enhancement Feature requests and code enhancements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants