search

pixelmatix / SmartMatrix

SmartMatrix Library for Teensy 3, Teensy 4, and ESP32
http://docs.pixelmatix.com/SmartMatrix
630 stars 162 forks source link

Support for smaller color spaces (rgb8, rgb565, etc) #23

Open embedded-creations opened 9 years ago

embedded-creations commented 9 years ago

Part of @mrwastl's #17 https://github.com/mrwastl/SmartMatrix/commit/a85836fe27a4f080a42242d0d7983c05775af62a https://github.com/mrwastl/SmartMatrix/commit/a280bf6e7e311dfa705426e974a2db2c8a5f7b3b

embedded-creations commented 9 years ago

@mrwastl we have a place to discuss smaller color spaces here.

you should be able to replace this code in SMLayerBackground<RGB, optionFlags>::fillRefreshRow

            refreshRow[i] = rgb48(backgroundColorCorrectionLUT[currentPixel.red],
                backgroundColorCorrectionLUT[currentPixel.green],
                backgroundColorCorrectionLUT[currentPixel.blue]);

with 

            colorCorrection(currentPixel, refreshRow[i]);

Add a new colorCorrection function for your smaller color spaces, and it should work. All the rest of the changes should be in MatrixCommon.h if I'm not mistaken.

mrwastl commented 9 years ago

@embedded-creations just that i get it right:

i've already played around with rgb8 and rgb16, but there must be an error in reasoning or so somewhere ... (usually in a place where i don't look ;-)

embedded-creations commented 9 years ago

@mrwastl Yes, that looks correct. Have you tested out your rgb8/16 conversion functions? Try isolating some code, maybe you could change working storage_depth=24 code to convert rgb24 to rgb16 and back to rgb24 within the fillRefreshRow() function to see if it's the conversion functions or something else that may not be working properly.

mrwastl commented 9 years ago

@embedded-creations do you have an idea, why the operator=-methods found in your library aren't working with implicit casts (i don't have that much experience with operator-overloading with structs):

  rgb8 colred8 = {7, 0, 0};
  rgb24 colred = colred8;

the 2nd assignment isn't working. i get this error-message: error: conversion from 'rgb8' to non-scalar type 'rgb24' requested

but if i add an additional cast-constructor, above example is working:

typedef struct rgb24 {
...
    rgb24& operator=(const rgb8& col);
    rgb24& operator=(const rgb16& col);
    rgb24& operator=(const rgb24& col);
    rgb24& operator=(const rgb48& col);

    // cast-constructors added
    rgb24( const rgb8& col );
    rgb24( const rgb16& col );
    rgb24( const rgb24& col );
    rgb24( const rgb48& col );
...
}

rgb24& operator=(const rgb8& col); and rgb24( const rgb8& col ); contain exactly the same code (except an additional return *this; in operator=).

it would be logical if the operator= overloading methods (defined for all supported colour-types!) would just do that?

maybe you know a better way how to solve this. or how to avoid code redundancy.

embedded-creations commented 9 years ago

@mrwastl Sorry I have no idea. This is beyond my C++ experience, we need an expert.

@gregfriedland do you have any idea why the implicitly defined assignment operators for rgb* types aren't working? For rgb24, I needed to create an assignment operator or drawing to the last pixel of the background buffer would crash the Teensy: https://github.com/pixelmatix/SmartMatrix/blob/sm3.0/src/MatrixCommon.h#L64

gregfriedland commented 9 years ago

@pixelmatix: it's hard to tell without looking at the call line but my guess is that the other assignment operator was getting triggered somehow and it was reading an rgb24 as if it were an rgb48.

Sent from my phone. Please forgive any terseness.

On Oct 12, 2015, at 6:16 PM, Louis Beaudoin notifications@github.com wrote:

@mrwastl Sorry I have no idea. This is beyond my C++ experience, we need an expert.

@gregfriedland do you have any idea why the implicitly defined assignment operators for rgb* types aren't working? For rgb24, I needed to create an assignment operator or drawing to the last pixel of the background buffer would crash the Teensy: https://github.com/pixelmatix/SmartMatrix/blob/sm3.0/src/MatrixCommon.h#L64

— Reply to this email directly or view it on GitHub.

mrwastl commented 9 years ago

i've pushed a new branch: sm3.0-colourspaces.

this branch adds support for:

colour space can be changed for layers (COLOR_DEPTH aka storage_depth), but not for pwm_depth (aka kRefreshDepth).

tested and working:

not yet working or not implemented yet:

drawbacks:

differences to original sm3.0-branch: https://github.com/mrwastl/SmartMatrix/compare/sm3.0...mrwastl:sm3.0-colourspaces

update: 2015-10-17: new push

embedded-creations commented 9 years ago

Great!

Do you think there's much of a reason to add support to indexedLayer and scrollingLayer? The memory saved in those layers by using smaller colors is only going to be a couple bytes. Maybe there's a good reason for consistency.

slower than rgb24 at the moment

I wonder if there are any tricks people have come up with (maybe using assembly instead of C code) in the past to quickly unpack the individual color channel bits in rgb8 or rgb16

mrwastl commented 9 years ago

new commit, see corrected / extended posting above.

there aren't any memory savings for indexedLayer nor scrollingLayer (1 bit / pixel can hardly be improved :), but some small conversion fixes had to be done.

speed: well, a little drawback, but still usable. maybe there're some unnecessary implicit colour space casts.

normalised colour initialisation: i've added constructors that allow setting colour space independent colour values (range [0.0 - 1.0] for each colour channel). problem: c/c++ cannot differentiate between constructor({integer, integer, integer}) and constructor({float, float, float}) (eg: RGB({ 255, 255, 255 }) and RGB({1.0, 1.0, 1.0})) - for c/c++ this is ambiguous ... so i had to introduce a 4th dummy float that is not used ('t' as in 'transparency').

maybe there's a better solution but i couldn't find one ...

mrwastl commented 9 years ago

new commit:

update (2015-10-19): the speed issue doesn't seem to be such a big issue as i've thought before: i've adapted my sketch that i use for playing my test video to support rgb8 and rgb16 for transmission of data between my library and the sketch: resulting fps are ok now (maybe a little less than expected). my earlier tests transmitted RGB888-data and than converted these data to rgb8 or rgb16. now i transmit RGB332/RGB565/RGB888 according to COLOR_DEPTH.

conclusion:

ecurtz commented 9 years ago

I've added the obvious and trivial static void loadMatrixBuffers12(unsigned char currentRow, unsigned char freeRowBuffer); in my local working copy, which would probably be a good fit here if you haven't done it already.

ecurtz commented 9 years ago

Also, are you using a version of the old streaming demo for your testing? If so, using smarter Serial methods rather than reading a byte at a time makes a huge difference in the framerate.

char* buffer = (char*)backgroundLayer.backBuffer(); int bytesAvail = Serial.available(); boolean swap = false;

if ((bytesAvail > 1) && (dataPos > 0)) { if (bytesAvail > dataExpected) bytesAvail = dataExpected; int count = Serial.readBytes(&buffer[dataPos], bytesAvail); dataPos += count; dataExpected -= count; if (dataExpected == 0) { swap = true; } } else if (bytesAvail) { char val = Serial.read(); if ( val == 1 ) { dataPos=0; dataExpected = kMatrixWidth * kMatrixHeight * 3; } else { buffer[dataPos++] = val; dataExpected--; if (dataExpected == 0) { swap = true; } } }

mrwastl commented 9 years ago

@ecurtz: loadMatrixBuffers12: nope. as written above my focus was on storage depth and not on pmw depth yet. could you post your method so that i can test it in my branch?

serial transfer: my library (VSSDCP) that i use for transferring data is already sending chunks (256 bytes in one go for teensy).

ecurtz commented 9 years ago

https://github.com/ecurtz/SmartMatrix/commit/30dd3c195a72a5f65becc70d0a1f1da396127ad0

mrwastl commented 9 years ago

@ecurtz i've tested your new method, but there seems to be a problem with the correct usage/mapping of colours (eg. when testing FeatureDemo.ino: black background (instead of dark red), wrong colours used when drawing objects, ...)

i'll have a look at this somewhen later, but i'm currently working on a different project.

ecurtz commented 9 years ago

Are you sure it isn't just the expected loss of color fidelity from reducing output from 8bit per channel to 4bit? The code is really basic.

mrwastl commented 4 years ago

@embedded-creations nice! now i no longer need to add this to my fork (or need to understand all teensy4 relevant changings :)

@ecurtz (sorry that my answer is 5 years late:) i (still) fool around with streaming video content to my displays (including rgb matrixes) and for those displays 4bit usually is a good compromise between video quality and throughput (and thus refreshrate).

embedded-creations commented 4 years ago

@mrwastl I haven't fully integrated your changes yet, but plan to. Some more details here:

https://community.pixelmatix.com/t/smartled-shield-for-teensy-4/740/12?u=louis

embedded-creations commented 4 years ago

@mrwastl How are you streaming content? Wireless? USB?

mrwastl commented 4 years ago

@embedded-creations i've implemented my own protocol (that I'm not very happy with): https://github.com/mrwastl/VSSDCP (github-version is very outdated; only serial/USB is working correctly at the moment, I will push my local changes that contain support for ESP32, different teensys a.s.o.). My library serdisplib contains support for this protocol and years ago I've made a patch for mplayer to add support for serdisplib (https://github.com/mrwastl/mplayer-vo_serdisp). With all these parts together I'm able to stream video content.

But I think I'll have a look at TPM2 (suggested by drbeefsupreme at the pixelmatrix-forum).

Wireless: I have tried to stream via wireless (ESP32, teensy using external modules) and ethernet (TCP/UDP), but the results have been very disappointing (lots of streaming errors). And I already have an ethernet adapter for teeensy4 - so this will be one of my next experiments.

@embedded-creations (offtopic): i have tried to login at community.pixelmatrix.com using my github-account but I get a 'Sorry, there was an error authorizing your account. Please try again.' - Any idea what might cause this?

embedded-creations commented 4 years ago

I committed a lot of changes to add two new layers with Adafruit_GFX support. I posted details here:

https://community.pixelmatix.com/t/adafruit-gfx-compatible-layers-modern-font-support-coming-to-smartmatrix-library-4-0/768

@mrwastl Cool, I didn't know that serdisplib is your project!

I know WLED has some streaming support, but I haven't tried it yet. This project looks promising too, specifically for using some lightweight compression on the video to increase the pixel count:

I think the Login with GitHub feature is broken. To be honest I didn't even know that was an option on the forum. I recommend just signing up with an email.