ev3dev-lang-python icon indicating copy to clipboard operation
ev3dev-lang-python copied to clipboard

Use native implementation of grey-to-RGB conversion for display update

Open WasabiFan opened this issue 7 years ago • 28 comments

Continuation of https://github.com/ev3dev/ev3dev-lang-python/pull/497.

This change should do it for the EV3; IIRC, other platforms will need their own solution to handle the extra bit used for the green channel.

The things that blocked me from opening a PR for that change originally were A) needing to test on the EV3 and B) needing to try with python3. (or, I suppose B is a prereq to testing with our library on A.) Hopefully I can get some time to build in Docker and figure out the Python targets.

WasabiFan avatar Aug 31 '18 21:08 WasabiFan

Do we have interest in pursuing this? I never actually got to testing it on the EV3 but I can if the perf boost is beneficial.

WasabiFan avatar Mar 30 '19 03:03 WasabiFan

I'll give it a try on my EV3

dwalton76 avatar Apr 18 '19 00:04 dwalton76

Testing with this:

#!/usr/bin/env python3

from ev3dev2.display import Display
from time import sleep
import datetime as dt
import logging

logging.basicConfig(level=logging.INFO,
                    format='%(asctime)s %(filename)22s %(levelname)8s: %(message)s')
log = logging.getLogger(__name__)


display = Display()
display_font = "luBS24"
x_grid = 4
y_grid = 4
total_ms = 0

log.info("start")
for x in range(20):
    start_time = dt.datetime.now()
    display.text_grid("%s%s%s" % (x, x, x), clear_screen=True, x=x_grid, y=y_grid, font=display_font)
    display.update()
    end_time = dt.datetime.now()
    delta = end_time - start_time
    total_ms += (delta.seconds * 1000) + int(delta.microseconds / 1000)

log.info("end")
log.info("total_ms: %s" % total_ms)
display.reset_screen()
display.update()

I ran it three times to get a baseline:

  • 12401ms
  • 12144ms
  • 12586ms

dwalton76 avatar Apr 18 '19 01:04 dwalton76

That's without the Pillow patch, right?

WasabiFan avatar Apr 18 '19 01:04 WasabiFan

right....Pillow is still building :( Am going to test on master on your fork then greyscale-to-rgb-packer

dwalton76 avatar Apr 18 '19 02:04 dwalton76

With master on your fork I got

  • 20500ms
  • 22233ms
  • 21487ms

But this does not include the time that it takes to import our Display library...that has gone up significantly. I did not time it before but it now takes 47s just to from ev3dev2.display import Display :(

dwalton76 avatar Apr 18 '19 23:04 dwalton76

With the patch I get

  • 21942ms
  • 21294ms
  • 21157ms

dwalton76 avatar Apr 19 '19 02:04 dwalton76

What version of Pillow ships with ev3dev? (not in front of my ev3 at the moment). Something to try would be to apply @WasabiFan 's patch to that version and see what we get.

dwalton76 avatar Apr 19 '19 13:04 dwalton76

I believe this also requires changing the Python library to use the native packer. Should be...

im.tobytes("raw", "RGB")

WasabiFan avatar Apr 19 '19 17:04 WasabiFan

Today when we do

self.mmap[:] = self._img.convert("RGB").tobytes("raw", "XRGB")

we get back a list of size 91136 but with

self.mmap[:] = self._img.convert("RGB").tobytes("raw", "RGB")

the list is only of length 68352, this causes an IndexError: mmap slice assignment is wrong size exception. The original size of self.mmap is determined by FbMem._get_fix_info(fid)'s smem_len...the fix_info.smem_len is used in _map_fb_memory.

For grins I hacked _map_fb_memory to use a size of 68352, re-ran my test with "RGB" and got 22050ms, 20909ms, and 21051ms.

dwalton76 avatar Apr 19 '19 20:04 dwalton76

Which platform are you running this on? It is only applicable on EV3.

WasabiFan avatar Apr 19 '19 20:04 WasabiFan

Am running on an EV3.

System info (from ev3dev-sysinfo)

Image file:         ev3dev-stretch-ev3-generic-2019-03-03
Kernel version:     4.14.96-ev3dev-2.3.2-ev3
Brickman:           0.10.0
BogoMIPS:           148.88
Bluetooth:          
Board:              board0
BOARD_INFO_HW_REV=7
BOARD_INFO_MODEL=LEGO MINDSTORMS EV3
BOARD_INFO_ROM_REV=6
BOARD_INFO_SERIAL_NUM=00165346201F
BOARD_INFO_TYPE=main

dwalton76 avatar Apr 19 '19 20:04 dwalton76

What version of Pillow ships with ev3dev? (not in front of my ev3 at the moment). Something to try would be to apply @WasabiFan 's patch to that version and see what we get.

We are running 4.0.0-4

robot@ev3dev:~$ dpkg -l | grep "Python Imaging"
ii  python3-pil:armel                           4.0.0-4                           armel        Python Imaging Library (Python3)
robot@ev3dev:~$ 

dwalton76 avatar Apr 19 '19 20:04 dwalton76

Right now, your measurements are showing that the change doesn't improve performance, right?

WasabiFan avatar Apr 19 '19 20:04 WasabiFan

Right...findings to this point are:

  • pillow takes drastically longer to import and about 2x as long to update the screen in v5.3 (what your master is based off of) vs v4.0.0 (what ev3dev ships with today)
  • I don't see any speed benefit when switching from XRGB to RGB (with your patch)

dwalton76 avatar Apr 19 '19 20:04 dwalton76

Are you sure it's a matter of the different version rather than different build parameters?

Are you saying that you think the implementation for L->RBG packing I added isn't what is needed? Does it not do what we previously intended to do? (You can look at the Pillow test I added to see what it does.)

WasabiFan avatar Apr 19 '19 20:04 WasabiFan

What build parameters are used for the pillow install that ships on ev3dev? To build I changed all of pip to pip3 and python to python3 in the Makefile. That was all I changed.

The test cases appear to be a dumpster fire :(

robot@ev3dev:~/Pillow$ python3 Tests/test_lib_pack.py 
...F......FFF.F.F..F.......F.F.F.
======================================================================
FAIL: TestLibPack.test_HSV
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Tests/test_lib_pack.py", line 179, in test_HSV
    self.assert_pack("HSV", "HSV", 3, (1, 2, 3), (4, 5, 6), (7, 8, 9))
  File "Tests/test_lib_pack.py", line 24, in assert_pack
    self.assertEqual(data, im.tobytes("raw", rawmode))
AssertionError: b'\x01\x02\x03\x04\x05\x06\x07\x08\t' != b'\x04\x05\x06\xff\x05\x06\x07\x08\t'

======================================================================
FAIL: TestLibPack.test_RGB
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Tests/test_lib_pack.py", line 62, in test_RGB
    self.assert_pack("RGB", "RGB", 3, (1, 2, 3), (4, 5, 6), (7, 8, 9))
  File "Tests/test_lib_pack.py", line 24, in assert_pack
    self.assertEqual(data, im.tobytes("raw", rawmode))
AssertionError: b'\x01\x02\x03\x04\x05\x06\x07\x08\t' != b'\x04\x05\x06\xff\x05\x06\x07\x08\t'

======================================================================
FAIL: TestLibPack.test_RGBA
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Tests/test_lib_pack.py", line 87, in test_RGBA
    "RGBA", "RGB", 3, (1, 2, 3, 14), (4, 5, 6, 15), (7, 8, 9, 16))
  File "Tests/test_lib_pack.py", line 24, in assert_pack
    self.assertEqual(data, im.tobytes("raw", rawmode))
AssertionError: b'\x01\x02\x03\x04\x05\x06\x07\x08\t' != b'\x04\x05\x06\x0f\x05\x06\x07\x08\t'

======================================================================
FAIL: TestLibPack.test_RGBX
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Tests/test_lib_pack.py", line 121, in test_RGBX
    "RGBX", "RGB", 3, (1, 2, 3, X), (4, 5, 6, X), (7, 8, 9, X))
  File "Tests/test_lib_pack.py", line 24, in assert_pack
    self.assertEqual(data, im.tobytes("raw", rawmode))
AssertionError: b'\x01\x02\x03\x04\x05\x06\x07\x08\t' != b'\x04\x05\x06\xff\x05\x06\x07\x08\t'

======================================================================
FAIL: TestLibPack.test_YCbCr
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Tests/test_lib_pack.py", line 153, in test_YCbCr
    self.assert_pack("YCbCr", "YCbCr", 3, (1, 2, 3), (4, 5, 6), (7, 8, 9))
  File "Tests/test_lib_pack.py", line 24, in assert_pack
    self.assertEqual(data, im.tobytes("raw", rawmode))
AssertionError: b'\x01\x02\x03\x04\x05\x06\x07\x08\t' != b'\x04\x05\x06\xff\x05\x06\x07\x08\t'

======================================================================
FAIL: TestLibUnpack.test_CMYK
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Tests/test_lib_pack.py", line 432, in test_CMYK
    "CMYK", "CMYKX", 5, (1, 2, 3, 4), (6, 7, 8, 9), (11, 12, 13, 14))
  File "Tests/test_lib_pack.py", line 229, in assert_unpack
    self.assertEqual(pixel, im.getpixel((x, 0)))
AssertionError: Tuples differ: (6, 7, 8, 9) != (6, 7, 8, 5)

First differing element 3:
9
5

- (6, 7, 8, 9)
?           ^

+ (6, 7, 8, 5)
?           ^


======================================================================
FAIL: TestLibUnpack.test_HSV
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Tests/test_lib_pack.py", line 479, in test_HSV
    self.assert_unpack("HSV", "HSV", 3, (1, 2, 3), (4, 5, 6), (7, 8, 9))
  File "Tests/test_lib_pack.py", line 229, in assert_unpack
    self.assertEqual(pixel, im.getpixel((x, 0)))
AssertionError: Tuples differ: (4, 5, 6) != (4, 1, 2)

First differing element 1:
5
1

- (4, 5, 6)
?     ^  ^

+ (4, 1, 2)
?     ^  ^


======================================================================
FAIL: TestLibUnpack.test_RGB
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Tests/test_lib_pack.py", line 281, in test_RGB
    self.assert_unpack("RGB", "RGB", 3, (1, 2, 3), (4, 5, 6), (7, 8, 9))
  File "Tests/test_lib_pack.py", line 229, in assert_unpack
    self.assertEqual(pixel, im.getpixel((x, 0)))
AssertionError: Tuples differ: (4, 5, 6) != (4, 1, 2)

First differing element 1:
5
1

- (4, 5, 6)
?     ^  ^

+ (4, 1, 2)
?     ^  ^


======================================================================
FAIL: TestLibUnpack.test_RGBX
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Tests/test_lib_pack.py", line 388, in test_RGBX
    (1, 2, 3, X), (4, 5, 6, X), (7, 8, 9, X))
  File "Tests/test_lib_pack.py", line 229, in assert_unpack
    self.assertEqual(pixel, im.getpixel((x, 0)))
AssertionError: Tuples differ: (4, 5, 6, 255) != (4, 1, 2, 255)

First differing element 1:
5
1

- (4, 5, 6, 255)
?     ^  ^

+ (4, 1, 2, 255)
?     ^  ^


======================================================================
FAIL: TestLibUnpack.test_YCbCr
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Tests/test_lib_pack.py", line 459, in test_YCbCr
    "YCbCr", "YCbCr", 3, (1, 2, 3), (4, 5, 6), (7, 8, 9))
  File "Tests/test_lib_pack.py", line 229, in assert_unpack
    self.assertEqual(pixel, im.getpixel((x, 0)))
AssertionError: Tuples differ: (4, 5, 6) != (4, 1, 2)

First differing element 1:
5
1

- (4, 5, 6)
?     ^  ^

+ (4, 1, 2)
?     ^  ^


----------------------------------------------------------------------
Ran 33 tests in 3.449s

FAILED (failures=10)
robot@ev3dev:~/Pillow$ 

dwalton76 avatar Apr 19 '19 21:04 dwalton76

Well that's... Cool... I'm not sure what's going on there. Perhaps a Python version issue — maybe there's a better-supported way of working with python3. Or maybe it's an endianness or word size issue. I know the tests passed when I initially implemented it, but I think I was doing that on a desktop.

Also, I think I was wrong earlier; we want to use "RGBA" not RGB".

WasabiFan avatar Apr 19 '19 21:04 WasabiFan

Lets try your patch against their 6.0.x branch and see what we get.

dwalton76 avatar Apr 19 '19 21:04 dwalton76

I am scratching my head over how they build for python3 though...changing the Makefile like this feels a bit hackish

dwalton76 avatar Apr 19 '19 21:04 dwalton76

On 6.0.x with your patch applied I got 23277ms, that was with this which does not have the mmap size mismatch issue.

self.mmap[:] = self._img.convert("RGBA").tobytes("raw", "RGBA")

When I was using

self.mmap[:] = self._img.convert("RGB").tobytes("raw", "RGBA")

I got an ValueError: No packer found from RGB to RGBA exception. Side NOTE, our Display library is still taking about 50s to import.

I am going to drop down to 4.0.x and see what sort of times we get to see if there is something about how I am building this that is having an impact.

dwalton76 avatar Apr 19 '19 23:04 dwalton76

On 4.0.x I got 21707ms so something about how I am building this has a significant impact. This is starting to feel like more trouble than it is worth...

dwalton76 avatar Apr 20 '19 01:04 dwalton76

Don't do the "convert". Just toBytes.

WasabiFan avatar Apr 20 '19 01:04 WasabiFan

ok back to 6.0.x with your patch and

diff --git a/ev3dev2/display.py b/ev3dev2/display.py
index 41d4ccf..1d09a3e 100644
--- a/ev3dev2/display.py
+++ b/ev3dev2/display.py
@@ -304,7 +304,8 @@ class Display(FbMem):
             self.mmap[:] = self._img_to_rgb565_bytes()
 
         elif self.var_info.bits_per_pixel == 32:
-            self.mmap[:] = self._img.convert("RGB").tobytes("raw", "XRGB")
+            #self.mmap[:] = self._img.convert("RGB").tobytes("raw", "XRGB")
+            self.mmap[:] = self._img.tobytes("raw", "RGBA")
 
         else:
             raise Exception("Not supported - platform %s with bits_per_pixel %s" %

I got 21682ms. I went back to using XRGB and got 22923ms.

dwalton76 avatar Apr 20 '19 02:04 dwalton76

To be clear... This is still 2X slower than what we have now, right? If so, there's something wrong. I might be able to take a closer look this weekend.

WasabiFan avatar Apr 20 '19 04:04 WasabiFan

yeah 4.0.0 that ships with ev3dev takes ~12000ms, 4.0.0 that I bulid/install from their git repo takes ~22000ms.

dwalton76 avatar Apr 20 '19 11:04 dwalton76

@WasabiFan thoughts on this?

dwalton76 avatar Jun 08 '19 17:06 dwalton76

I got it to compile that week but then ran into an issue getting the module recognized by python3. I'll self-assign this and take a look after finals are over.

WasabiFan avatar Jun 09 '19 03:06 WasabiFan