ponty / pyscreenshot

Python screenshot library, replacement for the Pillow ImageGrab module on Linux.
BSD 2-Clause "Simplified" License
499 stars 89 forks source link

scrot 1.1.1 causing OSError: cannot identify image file #67

Closed EliasEriksson closed 4 years ago

EliasEriksson commented 4 years ago

System: Pop!_OS v19.10 python3.7

Using pyscreenshot.grab() causes an OSError using scrot v1.1.1 on my system as it tries to open a file called /tmp/pyscreenshot_scrot_alnh_dsl.png but in the system its called /tmp/pyscreenshot_scrot_alnh_dsl_000.png. This does not occur when using grab(backend=gnome-screenshot) and it does not crash using scrot 0.8 compiled from source.

I have worked around this issue by adding filename = "_000.".join(filename.split(".")) on line 24 in pyscreenshot/plugins/scrot.py but its not really pretty and one of the created files are not removed from /tmp. It looks like some naming is going very wrong somewhere?

Full traceback:

  File "/home/elias-eriksson/Dev/Project/main2.py", line 7, in <module>
    a = grab(backend="scrot")
  File "/home/elias-eriksson/Dev/Project/venv/lib/python3.7/site-packages/pyscreenshot/__init__.py", line 67, in grab
    to_file=False, childprocess=childprocess, backend=backend, bbox=bbox)
  File "/home/elias-eriksson/Dev/Project/venv/lib/python3.7/site-packages/pyscreenshot/__init__.py", line 46, in _grab
    _grab_simple, imcodec.codec, to_file, backend, bbox, filename)
  File "/home/elias-eriksson/Dev/Project/venv/lib/python3.7/site-packages/pyscreenshot/procutil.py", line 37, in run_in_childprocess
    raise e
OSError: cannot identify image file '/tmp/pyscreenshot_scrot_alnh_dsl.png'
EliasEriksson commented 4 years ago

After further digging it looks like the new version of scrot requires passing --overwrite to work otherwise it will recognize that the tempfile already exists and makes a new one resulting in name error.

-o, --overwrite           By default scrot does not overwrite the files, use this option to allow it.

Including this option made it work like a charm again.

ponty commented 4 years ago

It is strange because file names are randomly generated, so there should be no existing file. Old tmp files are always removed using NamedTemporaryFile. I test on Ubuntu 18.10 Py37. Please check your pyscreenshot version.

from pyscreenshot import __version__
print(__version__)

Can you send me a small demo program where tmp files are not removed?

EliasEriksson commented 4 years ago

I am on version 0.5.1.

This happends beccause the newer versions of scrot does not try to overwrite the tempfile created by pyscreenshot (/tmp/pyscreenshot_scrot_alnh_dsl.png) instead scrot recognises that a file with that name already exists and creates a new file (/tmp/pyscreenshot_scrot_alnh_dsl_000.png). pyscreenshot then tries to read the now empty tempfile with pillow (/tmp/pyscreenshot_scrot_alnh_dsl.png) and the program crashes resulting in OSError: cannot identify image. The change to scrots behaveour seams to have changed in version 1.0 described here so to replicate this you need a version of scrot nwerer than 1.0. To fix this issue the only change that is needed is the --overwrite option so scrut now again overwrites the tempfile (/tmp/pyscreenshot_scrot_alnh_dsl.png) instead of creating a new file (/tmp/pyscreenshot_scrot_alnh_dsl_000.png)

My initial work around made pyscreenshot read the screenshot but only the tempfile (/tmp/pyscreenshot_scrot_alnh_dsl.png) got deleted and the screenshot (/tmp/pyscreenshot_scrot_alnh_dsl_000.png) was left existing in /tmp

The reason I have scrot 1.1.1 is beccause thats the version supplied by my package manager. My laptop which runs Pop!_OS 18.04 LTS gets scrot 0.8 when instaleld with apt and this is not an issue on my laptop.

Demo program to replicate crash with scrot newer than 1.0

import pyscreenshot
pyscreenshot.grab(backend="scrot")
ponty commented 4 years ago

Thanks for the detailed info. The problem was that NamedTemporaryFile created an empty file. I changed this to use temporary folders, so no empty files are created. I will not add the --overwrite flag, because it is not compatible with older scrot versions.