openatx / adbutils

pure python adb library for google adb service.
MIT License
736 stars 173 forks source link

Remove temporary file usage in `AdbDevice.screenshot()` #78

Closed PythonTryHard closed 1 year ago

PythonTryHard commented 1 year ago

This PR aims to improve adbutils.AdbDevice.screenshot() by removing usage of temporary screenshots.

Currently, the behaviour of screenshot() is to write the screenshot to a temp file then pull the screenshot out to load in and return. This behaviour is marked as "not thread-safe", to which I assume is related to the use of temp file.

screencap -p when not given the FILENAME argument will output the PNG screenshot to stdout which can then be read and loaded into PIL.Image. This behaviour is utilised to eliminate the need of a temp file on both the connected Android machine and the local machine.

As the screenshot is supposed to be raw bytes, using built-in methods such as read_until_close() with automatic decode to UTF-8 will mess with the bytes, therefore stream=True is enabled in the .shell() call, and manually reading the output in a similar fashion to read_until_close(). While this PR could have included a backward-compatible change in read_until_close() to allow opting out of decoding, therefore reducing duplicated code, by:

class AdbConnection(object):
    ...
    def read_until_close(self, decode=True) -> Union[str, bytes]:
    # Added                    ^^^^^^^^^^^     ^^^^^^     ^^^^^^
        ...
        return content.decode('utf-8', errors='ignore') if decode else content
        # Added                                        ^^^^^^^^^^^^^^^^^^^^^^^

I decided against it as it is out-of-scope for this PR, and might cause breakage somewhere else.

codeskyblue commented 1 year ago

Has you tested on windows?

PythonTryHard commented 1 year ago

Yes. The commits are signed by my Windows-only key, and I remember benchmarking for speed improvement. I can't remember the result though.

PythonTryHard commented 1 year ago

My friend has also independently confirmed the PR to be working on Windows 10 21H2 (build number 19044.2130). Neither of us have access to Windows 11.

codeskyblue commented 1 year ago

thanks, merged

codeskyblue commented 1 year ago

will be published with version 1.0.10