Closed cornim closed 3 years ago
Cornelius. Thank you very much for this. I will look at the code tomorrow. I'm actually rewriting things now so your input will be really useful. See the discussion on this repo and on GitHub helgeerbe
Cornelius. I've had a bit longer to look at your code this morning and I have to say it's much tidier than I would have done - even knowing all the extra functionality that's been added over the years. I would be more than happy to merge your mod but it can't break existing setups too badly!
At a trivial level I have tried to keep all the pi3d and pi3d_demos code python2 compatible. Although we are now over a year past the overdue death of p2, and I would dearly like to dump it, in debian systems when you type python
or pip
at the command line you get python2. Which means that a lot of people new to python get stuck down this blind alley.
More tricky is implementation of MQTT and specifically the back
and delete
functions which I think would be difficult to implement using the filter as a generator and lazy exif_data in Picture. While looking at using sqlite as a possible way to effectively hold the picture_list I persuaded myself that the only sensible way to do this was with a python list. With your system it might be possible to do something along the lines of making a previous_pictures
list as a byproduct of pic_nums_to_display
though I can see from your code that you (rightly) prefer non-byproduct functions.
If you look at the discussion here and https://github.com/pi3d/pi3d_demos/discussions/55 here https://github.com/helgeerbe/picture_frame/discussions/5 you will see that the hope was to get round the delayed exif reading as part of the filtering by keeping a "database" of the info on the SD card. The question is should that be done as a one-off job when the picture frame starts (it could take a few seconds the first time, or tens of minutes if there many thousands of pictures to catalogue) or should it be done only when the image is being read (like your system but written to a database for subsequent occasions) or (probably my preferred) should it be done when the image is being read and in a background thread or subprocess (which would have to a communication pipe to pause it during transitions)
Anyway, your input would be greatly appreciated.
Paddy
Hi Paddy,
thanks for having a look and thanks for creating the original setup. I know very little of openGL programming and could not have developed it from scratch.
First of, for it to break no existing setups, was the main reason why I implemented in a completely separate set of files. That way, people can decide when they want to switch over (assuming missing functionality will be added in time).
Regarding the python2 compatibility: I guess the easiest way to deal with that would be to move the entire thing into a PiPy package. That way necessary requirements could be listed an checked via pip at install time.
Back and forth functionality should be easy to implement by collecting shown pictures into a list (as you have said). The other mqtt functionalities should also be fairly easy to implement based on the fact the all loop variables are now grouped in a separate class. I don't see any major hurdles there other than the time it takes to do the work.
I read through parts of the discussion you mentioned and here are some of my thoughts.
Hence, if I were to extend/improve the loading functionality I would approach it like this:
I would assume that setting something like this up to work reliably is easily as much work as the entire application was so far and I have to admit that I'm not an expert in inter-process communications in python. My approach would probably be to build it based on queue first (the python stdlib seems to have a good implementation) and introduce caching if performance is still an issue then.
Cornelius, The main driver for using a cache is as more selection filters are added. You have identified one additional filter but others have suggested location as well as different EXIF data. If the full 10,000 images are being filtered lazily and the date is very specific (Christmas day 2014, or just mistyped, or any other filter) then the picture frame grinds to a halt.
The other reason for having a cache is that it can allow functionality such as the portrait pairs
, as you have found that is difficult to do with a sequential system. I'm not sure what other things people will think of in the future but sorting by the date the image was taken or by the average RGB value etc become feasible.
I agree that reading upfront might take an annoying length of time but maybe if the process was wrapped up in a way that could be run stand-alone or started as a background process as part of picture frame it could potentially get the best of both worlds. Sqlite isn't multi-user (like mysql or progresql) but it has table locking and I'm pretty sure that multiple process can access it safely. I have some (but not a great deal) of experience with the multiprocessing module but I have found that it doesn't seem to speed up IO intensive activities as the operating system automatically optimizes those (yes even though python supposedly suffers from GIL) often threading is just as fast. I imagine that the picture frame display/viewer/controller would only read from the cache and all writing would be done by the background process/thread. By default it would just work its way through a directory listing but there would be a queue allowing messages from the viewer: 'stop while I do a transition', 'start again', 'add details to the cache for xxx'. The exif info can be entered as fast as possible but the http requests to nominatim OSM must be kept well below 1 per second so that might need its own process/thread.
Your case regarding very specific filters makes sense. I just wanted to point out that caching, incl. delta updates and invalidation is a work intensive topic. Could be slightly easier here as file mtime could be used.
My suggestion regarding processes in favor of threading was because threading uses only one cpu in python whereas processes would run on a different cores and therefore alleviate the issue of stopping the per-processing while transitioning from one picture to the next.
I read up a little on multi-process access to SQLite and the answer is: It is supported, but... (see question 5 here). At the very least a robust retry logic would have to be build and the DB would have to be queried for every picture update to not miss out on (possibly frequently occurring) updates to the DB by other processes. In case of multiple updating threads there would also have to be some kind of completion tracking.
Paddy, as you are apparently not planning to move forward with this pull request in the short term, would you be ok with me publishing my rewrite (nothing else of course) as a GPL licensed PyPi package? Cornelius
Cornelius, of course not. It's all supposed to be open source for everyone to use without restriction. I'm sorry I haven't corresponded more but I've been concentrating on the re-write based on @helgeerbe's re-write. There are disadvantages with that structure too (personally I wouldn't have used as many underscores, setters and getters) but I thought there would be less things to add in later (MQTT, portait pairs, home-assistant communication, geolocation etc)
The sqlite image cache system seems to run pretty fast - I will maybe add a couple more tweaks to make it even less obtrusive on the very first run. The logic isn't too complicated at all really with just one threaded loop https://github.com/paddywwoof/picture_frame/blob/develop/picframe/image_cache.py#L36
On the subject of threading in python: people often dismiss them because they're not proper pthreads but I have often found them to be no slower than more complicated systems using multiprocessing, which only really seems to give a speed advantage where there are quite long-running CPU intensive jobs to be farmed out.
Anyway, keep in touch if you have any questions - or good ideas!
Paddy
Thanks for the quick reply. I'll let you know once I published something.
Paddy, just wanted to let you know that I published a version on Pypi. I added a feature which allowed me to adjust the probability of a picture to be shown based on the number of times it has been shown in the past. Therefore I had to build something which stores all pictures. I used SqlAlchemy so any kind of db can be used. Prefilling of db runs in a background process. Feel free to have a look and let me know should you have any questions. Cornelius
Cornelius, thanks for that, I will have a look later. I noticed you have the libatlas BLAS libraries as a dependency for numpy - are they not already there? I always assumed they were, at least on the standard raspberrypios. pi3d uses numpy a lot so that might make a difference to general animation performance (where there are lots of Shapes happening) Paddy
On my Pi3B+ with a clean installation of raspbian buster libatlas was missing which caused an exception when automatically trying to install numpy with pip. This is why I added it to the installation instructions.
Ah, were you starting from raspbian lite (I'm sure numpy is already on the full-fat version)? Mind you I've done that many times and never had to install the BLAS libraries - just numpy. Maybe it's something that's been split out on the latest ISO to increase liteness. Time to burn a new SD card and double check my FAQ page.
I used this version which is called Raspberry Pi OS with desktop (not the Raspberry Pi OS with desktop and recommended software version). I guess it is also important to note that I installed numpy via pip not apt-get. One question from my side: Is numpy an optional or a mandatory requirement of pi3d? I'm asking as it is not listed as a requirement for the pi3d pypi package.
Cornelius, well that is odd. I've not made an image from that update but certainly on all previous (and I think on the more recent lite images) numpy was already installed. I will check that out now.
Yes numpy is a requirement now and it should go into the setup.py and on pypi, well spotted. Originally pi3d used python and ctypes to do the matrix multiplication and array stuff but numpy is much faster.
Cornelius, following up. I burnt a new SD with the 2021-Jan-12 desktop without extra junk version and it has numpy already on. When I check what /usr/lib/python3/dist-packages/numpy/core/_multiarray.....so was built with it shows libblas.so.3 (and others) so I think everything required for numpy is also there.
pip3 install numpy
says it's already installed.
So it does seem strange that it was missing on your earlier download.
Anyway I will add the requirement to pi3d, so thanks for pointing that out.
That is indeed strange. Thanks for letting me know though. I'll remove the requirement from the installation instructions then.
Hi there,
I used your PictureFrame2020 on my Raspberry and I liked it a lot. There were only two things that bothered me:
The first issue was easy to fix but for the second it was necessary to set up preloading of the next image while there was no transition activity. Given the structure of PictureFrame2020.py (e.g. few functions, no classes, lots of global variables) restructuring everything would have been quite messy. I therefore rewrote the entire application (as PictureFrame2021) to be more pythonic, which is the content of this pull request. Some features are still broken (e.g. mqtt support, keyboard support, geo location etc.) and dual portrait pictures I dropped all-together as it was incompatible with lazy loading the pictures.
All in all I believe the application is now much better structured and therefore easier to extend but is currently lacking some features and (as the original) does not have sufficient documentation.
If you are interested in merging this, I can repair/improve on some of these aspects but as I don't use/need them and therefore won't do it if there is no interest in merging this on your side.
Best have a look and let me know what you think.