这是indexloc提供的服务，不要输入任何密码
Skip to content

warn user when GIF disposal mode is not supported #3918

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

warn user when GIF disposal mode is not supported #3918

wants to merge 3 commits into from

Conversation

@Amiralgaby
Copy link

@Amiralgaby Amiralgaby commented Nov 4, 2024 •
edited
Loading

Description

With cjxl, if a GIF has dispose-to-0 mode, an error Getting data error is printed (#3134)

This pull request change

$ cjxl -d 0 296631940-d5a56cb4-b9e4-47c8-a20f-53bcf1f5a098.gif test.jxl
JPEG XL encoder v0.12.0 adeeccc4 [SSE2]
Getting pixel data failed.

to

$ ./cjxl ~/Images/icons/296631940-d5a56cb4-b9e4-47c8-a20f-53bcf1f5a098.gif test.jxl -d 0
JPEG XL encoder v0.12.0 adeeccc4 [SSE2]
GIF with dispose-to-0 is not supported for non-full or blended frames
Getting pixel data failed.

Pull Request Checklist

  • CLA Signed: Have you signed the Contributor License Agreement
  • Authors: Have you considered adding your name to the AUTHORS file?
  • Code Style: Have you ensured your code adheres to the project's coding style guidelines? You can use ./ci.sh lint for automatic code formatting.

@jonsneyers
Copy link
Member

jonsneyers commented Nov 20, 2024

If this happens in practice, then maybe we should get rid of that TODO and actually make it work, rather than just improving the error messaging...

@veluca93 do you remember what the complication was that caused this case not to be handled?

@veluca93
Copy link
Member

veluca93 commented Nov 20, 2024

If this happens in practice, then maybe we should get rid of that TODO and actually make it work, rather than just improving the error messaging...

@veluca93 do you remember what the complication was that caused this case not to be handled?

I don't think it would be too complicated, but I suspect it would need inserting dummy frames, which would be why we did not do it to begin with...

@jonnyawsom3 jonnyawsom3 mentioned this pull request Jan 19, 2025
Open
jonsneyers
jonsneyers reviewed Apr 14, 2025
View reviewed changes
lib/extras/dec/gif.cc
total_rect.xsize() != canvas.color.xsize ||
total_rect.ysize() != canvas.color.ysize || !replace)) {
if (!JXL_IS_DEBUG_BUILD) {
printf(
Copy link
Member

@jonsneyers jonsneyers Apr 14, 2025

Choose a reason for hiding this comment

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

Suggested change
printf(
fprintf(stderr,

@jonnyawsom3
Copy link
Collaborator

jonnyawsom3 commented Apr 29, 2025

I don't think it would be too complicated, but I suspect it would need inserting dummy frames, which would be why we did not do it to begin with...

Could reference frames or patches be used instead? Should have minimal impact on density then, albeit with a more complex solution.

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

Reviewers

@jonsneyers jonsneyers jonsneyers left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Amiralgaby @jonsneyers @veluca93 @jonnyawsom3