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

Lossy #16

Closed
wants to merge 4 commits into from
Closed

Lossy #16

wants to merge 4 commits into from

Conversation

@kornelski
Copy link
Contributor

@kornelski kornelski commented Mar 29, 2014

--lossy option that allows inexact match against LZW dictionary, which improves compression ratio. Lossy matching does a bit of 1-dimensional dithering.

This is a very basic implementation that does recursive search of dictionary nodes.

write_compressed_data contains some duplicated code, because the lossy search function needs to use less optimized code (ignores imageline), although this probably could be refactored a bit.

edit: fixed transparency

The results are pretty good:

3.3MB original

fat smooth anim

1.25MB lossy

noisy small anim

@kohler
Copy link
Owner

@kohler kohler commented Apr 4, 2014

I am really psyched for these contributions! It may take me another week or so to go through them in depth, but wanted to confirm I've seen them.

}

static inline const uint8_t gif_pixel_at_pos(Gif_Image *gfi, unsigned pos);

This comment has been minimized.

@kohler

kohler Apr 4, 2014
Owner

I would prefer that we reuse some of the color functions that are currently in the main code. For instance it supports gamma-corrected diffs.

This comment has been minimized.

@kornelski

kornelski Apr 4, 2014
Author Contributor

Unfortunately switching to kcolor didn't improve quality.

I suspect it's because it's computing distance in linear RGB colorspace, which is less perceptually accurate than computation straight in sRGB. This is because linear color gives linear luminance, but luminance has non-linear relationship with perceived lightness

Converted to linear RGB (notice disproportionately heavy deformation in dark areas):
t-gamma22

Kept in gamma 2.2:
t-gamma10

@kornelski
Copy link
Contributor Author

@kornelski kornelski commented Apr 30, 2014

I've been using it for a while and I think the implementation is solid.

@iirelu
Copy link

@iirelu iirelu commented May 25, 2014

When will this be merged? It's extremely powerful and halves the sizes of my gifs.

@vvo
Copy link

@vvo vvo commented Oct 18, 2014

@kohler Can you review this again? Extremely usefull! @pornel

@kornelski
Copy link
Contributor Author

@kornelski kornelski commented Oct 19, 2014

I've noticed there's lossy branch in the gifsicle project, so @kohler is working on it :)

I know at least ezgif.com and compressor.io are using the lossy code in production and I haven't heard any complaints, so I presume it's pretty stable.

@nunofgs
Copy link

@nunofgs nunofgs commented Dec 9, 2014

@kohler any news on this PR? :)

@ryanmjacobs
Copy link

@ryanmjacobs ryanmjacobs commented Dec 18, 2014

@kohler merge this please, it's very useful. 😄

@ggamel
Copy link

@ggamel ggamel commented Dec 24, 2014

👍 Looking forward to lossy gifs. Hoping it can be merged soon!

@alanhamlett
Copy link

@alanhamlett alanhamlett commented Feb 2, 2015

Merge this please!

@masom
Copy link

@masom masom commented Mar 9, 2015

Any progress on this issue? We are using the lossy fork but we would prefer to be using the official upstream.

@kornelski
Copy link
Contributor Author

@kornelski kornelski commented Sep 29, 2015

FYI I'm tweaking that branch to separate the compression function into lossy and non-lossy versions, as we've discussed over email.

@vvo
Copy link

@vvo vvo commented Feb 6, 2016

Any news @pornel on getting this merged? Thanks

@andreyvit
Copy link

@andreyvit andreyvit commented Feb 8, 2016

+1 for merging this. Pretty please?

@kornelski
Copy link
Contributor Author

@kornelski kornelski commented Feb 8, 2016

If someone wants to help, there's a merge conflict that needs to be resolved.

denji pushed a commit to denji/gifsicle that referenced this pull request Jul 17, 2016
--lossy option that allows inexact match against LZW dictionary,
which improves compression ratio.
Lossy matching does a bit of 1-dimensional dithering.

This is a very basic implementation that does recursive search of dictionary nodes.

write_compressed_data contains some duplicated code,
because the lossy search function needs to use less optimized code (ignores imageline),
although this probably could be refactored a bit.

The results are pretty good:
* Original: 3.3MB
* Lossy: 1.25MB

Based on gifsicle which implements lossy LZW compression.
It can reduce animgif file sizes by 30%—50% at a cost of some dithering/noise.
* https://pornel.net/lossygif
* https://github.com/pornel/giflossy

Closed: kohler#16
denji pushed a commit to denji/gifsicle that referenced this pull request Jul 17, 2016
--lossy option that allows inexact match against LZW dictionary,
which improves compression ratio.
Lossy matching does a bit of 1-dimensional dithering.

This is a very basic implementation that does recursive search of dictionary nodes.

write_compressed_data contains some duplicated code,
because the lossy search function needs to use less optimized code (ignores imageline),
although this probably could be refactored a bit.

The results are pretty good:
— Original: 3.3MB
— Lossy: 1.25MB

Based on gifsicle which implements lossy LZW compression.
It can reduce animgif file sizes by 30%—50% at a cost of some dithering/noise.
— https://pornel.net/lossygifhttps://github.com/pornel/giflossy

Closed: kohler#16
denji pushed a commit to denji/gifsicle that referenced this pull request Jul 17, 2016
--lossy option that allows inexact match against LZW dictionary,
which improves compression ratio.
Lossy matching does a bit of 1-dimensional dithering.

This is a very basic implementation that does recursive search of dictionary nodes.

write_compressed_data contains some duplicated code,
because the lossy search function needs to use less optimized code (ignores imageline),
although this probably could be refactored a bit.

The results are pretty good:
— Original: 3.3MB
— Lossy: 1.25MB

Based on gifsicle which implements lossy LZW compression.
It can reduce animgif file sizes by 30%—50% at a cost of some dithering/noise.
— https://pornel.net/lossygifhttps://github.com/pornel/giflossy

Closed: kohler#16
denji added a commit to denji/gifsicle that referenced this pull request Sep 9, 2016
--lossy option that allows inexact match against LZW dictionary,
which improves compression ratio.
Lossy matching does a bit of 1-dimensional dithering.

This is a very basic implementation that does recursive search of dictionary nodes.

write_compressed_data contains some duplicated code,
because the lossy search function needs to use less optimized code (ignores imageline),
although this probably could be refactored a bit.

The results are pretty good:
— Original: 3.3MB
— Lossy: 1.25MB

Based on gifsicle which implements lossy LZW compression.
It can reduce animgif file sizes by 30%—50% at a cost of some dithering/noise.
— https://pornel.net/lossygifhttps://github.com/pornel/giflossy

Closed: kohler#16
kornelski added 3 commits Jul 17, 2016
--lossy option that allows inexact match against LZW dictionary,
which improves compression ratio.
Lossy matching does a bit of 1-dimensional dithering.

This is a very basic implementation that does recursive search of dictionary nodes.

write_compressed_data contains some duplicated code,
because the lossy search function needs to use less optimized code (ignores imageline),
although this probably could be refactored a bit.

The results are pretty good:
— Original: 3.3MB
— Lossy: 1.25MB

Based on gifsicle which implements lossy LZW compression.
It can reduce animgif file sizes by 30%—50% at a cost of some dithering/noise.
— https://pornel.net/lossygifhttps://github.com/pornel/giflossy

Closed: #16
@saurabheights
Copy link

@saurabheights saurabheights commented Apr 18, 2017

@pornel @kohler : Currently, any changes/fixes made in kohler/gifsicle are not pulled into lossy pornel repo. Thus, getting this PR merged would reduce such issues.

I tried looking into the merge issue but didn't properly understand. There are two PR #16(this one) and #72. Also, I see denji:lossy-kornel, pornel:lossy, kohler:lossy branches. It's quite perplexing to understand which one needs resolution. If you could inform which branch needs the fix, I would be glad to get this merged.

Also, there has been some conversation between kohler and pornel. If anything of relevance I should know, it would help.

@kornelski
Copy link
Contributor Author

@kornelski kornelski commented Sep 16, 2017

Having a pull request stuck in limbo for 3 years is disheartening. I'm giving up on contributing.

If anyone wants it, it's merged into my fork: https://github.com/pornel/giflossy

@kornelski kornelski closed this Sep 16, 2017
@vvo
Copy link

@vvo vvo commented Sep 18, 2017

I am really psyched for these contributions! It may take me another week or so to go through them in depth, but wanted to confirm I've seen them.

@kohler and @pornel Thanks both for great contributions. It's very hard to play the open source game but both gifiscle and giflossy are great! Everyone wants them to merge so we can remove any specifics in readmes explaining how to install giflossy/gisicle and the differences.

I hope @kohler you can give some more help and finally make the tedious work of merging the changes. Thanks ..

@seefood
Copy link

@seefood seefood commented Nov 19, 2017

Another +1 for this PR. 3 years is a ridiculous amount of time. @kohler Can you at least clue us in to the reason for the holdup?

@fatso83
Copy link

@fatso83 fatso83 commented Jan 30, 2018

Oh, well, no need to get antsy about this not getting merged. Giflossy is the new gifsicle anyway.

@seefood
Copy link

@seefood seefood commented Feb 1, 2018

Sadly not when you apt install gifsicle. Yet.

@gingerbeardman
Copy link

@gingerbeardman gingerbeardman commented Mar 21, 2018

Really poor form not merging this @kohler - what's up with that?

@wheelerlaw
Copy link

@wheelerlaw wheelerlaw commented Jan 15, 2019

Looks like this project is dead
@kornelski Could you enable issues on your fork? Since this project is dead, it would be nice to have a place where issues could be addressed.

@seefood
Copy link

@seefood seefood commented Jan 16, 2019

@wheelerlaw the fact he wrote somw code for it does not mean he wants to adopt the whole project and become the new maintainer, you know :-)

@theelectricdave
Copy link

@theelectricdave theelectricdave commented Apr 18, 2019

Thanks @kornelski !!

kohler added a commit that referenced this pull request Apr 18, 2019
--lossy option that allows inexact match against LZW dictionary,
which improves compression ratio.
Lossy matching does a bit of 1-dimensional dithering.

This is a very basic implementation that does recursive search of dictionary nodes.

write_compressed_data contains some duplicated code,
because the lossy search function needs to use less optimized code (ignores imageline),
although this probably could be refactored a bit.

The results are pretty good:
— Original: 3.3MB
— Lossy: 1.25MB

Based on gifsicle which implements lossy LZW compression.
It can reduce animgif file sizes by 30%—50% at a cost of some dithering/noise.
— https://pornel.net/lossygifhttps://github.com/pornel/giflossy

Closed: #16
Repository owner locked as too heated and limited conversation to collaborators Apr 18, 2019
@kohler
Copy link
Owner

@kohler kohler commented Apr 18, 2019

Thanks very much to @kornelski for the original patch.

The patch was not merged initially because it uses a completely different approach to color comparison than the rest of Gifsicle does. Kornel added his own color comparison subsystem exclusively to gifwrite.c, not using the existing subsystem.

There were other issues which I mentioned to Kornel (like the name of the option and some misspellings) which he did not change, but those issues weren't important.

In long-ago offline conversation we discussed this. Kornel has some concerns about the current color comparison subsystem, and evidence that it behaves badly esp. on dark areas of images, but there are also images on which it does much better than median-cut. I was hoping we would come to a shared, better color subsystem, but we did not.

I just merged it. As people said, it was about time.

Thank you to the commenters who said nicely that it was about time. And especially thanks to Kornel.

No thank you to the commenters who complained heatedly or acted like they deserved something. You are part of what makes open source hard.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

You can’t perform that action at this time.