Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 824018 - media-libs/libpng: dropping "apng" patches
Summary: media-libs/libpng: dropping "apng" patches
Status: IN_PROGRESS
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Gentoo's Team for Core System packages
URL:
Whiteboard:
Keywords:
Depends on: 824822 824826 824830 824834 824838
Blocks:
  Show dependency tree
 
Reported: 2021-11-16 13:41 UTC by Joonas Niilola
Modified: 2022-04-14 22:53 UTC (History)
11 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
libpng-1.6.37-apng-runtime-optional.patch (libpng-1.6.37-apng-runtime-optional.patch,10.42 KB, patch)
2021-11-19 15:29 UTC, Arfrever Frehtes Taifersar Arahesis
Details | Diff
square.png (square.png,1.91 KB, image/png)
2021-11-25 21:02 UTC, Michał Górny
Details
square-fs8.png (after pngquant) (square-fs8.png,1.23 KB, image/png)
2021-11-25 21:03 UTC, Michał Górny
Details
Elephant Diff (diff.png,7.76 KB, image/png)
2021-11-29 19:32 UTC, Thomas Deutschmann
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Joonas Niilola gentoo-dev 2021-11-16 13:41:35 UTC
Hey, 

to start off the discussion. 

Now this is what I've gathered: 
Apparently chromium cant' be built with system-libpng due to the apng patches we carry over. 
And firefox can't be built with system-libpng without it. 

This essentially creates a conflict where only one browser can be built using system-libpng. I'm not aware of any other packages behaving badly due to the patches, but from a fast lookup, we seem to be the only distro providing apng patches with libpng. So I doubt any other package would break if we removed those patches.

What's with firefox then? It seems both firefox-91esr and -94 bundle the latest libpng with them, and the bundled libpng does get patched with apng. Same seems to hold true for thunderbird-91.3.0. So I wouldn't worry about Mozilla "lagging behind" with libpng. 

Of course, bundled is always bundled. 

What should we do to solve this?
Comment 1 Lars Wendler (Polynomial-C) gentoo-dev 2021-11-16 13:46:21 UTC
https://wiki.gentoo.org/wiki/Why_not_bundle_dependencies

I'm strongly against unsing bundled png in mozilla products. It's simple math: One package that fails with libpng[apng] vs. many that do not fail. And there is an active upstream that provides apng patches for every new libpng release.
Comment 2 Stephan Hartmann gentoo-dev 2021-11-16 14:18:48 UTC
Small correction:
We can build Chromium with system-libpng, but if libpng is build with apng patch applied, then animations are flickering and we see a lot of libpng warnings in log files. Chromium has its own decoder for APNG on top of libpng and they don't require libpng patching.

As mentioned on IRC, at least Arch Linux dropped the apng patch months ago.
Comment 3 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-16 15:16:17 UTC
(In reply to Lars Wendler (Polynomial-C) from comment #1)
> https://wiki.gentoo.org/wiki/Why_not_bundle_dependencies
> 
> I'm strongly against unsing bundled png in mozilla products. It's simple
> math: One package that fails with libpng[apng] vs. many that do not fail.
> And there is an active upstream that provides apng patches for every new
> libpng release.

Actually, the simple math is: one vendor requires custom breaking patches, while everything else works fine without them.

My suggestion would be to last-rite Mozilla products entirely.
Comment 4 Joonas Niilola gentoo-dev 2021-11-16 16:13:16 UTC
(In reply to Michał Górny from comment #3)
> 
> My suggestion would be to last-rite Mozilla products entirely.

Only serious discussion please. This point was good though:

> Actually, the simple math is: one vendor requires custom breaking patches, 
> while everything else works fine without them.
Comment 5 Mike Gilbert gentoo-dev 2021-11-16 16:27:03 UTC
Questions:

1. Why has this "apng" patchset not been merged into the libpng project upstream?

2. Are there any plans to merge it?

3. How does this patchset break chromium?

4. Could the apng patchset or chromium be changed to work around the problem?

5. Why does Mozilla require this patchset?

If there are no plans to merge apng into the upstream project, it doesn't make sense to me that we would continue applying it to our media-libs/libpng package.
Comment 6 Stephan Hartmann gentoo-dev 2021-11-16 17:16:00 UTC
(In reply to Mike Gilbert from comment #5)
> Questions:
> 
> 1. Why has this "apng" patchset not been merged into the libpng project
> upstream?

Vote to support APNG chunks failed in the PNG group in 2007.
http://www.libpng.org/pub/png/png2007.html

I read somewhere that libpng only wants to process images (can't find the reference at the moment). Apple demonstrated in Webkit to implement a decoder for APNG without patching libpng.

> 
> 2. Are there any plans to merge it?

Second attempt did not receive much response, so unlikely.
https://sourceforge.net/p/png-mng/mailman/png-mng-misc/?viewmonth=201709

> 
> 3. How does this patchset break chromium?
> 
> 4. Could the apng patchset or chromium be changed to work around the problem?

https://crbug.com/1142228

> 
> 5. Why does Mozilla require this patchset?

They invented APNG, but failed to merge it into libpng for above reasons.

> 
> If there are no plans to merge apng into the upstream project, it doesn't
> make sense to me that we would continue applying it to our media-libs/libpng
> package.
Comment 7 Thomas Deutschmann gentoo-dev 2021-11-16 18:40:35 UTC
Why is that showing up now? I think we have that patch for years and chromium package is also in tree for some time. Sounds like a change in chromium is breaking something?
Comment 8 Stephan Hartmann gentoo-dev 2021-11-16 18:53:55 UTC
(In reply to Thomas Deutschmann from comment #7)
> Why is that showing up now? I think we have that patch for years and
> chromium package is also in tree for some time. Sounds like a change in
> chromium is breaking something?

There was no change in chromium, just nobody saw it so far.
Comment 9 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-16 20:20:33 UTC
(In reply to Stephan Hartmann from comment #6)
> (In reply to Mike Gilbert from comment #5)
> > Questions:
> > 
> > 1. Why has this "apng" patchset not been merged into the libpng project
> > upstream?
> 
> Vote to support APNG chunks failed in the PNG group in 2007.
> http://www.libpng.org/pub/png/png2007.html
> 
> I read somewhere that libpng only wants to process images (can't find the
> reference at the moment). Apple demonstrated in Webkit to implement a
> decoder for APNG without patching libpng.

The comment @ [1] suggests that it was rejected because people insisted on conflating it into .png rather than using a separate suffix.  Doesn't inspire you with confidence on both ends.

[1] https://github.com/ImageMagick/ImageMagick/issues/24#issuecomment-136362187
Comment 10 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2021-11-17 06:59:41 UTC
(In reply to Michał Górny from comment #3)
> (In reply to Lars Wendler (Polynomial-C) from comment #1)
> > https://wiki.gentoo.org/wiki/Why_not_bundle_dependencies
> > 
> > I'm strongly against unsing bundled png in mozilla products. It's simple
> > math: One package that fails with libpng[apng] vs. many that do not fail.
> > And there is an active upstream that provides apng patches for every new
> > libpng release.
> 
> Actually, the simple math is: one vendor requires custom breaking patches,
> while everything else works fine without them.
> 

Right.

This started when I mentioned some Chromium errors that had been noticed from running 'chromium' in the terminal. Discussed it in #gentoo-chromium and we found https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=973240.

At first, we suspected Chromium might have patches in its internal copy of libpng, but ended up finding the errors were caused by our patches to the system copy of libpng instead.

Chromium uses its own decoder _separately_ from libpng to handle apng.

Right now, we're patching the system copy with unofficial patches for the purpose of one package (Firefox), while another package (Chromium) which does things correctly (implementing the behaviour separately) is experiencing broken behaviour as a result.

I don't think if this were any other package we'd accept patching something for the benefit of 1 package and breaking others.
Comment 11 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2021-11-17 07:05:27 UTC
(In reply to Sam James from comment #10)

... an alternative to using the bundled copy in Firefox would be to try work with Mozilla / the apng patch authors and see if there's a way to enable it at runtime with some additional function call or parameter.
Comment 12 Thomas Deutschmann gentoo-dev 2021-11-17 14:52:40 UTC
(In reply to Sam James from comment #10)
> Right now, we're patching the system copy with unofficial patches for the
> purpose of one package (Firefox), while another package (Chromium) which
> does things correctly (implementing the behaviour separately) is
> experiencing broken behaviour as a result.

Are you indicating the patch is invalid? I don't agree with this.

Maybe chromium is trying to be extra clever... anyway, we better try to understand _why_ this is causing flickering for Chromium. Maybe either Chromium can be patched to fix the problem or apng can be improved to prevent this behavior in Chromium. But first we have to understand _why_ it is flickering in Chromium... apng in libpng itself is implemented in a transparent way: When reading image, it is looking for apng information. If it won't find this information, it will end additional processing. Haven't looked how Chromium implemented this yet but I would suspect that the image is processed twice (from libpng which is handling apng information already and their apng implementation) based on comment #2 which is causing the flickering.
Comment 13 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-17 15:38:07 UTC
I'm saying that it's the Gentoo policy to avoid applying custom patches, and especially carrying behavior-changing patches that were rejected upstream.  Even if we ignore everything else, this goes against the vanilla Gentoo principle, making it unsuitable for developing portable applications (as application built against Gentoo's patched libpng may not work against vanilla libpng).
Comment 14 Thomas Deutschmann gentoo-dev 2021-11-17 18:31:23 UTC
(In reply to Michał Górny from comment #13)
> I'm saying that it's the Gentoo policy to avoid applying custom patches, and
> especially carrying behavior-changing patches that were rejected upstream.

Conditional patching via USE flag is _not_ violating any Gentoo policies.

While the end result maybe the same, I think it is important to highlight that the registration attempt for APNG extension was downvoted in 2007 because of the lack of a real implementation.

Since Blink has implemented APNG in 2017, it is now standardized by coercion and a real thing in the web (image/apng is a thing in all browsers but not at IANA yet).

W3C is currently working on getting APNG somehow officially approved in retrospective (https://github.com/w3c/PNG-spec/issues/26).
Comment 15 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-17 19:44:53 UTC
(In reply to Thomas Deutschmann from comment #14)
> While the end result maybe the same, I think it is important to highlight
> that the registration attempt for APNG extension was downvoted in 2007
> because of the lack of a real implementation.

Do you have any evidence to claim that?  The citations above suggest otherwise.
Comment 16 Stephan Hartmann gentoo-dev 2021-11-17 20:03:47 UTC
(In reply to Thomas Deutschmann from comment #14)
> (In reply to Michał Górny from comment #13)
> > I'm saying that it's the Gentoo policy to avoid applying custom patches, and
> > especially carrying behavior-changing patches that were rejected upstream.
> 
> While the end result maybe the same, I think it is important to highlight
> that the registration attempt for APNG extension was downvoted in 2007
> because of the lack of a real implementation.
> 

The specification was published in 2004 with a patch. It was downvoted in 2007, because PNG group supported MNG at that time.
Comment 17 Andreas K. Hüttel archtester gentoo-dev 2021-11-19 01:04:31 UTC
(In reply to Thomas Deutschmann from comment #14)
> (In reply to Michał Górny from comment #13)
> > I'm saying that it's the Gentoo policy to avoid applying custom patches, and
> > especially carrying behavior-changing patches that were rejected upstream.
> 
> Conditional patching via USE flag is _not_ violating any Gentoo policies.
> 

As you know, not all Gentoo policies were written down in detail in the past.

Conditional patching via USE flag was *at least* strongly discouraged, if not outright banned except for unusual circumstances. (That's what I learned during my quizzes.)
Comment 18 Thomas Deutschmann gentoo-dev 2021-11-19 03:08:06 UTC
Discouraged yes, but for different reasons (like hard to test, especially for rarely used USE flags, similar why conditional patching is discouraged because during minor bumps, the dev is probably not going to test all possible combinations. especially for rare cases like prefix, hardening, s390, musl... to name just a few. Or that OpenSSL USE=bindist crap...). Keep in mind that we also have USE=vanilla... or see the OpenSSH patch set which will make OpenSSH incompatible depending on used USE flags combination but this is required to get the desired functionality (like X.509)...
Comment 19 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2021-11-19 03:18:40 UTC
(In reply to Thomas Deutschmann from comment #12)
> (In reply to Sam James from comment #10)
> > Right now, we're patching the system copy with unofficial patches for the
> > purpose of one package (Firefox), while another package (Chromium) which
> > does things correctly (implementing the behaviour separately) is
> > experiencing broken behaviour as a result.
> 
> Are you indicating the patch is invalid? I don't agree with this.
> 

I'm saying that it appears to be invalid given that it's forcing behaviour on library consumers despite it being rejected by upstream. The patch should make it runtime conditional with a flag on certain functions or some way to enable it for the duration of the process.

It sounds like Chromium is doing what is intended (not patching a library which rejected it, and instead implementing it separately).
Comment 20 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-19 12:45:53 UTC
So the way I see it, we have a few options.

Of course, the preferable option would be for Mozilla to stop relying on patched version of libpng but given Mozilla's track record this is unlikely to happen (see also: sqlite).

The more likely options are:

1. Switching back to bundled libpng.  While I hate bundling, Mozilla packages are already doing it anyway, so I can live with it.

2. Updating libpng patches to make APNG modifications enabled at runtime.  I'm basically talking about having some png_enable_apng() function and disabling all the extra code unless it's been called first.

Ideally, this would be done "upstream", so that we wouldn't have to patch everything but even if we had to, it's still not that bad.  That said, it still involves carrying custom patches, so I hate it nevertheless.

3. Fork media-libs/libpng into media-libs/libpng_apng.  Either rename the header and library, or put it into subdirectory.  The former implies patching revdeps, the latter adding proper -I, -L which should be easier.  This is the option I hate the least as it avoids bundling and lets users have vanilla and patched libpng simultaneously (*).

(*) I'm not sure if this will actually work.  It's inevitable that libpng and libpng_apng will both be loaded simultaneously, and I can't predict the outcome.
Comment 21 Joonas Niilola gentoo-dev 2021-11-19 13:04:17 UTC
Should we already prep up votify to solve this? :) who're eligible voters, base-system, codecs project... mozilla and chromium? Other consumers?

Just a throwaway thought seeing there's no clear consensus.
Comment 22 Thomas Deutschmann gentoo-dev 2021-11-19 13:55:52 UTC
(In reply to Joonas Niilola from comment #21)
> Should we already prep up votify to solve this? :) who're eligible voters,
> base-system, codecs project... mozilla and chromium? Other consumers?

We are far away from being ready to make a decision.

Comment #20 is missing one important key: Many applications from https://packages.gentoo.org/packages/media-libs/libpng/reverse-dependencies gained APNG support not even knowing through libpng[apng].

Dropping APNG support from libpng will have consequences for hundreds of packages because there is no single just APNG supporting library available (and of course, all of these applications currently benefiting from libpng[apng] would need porting).

So saying, "Oh, this one is easy, let's re-use bundled library again" misjudges the situation.  This is only possible for Chromium/Webkit related applications which are new and are the only one experiencing the problem (maybe because they wanted to do it right and invented their own APNG decoded on top which is now clashing).

Also, what do you want to do when APNG extension will land in libpng (comment #14)? Did someone bring this up to Chromium for example already? Maybe it is easier for them to prevent double-processing at their end...



PS: I would do the opposite from #2 from comment #20, i.e. keep libpng[apng] enabled by default but patch applications with own APNG decoder to disable that code path at runtime because there are way less applications with own APNG-decoders. This way, you can keep status quo, i.e. applications who silently gained APNG support through libpng[apng]...
Comment 23 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-19 14:07:33 UTC
(In reply to Thomas Deutschmann from comment #22)
> (In reply to Joonas Niilola from comment #21)
> > Should we already prep up votify to solve this? :) who're eligible voters,
> > base-system, codecs project... mozilla and chromium? Other consumers?
> 
> We are far away from being ready to make a decision.
> 
> Comment #20 is missing one important key: Many applications from
> https://packages.gentoo.org/packages/media-libs/libpng/reverse-dependencies
> gained APNG support not even knowing through libpng[apng].

Do you realize that the APNG patches don't magically make animated PNGs work in applications that don't use the custom API?
Comment 24 Thomas Deutschmann gentoo-dev 2021-11-19 15:01:45 UTC
(In reply to Michał Górny from comment #23)
> Do you realize that the APNG patches don't magically make animated PNGs work
> in applications that don't use the custom API?

How about reading the entire patch and get familiar with the format itself? Sure, there is the animated thing requiring use of functions like png_read_frame_* but PNG gained better color depth, 24-bit transparency support from APNG, just to mention a few.
Comment 25 Thomas Deutschmann gentoo-dev 2021-11-19 15:03:19 UTC
Will be funny for many users if they open an existing PNG, save it again  with libpng[-apng] and suddenly the PNG has changed...
Comment 26 Thomas Deutschmann gentoo-dev 2021-11-19 15:07:12 UTC
Don't get me wrong here: If we wouldn't have libpng[apng] support _today_ and this would be about new Firefox requiring a patched libpng, I agree with anyone saying "we cannot patch libpng".

But we have to take into account that we are carrying this patch many many years. Reverting this is not possible without data corruption/information loss.
Comment 27 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-19 15:20:50 UTC
(In reply to Thomas Deutschmann from comment #25)
> Will be funny for many users if they open an existing PNG, save it again 
> with libpng[-apng] and suddenly the PNG has changed...

You do realize people use systems other than Gentoo, and these systems don't carry custom patches, right?  You're only proving how much harm this inconsiderate patching has caused already.

The worst part is, people supporting this patch don't even seem to realize how much harm they're doing, and instead behave as if it was the best thing in the world and everyone else was wrong for not worshipping it.
Comment 28 Arfrever Frehtes Taifersar Arahesis 2021-11-19 15:29:42 UTC
Created attachment 752938 [details, diff]
libpng-1.6.37-apng-runtime-optional.patch

My initial attempt at making APNG feature runtime-optional.

In this version of patch, APNG feature is enabled by default, and programs which do not want it should call:
png_use_apng(0);
Comment 29 Thomas Deutschmann gentoo-dev 2021-11-19 16:29:59 UTC
(In reply to Michał Górny from comment #27)
> You do realize people use systems other than Gentoo, and these systems don't
> carry custom patches, right?  You're only proving how much harm this
> inconsiderate patching has caused already.
> 
> The worst part is, people supporting this patch don't even seem to realize
> how much harm they're doing, and instead behave as if it was the best thing
> in the world and everyone else was wrong for not worshipping it.

Again, I agree with you if this would be about us talking about adding the patch _today_.

But we have this patch for more than a decade.

If you drop this patch today, people exchanging PNG files from any Debian or Ubuntu system with Gentoo will lose image information when they try to modify and save again. Not to mention all the locally created PNG images which will get destroyed when they try to save them again because they didn't know that when they picked Gimp to save that transparent image as PNG, that this wouldn't work as expected but just worked because they used libpng[apng] at the time of creation.



@Arfrever Frehtes Taifersar Arahesis: I am also experimenting with such an attempt. However, I am only patching pngread.c to disable the PNG_READ_APNG_SUPPORTED clause at runtime which should be enough to address the double processing in Webkit.
Comment 30 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-20 07:33:05 UTC
(In reply to Thomas Deutschmann from comment #24)
> How about reading the entire patch and get familiar with the format itself?
> Sure, there is the animated thing requiring use of functions like
> png_read_frame_* but PNG gained better color depth, 24-bit transparency
> support from APNG, just to mention a few.

So I've read the whole patch, I've read the specification [1] and I have found no evidence of what you're talking about.  Could you please point me at specific patch lines and/or points in the specification?

[1] https://wiki.mozilla.org/APNG_Specification
Comment 31 Arfrever Frehtes Taifersar Arahesis 2021-11-20 12:47:16 UTC
(In reply to Arfrever Frehtes Taifersar Arahesis from comment #28)
> Created attachment 752938 [details, diff] [details, diff]
> libpng-1.6.37-apng-runtime-optional.patch
> 
> My initial attempt at making APNG feature runtime-optional.
> 
> In this version of patch, APNG feature is enabled by default, and programs
> which do not want it should call:
> png_use_apng(0);

Maybe add support for environmental variable (e.g. LIBPNG_APNG) which would affect initial default value.
Explicit call to png_use_apng() C function would still override it.

Chromium, Firefox, Thunderbird have shell launcher scripts.
Exporting a variable there would be easier than patching their source code.
Comment 32 Max Stepin 2021-11-21 22:09:06 UTC
(In reply to Thomas Deutschmann from comment #12)
> Haven't
> looked how Chromium implemented this yet but I would suspect that the image
> is processed twice (from libpng which is handling apng information already
> and their apng implementation) based on comment #2 which is causing the
> flickering.

Chromium's implementation works like lightweight remuxing of APNG into a bunch of individual PNGs. One frame, one PNG. Then those PNGs are sent to libpng for decoding. 

So in theory it shouldn't matter which libpng version is used, since both patched and unpatched versions decode static PNGs just fine.

Most likely the problem is with Chromium's remuxing code. 

Or maybe it's Chromium code for composing one frame on top of another. If transparency is handled incorrectly there, it might appear as flicker. 

Their codebase for this is in  
chromium/third_party/blink/renderer/platform/image-decoders/png/
Comment 33 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-22 09:29:13 UTC
For the record, I'm still waiting for whissi to justify his claims but let's presume that my analysis is correct for the moment.

What's follows is my personal opinion.  While others have the right to disagree, unless they're able to bring good counter-arguments, this is the opinion I'm going to pursue.  If there is still disagreement about how to proceed, I am going to call for a QA team vote on pursuing this.

My opinion is based on the following goals:

1. Users should be able to use either browser (or both) without dependency conflicts and rendering issues.

2. (less important) Gentoo should be avoiding diverging from upstream.


To recap:

1. The APNG standard has been rejected by the PNG Group.  It has been criticized for repeating the mistakes of GIF, and with its not very wide deployment it's even worse.  Long story short, many applications will only display the static image and this can be used to sneak additional content in.  This is bad but the cat's out of the bag now, thanks to Mozilla and Apple.

2. Gentoo's libpng defaults to applying Mozilla's APNG patches which have been rejected by libpng for a long time now.  These patches should not affect static PNGs, and switching between -apng and apng should not be a problem outside of APNG domain.

3. Mozilla products plus two other packages (apngasm, kimtoy) require patched libpng for APNG support.  All of them vendor patched libpng which we're unbundling.

4. WebKit-based browsers implement APNG support on top of vanilla libpng.  APNG patches are causing issues with this implementation that are non-trivial to resolve.


Even if we said that chromium is in the wrong for doing hacky stuff, Mozilla is no better for relying on rejected patches to a high-profile library.  On top of that, Gentoo is wrong to be defaulting to deploying these patches by default and potentially creating a case for applications wrongly depending on these patches.

I believe that the following approach will be the best solution for the time being:

1. Packages depending on APNG patches gain IUSE=system-png that is OFF by default.  That is, they use vendored libpng by default to free the constraints on libpng.

2. WebKit-based packages optionally gain IUSE=+system-png that is ON by default.  When using system libpng, they eventually start requiring libpng[-apng] in order to avoid the breakage involved in using APNG patches.

3. We change the desktop profiles not to enable APNG patches by default anymore.

[4. Long-term, we may want to pursue masking or removing the APNG patches altogether but that's not something that we need to decide on today.]


I believe that this is a fair approach, as it makes it possible for users to use both browser families without issues.  As much as I hate bundled libraries, the milk is already spilled in both cases and I don't see why libpng should be made special here.

The Chromium maintainer has agreed to add system-png flag in the dev channel release for Chromium already.  Since the issue is not critical, it will naturally get deployed to other versions.  The Mozilla and profile part is done in the linked PR.

I would like to hear the opinion of Mozilla maintainers by 2021-11-29 (i.e. next Monday).  If I receive no reply by then, I'm going to request QA intervention.
Comment 34 Thomas Deutschmann gentoo-dev 2021-11-25 12:57:04 UTC
APNG brings 24-bit images and full 8-bit transparency (real alpha channel) to PNG.

If you take the original Gentoo blender logo for example (not PNG from the wiki!), you can create a really stunning version of the logo which makes full use of the 24-bit format resulting in better looking metallic(?) effects for example, especially on HighDPI displays.

Now create the alpha channel and add full transparency to the image (not as color/palette like you can do with plain PNG).

You can use the logo on every background.

Now re-compile your entire system without libpng[apng] support. Open the image and save it again. Look what happened: You ended up with a logo with reduced colors and black background. The logo can't be used on any background anymore (except black).

Another real world example: There are image optimizer like media-gfx/pngquant. Many platforms or programs like GitLab, Jenkins, WordPress (via plugins), Nikola... come with tasks/recipes to auto-optimize images on build.

Like you probably read, PNG format supports extensions. I.e. the specs are saying "This is how we define chunks; If you stumble across a chunk you don't know, just skip" and APNG patch is also implemented as extension, i.e. fully transparent. However, this only applies for reading/processing PNGs. Converts/optimizers, i.e. tools which will read the source image and write out an hopefully optimized version are working different. When they are unaware of APNG chunks, they will not write them to disk again. In short: pngquant which uses ibimagequant which gains APNG support through libpng[apng] will kill all APNGs when you re-process the file with libpng[-apng].

Now imagine a webserver running Gentoo Linux with WordPress and plugins like EWWW Image Optimizer. If the web customer will switch WordPress theme which causes new thumbnail sizes and system was changed to libpng[-apng] in the meanwhile, the customer will end up with broken images.


(In reply to Michał Górny from comment #33)
> To recap:
> 
> 1. The APNG standard has been rejected by the PNG Group.  It has been
> criticized for repeating the mistakes of GIF, and with its not very wide
> deployment it's even worse.  Long story short, many applications will only
> display the static image and this can be used to sneak additional content
> in.  This is bad but the cat's out of the bag now, thanks to Mozilla and
> Apple.

This is not true.

As said in previous comments, application was only rejected in 2007 for formal reasons. 

In 2017, the format became popular and adopted by all major web browsers, it is now standardized by coercion. The paper work to make this 'official' was started this summer.


> 2. Gentoo's libpng defaults to applying Mozilla's APNG patches which have
> been rejected by libpng for a long time now.  These patches should not
> affect static PNGs, and switching between -apng and apng should not be a
> problem outside of APNG domain.

No. Please stop repeating that the patch has been rejected.

And no, like shown, it can affect any image written to disk as PNG via libpng[apng]. _That iss the problem_. I.e. the blender example uses just one frame, no animation. Like your 'static' PNG.


> 4. WebKit-based browsers implement APNG support on top of vanilla libpng. 
> APNG patches are causing issues with this implementation that are
> non-trivial to resolve.

How so? At no point did you try to check with Chromium. If comment #32 is correct, this is a problem in that engine. Remember: You noticed on your own that the animated APNG stuff will require special API calls. But Chromium doesn't use them. So how can the patch cause problems if this functionality isn't used?


> 1. Packages depending on APNG patches gain IUSE=system-png that is OFF by
> default.  That is, they use vendored libpng by default to free the
> constraints on libpng.

This is maintainer's decision. Sure, some will change IUSE defaults, but some won't. In the end, Gentoo is about choices and people can decide on their own if they want to go with bundled libs or not if provided by the package.


> I believe that this is a fair approach, as it makes it possible for users to
> use both browser families without issues.  As much as I hate bundled
> libraries, the milk is already spilled in both cases and I don't see why
> libpng should be made special here.

You are really missing the points I outlined above. How do you want to prevent data loss? You do not provide a migration path. That's the real problem here. You cannot "just drop" it.
Comment 35 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-25 13:11:55 UTC
(In reply to Thomas Deutschmann from comment #34)
> APNG brings 24-bit images and full 8-bit transparency (real alpha channel)
> to PNG.
> 
> If you take the original Gentoo blender logo for example (not PNG from the
> wiki!), you can create a really stunning version of the logo which makes
> full use of the 24-bit format resulting in better looking metallic(?)
> effects for example, especially on HighDPI displays.
> 
> Now create the alpha channel and add full transparency to the image (not as
> color/palette like you can do with plain PNG).
> 
> You can use the logo on every background.
> 
> Now re-compile your entire system without libpng[apng] support. Open the
> image and save it again. Look what happened: You ended up with a logo with
> reduced colors and black background. The logo can't be used on any
> background anymore (except black).

Have you actually done that?  I've been asking for proof, either in form of links to specific code, specification or even example images as you claim here.
Comment 36 Thomas Deutschmann gentoo-dev 2021-11-25 15:36:41 UTC
I did a mix.

I created the logo using Windows and Adobe programs.
However, I opened and re-saved file on my Gentoo notebook with libpng[apng] and confirmed that it only dropped some extensions added by Adobe.

Then I confirmed the pngquant issue. E.g. running the image through pngquant on the libpng[apng] system did not kill the alpha channel. But running through pngquant in chroot with libpng[-apng] killed it.
Comment 37 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-25 16:12:08 UTC
Could you please attach the files so that someone can prove your claims?
Comment 38 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-25 20:59:29 UTC
(In reply to Thomas Deutschmann from comment #36)
> I did a mix.
> 
> I created the logo using Windows and Adobe programs.
> However, I opened and re-saved file on my Gentoo notebook with libpng[apng]
> and confirmed that it only dropped some extensions added by Adobe.
> 
> Then I confirmed the pngquant issue. E.g. running the image through pngquant
> on the libpng[apng] system did not kill the alpha channel. But running
> through pngquant in chroot with libpng[-apng] killed it.

I'm afraid I can't reproduce.  However, I haven't used some "Windows and Adobe programs" but I've exported a PNG image from Inkscape drawing with alpha channel.  Then I've pushed the image through convert(1) and pngquant(1) on a system with libpng[-apng].  The original image and the two output images all have their alpha channel preserved.

If this is really not an user error, then my only possible suspicion is that Adobe is outputting non-standard image that just happens to be accidentally preserved through APNG patches.

However, we can't really prove anything given that you 1) do not describe the method thoroughly, and 2) do not provide actual samples.

That said, pngquant(1) is supposed to be lossy.  So I wouldn't be surprised if it reduced color depth in some special cases.  However, this should not depend on APNG patches.  If it does, then it is more likely that APNG patches actually *break* pngquant(1) and prevent it from working correctly.


(In reply to Thomas Deutschmann from comment #34)
> APNG brings 24-bit images and full 8-bit transparency (real alpha channel)
> to PNG.

I'm starting to seriously wonder if you're really this wrong and you're insisting on being wrong, or you're deliberately spreading FUD.  Let's make this clear.

32-bit RGBA color space is a feature of PNG format.  APNG has nothing to do with it.  If you read the APNG spec, you'd notice that *they do not change anything about the image data*.  In fact, the additional animation frames use *exactly the same format* as regular IDAT frame.

The APNG specification mentions 24-bit color space and 8-bit alpha channel in comparison to *animated GIF*.  The things you claim of PNG are actually limitations of GIF files.  The spec literally says that APNG is used to combine advantages of PNG (over GIF) with the ability to do animation.
Comment 39 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-25 21:02:21 UTC
Created attachment 756342 [details]
square.png
Comment 40 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-25 21:03:42 UTC
Created attachment 756346 [details]
square-fs8.png (after pngquant)

Here are my test images.

square.png is the original image, square-fs8.png is the result of pngquant.

I've opened both images in eog(1), and in both cases I see semi-transparent rectangle on transparent background.
Comment 41 Georgy Yakovlev archtester gentoo-dev 2021-11-25 21:21:03 UTC
I also did some testing with imagemagick's convert and don't see behavior described in comment #36

alpha channel remains untouched.
image type remains TrueColorAlpha (from identify -verbose)
Comment 42 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-29 13:37:39 UTC
Given no reachable agreement between interested parties, I would like to request the Council to vote on proceeding with disabling APNG by default as outlined in #c33 and implemented in [1].

In particular:

1. Adding USE=system-png to firefox & thunderbird (I've skipped seamonkey because it is broken and does not build, I can add patches if need be once it's fixed).

2. Modifying the profiles not to enable APNG patches by default.

The changes are already done on chromium side.

[1] https://github.com/gentoo/gentoo/pull/23005
Comment 43 Thomas Deutschmann gentoo-dev 2021-11-29 19:32:57 UTC
Created attachment 757021 [details]
Elephant Diff

It is too early to call for a decision -- like mentioned in previous comments, it could also be a problem in chromium and maybe something we can be easily fixed upstream.

Anyway, regarding the reproducer:

Try https://apng.onevcat.com/demo/. Confirm in a tool of your choice that you are able to detect alpha channels and that you understand the difference between using 8bit alpha channel vs using color/palette, i.e. understand what a non-associated alpha channel is and how you detect the difference with the tools of your choice).

Note: pngquant is probably not a good (or hard to understand) example because the tool itself is not apng-aware. I.e. when you will process an APNG animation from the example above, you will lose all the additional frames (which is somehow expected like you noticed yourself in comment #23) and it is also changing color depth to reduce file size. However, pay attention to the alpha channel.

If you repeat processing image with and without libpng[apng] support, you should end up with two different files (~400 bytes difference).

Please see the attached attempt of showing the visual difference.
Comment 44 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-29 19:46:38 UTC
(In reply to Thomas Deutschmann from comment #43)
> If you repeat processing image with and without libpng[apng] support, you
> should end up with two different files (~400 bytes difference).

I saved the image and used convert(1) with libpng[-apng] and libpng[apng].  In both cases I've gotten a byte-by-byte identical file.  All files preserved RGBA color space, and visually similar to the original (as seen in non-APNG eog(1)).  There's obviously a size difference because the extra frames are dropped (in both cases).

$ cmp elephant[23].png 
$ file elephant*
elephant2.png: PNG image data, 480 x 400, 8-bit/color RGBA, non-interlaced
elephant3.png: PNG image data, 480 x 400, 8-bit/color RGBA, non-interlaced
elephant.png:  PNG image data, 480 x 400, 8-bit/color RGBA, non-interlaced

How do you expect others to verify your result if you keep refusing to provide precise instructions on how to reproduce your exact result?
Comment 45 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-29 19:58:26 UTC
This is a script that reproduces my result (note: you may need to enable binpkg-multi-instance or remove -k).

```
#!/usr/bin/env bash
set -e -x

rm -f elephant{,-apng,-noapng}.png

wget https://apng.onevcat.com/assets/elephant.png
sudo env USE=apng emerge -1vk media-libs/libpng
convert elephant.png elephant-apng.png
sudo env USE=-apng emerge -1vk media-libs/libpng
convert elephant.png elephant-noapng.png
cmp elephant-apng.png elephant-noapng.png
file elephant{,-apng,-noapng}.png
```

See?  This is how your share your results, not via some ambiguous undocumented claims.
Comment 46 Thomas Deutschmann gentoo-dev 2021-11-29 20:55:58 UTC
I am really not sure if it is that much helpful to try to show world at all costs that I am wrong. I mean, I would be happy if I would be wrong and we could drop an additional patch and wouldn't need to care about data loss... How about working together and not trying to make someone looking bad and posting sneaky comments in IRC?

But well.

Do you really don't know how "file" command works? It's not that accurate like you would hope for. And in case you aren't aware that imagemagick has some independent APNG suppport, please check`magick identify -list format | grep APNG`. In the end I really didn't just mention "not as color/palette" for fun:

> whissi@lore /tmp $ wget -O source.png "https://bugs.gentoo.org/attachment.cgi?id=756342"
> --2021-11-29 21:16:20--  https://bugs.gentoo.org/attachment.cgi?id=756342
> SSL_INIT
> Resolving bugs.gentoo.org (bugs.gentoo.org)... 2607:fcc0:4:ffff::4, 204.187.15.4
> Connecting to bugs.gentoo.org (bugs.gentoo.org)|2607:fcc0:4:ffff::4|:443... connected.
> HTTP request sent, awaiting response... 302 Found
> Location: https://824018.bugs.gentoo.org/attachment.cgi?id=756342 [following]
> --2021-11-29 21:16:21--  https://824018.bugs.gentoo.org/attachment.cgi?id=756342
> SSL_INIT
> Resolving 824018.bugs.gentoo.org (824018.bugs.gentoo.org)... 2607:fcc0:4:ffff::4, 204.187.15.4
> Connecting to 824018.bugs.gentoo.org (824018.bugs.gentoo.org)|2607:fcc0:4:ffff::4|:443... connected.
> HTTP request sent, awaiting response... 200 OK
> Length: 1958 (1,9K) [image/png]
> Saving to: ‘source.png’
> 
> source.png                    100%[=================================================>]   1,91K  --.-KB/s    in 0s
> 
> 2021-11-29 21:16:23 (26,8 MB/s) - ‘source.png’ saved [1958/1958]
> 
> whissi@lore /tmp $ wget -O result.png "https://bugs.gentoo.org/attachment.cgi?id=756346"
> --2021-11-29 21:16:36--  https://bugs.gentoo.org/attachment.cgi?id=756346
> SSL_INIT
> Resolving bugs.gentoo.org (bugs.gentoo.org)... 2607:fcc0:4:ffff::4, 204.187.15.4
> Connecting to bugs.gentoo.org (bugs.gentoo.org)|2607:fcc0:4:ffff::4|:443... connected.
> HTTP request sent, awaiting response... 302 Found
> Location: https://824018.bugs.gentoo.org/attachment.cgi?id=756346 [following]
> --2021-11-29 21:16:37--  https://824018.bugs.gentoo.org/attachment.cgi?id=756346
> SSL_INIT
> Resolving 824018.bugs.gentoo.org (824018.bugs.gentoo.org)... 2607:fcc0:4:ffff::4, 204.187.15.4
> Connecting to 824018.bugs.gentoo.org (824018.bugs.gentoo.org)|2607:fcc0:4:ffff::4|:443... connected.
> HTTP request sent, awaiting response... 200 OK
> Length: 1261 (1,2K) [image/png]
> Saving to: ‘result.png’
> 
> result.png                    100%[=================================================>]   1,23K  --.-KB/s    in 0s
> 
> 2021-11-29 21:16:38 (15,1 MB/s) - ‘result.png’ saved [1261/1261]
> 
> whissi@lore /tmp $ pngcheck -v source.png
> File: source.png (1958 bytes)
>   chunk IHDR at offset 0x0000c, length 13
>     544 x 480 image, 32-bit RGB+alpha, non-interlaced
>                      ^^^^^^^^^^^^^^^^
>   chunk pHYs at offset 0x00025, length 9: 3779x3779 pixels/meter (96 dpi)
>   chunk tEXt at offset 0x0003a, length 25, keyword: Software
>   chunk IDAT at offset 0x0005f, length 1843
>     zlib: deflated, 32K window, default compression
>   chunk IEND at offset 0x0079e, length 0
> No errors detected in source.png (5 chunks, 99.8% compression).
> whissi@lore /tmp $ pngcheck -v result.png
> File: result.png (1261 bytes)
>   chunk IHDR at offset 0x0000c, length 13
>     544 x 480 image, 8-bit palette, non-interlaced
>                      ^^^^^^^^^^^^^
>   chunk tEXt at offset 0x00025, length 25, keyword: Software
>   chunk pHYs at offset 0x0004a, length 9: 3779x3779 pixels/meter (96 dpi)
>   chunk PLTE at offset 0x0005f, length 60: 20 palette entries
>   chunk tRNS at offset 0x000a7, length 20: 20 transparency entries
>   chunk IDAT at offset 0x000c7, length 1042
>     zlib: deflated, 32K window, maximum compression
>   chunk IEND at offset 0x004e5, length 0
> No errors detected in result.png (7 chunks, 99.5% compression).


PS: And like said before, pngquant is probably more confusing/misleading if you aren't familiar with PNG format itself and libimagequant limitations/options. We should probably find a better how to demonstrate the issue.
Comment 47 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-11-29 21:39:03 UTC
(In reply to Thomas Deutschmann from comment #46)
> PS: And like said before, pngquant is probably more confusing/misleading if
> you aren't familiar with PNG format itself and libimagequant
> limitations/options. We should probably find a better how to demonstrate the
> issue.

So your claim is that pngquant is lossy?  Guess what, that's what it says on the can!  So I ran pngquant(1) on elephant.png with and without apng, and unsurprisingly the result is identical.  In both cases the color depth is trimmed.  However, that's not relevant to APNG at all but to what pngquant does.

If you really cared to work together, you could have shared *precise* commands two weeks ago.  What you do instead is stall, attach some files (how were they created?) and then blame [-apng] for everything.

Still, two weeks have passed and all we have are wild claims, zero pointers to specification that would prove your claims, zero pointers to code that would prove your claims (yet you claim that *I* haven't looked at the patch) and some sneaky samples that still prove nothing.  If your goal is to waste my time and make me mad, then you're doing well.  However, if that's how you prove your arguments... then maybe it's time to give up and stop wasting everyone's time.
Comment 48 Amel Hodzic 2021-12-23 20:10:35 UTC
Is there consensus to bring this issue back on track away from emotional/personal issues? Maybe a concerted effort to try and prove the potential issues raised in comment #22, or any adverse effects from the approach suggested and outlined in comment #20 would serve us all better in the long-run, even if you have to play devil's advocate.  Alternatively, a comment or link to any additional work or discussions external to this bug report could be added as a way to maintain the status of this bug.  Comment #21 hints at some of the relevant projects or people that could help with this. 

As this is not a matter of urgency, maybe it behooves us all to slow down a bit here.

Just some thoughts from a lowly end-user with much love and respect.
Comment 49 Marcus Comstedt 2021-12-29 10:09:28 UTC
(In reply to Thomas Deutschmann from comment #34)
> APNG brings 24-bit images and full 8-bit transparency (real alpha channel)
> to PNG.

This seems to be a simple misunderstanding.

PNG has always had 24-bit images and full 8-bit transparency.  That is not what the APNG patch brings to the table.

What it _does_ do is to allow PNG (which has 24-bit images and 8-bit transparency) to be used instead of _GIF_ (which does not) in animations.  Doing so requires use of a non-standard API, so it does not benefit applications using the libpng standard API.
Comment 50 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2021-12-29 10:15:57 UTC
(In reply to Amel Hodzic from comment #48)
> Is there consensus to bring this issue back on track away from
> emotional/personal issues? Maybe a concerted effort to try and prove the
> potential issues raised in comment #22, or any adverse effects from the
> approach suggested and outlined in comment #20 would serve us all better in
> the long-run, even if you have to play devil's advocate.  Alternatively, a
> comment or link to any additional work or discussions external to this bug
> report could be added as a way to maintain the status of this bug.  Comment
> #21 hints at some of the relevant projects or people that could help with
> this. 
> 
> As this is not a matter of urgency, maybe it behooves us all to slow down a
> bit here.
> 
> Just some thoughts from a lowly end-user with much love and respect.

Away from personal issues, what's happened here is:
- the council voted and made a decision [0] to have Firefox use -system-png (bundled copy) and Chromium optionally use the system copy (on by default), so they don't clash, while not putting apng onto people where possible globally;

- no patches have been offered to adjust Chromium/WebKit's logic here (but I'm not convinced this woudl really be enough as it could affect any application);

- a WIP patch was posted somewhere by Arfrever (it may have just been IRC?) to make it runtime optional but nobody seems to have played with integrating into Mozilla. Whissi had also mentioned playing with this but I don't think he posted anything.

The status quo is that Mozilla products work using bundled libpng with the apng modifications they require and everything else on the system has vanilla libpng by default. It's the best of a bad situation, I think.

[0] https://bugs.gentoo.org/824834#c1
Comment 51 Amel Hodzic 2021-12-29 12:35:05 UTC
Thank you very much for the clarification and for the update.  I know I, at least, was a bit confused about the apparent reversal of the previously-announced apng removal.  Thanks again for the continued methodical efforts.
Comment 52 Marek Szuba archtester gentoo-dev 2022-01-09 19:33:46 UTC
No further Council involvement expected at present.
Comment 53 Perfect Gentleman 2022-01-13 17:56:07 UTC
I've built ungoogled-chromium with libpng[apng]. It works fine.