A lot of enhancements to the wonderful flaschen-taschen project: #44

werling · 2018-10-21T18:49:23Z

possible to send directly images in python
images are encodes as png to avoid udp packet size problems
c++ framework has now dependency of ImageMagick++ (used for converting bmp <-> png)
new python class for sending images
new header "Q7"
python api depends now on pillow
threading is used in python class to send multiple images parallel

- possible to send directly images in python - images are encodes as png to avoid udp packet size problems - c++ framework has now dependency of ImageMagick++ (used for converting bmp <-> png) - new python class for sending images - new header "Q7" - python api depends now on pillow - threading is used in python class to send multiple images parallel

hzeller

Hi, thanks for your improvements.

I am not convinced yet though that we want the Q7 header and additional arbitrary image formats. I could be convinced though to allow for exactly one defined format (PNG) with FlaschenTaschen extensions (offset x and y) at the end.

The FlaschenTaschen protocol is meant to be as simple as possible, so that it is very simple to encode without any issues for the clients and decode on the server-side with minimal possibility of security issues and dependency on external libraries. This is why we use a very simple protocol that is essentially the raw, uncompressed RGB bytes, and also very easy to generate with a script or a few lines of code without needing additional libraries.

Simplicity and accessibility from any language is they key goal; the original FlaschenTaschen is used in an interactive art project that invites people nearby to send images and interact with it.

Case in point: we have implemented the FlaschenTaschen protocol also on an ESP8266, which of course will never be able to decode compressed images; it barely gets along reading the raw bytes. Having the protocol only partially implemented in the smaller devices will result in a compatibility mess.

Using ImageMagick to decode arbitrary unverified bytes received from the network is also a security concern, this needs a lot more vetting. So in the current state, this pull request can not be integrated into FlaschenTaschen (if you use it for your own server, make sure to fix the unsigned issues as pointed out, the memory leak and add more vetting to the bytes received).

So to get this in a good state: if we add an compressed image format, we should settle on exactly one (PNG is the only viable), and avoid having ImageMagick attempting to decode random bytes whose success also depends on which libraries ImageMagick happens to be linked to. So if you're thinking of adding that, only link libpng and make sure there is enough vettting of bytes going on.
We should also keep the simplicity aspect in mind, so ideally it would just be possible to send a PNG image to the server without having to add an additional header. Then sending an image is as simple as sending a PNM file now, just with using cat in bash:

   cat myimage.png > /dev/udp/ft.noise/1337

So that means, the FlaschenTaschen feature of having an offset should be added as a footer to the data - so that it is possible to decode the first part as image, then interpret the rest as offset bytes (similar to how it is done now in the PNM case). Of course we need to make sure that either the PNG library skips the additional bytes at the end, or we can sufficiently pick them out ourselves first before sending the blob to libpng to decode.

So the additional header in this case to look for would be `0x89PNG\r\n0x1A\r'

In General: Paradigm is to have the client-side do the hard work (decode image etc.) and then send in the simple format. So move the complicated part of decoding and scaling the image to the client side, and then send the simplified image over to the server. That also makes sure that you can do the decoding and scaling, which is an expensive operation, on a decent machine and not the Raspberry Pi which might take hundreds of milliseconds to decode images. So if you want to provide more additional Python helper classes that do this, this is a useful pull request in itself.

I agree that it is problematic that UDP does have a limit in byte-size, and the number of bytes transferred over the network is less compressed than it could be. But it is always possible to 'tile-fill' a large display using the offset feature (it is not a problem in the original 35x45 pixel art installation, and also not in smallish size LED panels (I am using it to watch videos on a 160x96 LED panel, streamed from VLC which has native FlaschenTaschen protocol support).

However, compressed images also will only improve the situation somewhat, at some point the 64kByte limit is still in the way. But as I said, I could be convinced to support PNG.

(A careful enhancement could be to also include TCP streams in the future).

hzeller · 2018-10-22T18:29:30Z

server/ppm-reader.h

-    int width;
-    int height;
-    int range;      // Range of gray-levels. We only handle 255 correctly(=1byte)
+    unsigned int width;


Never use unsigned unless you want to represent bitmaps or need the extra 2 billion at the top.
Using unsinged will create all kinds of subtle issues when using addition.

For instance now you removed the feature to use negative offsets, but making the offsets below unsigned and create non-intentional super-large values when the offsets are applied in display->SetPixel().

hzeller · 2018-10-22T18:31:37Z

server/udp-server.cc

        mutex->Lock();
        display->SetLayer(img_info.layer);
-        for (int y = 0; y < img_info.height; ++y) {
-            for (int x = 0; x < img_info.width; ++x) {
+        for (unsigned int y = 0; y < img_info.height; y++) {


This loop still should be using int not unsinged see above.
Also please keep the prefix iterator (++y) instead of postfix iterator (y++), as it expresses the intent better (never never intend to use the previous result and post-increment, we are only interested in it being incremented.)

hzeller · 2018-10-22T18:39:38Z

server/udp-server.cc

+                image.scale(geo);
+            }
+
+            void* ptr = (void*)new char[3 * img_info.width * img_info.height];


I don't see where you deallocate this memory, so this will leak memory with every image you send via Q7.

werling · 2018-10-23T17:11:47Z

Thanks a lot for your detailed answer and your ideas!
Yes you're right about the matter of simplicity and I'm also not a fan of having unnecessary dependencies, but in the case of ImageMagick it was much easier to handle than libpng. As you can see by my mistake to deallocate the memory I'm not an expert in C++. Usually I'm coding in python.

I build a RGB display with 256x96 pixels which exceeds the udp packet size. First I tried to split the image in two parts and send the information separately. But using python and setting each pixel individually is really slow and I'm planing to send (multilayered) animations. I also tried snappy, which you suggested here: #33 (comment)
But my tests showed using snappy is slower and not as efficient as converting to png.

The solution with ImageMagick on the c++ side and Pillow in python works for me perfectly. As a matter of fact, I'm planning to create some python helper functions for animated stuff. Maybe I will do another pull request for it in the future.

I'm also not planning to use the program via the internet, so the security concerns are not too important for me. But I will adjust the code based on your suggestions about the unsigned integers, prefix iterator and data leak. For the more sophisticated things like using libpng and using a footer as data storage I'm unfortunately lacking of time.

added flaschenmulticlient added examples for flaschenmulticlient and flaschenQ7client

Internally for every new socket connection a new thread will be created. Also new compiling parameter is CONSTANT_FPS=X. Where X can be any positive number. This is needed if multiple clients sending pictures to server at once. If X=0 the server is trying to show everny image received, which can lead to artifacts on the display.

werling · 2018-11-12T20:21:04Z

Hi,
I'd like to show you what I worked on recently:
https://github.com/werling/flaschenQ7client

And also have a look at the new server files.
I added tcp support and other features.
The next think I like to change is using libpng instead of Magick++.

removed magick++ dependencies added different image reader files

merging docu files

werling · 2018-11-21T19:53:05Z

Hi,
I recently had some time to make some changes.
Now the server has no magick++ dependencies anymore. Only libpng is used for decoding png files.
Furthermore your request
cat myimage.png > /dev/udp/ft.noise/1337
is now working.
Any more changes for accepting the pull request?

/Edit: Also the "Q7" header type is removed since its not needed anymore.

hzeller · 2018-11-30T11:40:51Z

Cool!
I am currently travelling, but once I have a little time, I'll have a closer look.

changed the readme file

…l one layer below current layer

hzeller requested changes Oct 22, 2018

View reviewed changes

werling added 6 commits October 24, 2018 21:05

some changes which where discussed here: hzeller#44

05f13f3

initialization of ptr inside q7 image type bracket

cb177ce

renamed flaschenimage -> flaschenQ7client

e727636

added flaschenmulticlient added examples for flaschenmulticlient and flaschenQ7client

Update README.md

8721548

Update README.md

835369b

werling added 3 commits November 21, 2018 20:40

added libpng for decoding png files

e0a781a

removed magick++ dependencies added different image reader files

Merge branch 'master' of https://github.com/werling/flaschen-taschen

88642d3

merging docu files

reverted python api files

49c8ce7

werling added 5 commits December 27, 2018 21:01

added missing libpng link

4d36d65

Update README.md

b7989e3

changed the readme file

bugfix; now the program is starting to look for an intransparent pixe…

2e53eee

…l one layer below current layer

revert

efe2162

bugfix in server/servers.cc buffer array has to be cleared every loop

c752568

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

A lot of enhancements to the wonderful flaschen-taschen project: #44

A lot of enhancements to the wonderful flaschen-taschen project: #44

werling commented Oct 21, 2018

hzeller left a comment •

edited

Loading

hzeller Oct 22, 2018

hzeller Oct 22, 2018

hzeller Oct 22, 2018

werling commented Oct 23, 2018

werling commented Nov 12, 2018

werling commented Nov 21, 2018 •

edited

Loading

hzeller commented Nov 30, 2018

A lot of enhancements to the wonderful flaschen-taschen project: #44

Are you sure you want to change the base?

A lot of enhancements to the wonderful flaschen-taschen project: #44

Conversation

werling commented Oct 21, 2018

hzeller left a comment • edited Loading

Choose a reason for hiding this comment

hzeller Oct 22, 2018

Choose a reason for hiding this comment

hzeller Oct 22, 2018

Choose a reason for hiding this comment

hzeller Oct 22, 2018

Choose a reason for hiding this comment

werling commented Oct 23, 2018

werling commented Nov 12, 2018

werling commented Nov 21, 2018 • edited Loading

hzeller commented Nov 30, 2018

hzeller left a comment •

edited

Loading

werling commented Nov 21, 2018 •

edited

Loading