Bug 1765388 - PngQuant is corrupting photos due to how you've built it
Summary: PngQuant is corrupting photos due to how you've built it
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora EPEL
Classification: Fedora
Component: pngquant
Version: epel7
Hardware: x86_64
OS: Linux
unspecified
urgent
Target Milestone: ---
Assignee: Sergio Basto
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-10-25 02:29 UTC by Joe
Modified: 2020-01-04 20:11 UTC (History)
3 users (show)

Fixed In Version: pngquant-2.7.2-3.el7
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-01-04 20:11:35 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Github kornelski/pngquant/ 0 None None None 2019-12-10 04:45:53 UTC

Description Joe 2019-10-25 02:29:29 UTC
Description of problem:

PngQuant is corrupting photos. This is not a bug with the PngQuant source code but with how you've built it.


Version-Release number of selected component (if applicable):

2.12.5


Steps to Reproduce:

1. On CentOS 7 yum install pngquant from EPEL repo.
2. pngquant --output pngquant-output.png --speed 1 --force -v -- pngquant-input.png
3. View pngquant-output.png it is distorted.


Actual results:

pngquant-input.png:
  read 2019KB file
  used embedded ICC profile to transform image to sRGB colorspace
  made histogram...44630 colors found
  selecting colors...3%
  selecting colors...7%
  selecting colors...11%
  selecting colors...14%
  selecting colors...18%
  selecting colors...22%
  selecting colors...25%
  selecting colors...48%
  selecting colors...70%
  selecting colors...92%
  selecting colors...100%
  moving colormap towards local minimum
  eliminated opaque tRNS-chunk entries...0 entries transparent
  mapped image to new colors...MSE=553.309 (Q=0)
  writing 2-color image as pngquant-output.png
Quantized 1 image.


Expected results:

pngquant-input.png:
  read 2019KB file
  used embedded ICC profile to transform image to sRGB colorspace
  made histogram...44630 colors found
  selecting colors...3%
  selecting colors...7%
  selecting colors...11%
  selecting colors...33%
  selecting colors...37%
  selecting colors...59%
  selecting colors...81%
  selecting colors...100%
  moving colormap towards local minimum
  eliminated opaque tRNS-chunk entries...0 entries transparent
  mapped image to new colors...MSE=1.740 (Q=94)
  writing 256-color image as pngquant-output.png
  copied 2KB of additional PNG metadata
Quantized 1 image.


Additional info:

It seems to be a problem with the build process of pngquant using gcc version 4.8.5 which is the default version of gcc on CentosOS 7.

I compiled using gcc6 with success, see configure command here https://github.com/joejordanbrown/pngquant-epel-package-issue/blob/master/builds/successful/Dockerfile#L25.

See this repo for more details, test and build examples   https://github.com/joejordanbrown/pngquant-epel-package-issue


Also, you haven't compiled it with the latest libpng, see the below warning message when running pngquant -h.

pngquant, 2.12.5 (July 2019), by Kornel Lesinski, Greg Roelofs.
   Compiled with OpenMP (multicore support).
   Color profiles are supported via Little CMS. Using libpng 1.5.13.

WARNING: Your version of libpng is old and has buggy support for custom chunks.
Please recompile pngquant with the current version of libpng (1.6 or later).

Comment 1 Sergio Basto 2019-10-25 03:16:30 UTC
Hi,

Thanks for the report , maybe was an error update pngquant on el 7 ...  the previous version of pnqquant on el7 (built on 02 Dec 2016)  

Can you try pngquant 2.7.2 [1] ? 

Or you suggest use devtoolset-6-gcc ? are you sure that fixes all the problems ? shouldn't be easier rollback to 2.7.2 ?  

Thanks 

[1] 
https://koji.fedoraproject.org/koji/buildinfo?buildID=821860

[2]
https://src.fedoraproject.org/rpms/pngquant/pull-requests

Comment 2 Joe 2019-10-25 19:00:03 UTC
Hello Sergio,

I think it's best to update to the latest version than revert back to 2.7.2, it includes multiple improvements.

----------

I've found out exactly what is causing the issue now. it relates to a bug with gcc and OpenMP.

When compiling with gcc 4.8.5 and OpenMP 3.1 I notice this warning.

mediancut.c: In function 'mediancut':
mediancut.c:346:0: warning: ignoring #pragma omp taskgroup [-Wunknown-pragmas]
         #pragma omp taskgroup
 ^
mediancut.c:394:0: warning: ignoring #pragma omp taskgroup [-Wunknown-pragmas]
             #pragma omp taskgroup
 ^

I did some research and there seem to be a few compatibility issues fixed in a later version of gcc.

I've now tested this with a gcc 4.9.2 from devtoolset-3-gcc-4.9.2-6.el7.x86_64.rpm, found here https://centos.pkgs.org/7/centos-sclo-rh-x86_64/devtoolset-3-gcc-4.9.2-6.el7.x86_64.rpm.html and it works perfectly.

----------

Here is what was required to build successfully. 

# Install required repo packages
yum install epel-release centos-release-scl -y

# Update packages
yum update -y

# Install required packages
yum install git wget libpng-devel lcms2-devel devtoolset-3 -y

# Clone PngQuant github repo
cd /tmp && git clone --recursive https://github.com/kornelski/pngquant.git

# Checkout PngQuant tagged version
cd /tmp/pngquant && git checkout tags/2.12.5

# Download libpng
cd /tmp && wget http://prdownloads.sourceforge.net/libpng/libpng-1.6.37.tar.gz && tar xvzf libpng-1.6.37.tar.gz

# Configure libpng for PngQuant static
mv /tmp/libpng-1.6.37 /tmp/pngquant/libpng && cd /tmp/pngquant/libpng && ./configure --enable-static CC=/opt/rh/devtoolset-3/root/usr/bin/gcc && make

# Build using make
cd /tmp/pngquant && ./configure --with-openmp --with-lcms2 --prefix=/usr CC=/opt/rh/devtoolset-3/root/usr/bin/gcc && make && make install

Let me know if you have any other questions.

Regards,
Joe

Comment 3 Sergio Basto 2019-11-14 02:52:37 UTC
we need simplify this on spec file 
we need BuildRequires devtoolset-3 , and the rest ?

Comment 4 Joe 2019-11-14 18:58:19 UTC
I actually already have a spec file created for this, we rolled our own RPM internally to solve this issue. I will clean up our repo and publish it to GitHub then you'll have the spec to generate a successful build.

I used mock to test and build the RPM with a custom config file centos-7-x86_64.cfg to add the repos that were required during the build. 

I'm not familiar with koji build system, I know it uses mock but does it allow you to use a custom config file or add extra repos? If you can then you will be able to use BuildRequires devtoolset-3 to install the package. If it doesn't the spec will need to install the repos via rpm and the devtoolset-3, which can be done in the %prep section of the spec.

Comment 5 Sergio Basto 2019-11-14 19:03:22 UTC
(In reply to Joe from comment #4)
> I actually already have a spec file created for this, we rolled our own RPM
> internally to solve this issue. 


> I will clean up our repo and publish it to
> GitHub then you'll have the spec to generate a successful build.

No need , send me please. or do a pull request here https://src.fedoraproject.org/rpms/pngquant. click in fork , after git clone your fork , commit , push and via web do a pull request .

So give me a link with srp.rpm or a link for the pngquant.spec 

Thanks .

Comment 6 Sergio Basto 2019-11-14 19:22:12 UTC
(*) or give me a link with srp.rpm or a link for the pngquant.spec.

i.e. send me something as you like ! 

Thanks

Comment 7 Joe 2019-11-15 00:48:35 UTC
https://github.com/joejordanbrown/mock-rpm-pngquant look in data for the spec etc.

Comment 9 Joe 2019-11-15 18:29:59 UTC
Also, it's set to use devtoolset-7-gcc because the author of PngQuant said it should only be compiled on 7+ see here https://github.com/kornelski/pngquant/issues/346#issuecomment-547094923

Comment 10 Sergio Basto 2019-11-17 21:57:49 UTC
but by [1] you should use [2] not custom hacks ... , I'll will see if find time to fix pngquant on EPEL 7 soon. 



[1]
https://www.softwarecollections.org/en/scls/rhscl/devtoolset-7/

[2] 
scl enable devtoolset-7 bash

Comment 11 Joe 2019-11-19 21:53:11 UTC
Good catch, that shouldn't be like that. I refactored that before and commented that out but seems to have crept back in when refactoring. 

The whole time I thought the EPEL mock conf didn't have the repos required. By mistake, I'd been testing the spec on a very old mock version `mock-1.2.17-1.el7.centos` (default with CentOS repos) which dates back to 2016. I only realised the other day when refactoring and checking the mock GitHub source I found that the mock configs had the scls repository which was needed for devtoolset such a relief since it simplified the spec.

If you're happy with the spec I will create a pull request on your https://src.fedoraproject.org/rpms/pngquant/tree/epel7. Should I leave you to do the version change in the spec?

Comment 12 Fedora Update System 2019-12-10 04:46:38 UTC
FEDORA-EPEL-2019-7c9f06a0a4 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2019-7c9f06a0a4

Comment 13 Fedora Update System 2019-12-11 17:24:39 UTC
pngquant-2.7.2-2.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2019-7c9f06a0a4

Comment 14 Fedora Update System 2019-12-14 00:07:48 UTC
pngquant-2.7.2-3.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2019-d6bedfa33e

Comment 15 Fedora Update System 2020-01-04 20:11:35 UTC
pngquant-2.7.2-3.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.