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

raise limits to skip webp alpha tests for libwebp <= 0.2.1 #1098

Closed
wants to merge 3 commits into from

Conversation

frispete
Copy link
Contributor

@frispete frispete commented Feb 5, 2015

Dear Alex & friends,

in my public openSUSE python repo, I maintain a huge stack of python related packages.
One nice thing about build serrvice is the possibility to build the packages for various distributions.
For my own needs, I limit them to openSUSE releases of various ages (as distribution independant build specifications are a great PITA, even for rpm based systems).

https://build.opensuse.org/package/show/home:frispete:python/python-Pillow

The one, that failed consistently was 12.3:

[ 98s] FAIL: Can we write a RGBA mode file to webp without error.
[ 98s] ----------------------------------------------------------------------
[ 98s] Traceback (most recent call last):
[ 98s] File "/home/abuild/rpmbuild/BUILD/Pillow-2.7.0/Tests/test_file_webp_alpha.py", line 86, in test_write_rgba
[ 98s] self.assert_image_similar(image, pil_image, 1.0)
[ 98s] File "/home/abuild/rpmbuild/BUILD/Pillow-2.7.0/Tests/helper.py", line 105, in assert_image_similar
[ 98s] ave_diff, epsilon))
[ 98s]

Obviously, this is due to a failing libwebp version.
Since you already have a skip for version 0.1.3, question is, why not raise the limits for webp alpha tests. Side effect: the (rare) users of this lib are being warned that way, too, if they check WebPDecoderBuggyAlpha.

@landscape-bot
Copy link

Code Health
Repository health increased by 0.95% when pulling dd2266d on frispete:master into 5fa52f8 on python-pillow:master.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling dd2266d on frispete:master into * on python-pillow:master*.

1 similar comment
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling dd2266d on frispete:master into * on python-pillow:master*.

@wiredfool
Copy link
Member

I don't think that this is the right way to go about it.

With webp 0.1.3, attempting to write a RGBA image as lossless webp resulted in an RGB image, and reading a RGBA image failed completely.

With 0.2.3, it appears that the algorithm works, but is a little different from later versions. There is a little more variation from the expected image. In this case it appears a difference of 3 rather than 1, which is totally reasonable for a lossy format, especially with an early version knowing that later versions do it better.

@frispete
Copy link
Contributor Author

frispete commented Feb 6, 2015

On Donnerstag, 5. Februar 2015 22:17:14 wiredfool wrote:

I don't think that this is the right way to go about it.

With webp 0.1.3, attempting to write a RGBA image as lossless webp
resulted in an RGB image, and reading a RGBA image failed completely.

With 0.2.3, it appears that the algorithm works, but is a little different
from later versions. There is a little more variation from the expected
image. In this case it appears a difference of 3 rather than 1, which is
totally reasonable for a lossy format, especially with an early version
knowing that later versions do it better.

Sure, the primary question is, where do we want to draw the line concerning
the term buggy: do we want to test and handle those deviations specially, or
do we generally suppress testing alpha channel mode results for these early
lib versions. Nobody is going to test all released lib versions, hence it
potentially ends in broken builds for certain environments.

Anyway, I've reverted the fix, and supplied the former (IMHO uglier) approach.
Test build with 5 different lib versions and two archs were successful.

@frispete frispete closed this Feb 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants