Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement pixel buffer for accurate getPixelColor() #820

Closed
apleschu opened this issue Apr 1, 2020 · 16 comments
Closed

Implement pixel buffer for accurate getPixelColor() #820

apleschu opened this issue Apr 1, 2020 · 16 comments

Comments

@apleschu
Copy link

apleschu commented Apr 1, 2020

The following code moves the pixels down the line, but as it gets to the end it also gets more and more red, even though there is nothing in the code that should do that.

SOMETHING in the getPixelColor/SetPixelColor is not 100% transparent.

for (int i = SEGLEN - 1; i > SEGLEN/2; i--) { // Move to the right.
setPixelColor(i,getPixelColor(i-1));
}

@Aircoookie
Copy link
Owner

Hi,
unfortunately this is a side effect of using NeoPixelBrightnessBus without an extra buffer. The color is scaled for brightness adjustment. getPixelColor() can then not perfectly reconstruct the set color. We have no other options though, as the extra buffer would cost too much memory.
What would you like to implement? There may be a solution without relying on getPixelColor()

@apleschu
Copy link
Author

apleschu commented Apr 1, 2020 via email

@apleschu
Copy link
Author

apleschu commented Apr 2, 2020 via email

@huggy-d1
Copy link
Contributor

huggy-d1 commented Apr 2, 2020

@apleschu how does the ESP8266 get input for sound/noise? Do you have a github link to the pushed source?

@apleschu
Copy link
Author

apleschu commented Apr 2, 2020

@huggy-d1 Yes, we have a published source. Currently waiting if aircookie wants to merge or if we have to continue to maintain our own branch.

The public code is at: https://github.com/atuline/WLED

Please note that 8266 is limited to simple audio functions. In order to run more comples audio functions (FFT) we need an ESP32 so that we have the second core to run the FFT without slowing down WLED.

@Aircoookie
Copy link
Owner

Ok, in that case our best bet is probably using segment.data, a dynamically allocated auxiliary array. You can have a look at the Fire2012 method to see how it is used :)

@huggy-d1
Copy link
Contributor

huggy-d1 commented Apr 5, 2020

Ok, in that case our best bet is probably using segment.data, a dynamically allocated auxiliary array. You can have a look at the Fire2012 method to see how it is used :)

@aircookie do you mean SEGENV.data?

@apleschu
Copy link
Author

apleschu commented Apr 5, 2020 via email

@atuline
Copy link

atuline commented Apr 5, 2020

Hmm, I'll have to look into that SEGENV.data method for moving values instead of getPixelColor().

In the meantime, I wrote a little helper routine that will set pixel colours based on the the palette, but if the SEGMENT.palette == 0, then set it based on a SEGCOLOR(0) scaled down (err, blended with black) with a volume level. That way, I can also support the 'w' channel.

@apleschu
Copy link
Author

apleschu commented Apr 5, 2020 via email

@Aircoookie
Copy link
Owner

@apleschu There is, bus->GetPixels(), but it doesn't help since the underlying LED structures already contain the values scaled to brightness (you will notice that the color degration issue doesn't happen with the max. brightness 255) and there is just no way to get the color originally used in setPixelColor() losslessly without an extra buffer.

SEGENV.data can act as exactly such a buffer without consuming the memory on other effects that don't need it. You allocate the memory you need (maximum of 2048 bytes on 8266, quadruple that on ESP32) and you can cast it to any data structure you like. 1D fireworks for example, uses a Spark object to store light particles. For colors, you could do something like this:

uint16_t WS2812FX::mode_soundstream()
{
  uint16_t dataSize = 4 * SEGLEN;
  if (!SEGENV.allocateData(dataSize)) return mode_static(); //allocation failed
  
  uint32_t* colors = reinterpret_cast<uint32_t*>(SEGENV.data);

  //...

  for (int i = SEGLEN - 1; i > SEGLEN/2; i--) {
    oldColor = colors[i-1];

    setPixelColor(i, color);
    colors[i] = color;

Hope this helps! (although if you are working with palettes it would be better to just save the palette index (and brightness if needed) and call the palette function again rather than storing the entire color, it halves the amount of required data memory allocation, and the more is allocated, the more likely stability issues may arise)

Thank you for the awesome work! I just received my MAX9814 today and will try out your code tomorrow.

@apleschu
Copy link
Author

apleschu commented Apr 5, 2020 via email

@Aircoookie
Copy link
Owner

Oh, that sucks! Hope you'll have a working setup soon and maybe even get your PC to work again. Also stay healthy!

SEGENV.allocateData(dataSize) does do a memset(0), but only if the supplied dataSize (should stay the same unless segment parameters like SEGLEN change) or the effect mode itself changes. So it is good for storing general purpose data between function calls of the currently running effect :)

@apleschu
Copy link
Author

apleschu commented Apr 5, 2020 via email

@Aircoookie
Copy link
Owner

Changing this bug report to a feature request for fixing the underlying problem.
To solve this problem we would need a secondary pixel data buffer. Not an issue at all on ESP32, but on ESP8266 it would cut the number of possible LEDs in half, so it needs to be optional

@Aircoookie Aircoookie changed the title getPixelColor()/ setPixelColor() slowly change the colors to more and more red Implement pixel buffer for accurate getPixelColor() May 24, 2020
@Aircoookie Aircoookie added this to the 0.12.0 milestone May 24, 2020
@Aircoookie Aircoookie modified the milestones: 0.12.0, 1.0.0 Apr 6, 2021
@blazoncek
Copy link
Collaborator

Implemented in 0.14.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants