Led gamma setting

[Romonaga]

ledstring.channel[0].gamma = 0; ledstring.channel[1].gamma = 0;

The library will allocate memory using malloc to create a default gamma correction table. The issue is when one wants to provide a custom gamma correction table.

1. Leads to memory leak as no way to free the existing memory.
2. Leads to error when cleaning up, as you are now trying to free memory that you no longer own, once you supply your own gamma correction table.

Yes, one can assign nullptr to the gamma, however this is not very clear in the implementation of this functionality, and does lead to the above issues. I guess I do not understand the objective of mallocing this array.

[Xartrick]

I guess you can free the gamma field and then create your own correction table between init and cleanup. cleanup will clear your table.

[Gadgetoid]

I wrote the gamma functionality a few years ago, and it's safe to say I'm not exactly a genius when it comes to grokking free/malloc. It was largely developed for our Unicorn HAT board- and I upstreamed to avoid having to maintain and distribute a fork of this project.

I can see exactly the issue you describe, but I'm not sure how to put together a forward-compatible fix for it. My usual approach for this kind of thing would be to have a setter to handle assigning the internal gamma table. While a fully custom table is a nice-to-have I suspect in most cases a user might want to specify the gamma value and have it calculated automatically anyway.

[Romonaga]

In the end, I was having to much issues with dealing with the allocated memory, my initial attempt to work around still lead to seg faults shutting down. In the end, I ended up making a helper function that simply walked my array, and replaced my values with the internal ones. This solved my issue.

As to how this can be fixed, can we simply not allocate memory for the array? Your idea for a solution could work, as we know the formula is very simple, if one were to simply provide the factor, and we adjusted the array, this would solve the issue I think. I will do some work on this when I have some time, and put in a pull request.

[Romonaga]

I guess you can free the gamma field and then create your own correction table between init and cleanup. cleanup will clear your table. Yep this is the first thing I tried, even NULL'ed the values out, however it still seg faulted when shutting down. In the end I was able to solve by simply setting the values of the internal array.

[Romonaga]

In the end I ended up doing this to work around the issue. I could see a function where we pass in the gamma correction value and allow the internals to be set without the worry of freeing and managing that memory. I can make a pull request and add this function to the lib if people think it is a good solution. My application... [code] void ILightShow::gammaCorrection() {

for(int  counter = 0; counter <= 255; counter++)
{
    _gammaCorrection[counter] = (!_useGammaCorrection) ? counter :
            (int)(pow((float)counter / (float)255.00, _settings->getGamma()) * 255.00 + 0.5);
}

_ledWrapper->setCustomGammaCorrection(_gammaCorrection);

}

My wrapper void Ws2811Wrapper::setCustomGammaCorrection(uint8_t* gamma8) { for(int counter = 0; counter < 256; counter++) { _ledstring.channel[0].gamma[counter] = gamma8[counter]; _ledstring.channel[1].gamma[counter] = gamma8[counter]; } }

[code]

[Romonaga]

409

Added this pull request to address the issue we are talking about here. I took @Gadgetoid suggestion and added a function to set the gamma based on the correction factor.