forkineye / ESPixelStick

Firmware for the ESPixelStick
http://forkineye.com/
528 stars 169 forks source link

ESP V4 allows undersized frames to be played if they are undersized by the same number (or less) of bytes in the fseq header of the file. #717

Closed kingofmonkeys closed 6 months ago

kingofmonkeys commented 6 months ago

--------- Instructions -------- Please provide answers directly below each section. --------- Instructions ---------

ESPixelStick Firmware Version Latest code on main branch

Hardware Version ESP32 board

Binary release or compiled yourself? compiled myself

Operating System (and version) Windows 11

Web Browser (and version) Chrome

Access Point

Describe the bug I have been uploading a Fseq file using FPP connect from xlights and getting the "File does not contain enough data to meet the Stated Channel Count Number of Frames value. Expected..." error. I have other boards working fine and after some troubleshooting I decided to dig into the code to attempt to determine why the file was bad. What I found was the check for this error message comparing the frameschannels with the actual file size. After some more troubleshooting I decided to comment out that check and see if the files still worked or stopped short. What I found is the file appears to work fine as is. Is it possible that comparing the file size to channels * frames isn't always a valid check?

I did double/triple check I have xlights set to output as V2 Sparse/Uncompressed.

kingofmonkeys commented 6 months ago

Pier Lights.fseq.txt

Attaching the .fseq file. This isn't even an animation so that is likely why it is so small but still valid. I made a more complex one that was barely over the size check and it worked with the check in place. Maybe xlights changed how it exports these files to limit the size recently?

MartinMueller2003 commented 6 months ago

Just looked at the code and you are correct, there is an error in the calculation. However, the error is such that it would allow a slightly undersized file to be accepted. I will test your file shortly

MartinMueller2003 commented 6 months ago

Your file is 208KB long

Using the tool for data extraction we get: C:\z\ESPixelStick\tools>python fseqinfo.py Pier.Lights.fseq Pier.Lights.fseq: V2.0 sparse uncompressed 1500 channels with 1200 frames @ 25ms from xLights Windows 2023.20 64bit

1500 * 1200 = 1,800,000 bytes = 1,758KB = 1.7MB Needed data size inside the file must be at least greater than 1.7MB while your file is 0.2MB. Your file is undersize.

Even if I fix the calculation (which I will do), your file is too small and output will terminate sooner than expected. The check is in there to make it evident that there is a problem with the data file. Yes it will play for a while, but the output will stop and you would be looking at your show asking why did it abort my sequence. Best not to play the sequence in the first place so the problem is dealt with up front.

kingofmonkeys commented 6 months ago

But the file is working exactly as it should when I remove that check. When I get time I can play with animation but I have a feeling xlights is limiting the file to only what is needed (if nothing changes in a frame its not doing anything in that frame).

MartinMueller2003 commented 6 months ago

The ESP expects a byte of data for every frame reported in the fseq header. Truncating the data should include truncating the number of frames declared in the header. That would be an xLights bug.

cybercop23 commented 6 months ago

Do you have Suppress Duplicate Frames? Given that you can read the file, seems like xlights sent it all, but maybe you have the suppress checkbox on image

kingofmonkeys commented 6 months ago

You had me excited that it was something simple but I just checked and no Suppress Duplicate Frames is not checked on any of my controllers. I am still investigating settings in xlights for the file generation but I haven't had much time to dig into it.

MartinMueller2003 commented 6 months ago

FYI: I looked at a tool in FPP for creating V2 Uncompressed files and I do not see anything that would remove redundant frames. You might try using that tool to generate the file and see if they come out the same size. fsequtil

MartinMueller2003 commented 6 months ago

The fix for this has been integrated into main.