Use native implementation of grey-to-RGB conversion for display update
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.
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.
I'll give it a try on my EV3
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
That's without the Pillow patch, right?
right....Pillow is still building :( Am going to test on master on your fork then greyscale-to-rgb-packer
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 :(
With the patch I get
- 21942ms
- 21294ms
- 21157ms
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.
I believe this also requires changing the Python library to use the native packer. Should be...
im.tobytes("raw", "RGB")
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.
Which platform are you running this on? It is only applicable on EV3.
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
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:~$
Right now, your measurements are showing that the change doesn't improve performance, right?
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)
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.)
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$
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".
Lets try your patch against their 6.0.x branch and see what we get.
I am scratching my head over how they build for python3 though...changing the Makefile like this feels a bit hackish
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.
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...
Don't do the "convert". Just toBytes.
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.
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.
yeah 4.0.0 that ships with ev3dev takes ~12000ms, 4.0.0 that I bulid/install from their git repo takes ~22000ms.
@WasabiFan thoughts on this?
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.