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

Add GIF Player UI extension (/gifplayer.htm) #3921

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

Manut38
Copy link

@Manut38 Manut38 commented Apr 21, 2024

I wrote a little UI extension page for the new GIF support and I would love to have it included in this project.

Thanks to @ajotanc, whose PixelMagicTool I used as a base for my layout and structure!

I also included a "GIF" button right next to the pxmagic.htm button.
This is merely an idea and of course I'm happy to take suggestions about the placement or about the implementation in general.

Source: https://github.com/Manut38/WLED-GifPlayer-html

Thank you all so much for your work on WLED, it's honestly amazing!!
I've been using it for the last couple of years now and I love to see how it developed over that time. GIF support finally being added is all I that dreamed about 🔥


A WLED UI extension for an easy control of the awesome new "Image" effect, capable of displaying animated GIFs on your LED matrix.

It is using the ESP filesystem editor at /edit to list and change files on the local ESP storage.

Features

  • Responsive layout
  • Click to change currently displayed image
  • Upload/Delete image files
  • Save images as presets
  • Forced caching of images and request limiting (ESP crashes when there are too many concurrend HTTP requests)
  • Button to refresh all images, useful when an image was replaced with another one of the same name

Limitations

Warning

Only upload images in your matrix native resolution! (e.g. 16x16)

Due to the limited flash storage space and processing power, .gif files of larger dimensions will make the effect lag and kill WLED performance

  • Currently the "Image" effect only supports .gif files
  • All image files will be uploaded to/read from the root path of WLEDs LittleFS because it does not support subfolders
  • SD card support untested

Screenshots

Screenshot Image List
Screenshot Image Select

@dosipod
Copy link
Contributor

dosipod commented Apr 22, 2024

Works okay with one note ( may be not related directly to the html page ) is that the storage size reported of 241/983 KB is not accurate and it will only report the correct number once you reboot the esp which is the same behavior on the filesystem info page .

@softhack007
Copy link
Collaborator

softhack007 commented Apr 22, 2024

@Manut38 Does your PR build on top of the PR from Aircoookie for GIF support, or is it independent?

@Manut38
Copy link
Author

Manut38 commented Apr 22, 2024

@dosipod Yes, I did notice that behavior too. I'm just getting this info straight from the /json/info endpoint. I don't know of any other way to check for the current values.

@softhack007 This is essentially a separate interface, specialized in controlling the new "Image" effect, added by Aircoookie in the gif branch. So it is built on top of that functionality.
Right now it is using the FX ID 53 for sending the requests.
const FX_MODE_IMAGE = 53; // set effect mode "Image"
This needs to be adjusted, if the ID is changed before the gif branch is merged to main

Manut38 added 4 commits April 24, 2024 11:08
they will now use the existing presets, even if they were previously renamed
essentially sending the "off" command
- move advanced features to config dialog
@Manut38
Copy link
Author

Manut38 commented May 3, 2024

I made a few changes to the code since I took the screenshots that I would love to hear your opinion on.

  1. I added a pause and stop button to the UI
    My reasoning for this is that it allows for a much faster control the displayed image, without having to go back to the main WLED UI. My intention for this page is to basically build an clean interface specialized for the new "Image" effect, so I felt that it made sense to include this functionality; Especially if displaying images is the primary use of your LED matrix.
  2. The image buttons are now aware of already existing presets that hold the same image filename in their segment config. Users can now e.g. customize the playback speed of the gif using a preset and still expect GifPlayer to load the correct preset.

Another thing I want to note is that I am very aware that this is building on top of a feature that is still currently in-development. Especially because of that I would be interested in @Aircoookie's opinion, because as far as I know he was the one mostly working on the "Image" effect.
I could also see how adding more and more isolated htmls (pixart.htm, pxmagic.htm, gifplayer.htm) might not be the way to go forward with this, however I'm very curious about your all of your vision here.

@Manut38
Copy link
Author

Manut38 commented May 19, 2024

Updated the images to reflect the changes to the button layout.
Any news on this? 😄

@dosipod
Copy link
Contributor

dosipod commented May 20, 2024

@Manut38 I am personally not in favor of adding such external tools to wled at build time just from the points that future updates to wled might break things( this has been seen with other tools a lot ) and if your tool is separate then it could be updated faster which we also seen first hand . Does not harm to maintain your page though , please post about the same on wled discord 2D channel

At the moment we use a lot of tools like that and we just download the latest version and copy under edit , i am not sure what is wrong with that if any . That is beside the fact that gif is not merged yet

@Manut38
Copy link
Author

Manut38 commented Jun 5, 2024

@dosipod I kind of agree with this honestly. This pull request was motivated by the fact that there are some other tools that have been integrated into the source directly and also got mentioned on the official wiki.
Of course this way additional tools like this would get a lot more exposure and users.

I saw that the "older" WLED-PixelArtConverter [pixartmin.htm] got a separate entry in the wiki at https://kno.wled.ge/features/pixel-art-converter/
Maybe this could be a way to present some links to tools like this on the official wiki?
Because I remember that I found these extensions pretty much randomly on GitHub rather than on an official post.
What is your take on this?
Thanks for all the amazing work btw 👍

@blazoncek
Copy link
Collaborator

@Manut38 when (or if) GIF player gets merged and @Aircoookie decides that this UI extension is usable and merges this PR you can make a PR for WLED documentation so that it gets attention of users.

@ajotanc
Copy link
Contributor

ajotanc commented Jun 14, 2024

The idea was really good, huh? hahah! I'll test it and let you know what I think, I'm out of date... I didn't even know it had gif support already. It looks very promising.

If possible @Manut38 , can I adapt what you did to my project? Looking at yours gave some very interesting ideas!

@Manut38
Copy link
Author

Manut38 commented Jun 25, 2024

@ajotanc Oh I'm sorry, I missed this reply!
Yeah, your idea definitely was great, and I've been using your solution before I found the on-the-fly decoder that they wrote here.
Thank you a lot for your pioneering on this!

However I'm pretty sure, that whenever the gif branch will be merged it will be the more versatile and faster solution for animations.
I'm honestly still super stoked about this 😁

Of course you can take any inspiration you like from my code, it's open source after all and it's also the exact same thing I did with your code before 😄

@netmindz netmindz changed the base branch from gif to main January 26, 2025 15:47
@netmindz netmindz added Awaiting testing and removed rebase needed This PR needs to be re-based to the current development branch labels Jan 26, 2025
Comment on lines +84 to +85
#undef WLED_DISABLE_GIF
#define WLED_DISABLE_GIF
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove

@@ -257,7 +257,6 @@ void handleIR();
#include "ESPAsyncWebServer.h"
#include "src/dependencies/json/ArduinoJson-v6.h"
#include "src/dependencies/json/AsyncJson-v6.h"
#include "FX.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please revert this change

@@ -9,6 +9,9 @@
#ifndef WLED_DISABLE_PXMAGIC
#include "html_pxmagic.h"
#endif
#ifndef WLED_DISABLE_GIFPLAYER
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please wrap with a check for WLED_ENABLE_GIF is defined as no point having the player if you don't have GIF support

@DedeHai
Copy link
Collaborator

DedeHai commented Jan 26, 2025

this is a nice addition to the gif player. I looked at flash usage: this UI uses more flash than the gif player itself... just wondering if it can be slimmed down a little by not using all those fancy graphics.
gif player: 6k of flash
this html UI: 8k of flash

@Manut38
Copy link
Author

Manut38 commented Feb 5, 2025

Thank you for your considerations to add this!
I'll be quite busy for the rest of the week, but I'll promise I'm going to take a look at the proposed changes as soon as I can.

@DedeHai
Copy link
Collaborator

DedeHai commented Feb 7, 2025

IMHO this addition makes the gif player much more user friendly but if it is added by default it must be as slim as possible, ideally 1k or less of flash.
Just an idea: have the most basic version possible built in and if internet is available, load an extended version from git repo?

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

Successfully merging this pull request may close these issues.

8 participants