-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Jpg config image extension #1863
Jpg config image extension #1863
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the implementation.
The config should only be read on app start and not be adjusted during runtime, as this is a global setting.
For UI elements Fooocus currently uses the AsyncJob object, which contains an array of parameters and which is introducing isolation per user/session. Using your approach (config) leads to User 1 updating the output extension also for User 2, who might not be aware / is not made aware of in the UI that the change happened and this way the feature isn't usable in isolation.
This is why i'd propose to add the UI element to the ctrls in webui.py, add a pop() call in async_worker.py and hand it over to the private_logger.py's log() method as a parameter. The radio buttons should also only be shown/interwctive when history log is enabled (see args).
The config is useful and can still be used as default for the instance.
What do you think?
Hi thanks for the feedback and clarification. Did not realize the config was global so thanks for pointing that out. |
This is awsome guys, thanks for the work you are all doing. |
… to log and get_current_html_path functions per feedback
Updated with the changes requested. Please let me know if I missed something. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eddyizm i've fixed a bug (missing extension param), added webp support and adjusted the label, as the label "Image Extension" could be misunderstood as images shown in the webui are still always png (see issue #1421). The wording is still not 100% perfect, feel free to adjust to your liking.
Comparison between output formats, incl. webp (same seed & settings):
What do you think, should we integrate the proposed solution of #1421 to this feature?
@mashb1t I agree the naming is not correct. by using image extension we are implying format which it looks like originally I was just changing the naming convention, so more like image naming extension or file extension. When I was changing it I was looking at the gradio docs because there should really be some type of mouse over with a not or a star to say "this does not change the actual creation or compression logic, just simply changes the file extension for convenience" or something to that effect. Although after seeing the outputs, it does appear to be affecting the size of the file. Good catch on the log parameter, seems like I had it in there at some point and backed it out. Everything else looks good to me! EDIT: Yes, I seen the linked issue. It is also a problem I've been having with Firefox since day one until I stopped using it. But if we can fix it, let's do it. It is quite annoying. |
returns and uses filepaths instead of numpy image by saving to temp dir uses double the temp dir file storage on disk as it saves to temp dir and gradio temp dir when displaying the image, but reuses logged output image
…nfig_image_extension # Conflicts: # modules/async_worker.py # modules/private_logger.py
@eddyizm functionality from #1932 has been merged, works perfectly for me. Now also the preview / final images in gradio do have the selected file extension. Please check, happy to hear your feedback! |
this is now possible due to image extension support in gradio via lllyasviel#1932
Looks good to me. That last label change is perfect! |
quality=95 optimize=True progressive=True
@eddyizm A label and naming even better describing what it is is "Output Format". Hope this is also ok for you. |
Yup makes more sense. I was thinking about changing it as well. Good call. |
@eddyizm fyi now that #1940 has fully been implemented i've also added EXIF metadata handling for jpg and webp, see mashb1t@b281375 |
This bad boy keeps getting bigger and bigger! |
# Conflicts: # modules/async_worker.py # modules/flags.py # modules/private_logger.py # webui.py
@eddyizm would it be ok for you if i add myself as the committing author? |
@mashb1t sure. No problem at all! |
…1863) * feature: added flag, config and ui update for image extension change #1789 * moved function to config module * moved image extension to webui via async worker. Passing as parameter to log and get_current_html_path functions per feedback * check flag before displaying image extension radio button * disabled if image log flag is passed in * fix: add missing image_extension parameter to log call * refactor: change label * feat: add webp to image_extensions supported image extemsions: see https://pillow.readthedocs.io/en/stable/handbook/image-file-formats.html * feat: use consistent file name in gradio returns and uses filepaths instead of numpy image by saving to temp dir uses double the temp dir file storage on disk as it saves to temp dir and gradio temp dir when displaying the image, but reuses logged output image * feat: delete temp images after yielding to gradio * feat: use args temp path if given * chore: code cleanup, remove redundant if statement * feat: always show image_extension element this is now possible due to image extension support in gradio via #1932 * refactor: rename image_extension to image_file_extension * feat: use optimized jpg parameters when saving the image quality=95 optimize=True progressive=True * refactor: rename image_file_extension to output_format * feat: add exif handling * refactor: code cleanup, remove items from metadata output --------- Co-authored-by: Manuel Schmid <[email protected]> Co-authored-by: Manuel Schmid <[email protected]> Co-authored-by: Manuel Schmid <[email protected]> Co-authored by: eddyizm <[email protected]>
[Added]
feature: added flag, config and ui update for image extension change)[feature: added flag, config and ui update for image extension toggle.
#1789