Last Comment Bug 257197 - (apng) Add APNG support for Mozilla
(apng)
: Add APNG support for Mozilla
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: Trunk
: All All
: -- enhancement with 21 votes (vote)
: ---
Assigned To: Andrew Smith
:
Mentors:
http://wiki.mozilla.org/APNG_Specific...
: 375174 (view as bug list)
Depends on: apngspec
Blocks: 495609 8415 152292 326817
  Show dependency treegraph
 
Reported: 2004-08-27 17:07 PDT by Stuart Parmenter
Modified: 2014-09-22 09:17 PDT (History)
103 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
APNG decoder patch (28.36 KB, patch)
2004-08-27 17:11 PDT, Stuart Parmenter
no flags Details | Diff | Splinter Review
apng patch for libpng 1.2.10 (47.81 KB, patch)
2006-09-09 23:10 PDT, Andrew Smith
pavlov: review+
Details | Diff | Splinter Review
apng patch for libpng 1.2.12 (47.76 KB, patch)
2006-09-18 21:34 PDT, Andrew Smith
no flags Details | Diff | Splinter Review
apng patch for libpng 1.2.12 with changes as per comments #59, #60 from glenn (47.70 KB, patch)
2006-09-19 08:24 PDT, Andrew Smith
no flags Details | Diff | Splinter Review
apng mozilla patch, just for feedback (106.31 KB, patch)
2006-10-24 01:41 PDT, Andrew Smith
no flags Details | Diff | Splinter Review
almost complete patch, for comments/tracking only (110.85 KB, patch)
2006-11-07 22:59 PST, Andrew Smith
no flags Details | Diff | Splinter Review
apng patch for mozilla, ready for review (111.77 KB, patch)
2006-11-14 11:52 PST, Andrew Smith
no flags Details | Diff | Splinter Review
updated patch (111.79 KB, patch)
2006-12-06 17:39 PST, Andrew Smith
pavlov: review+
Details | Diff | Splinter Review
updated patch (117.69 KB, patch)
2007-01-29 15:27 PST, Andrew Smith
no flags Details | Diff | Splinter Review
patch updated for APNG 0.6 (119.68 KB, patch)
2007-02-08 13:15 PST, Andrew Smith
no flags Details | Diff | Splinter Review
patch for APNG 0.7 (121.02 KB, patch)
2007-03-06 02:08 PST, Andrew Smith
no flags Details | Diff | Splinter Review
same, but must have patched system-png (121.43 KB, patch)
2007-03-08 15:32 PST, Andrew Smith
no flags Details | Diff | Splinter Review
same, but must have patched system-png (121.59 KB, patch)
2007-03-08 22:48 PST, Andrew Smith
cbiesinger: review+
Details | Diff | Splinter Review
apng patch (149.04 KB, patch)
2007-03-18 19:55 PDT, Andrew Smith
pavlov: review+
Details | Diff | Splinter Review
[checked in] apng patch (149.12 KB, patch)
2007-03-19 20:06 PDT, Andrew Smith
pavlov: review+
pavlov: superreview+
Details | Diff | Splinter Review
[checked in] remove extra CRCs and make chunks not-safe-to-copy (48.51 KB, patch)
2007-03-21 01:00 PDT, Stuart Parmenter
asmith16: review+
Details | Diff | Splinter Review
split out dispose_op and blend_op from render_op (20.48 KB, patch)
2007-03-21 13:23 PDT, Stuart Parmenter
vladimir: superreview+
Details | Diff | Splinter Review
[checked in] split out dispose_op and blend_op from render_op (21.03 KB, patch)
2007-03-22 16:36 PDT, Stuart Parmenter
pavlov: review+
pavlov: superreview+
Details | Diff | Splinter Review
iteration sample image (429 bytes, image/png)
2007-03-23 11:25 PDT, N.Katô
no flags Details
various small fixes (14.65 KB, patch)
2007-03-29 12:21 PDT, Andrew Smith
no flags Details | Diff | Splinter Review
[checked in] various small fixes (26.07 KB, patch)
2007-04-14 21:10 PDT, Andrew Smith
pavlov: review+
Details | Diff | Splinter Review

Description Stuart Parmenter 2004-08-27 17:07:33 PDT
Add support for the new animated PNG spec to Mozilla.
Comment 1 Stuart Parmenter 2004-08-27 17:11:27 PDT
The spec can be found at
http://www.vlad1.com/~vladimir/projects/apng/AnimatedPNG.html
Comment 2 Stuart Parmenter 2004-08-27 17:11:48 PDT
Created attachment 157216 [details] [diff] [review]
APNG decoder patch

This patch adds APNG support for to the PNG decoder, makes the default
imgContainer implementation support basic animations and turns on unknown chunk
support in libpng.  Because most of the imgContainer code is the same as the
code in imgContainerGIF, I will be making imgContainerGIF inherit from
imgContainer to save code.

This is just a first pass at the decoder implementation, so it might need a bit
more work.
Comment 3 tor 2004-08-27 19:09:50 PDT
Please do the decoder changes for cairo gfx in a seperate bug.
Comment 4 Stuart Parmenter 2004-08-27 19:57:35 PDT
Yeah, those changes weren't supposed to be part of this patch.  Please ignore them..
Comment 5 Glenn Randers-Pehrson 2004-08-28 10:54:34 PDT
To avoid violating the PNG spec, aPNG should have its own signature bytes
(Just change "P" to "A") and header chunk (AHDR could be the same as your
proposed "aNIm" chunk, plus canvas width and height).  See clause 5.2 of
the ISO PNG spec which promises that a PNG file contains only a single
image.
Comment 6 Vladimir Vukicevic [:vlad] [:vladv] 2004-08-28 12:50:56 PDT
(In reply to comment #5)
> To avoid violating the PNG spec, aPNG should have its own signature bytes
> (Just change "P" to "A") and header chunk (AHDR could be the same as your
> proposed "aNIm" chunk, plus canvas width and height).  See clause 5.2 of
> the ISO PNG spec which promises that a PNG file contains only a single
> image.

Doing so would make APNG images unreadable by non-APNG-aware browsers, and would
severely hamper any adoption of the format; currently, the format can be
trivially read by the widely-used libpng library.  The PNG spec states that a
PNG file contains only a single image; from the point of of view of any
compliant decoder, this is still true -- anything after the IEND should simply
be ignored.  That said, we do violate 15.2.1 (c), making the complete APNG
streams non-conformant, but still parseable.
Comment 7 Vladimir Vukicevic [:vlad] [:vladv] 2004-08-28 13:05:18 PDT
I've opened bug 257263 for discussion of/feedback on the spec itself; let's
leave this bug for the mozilla patch and other integration bits.
Comment 8 Scott Baker 2004-08-28 14:18:01 PDT
I'm confused as to why we need a new animated spec, when the existing MNG spec
is more mature?

See bug #18547 for info about adding MNG support back into Firefox. Are these
two conflicting?
Comment 9 Brendan Eich [:brendan] 2004-08-28 14:40:59 PDT
APNG is like ~5K of code on top of PNG.  MNG is hundreds of K for a format
almost no one uses, overdesigned to the point that there is no MNG profile to do
just animated PNG.  Drivers@mozilla.org have said that we want just animated
PNG, not the whole of MNG, for inclusion in default builds (for an animated
image format with 8-bit alpha, and otherwise to avoid GIF).

There's no comparison between APNG and MNG, and no way MNG as a build option
preempts this work.

/be
Comment 10 Glenn Randers-Pehrson 2004-08-28 14:54:24 PDT
MNG is not necessarily hundreds of K.  The MNG_BUILD_WEB_NO_JNG configuration
is 108k and includes a complete PNG decoder.  By comparison, the libpng PNG
decoder by itself was 105k in Mozilla-1.4 (it's a good deal smaller now because
I cut it down to make room for MNG support).  Even the largest configuration,
MNG_BUILD_FULL_MNG_NO_JNG, is 168k.  Only if you include JNG does it exceed
200k (I'm using FreeBSD5.0/gcc-3.2.1; other platforms will do better).
Comment 11 Glenn Randers-Pehrson 2004-08-28 16:07:06 PDT
There is a MNG profile to do "just animated PNG."  It is MNG_VLC, which
was eventually rejected for moz (see bug #18574) because MNG_VLC doesn't do
GIF disposal methods.  Whoops, neither does aPNG.  The MNG-VLC decoder
libmng variant MNG_BUILD_VLC is 74.4k on FreeBSD5.0/gcc-3.2.1 and includes
a PNG decoder.
Comment 12 Stuart Parmenter 2004-08-28 16:54:00 PDT
Perhaps you missed that the spec was at version 0.2.. Frame disposal methods
will be added in the near future.  There are reserved bytes for it and can be
added whenever without breaking the format.
Comment 13 Brendan Eich [:brendan] 2004-08-28 17:00:00 PDT
> There is a MNG profile to do "just animated PNG."

I get tired of writing "suitable for replacing animated GIFs, and with 8-bit
alpha, etc."  VLC wasn't good enough, and here we are.  It's good to have MNG
experts helping.

/be
Comment 14 haferfrost 2004-08-29 02:54:57 PDT
> MNG is hundreds of K for a format
> almost no one uses, 

Versus APNG which really no one uses.

> overdesigned to the point that there is no MNG profile to 
> do just animated PNG.

Then why don't you join mng-list to develop a new MNG profile? Is it because you
are so much smarter than all of mng-list? 
Comment 15 Scott Baker 2004-08-29 09:23:15 PDT
(In reply to comment #8)
> I'm confused as to why we need a new animated spec, when the existing MNG spec
> is more mature?
> 
> See bug #18547 for info about adding MNG support back into Firefox. Are these
> two conflicting?

Sorry I had a dislexic moment, the bug regarding MNG support is bug #18574
Comment 16 Brendan Eich [:brendan] 2004-08-29 09:45:45 PDT
> Versus APNG which really no one uses.

Not yet, but soon.  APNG will be used in XUL apps where animated GIFs have been
used poorly till now.

> Then why don't you join mng-list to develop a new MNG profile? Is it because
> you are so much smarter than all of mng-list? 

This must be addressed to pav and vlad, so they can answer if necessary, but
it's silly -- Glenn is commenting here and in the apngspec bug, and if the right
thing is to make APNG an official MNG profile, the right people are cc'd to make
that happen.  In the mean time, Mozilla will have APNG sooner than your way.

Even if APNG becomes an MNG profile, it should not require full MNG integration
-- the entire point of this exercise is to provide an animated GIF replacement
that uses PNGs with full alpha channel, and not to do all of what MNG does, or
any other part of MNG.

Finally, trolling in this bug is a good way to get kicked from bugzilla, so do
not post cruft here again.

/be
Comment 17 Anne (:annevk) 2004-08-29 11:58:50 PDT
If it would become a MNG profile, would it remain backwards compatible? If not,
I think it shouldn't even be considered.
Comment 18 Glenn Randers-Pehrson 2004-08-29 15:25:51 PDT
>if the right thing is to make APNG an official MNG profile, the right
>people are cc'd to make that happen.

vlad has rejected that approach outright in the other bug.  They
prefer to proceed with the multiple-image PNG approach, which clearly
violates the PNG spec, clause 5.2.
Comment 19 Stuart Parmenter 2004-08-29 15:46:58 PDT
(In reply to comment #18)
> >if the right thing is to make APNG an official MNG profile, the right
> >people are cc'd to make that happen.
> 
> vlad has rejected that approach outright in the other bug.  They
> prefer to proceed with the multiple-image PNG approach, which clearly
> violates the PNG spec, clause 5.2.

Why are you saying one thing in this bug and another in the other bug?  You just
finished saying "That would be OK" in the other bug after vlad clearly explained
to you that the afRa chunks are just blobs of data.  The PNG image, as viewed by
the PNG spec is still a single image.  The extra chunks don't mean anything to a
standard PNG decoder.
Comment 20 Glenn Randers-Pehrson 2004-08-29 16:38:24 PDT
Here is the context in which I said "That would be OK":

That would be OK for a purely private development but in this case surely you
intend to deploy the format world wide.  
Comment 21 Brendan Eich [:brendan] 2004-08-29 17:39:42 PDT
If no PNG decoder in the world sees the APNG as more than a single image, then
what is the problem? That's exactly the graceful degradation we want.

Glenn, what goes wrong operationally with the approach of the current spec?

Purity issues about how many images dance on the head of an APNG are neither
here nor there. Specs exist to constrain or ensure interoperation and compatibility.

/be
Comment 22 Glenn Randers-Pehrson 2004-08-29 18:04:02 PDT
>If no PNG decoder in the world sees the APNG as more than a single image, then
>what is the problem? That's exactly the graceful degradation we want.

The problem is that suddenly every browser and every editor is expected to
be able to handle them, when we promised that png would always be single-image.

>Glenn, what goes wrong operationally with the approach of the current spec?

The second tier of chunks isn't visible to editors.  No other PNGs to my
knowledge -- certainly no standard ones -- have multiple tiers.  That is
an unnecessary complication that was added simply to try and work around
the spec prohibition against multiple images.

>Purity issues about how many images dance on the head of an APNG are neither
>here nor there. Specs exist to constrain or ensure interoperation and
>compatibility.

Purity is important to the PNG group and we have fended off a few similar
attempts already.   Be glad that some big corp hasn't already shoved
an animated-PNG down our throat.  It's a bit of a surprise to get what
we perceive as an attack on the spec from the open source community.  Well
maybe not an "attack" but certainly a plan to go against our intentions,
even after you have been clearly reminded of them.

Note that we have no objection to the plan (maybe criticism though) if
you don't call it PNG and don't use the PNG signature.  If you do use
our signature then you devalue the PNG signature (it no longer would
"indicate" the presence of single versus multiple images) and the image/png
MIME type.

If you want, go ahead and propose the new chunks to the PNG Registration
Authority (See Clause 4.9 of the ISO PNG Spec).  I can't promise a
favorable vote but I know several probably will vote for it.
Comment 23 timeless 2004-08-29 18:12:42 PDT
brendan: have you followed the apng qt interactions?
Comment 24 Vladimir Vukicevic [:vlad] [:vladv] 2004-08-29 18:48:15 PDT
(In reply to comment #23)
> brendan: have you followed the apng qt interactions?

The Qt interactions won't occur with the current proposal (0.3).
Comment 25 Mike Shaver (:shaver -- probably not reading bugmail closely) 2004-08-29 20:30:25 PDT
> Purity is important to the PNG group and we have fended off a few similar
> attempts already.   Be glad that some big corp hasn't already shoved
> an animated-PNG down our throat.  It's a bit of a surprise to get what
> we perceive as an attack on the spec from the open source community.  Well
> maybe not an "attack" but certainly a plan to go against our intentions,
> even after you have been clearly reminded of them.
> 
> Note that we have no objection to the plan (maybe criticism though) if
> you don't call it PNG and don't use the PNG signature.  If you do use
> our signature then you devalue the PNG signature (it no longer would
> "indicate" the presence of single versus multiple images) and the image/png
> MIME type.

To whom is the "we" meant to refer?  Willem van Schaik[1] and Thomas Boutell[2]
-- both influential members of the PNG community, if I read history correctly --
have expressed support for precisely this piggybacking approach to
multiple-frame PNGs, so I don't think it's appropriate to represent this as an
attack on PNG, or even as condemned by PNG-Group consensus.

[1] http://mail.mozilla.org/pipermail/apng/2004-August/000009.html
[2] http://mail.mozilla.org/pipermail/apng/2004-August/000015.html
Comment 26 Glenn Randers-Pehrson 2004-08-29 20:42:02 PDT
I meant the PNG development group.  Last time we discussed this type of
change was in June 2003 (thread "PNG with MNG-like features" in mng-list)
and at that time opposition was pretty solid.  I respect Willem's and Thomas'
opinions, too.  Their expressed support for something APNG-like is fairly
recent history.  You will find messages in opposition in recent history
as well from other PNG old-timers.

We have a voting mechanism in place when it comes time to decide what to do.

Glenn
Comment 27 Brendan Eich [:brendan] 2004-08-29 21:08:35 PDT
Re: comment 22: the "we" who are in favor of purity of concepts and separation
by suffix/mime-type are not the "we" leading Mozilla, who do want
backward-compatible extensions to existing formats (see http://www.whatwg.org/,
particularly the web forms 2 spec).  The lack of uptake of MNG on the web, and
the hardships faced by non-IE browsers, should suggest which values are better
for market share.

So there is a conflict of values here.  I don't know how to resolve it, or that
it must be resolved right now.

/be
Comment 28 Doug Wright 2004-08-30 04:32:58 PDT
I already said this in the spec bug, but I'll repeat it here.

The text explaining the PNG signature in 5.2 uses the word
*indicate*. This is just about vague enough IMO as to allow APNG to be a valid
extension of PNG, as long as the final spec meets all the other criteria.

Now if the spec had said *specifies*, that would be the end of it...
Comment 29 Robert Kaiser 2004-08-30 04:55:39 PDT
(In reply to comment #27)
> The lack of uptake of MNG on the web, and
> the hardships faced by non-IE browsers, should suggest which values are better
> for market share.

Don't use that argument. Neither here nor anywhere else. Neither against nor for
MNG, APNG or whatever. As long as there are no really suitable viewers (esp.
browsers) supporting an image format and distributed to a big enough mass of
people (see maket share), there won't be any "uptake" of it on the web. It
doesn't depend on it being MNG, APNG or anything else. Aditionally, there was a
big decrease in use of animated images for web design in the last years, as the
heavy use of such imagery almost always accounts for bad design.
So don't expect any new animated format to be used heavily on the web -
especially as long as the maket share majority of browser don't support it.
Because of that, your argument above is useless.

The real argument for including any such format in default builds can either be
"we want to help it to succeed" (but than state so) or - as said here by you in
comment #16 - "we want to use it internally". And I think the second is the real
argument here, so stick by it. Dissing other formats doesn't help but getting
people angry, esp. if the argument renders useless as this one (btw, if you's
replace "MNG" by "APNG" in that argument, it would even be more true - but
render equally useless by what I explained).

I guess it would be nice to have the code for supporting both formats in CVS,
perhaps have APNG in default builds but have the possibility for people doing
their MNG builds as well. Perhaps some code could even be combined in the long
run (I don't know either code nor specs well enough).
Comment 30 Christian :Biesinger (don't email me, ping me on IRC) 2004-08-30 08:26:47 PDT
(In reply to comment #16)
> Not yet, but soon.  APNG will be used in XUL apps where animated GIFs have been
> used poorly till now.

so you are against MNG because it is not compatible with other browsers and not
used on the web, but you see APNG's main usage in XUL apps?
Comment 31 Brendan Eich [:brendan] 2004-08-30 10:11:57 PDT
This has been said before in many places.  We want APNG for XUL apps.  We think
it may be useful for web content if implemented by other browser vendors --
that's the whatwg.org path.  MNG has failed so far to be implemented by other
browser vendors, and it's not in default Mozilla builds.  MNG is overkill, too
much code, for our "internal" (come on, people write XUL apps and extensions all
over the world) needs.

Robert Kaiser: Apple and especially Opera are never going to implement MNG, so
don't *you* use the bogus argument that all tiny-distribution formats are equal.
 With APNG, we have a shot via whatwg.org at getting multiple browser support. 
With MNG, there is no way.

People seem to play dumb about the above reasoning, but we've always made it
clear, ever since drivers@mozilla.org asked for an animated PNG profile of MNG
that could be used to replace the GIFs used in XUL.  VLC is not that profile;
there is no such profile, short of APNG becoming one -- but the guys doing APNG,
reasonably enough, would rather not change MIME type and suffix if they can do
what Netscape did in extending GIF.  The reason for that design decision is web
compatibility, via degradation to first frame instead of broken image icon.

It seems bogus to say that we must do only one of the two things discussed above
(innovate for XUL; innovate in ways that multiple minority-share browser vendors
can agree on for the web).  We can do both, we're definitely looking to use APNG
in XUL, and we are not going to close the door on the web right now, certainly
not just to preserve MNG's "territorial imperative".

/be
Comment 32 Mike Shaver (:shaver -- probably not reading bugmail closely) 2004-08-30 10:16:44 PDT
(In reply to comment #29)
> Don't use that argument. Neither here nor anywhere else. Neither against nor for
> MNG, APNG or whatever. As long as there are no really suitable viewers (esp.
> browsers) supporting an image format and distributed to a big enough mass of
> people (see maket share), there won't be any "uptake" of it on the web.

There are tens of millions of APNG-safe decoders deployed on the web, in the
form of PNG decoders; all APNG images are legal PNG images, by design.  An
author who wants to deploy an APNG animation has a simple task (aided by every
tool that currently works with PNG, which is to say pretty much all of them). 
They don't face the "broken image" barrier, as they would with JNG or MNG or
MJPEG.  They don't have to write complex and fragile <object> fallback code
(which is especially fun to make work with rollovers and list-style-image and
image maps and favicons) or do programmatic content negotiation.  That's a
primary design characteristic of APNG, as it was a primary design
_non_-characteristic with MNG.  They wanted a clean break, I suppose, which is
not necessarily bad engineering.  It just seems, from our experience (including
the experience with GIF and HTML and CSS) that it's not great "marketing".

MNG has other barriers as well, I think, such as the greater complexity of
working with the format.  As I understand it, the GIMP -- whose primary
maintainer is a great supporter of MNG, and initially disliked APNG because it
was a threat to MNG's adoption -- can produce MNG images, but can't load them to
edit, partly because it's difficult to extract individual frames for editing and
manipulate them appropriately to the format.

(In reply to comment #30)
> so you are against MNG because it is not compatible with other browsers and 
> not used on the web, but you see APNG's main usage in XUL apps?

That's a misrepresentation of the position, and I suspect you know it.  APNG's
_first_ major usage will likely be in XUL apps.  The backward-compat feature
means that it can have a relatively graceful move "from the nest", as content
authors become covetous of its features.

But this bug is not about MNG; I just wanted to explain that the compatibility
characteristics of APNG are at least as significant as its animation
characteristics, from the perspective of Mozilla interest in the format.  There
are a number of formats that could provide "better animation" in terms of
flexibility and opportunities for encoders to produce extremely efficient
representations of complex effects.  But there don't seem to be any that come
close to the compatibility profile of the proposed APNG, and that's why it's
still a problem in need of a solution.
Comment 33 Glenn Randers-Pehrson 2004-08-30 16:43:20 PDT
Re comment #28, the spec says "specifies" in Clause 1, Scope.  This International
Standard specifies a datastream and an asssociated file format, Portable
Network Graphics (PNG, pronounced "ping"), for a lossless, portable,
compressed individual computer graphics image transmitted across the Internet.
Comment 34 Doug Wright 2004-08-31 03:13:20 PDT
Comment #33, I posted a rebuttal in the spec bug (where further discussion on
that topic definataly belongs!)
Comment 35 Peter Weilbacher 2004-09-05 09:35:47 PDT
Has anyone actually tried the patch?

First, it is missing a definition of "gfxIFormats::ARGB". I invented one, added
it to gfxIFormats and then got a build. (I am unable to test it, although I
remember having seen an example image a few days ago, I cannot find that any
more. And as the spec target still changes it probably does not make much sense
trying it. I was just curious...)

Second, I did a measurement of the speed in which pages containing (normal) PNGs
are rendered, I find a slowdown of about a factor of 1.4 in the average (or ~1.6
in the median) pageload time:

Standard PNG decoder:
Avg. Median : 767 msec		Minimum     : 390 msec
Average     : 932 msec		Maximum     : 3780 msec

With aPNG patch:
Avg. Median : 1264 msec		Minimum     : 459 msec
Average     : 1286 msec		Maximum     : 3750 msec

As this test was done on OS/2, I am not sure that it is OK, because I didn't
actually check the patch... So in case somebody wants to try to reproduce this
result, I put up the test pages I used for the page-loader at
<http://weilbacher.org/Peter/png-testsurls.zip>.
Comment 36 Stuart Parmenter 2004-09-05 15:56:32 PDT
The patch had some other changes I was working on at the same time (namely the
ARGB stuff).  There are a fair number of optimizations left to be made in the
patch (not to mention that the patch is for APNG 0.1 and we're at 0.4 now). 
There will be a new patch in the next day or two.

Also, the name of the extensions is "APNG", not "aPNG"
Comment 37 Daniel Mota Leite 2004-09-05 21:25:27 PDT
just one thing:

how can i see that a png is a apng or a normal png?

if i receive a png image, will i have to open a apng enable viewer just to check if it have more things or not?

using the same signature for single image png and animated png is not bad, its evil... it makes me recall the RTF format extensions from MS...
Comment 38 Ryan Rubley 2004-09-05 21:30:41 PDT
I would assume you have to open it in a program that understands APNG or you
would only see the first frame. That is exactly how Animated GIF files work. 
Its no different then opening a EXIF jpeg in a program that only understands
JFIF jpegs and not being able to see the thumbnail image stored in the jpeg
either.  Even .bmp files support uncompressed/rle/png/jpg compression, and if
your .bmp viewer doesnt understand BMP v5 png/jpg compression you can't view the
.bmp that other newer viewers could.
Comment 39 Daniel Mota Leite 2004-09-06 02:56:48 PDT
i hate the gif /animated gif issue...

i cant map the gif mime to a light and very fast image viewer because 1% of the gif are animated and i cant know forehand (ans sometimes even after seing the gif) if its animated or not...

i call this breaking the mime definitions and turning it almost useless (isnt any different between image/* and image/(gif|png|jpg|...)  as we cant trust it to be correct)

for the jpeg, its only the thumbnail, if i'm seeing the full image, i probably will not care about the thumbnail
for the BMP, there as the gif exemple, its a mess, but at least a less important one: when you have the wrong format for your viewer it will not open and you see that you need to start the heavier but more complete image viewer instead of silently not displaying the full info
Comment 40 ArronM (:paper) 2005-01-17 18:11:36 PST
Stuart, wouldn't it be better to create a imgContainerFramed, and have
imgContainerGIF inherit from it?  I'm just thinking of all the wasted bytes for
non multiframed images (jpegs, bmps, etc)
Comment 41 Gábor Farkas 2005-03-21 10:44:28 PST
what about the conversion tools?

i mean, everyone is talking about VIEWERS..

for example:
i see a png file on a website... how nice..
i download it, but i decide to convert it to jpeg.
so i convert it with ..let's say imagemagick... to jpeg,
and save it somewhere on my disk. that's it.

but let's assume that in fact, that image was an apng!

by this point a completely lost the whole animation, because my conversion
program only converted the first frame. but i *had no idea* that the file was an
apng, because *noone told me that*.

but, if it would have had a different extension/mime-type/file-type/whatever,
i would have seen an undisplayable object on the webpage, and would install the
necessary extension to process it.
Comment 42 Josh Smith 2005-04-06 04:25:59 PDT
Isn't the conversion problem an issue with gif animation too, even for fairly
current software (Photoshop 6, Paint Shop Pro 7, even The GIMP if you're not
paying attention)?

The way I see it, this is a non-issue that boils down to tough luck on your
part, and in my opinion APNG is a great idea that can truly bring PNG to the
point of superiority to GIF in almost all respects.
Comment 43 Daniel Mota Leite 2005-04-06 15:51:09 PDT
i dont oppose the apng idea, i oppose the use of the same extension/mime/sig for
it...

.png is a static loseless image and should stay like this
apng should have a new extension (and mime-type) for it (say ang, for animated
network graphic)

trying to do the same to png as it was done to gif is repeat the same mistake
(just see how many programs really support animated gifs compared to the one
that work with normal gifs and you see this was far from perfect)
the problems in converting animated gifs today exist precisely because of this
mistake

not having a different file signature is even worse, its hijacking the png
format for mozilla needs at expense of everybody else needs...
Comment 44 Vladimir Vukicevic [:vlad] [:vladv] 2005-04-06 19:14:12 PDT
(In reply to comment #43)
> i dont oppose the apng idea, i oppose the use of the same extension/mime/sig for
> it...
> 

Please see png-list discussion from ~2-3 months ago.

This bug is rapidly turning into an advocacy spammage bug; will open a new
technical bug once pav has a new patch ready to go.
Comment 45 A 2005-09-18 21:58:52 PDT
Related: bug 48570
Comment 46 Andrew Smith 2006-09-09 23:10:47 PDT
Created attachment 237581 [details] [diff] [review]
apng patch for libpng 1.2.10
Comment 47 Andrew Smith 2006-09-09 23:19:17 PDT
Comment on attachment 237581 [details] [diff] [review]
apng patch for libpng 1.2.10

will ask glenn to look over the patch once stuart's had a pass at it.
Comment 48 Peter Weilbacher 2006-09-10 02:07:24 PDT
I don't want to start the discussion again but I thought that this idea had died two years ago when the PNG group through their votes showed that their spec was already clear enough to not allow inclusion of animations in PNG streams:
   http://article.gmane.org/gmane.comp.graphics.png.general/887
Were there newer votes on the matter or what else did I miss?
Comment 49 Mike Shaver (:shaver -- probably not reading bugmail closely) 2006-09-10 11:04:35 PDT
(In reply to comment #48)
> I don't want to start the discussion again

Then please don't, or not in bugzilla.  png-list or mozilla-general or whatever you feel is more appropriate.
Comment 50 Vladimir Vukicevic [:vlad] [:vladv] 2006-09-10 11:17:16 PDT
(In reply to comment #48)
> I don't want to start the discussion again but I thought that this idea had
> died two years ago when the PNG group through their votes showed that their
> spec was already clear enough to not allow inclusion of animations in PNG
> streams:
>    http://article.gmane.org/gmane.comp.graphics.png.general/887
> Were there newer votes on the matter or what else did I miss?

The two votes that were taken, one to explicitly "Forbid Animations" and one to explicitly "Allow Animations" both failed, and each by a large margin (10-2 and 9-2 voting no -- search for "20040902 FAIL" on gmane).  The only reason why APNG died out is that stuart and I became very busy working on other things, and didn't have the time necessary to devote to doing a proper integration of APNG as an optional patch to libpng (instead of hacking it in to the mozilla decoder).

Side note to anyone watching this bug: Please PLEASE keep any discussion that is not directly related to the patch or to integration with mozilla to the png list, bug 257263, or mozillazine, and not here.
Comment 51 Stuart Parmenter 2006-09-13 10:40:59 PDT
Comment on attachment 237581 [details] [diff] [review]
apng patch for libpng 1.2.10

this looks right to me.  i've looked over it a few times before this patch and made comments so i think we're good to go, but I'd like glenn to look over it.
Comment 52 Glenn Randers-Pehrson 2006-09-13 11:51:37 PDT
It's a nice job. The patch applied cleanly to my copy of libpng-1.2.10, and compilation was OK, on a FreeBSD platform.  I haven't tried to link the
resulting library into any application yet, but will as time permits, and
haven't tested on any other platform yet.

To be compliant with the PNG spec, the chunk names should
have a lower-case second letter.  Uppercase is reserved for registered
chunk names.

As discussed previously on the png-mng-implement list, the inner CRC values
are not of much use since the outer ones already guarantee the file isn't
currupted.  Some resources could be saved by ignoring them in decoders.
Comment 53 Glenn Randers-Pehrson 2006-09-13 12:05:20 PDT
Re my comment #52
s/should/must/
Comment 54 Andrew Smith 2006-09-13 13:48:04 PDT
yes, section 5.4 of the png standard requires that any chunk names that are maintained by the registration authority have a lowercase second letter.

this won't cause a problem, from the document: 'Decoders do not need to test the private-chunk property bit, since it has no functional significance; it is simply an administrative convenience to ensure that public and private chunk names will not conflict.'
Comment 55 Glenn Randers-Pehrson 2006-09-13 14:01:02 PDT
That are *not* maintained by the registration authority must have a lowercase
letter.

By "this won't cause a problem" I assume you mean changing the chunk names
to have lowercase second letters won't cause a problem.
Comment 56 Andrew Smith 2006-09-13 15:56:07 PDT
yes, that's what i meant
Comment 57 Glenn Randers-Pehrson 2006-09-14 07:45:21 PDT
It is fairly trivial to apply the patch to libpng-1.2.12.  A pngpread.c hunk has to be applied manually, without change, and in png.h the mode flag 0x2000 is taken so the two new mode flags should be 0x4000 and 0x8000L.

I believe the spec can be simply redesigned to use all top-level chunks instead of the embedded chunks.  I'll comment in the aPNG spec design bug about that.
Comment 58 Andrew Smith 2006-09-18 21:34:54 PDT
Created attachment 239112 [details] [diff] [review]
apng patch for libpng 1.2.12
Comment 59 Glenn Randers-Pehrson 2006-09-18 22:09:04 PDT
#define PNG_HAVE_fCTL             0x8000
should be
#define PNG_HAVE_fCTL             0x8000L

	
Comment 60 Glenn Randers-Pehrson 2006-09-18 22:12:35 PDT
You haven't changed the chunk names to have lower-case second letters.

	
Comment 61 Andrew Smith 2006-09-19 08:24:39 PDT
Created attachment 239179 [details] [diff] [review]
apng patch for libpng 1.2.12 with changes as per comments #59, #60 from glenn
Comment 62 Andrew Smith 2006-10-24 01:41:10 PDT
Created attachment 243315 [details] [diff] [review]
apng mozilla patch, just for feedback

attached what i have so far of my work on integrating the apng-patched libpng into mozilla. this patch does work (only tested on one machine though).

things to note:

- everything from imgContainerGIF has been copied to imgContainer. once i'm done there will be no imgContainerGIF

- i am next to completely clueless about render flags, i only understand dispose_none and dispose_background, thus those are the only two i tested

- the same enum with the render flags is defined in 4 places. i'm planning to put it into imgContainer.h (fully global)

- there are a couple of minor updates to the libpng patch, the one worth noting is i moved the frame callback from the end of the frame to right after each embedded fctl, so before any pixel data for the frame

- i'm using bits from a pruint8 as flags, am not completely sure this is ok (portability problems maybe?)

if you want to give it a try, go to http://littlesvr.ca/apng/ for sample images
Comment 63 Christian :Biesinger (don't email me, ping me on IRC) 2006-10-24 10:15:08 PDT
> - everything from imgContainerGIF has been copied to imgContainer. once i'm
> done there will be no imgContainerGIF

heh. so this undoes bug 77497 :-)
Comment 64 ArronM (:paper) 2006-10-24 21:46:39 PDT
I feel myself being erased :)
Comment 65 Andrew Smith 2006-11-07 22:59:08 PST
Created attachment 244979 [details] [diff] [review]
almost complete patch, for comments/tracking only

this patch is an almost full and working implementation of apng (all except for the blend flag). i'll post it here to make sure i don't mess things up when trying to change how compositing works.
- all references to imgContainerGIF removed, using imgContainer instead
- all render flags implemented except for the blend flag
- the enums i mentioned have been removed, added a bunch of consts to imgIContainer to replace them
- noone seems to have a problem with the PRUint8 so i'll stop mentioning it
Comment 66 Andrew Smith 2006-11-14 11:52:16 PST
Created attachment 245594 [details] [diff] [review]
apng patch for mozilla, ready for review

patch works on my default builds of minefield on linux and windows. full alpha compositing only works on a mac if compiled with --enable-default-toolkit=cairo-cocoa

the patch could use more testing to bring up strange cases when it might not work, especially to make sure nothing that used to work is now broken.

imgContainerGIF.cpp/.h is no longer used (at all as far as i can tell).
Comment 67 Andrew Smith 2006-12-06 17:39:20 PST
Created attachment 247757 [details] [diff] [review]
updated patch

updated the code style but more importantly removed two checks from imgContainer.cpp so now they look like:

  // this check was here when this code was in imgContainerGif
  // i think it's just a superfluous check but i'm not brave enough to
  // delete it completely since i haven't traced the entire execution
  //~ gfx_format format;
  //~ aCompositingFrame->GetFormat(&format);
  //~ if (format != gfxIFormats::RGB_A1 && format != gfxIFormats::BGR_A1) {
    //~ NS_NOTREACHED("GIFs only support 1 bit alpha");
    //~ aCompositingFrame->UnlockAlphaData();
    //~ aOverlayFrame->UnlockAlphaData();
    //~ return;
  //~ }

with these commented out full alpha compositing works on a default mac build
Comment 68 Stuart Parmenter 2006-12-13 12:58:36 PST
Comment on attachment 247757 [details] [diff] [review]
updated patch

i think this is good to go
Comment 69 Glenn Randers-Pehrson 2006-12-13 17:14:22 PST
There are a couple of suggestions about the spec over in the APNG spec bug, that have gone unanswered.

First is to add a chunk sequence number by which it can be detected whether the chunk order has been disturbed.  This is relatively easy to change in the proposed code.  It has the benefit that the chunk names can be changed to "copy-safe" names (4th symbol lower-case), so that applications that don't recognize them are not forced to drop them.

Second is somewhat more complicated to implement in the proposed code.  It does away with the chunk heirarchy, which makes it simpler for existing applications to handle apng files, and eliminates the size limitation that the current proposal imposes (each frame must go entirely into one chunk).
Comment 70 Vladimir Vukicevic [:vlad] [:vladv] 2007-01-10 10:14:48 PST
I don't have any problems with the chunk sequence number; I think an earlier version of our spec had sequence numbers embedded, and I think we took them out based on a request from the png list.

For the chunk hierarchy though, I'm not sure -- the problem is that, again, our original spec had things expanded out and all as toplevel chunks, but there were a number of issues raised on the list (I don't know if they were theoretical or not) about how putting those extra chunks would break some smaller custom decoders or something.  Are you sure that pulling them up won't cause more breakage than the hierarchical approach (even despite the frame size limitation)?
Comment 71 Stuart Parmenter 2007-01-10 15:35:41 PST
(In reply to comment #69)
> There are a couple of suggestions about the spec over in the APNG spec bug,
> that have gone unanswered.
> 
> First is to add a chunk sequence number by which it can be detected whether the
> chunk order has been disturbed.  This is relatively easy to change in the
> proposed code.  It has the benefit that the chunk names can be changed to
> "copy-safe" names (4th symbol lower-case), so that applications that don't
> recognize them are not forced to drop them.
> 

Are there any apps that actually copy chunks blindly?  If not is there any good reason to do this?

> Second is somewhat more complicated to implement in the proposed code.  It does
> away with the chunk heirarchy, which makes it simpler for existing applications
> to handle apng files, and eliminates the size limitation that the current
> proposal imposes (each frame must go entirely into one chunk).
> 

We removed this from the original spec because of our discussions on the png list.  I don't really see any good reason to revisit this at this point.
Comment 72 tor 2007-01-11 15:54:39 PST
This patch increases the size of imgContainer considerably.  Couldn't we move the new data fields needed into a structure which we only allocate when the container is being used to store an animation, to avoid bloating in the common case?

Anyone trying to use --enable-system-png is going to get a bit of a shock.  :)
Comment 73 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-01-11 18:32:00 PST
(In reply to comment #72)
> Anyone trying to use --enable-system-png is going to get a bit of a shock.  :)

Linux distros use this.  If we break it, we should remove it so builds don't break.
Comment 74 Christopher Aillon (sabbatical, not receiving bugmail) 2007-01-12 08:25:03 PST
Um, cough.  Don't break --enable-system-png.
Comment 75 Benjamin Smedberg [:bsmedberg] 2007-01-12 08:36:24 PST
We're not going to stop producing browser features just to support system-png. If the system png cntains the APNG patches then I guess that's not a problem.
Comment 76 Andrew Smith 2007-01-12 08:37:09 PST
nothing is going to break. the problem tor is referring to is that the imgContainer has many more variables in it now. these are only used for animated images. i'm looking at ways to avoid it, possibly by allocating them only when needed.
Comment 77 tor 2007-01-12 08:46:16 PST
I was referring to two different problems.  For the libpng one, have you talked to the libpng authors to see if they are open to adding APNG functionality and if so if your code passes their code review process?
Comment 78 Andrew Smith 2007-01-12 08:47:17 PST
oh sorry, i see what you're talking about. will do some research.
Comment 79 Glenn Randers-Pehrson 2007-01-12 18:29:21 PST
(In reply to comment #71)

> Are there any apps that actually copy chunks blindly? 

Yes.  That's what the copy-safe/copy-unsafe mechanism is for.

> > Second is somewhat more complicated to implement in the proposed code.  It does
> > away with the chunk heirarchy, which makes it simpler for existing applications
> > to handle apng files, and eliminates the size limitation that the current
> > proposal imposes (each frame must go entirely into one chunk).
> > 
> 
> We removed this from the original spec because of our discussions on the png
> list.  I don't really see any good reason to revisit this at this point.

The "compelling arguments" in png-list for chunk heirarchy included the notion of a single superchunk, to deal with the possibility of chunks getting out of
order.  In the 0.4 proposal there is one superchunk per frame, so that
possibility still exists and decoders have to deal with it anyhow.

Comment 80 Andrew Smith 2007-01-29 15:27:32 PST
Created attachment 253238 [details] [diff] [review]
updated patch

All the members only used by animated images have been moved into a structure that's not allocated until the second frame is added. Except mAnimationMode, mLoopCount and mObserver which can't be removed without major architecture changes. On the bright side containers for gif images now use a lot less memory then they did before so overall more memory is saved than waisted.

Added a few #ifdefs so it compiles and works as expected --with-system-png

All major changes are in imgContainer, everything else is the same except for the #ifdefs
Comment 81 Andrew Smith 2007-02-08 13:15:00 PST
Created attachment 254449 [details] [diff] [review]
patch updated for APNG 0.6

The specification and sample images have been updated for the 0.6 spec. The change from the previous version is that the chunk names have been made copy safe (so the cunk names changed and the acTl chunk now contains copies of the CRC from IHRD and possibly PLTE).

The attached patch works with APNG 0.6
Comment 82 Glenn Randers-Pehrson 2007-02-26 05:41:17 PST
Re comment#81:
In the current patch, "feND" should be "feNd" everywhere, and "68" should
be "100" in the definition of png_feNd.  This will be moot, though, if the
feNd chunk is eliminated from the next version of the spec.
Comment 83 Andrew Smith 2007-03-06 02:08:42 PST
Created attachment 257488 [details] [diff] [review]
patch for APNG 0.7

The specification, sample images and the patch have been updated to get rid of the chunk hierarchy and reintroduce width/height per frame.

Tested on Linux and Windows, will test on a Mac tomorrow but don't expect any problems.
Comment 84 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-03-06 08:54:01 PST
Per comment 72 through comment 75, you should patch configure.in (which requires regenerating configure using autoconf, but you might not want to include the configure diff in the patch) so that, when --enable-system-png is used, you test for the presence of the functions you are adding to libpng.  (Look at some other uses of AC_CHECK_FUNCS or AC_TRY_COMPILE (probably the former) in configure.in for how to do this, but note that you'll need to push/pop the libpng compilation options around the call.)

If the test fails, different people will disagree on what to do about it -- you should either give an error and exit, or print a warning and disable system libpng.  I prefer giving an error and exiting, but others might prefer the warning.
Comment 85 Andrew Smith 2007-03-08 15:32:29 PST
Created attachment 257877 [details] [diff] [review]
same, but must have patched system-png

The new patch adds a couple of lines to configure.in to make sure that the system-png (if requested) has APNG support. If it doesn't the request is ignored and the embedded library is used instead.

Unfortunately this was necessary to avoid an unexplicable-to-most-users inconsistency among platforms.
Comment 86 Andrew Smith 2007-03-08 22:48:24 PST
Created attachment 257929 [details] [diff] [review]
same, but must have patched system-png

Oops, i used the wrong variable in configure.in so it wouldn't link. It does now. Tested both with --with-system-png and without.
Comment 87 Christian :Biesinger (don't email me, ping me on IRC) 2007-03-09 08:22:16 PST
> If it doesn't the request is
> ignored and the embedded library is used instead.

IMO it would be better to show an error and abort configure (AC_MSG_ERROR). silently ignoring configuration options isn't nice.
Comment 88 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-03-09 09:58:16 PST
(In reply to comment #87)
> IMO it would be better to show an error and abort configure (AC_MSG_ERROR).
> silently ignoring configuration options isn't nice.

I agree, but it's probably better to make that a separate bug (just like I did for bug 372878 when discussing this with Andrew), since it's a common pattern over a bunch of tests.  (That said, in general, configure silently ignores unknown options too...)
Comment 89 Christian :Biesinger (don't email me, ping me on IRC) 2007-03-15 15:32:43 PDT
Comment on attachment 257929 [details] [diff] [review]
same, but must have patched system-png

in row_callback:
+  PRUint32 bpr, abpr;
+  decoder->mFrame->GetImageBytesPerRow(&bpr);
+  decoder->mFrame->GetAlphaBytesPerRow(&abpr);

these two variables don't seem to be used

+void nsPNGDecoder::create_frame(png_uint_32 x_offset, png_uint_32 y_offset, 

I think it would be nice not to mix nsPNGDecoder methods with the callbacks. I.e., I'd put the methods before all the callbacks in this file.

also: this function should be called CreateFrame, as mentioned on IRC; similar for nsPNGDecoder::set_anim_frame_info.

also, I'd move the variables in this function to the place where they are first used

+    PRInt32 num_iterations;
+    
+    num_iterations = png_get_num_iterations(png_ptr, info_ptr);

I'd merge these lines

modules/libpr0n/src/Makefile.in
+# nsGIFDecoder2.cpp requires access to this one:
+EXPORTS		= imgContainer.h

then it should use LOCAL_INCLUDES. this doesn't seem like a public file.

Please don't add MOZ_CAIRO_GFX ifdefs to imgContainer.cpp; just always use the cairo code.

imgContainer.h:
+      if (mFrames.Count())
+        return mFrames.ObjectAt(0);
+      else
+        return nsnull;

Couldn't that be:
  return mFrames.SafeObjectAt(0);

imgContainer.cpp:
-NS_IMETHODIMP imgContainerGIF::Clear()
+NS_IMETHODIMP imgContainer::Clear()
 {
-  mFrames.Clear();
-  return NS_OK;
+  return NS_ERROR_NOT_IMPLEMENTED;

why this change compared to the GIF version?

+  NS_ASSERTION(mAnim, "imgContainer::Notify() called but mAnim is null");
+  if (!mAnim)
+    return NS_ERROR_UNEXPECTED;

I wouldn't bother with the if
Comment 90 Steffen Wilberg 2007-03-16 07:47:46 PDT
--> Andrew, who's done all but the first patch here.
Comment 91 Andrew Smith 2007-03-18 19:55:56 PDT
Created attachment 258969 [details] [diff] [review]
apng patch

New patch with changes addressing concerns from comment #89 and some others brought up on IRC.

Also:
- chunks between APNG chunks are now ignored
- removed the hidden flag, the first frame is hidden if the fcTl is missing

These require spec changes, I will update it as soon as I get a chance. Sample images are up to date.
Comment 92 Glenn Randers-Pehrson 2007-03-19 05:58:20 PDT
The PNG group's fork of the proposal has been updated accordingly.

ftp://ftp.simplesystems.org/pub/png-group/documents/png-apng-proposal-20070315.txt

Besides the spec changes Andrew mentioned there is this:

APNG_RENDER_OP_BLEND_FLAG must be 0 for frame 0.

Also it introduces terminology definitions "canvas" and "output buffer" and
uses "output buffer" where the previous proposal said "canvas".  This isn't
a spec change but seems clearer.
Comment 93 Stuart Parmenter 2007-03-19 17:21:38 PDT
+  if (render_op & 0x04)
+      mFrame->SetFrameDisposalMethod(imgIContainer::kDisposeRestorePrevious);
+  else if (render_op & 0x02)
+      mFrame->SetFrameDisposalMethod(imgIContainer::kDisposeClear);
+  else
+      mFrame->SetFrameDisposalMethod(imgIContainer::kDisposeKeep);

these should probably use PNG_RENDER_FLAG*
Comment 94 Stuart Parmenter 2007-03-19 17:26:46 PDT
Comment on attachment 258969 [details] [diff] [review]
apng patch

I think this is probably good to go (with the previous comment fixed).  We can do followup patches as smaller more manageable fixes.


My only other real comment would be if we want to make the APNG chunks public now since changing them later will be harder.
Comment 95 Glenn Randers-Pehrson 2007-03-19 18:21:55 PDT
(In reply to comment #93)
> these should probably use PNG_RENDER_FLAG*

That looks like a constant.  render_op is a variable conveyed within the
APNG datastream.

Comment 96 Glenn Randers-Pehrson 2007-03-19 18:24:39 PDT
(In reply to comment #93)

> That looks like a constant.  render_op is a variable conveyed within the
> APNG datastream.
>

D'oh!  Never mind, you are right.  The 0x0N should be PNG_RENDER_FLAG*

Comment 97 Andrew Smith 2007-03-19 20:06:09 PDT
Created attachment 259059 [details] [diff] [review]
[checked in] apng patch

hex flags replaced with #defines
Comment 98 Chris Nokleberg 2007-03-19 21:56:55 PDT
Andrew, for when you incorporate the patch changes into the spec, I have some comments:
http://article.gmane.org/gmane.comp.graphics.png.general/1239

Also, as I mention in the message it doesn't appear that the patch does anything with the blend flag, is that correct?
Comment 99 Stuart Parmenter 2007-03-20 01:01:38 PDT
It looks like his patch uses the blend flag properly.
+  if (render_op & PNG_RENDER_OP_DISPOSE_PREVIOUS)
+      mFrame->SetFrameDisposalMethod(imgIContainer::kDisposeRestorePrevious);
+  else if (render_op & PNG_RENDER_OP_DISPOSE_BACKGROUND)
+      mFrame->SetFrameDisposalMethod(imgIContainer::kDisposeClear);
+  else
+      mFrame->SetFrameDisposalMethod(imgIContainer::kDisposeKeep);

Or am I missing something?
Comment 100 Chris Nokleberg 2007-03-20 01:16:31 PDT
(In reply to comment #99)
> It looks like his patch uses the blend flag properly.

That's not the blend flag, that's the dispose op. The blend flag is the 4th bit in the render op, look for APNG_RENDER_OP_BLEND_FLAG. It looks like some error conditions are checked for but it is never actually used for anything. I'm not sure whether the current impl always blends or never blends.

Part of the problem is that none of the sample images test it. I think a few more of those are necessary to work out the kinks. Ideally the patch here and my Java impl would have the same behavior in all regards before this gets baked in. If there is an easy way to get a build with this patch a pointer would be great, otherwise I'll put up a comparison page for which someone else's eyeballs will be required.
Comment 101 Stuart Parmenter 2007-03-20 01:34:37 PDT
Ah, you're right.  It shouldn't be hard to fix.  As for more images, yeah we should certainly create a bunch more to test things.  I can probably get you a build tomorrow sometime.
Comment 102 Stuart Parmenter 2007-03-20 16:58:29 PDT
ok, i've landed the current patch.  We should work to get more test cases up and file some additional bugs on things like the blend flag being ignored.  We should continue for the next week or two to address spec concerns and try to make minimal changes.  We'll make the PNG chunks public after once the PNG group says its ok.
Comment 103 Otsu Kimi 2007-03-20 21:22:44 PDT
Where is "the PNG group should allow APNG to use public chunks" bug button?
Comment 104 Stuart Parmenter 2007-03-21 00:56:28 PDT
I suggest we keep this bug open and put patches here until we hit APNG 1.0...
Comment 105 Stuart Parmenter 2007-03-21 01:00:11 PDT
Created attachment 259180 [details] [diff] [review]
[checked in] remove extra CRCs and make chunks not-safe-to-copy

I think this patch is right (haven't tested it yet)... mostly s/fooo/fooO/g but i updated the bytes and removed the crc setters/getters and updated the bytes written/read.
Comment 106 Andrew Smith 2007-03-21 07:32:06 PDT
Comment on attachment 259180 [details] [diff] [review]
[checked in] remove extra CRCs and make chunks not-safe-to-copy

In png_handle_acTL() the declaration 'png_byte data[16];' and the check 'else if (length != 16)'have to be changed to 8.

Otherwise it's good. I tested it but can't update the samples on the website or the spec until tomorrow.
Comment 107 Stuart Parmenter 2007-03-21 13:23:37 PDT
Created attachment 259246 [details] [diff] [review]
split out dispose_op and blend_op from render_op

this splits out dispose_op and blend_op from render_op
Comment 108 Vladimir Vukicevic [:vlad] [:vladv] 2007-03-21 14:33:31 PDT
Comment on attachment 259246 [details] [diff] [review]
split out dispose_op and blend_op from render_op

This looks ok to me, but it looks like the nsPNGDecoder bits don't handle the blend op at all -- so all APNGs will render as if with OP_SOURCE, which will make it impossible to layer stuff on top of the canvas.

Would definitly like andrew to confirm this though.
Comment 109 Andrew Smith 2007-03-22 13:38:50 PDT
Comment on attachment 259246 [details] [diff] [review]
split out dispose_op and blend_op from render_op

25 has to be replaced with 26 in two places:

pngrutil.c:
else if (length != 25)

pngwutil.c:
png_write_chunk(png_ptr, (png_bytep)png_fcTL, data, (png_size_t)25);

pngget.c this should be dispose_op:
png_debug(1, "in png_get_next_frame_render_op()\n");

otherwise it's good. applies smoothly over the remove crc patch.

by the way the sample images have been updated.
Comment 110 Stuart Parmenter 2007-03-22 16:36:42 PDT
Created attachment 259371 [details] [diff] [review]
[checked in] split out dispose_op and blend_op from render_op

attaching the patch I checked in
Comment 111 N.Katô 2007-03-23 11:25:16 PDT
Created attachment 259427 [details]
iteration sample image

It looks that looping is done (acTL's num_iterations + 1) times.

The attached APNG has 4 frames and 1 for num_iterations in acTL, but
the sequence is played twice.
Comment 112 Glenn Randers-Pehrson 2007-03-23 18:47:09 PDT
Please add a line to mozilla/modules/libimg/png/MOZCHANGES to indicate that APNG support was added on 20 March 2007.
Comment 113 Ria Klaassen (not reading all bugmail) 2007-03-24 04:28:17 PDT
*** Bug 375174 has been marked as a duplicate of this bug. ***
Comment 114 neil@parkwaycc.co.uk 2007-03-25 02:34:08 PDT
(In reply to comment #42)
>Isn't the conversion problem an issue with gif animation too, even for fairly
>current software (Photoshop 6, Paint Shop Pro 7, even The GIMP if you're not
>paying attention)?
Last time I looked, Paint Shop Pro came with a second app called Animation Shop which opens GIF and (apparently, since I have no reference MNG viewer) MNG.
Comment 115 Gerard Juyn 2007-03-28 05:51:44 PDT
Nope. AS is a behemoth. It will only open it's own MNG-files. It does not support all of the spec. But it does offer a starting point to create MNG files. My GIF2MNG converter also does MNG->MNG optimization. For those needing to create highly optimized MNG's, there is also a Windows DLL that contains the necessary code and takes a series of frames + delays from the parent app.
Comment 116 Glenn Randers-Pehrson 2007-03-29 04:46:31 PDT
  AC_CHECK_LIB(png, png_get_acTl, ,
should be
  AC_CHECK_LIB(png, png_get_acTL, ,
in configure.in
Comment 117 Andrew Smith 2007-03-29 12:21:26 PDT
Created attachment 260039 [details] [diff] [review]
various small fixes

- fixed num_iterations bug (see comment #111)
- added a line to png/MOZCHANGES (see comment #112)
- fixed the check in configure.in (see comment #116)
- renamed num_iterations to num_frames, this does not change the spec - just clarifies what was meant.
Comment 118 Glenn Randers-Pehrson 2007-03-29 15:08:08 PDT
That's "num_plays" not "num_frames". [checks patch] It's correct in the patch, so the typo is only in the comment.  [checks wiki] The wiki is OK too.
Comment 119 N.Katô 2007-03-31 10:22:12 PDT
+++ modules/libpr0n/decoders/png/nsPNGDecoder.cpp
+    if (num_plays <= 0) /* forever */
+      num_plays = -1;
+    decoder->mImage->SetLoopCount(num_plays - 1);

When num_plays <= 0 at the beginning, -2 is passed to SetLoopCount.  Is it
safe?
Comment 120 Andrew Smith 2007-03-31 17:30:00 PDT
Yes that's a problem. I'll fix it. Since there is also a problem with the interpretation of num_frames I'll do that and post a new cumulative patch. Will take a while by the looks of it.
Comment 121 Andrew Smith 2007-04-14 21:10:47 PDT
Created attachment 261579 [details] [diff] [review]
[checked in] various small fixes

Same as comment #117 plus:
- updated for the new libpng in the tree (1.2.16)
- fixed bug (see comment #119)
- num_frames no longer includes a hidden first frame

The stickman demo has been updated to work with the new hidden frame count. Tested on Linux and Windows.
Comment 122 Steve England [:stevee] 2007-04-19 15:12:55 PDT
Could this bug have caused bug 376446? (Note: the 'various small fixes' from comment 121 haven't changed bug 376446)
Comment 123 Alfred Kayser 2007-06-28 02:52:20 PDT
This one can be closed, no?
Comment 124 sv 2008-07-04 10:53:09 PDT
Put me down as extremely disappointed this was added to Firefox 3.0.

No offense, but what were you thinking? The standard was outright rejected by the W3C for extremely valid reasons. I don't agree with the arguments that an animation format requires sprites/etc, but I *do* agree with the fact this specification hijacks the PNG fileformat and encodes image data into meta data.

Just because the GIF file format had animation doesn't mean it's logical to add animation to any other image format. 

Many developers supported Firefox because of it's strong principles about adhering to W3C standards and not implementing custom, proprietary additions to existing standards. I'm sure everyone here remembers what happened when Netscape and Microsoft did this? We're *still* cleaning up the mess it created.

It's downright disgusting that you bombed the PNG specification this way and maliciously "embraced and extended" an existing international standard in a way that breaks it's fundamental design philosphy.

I'm a *strong* supporter of an animated-PNG-like specification, but I have serious objections to the approach that was used. Shame on all of you, you're doing the exact same thing Microsoft does/did.
Comment 125 Brendan Eich [:brendan] 2008-07-04 22:52:25 PDT
"shadowchaser": see comment 50, and read bugzilla etiquette (or re-read it):

https://bugzilla.mozilla.org/page.cgi?id=etiquette.html

Also, here's a clue: we do not blindly adhere to all W3C standards, never mind that APNG was not outright rejected by the relevant governing body, the png-list, per comment 50. Did you miss the XML utopia that cost way too much time in the W3C, and lost opportunity for the open web, starting in 1998? See

http://weblogs.mozillazine.org/roadmap/archives/005632.html

for a start.

Standards bodies including W3C are consortia whose pay-to-play members and paid staff have often starkly conflicting values and visions. The result is that standards are like sausage, including W3C ones. Arguments from W3C authority do not trump all other arguments in Mozilla discourse.

/be
Comment 126 sv 2008-07-04 23:23:54 PDT
You're right. W3C has been misguided for a long time - it was very evident upon the insane release of XHTML 1.1 and their pointless "componentization" efforts.

However, what concerns me is the fact that the Mozilla team effectively "bombed" a stable and widely used Internet file format. Yes, APNG is backwards compatible, but you still added proprietary extensions to a stable standard. Granted, I've been one of the developers begging for animated PNG support for a long time - I just question if this is the right way to handle it.

As misguided as the W3C is these days, their points about PNG being designed primarily as a non-animation (image) file format was perfectly valid and sensible. 

If you're going to fork from W3C, then fork - don't play the middle ground trying to retrofit (finally!) well supported file formats with proprietary technologies. 

My point stands. It's exactly what Microsoft used to do, and it's a shame that the Mozilla team has decided to pick up this horrible legacy web developers thought was long gone. 
Comment 127 Justin Dolske [:Dolske] 2008-07-04 23:35:27 PDT
None of this is relevant. The W3C has nothing to do with PNG or APNG, and Bugzilla is not a place for advocacy. I refer you, again, to the Bugzilla etiquette link Brendan provided in his last comment.

Shoo, troll, shoo.
Comment 128 Grey Hodge (jX) 2008-07-04 23:47:00 PDT
There are many standards bodies besides the W3C. Javascript is approved by ECMA as ECMA script, but I don't hear you whining that we're extending JS ahead of ECMA's proposals.

APNG is not proprietary, it's as open as it gets, with a public spec, and open source implementations. APNG finally finishes what PNG SHOULD have been all along anyway, a _full_ replacement for GIF with a few extra features like 24bit color and alpha transparency. If you don' tlike APNG, don't use it, and don't support it in your apps. APNGs are backwards compatible, as you even mention.

PNG is still perfectly valid and sensible, APNG extends it, it doesn't change what's already established. And unlike MS, we're not extending in proprietary ways.

As dolske said, go troll under another bridge. APNG is a settled matter that other vendors are embracing as well, like Opera. It's here, and the greater support peopel give it the faster it'll ebcome a standard like GIF was, like the original HTM: versions, etc., de facto.
Comment 129 Daniel Mota Leite 2008-07-06 18:33:28 PDT
this isnt trolling, its a valid problem that mozilla is trying to ignore (again).

>PNG is still perfectly valid and sensible, APNG extends it, it doesn't change
>what's already established. And unlike MS, we're not extending in proprietary
>ways.

false! apng is neutral only with apng enabled apps, to all others its a bad thing.

if i receive a .png file on a non-apng app, how can i know that i'm missing something? the aminated gif was a bad hack, apng is also a bad hack, it hijacks the png format. apng must differentiate it self from png so people know when all info is in there.

in a apng, take a first frame full white and then scroll something in the other frames. a non-apng app will only see the white picture, the user will never know what it should see more. now instead of a white picture (as the user might suspect that something is missing), put something and the user will not even try to load the (a)png file in any other program.

saying that the picture designer should make sure that the first frame have all the needed info is just like giving a load gun to a child... it will for sure (back)fire sooner or later.

if apng is to be used only in mozilla apps, then again no need to hijack the png, just turn apng a NEW format.

If mozilla is pushing apng as a hijack from png, its just as bad as MS. It doesnt matter being open, if no one listens and even goes against what the png group voted for.

APNG is needed and VERY welcome, but ONLY if it DOESNT hijack PNG. As Mozilla is the primary supporter to apng, it should fix this.
Comment 130 Byron Jones ‹:glob› 2008-07-06 18:43:32 PDT
This bug is about adding APNG support to Mozilla.  This has happened, and comments ON THIS BUG will only irritate.  Mozilla isn't ignoring you, however this isn't the right forum to raise your points.

Please keep any discussion on the PNG list, bug 257263, or MozillaZine, and not here.
Comment 131 Vladimir Vukicevic [:vlad] [:vladv] 2008-07-06 18:44:15 PDT
(In reply to comment #129)
> this isnt trolling, its a valid problem that mozilla is trying to ignore
> (again).

Bugzilla is /not/ a place for random advocacy.  If you have issues, feel free to take them to the newsgroups or to mozillazine.
Comment 132 Grey Hodge (jX) 2008-07-06 18:45:16 PDT
If the PNG group voted that PNG should kill puppies, would that be ok? What the PNG group says isn't always right. Also, APNG very nearly passed their vote, so there's at least SOME significant support inside their ranks. We're not hijacking anything. APNG gracefully degrades in non-APNG compatible apps, so there's no need to worry about "lost information".

We're not responsible for what bad designers do any more than we're responsible for people who couldn't code a valid HTML page to save their life. If you're so against bad design, go take down MySpace, then we'll talk about making sure APNG is completely nerfed.
Comment 133 Daniel Mota Leite 2008-07-06 21:02:23 PDT
>This has happened, and comments ON THIS BUG will only irritate.

Just like ignoring all the complains and hijacking png?!

the old same issues are raised again and again, the fall png back could be easily resolved with a javascript browser detection and so resolve the main problem: png and apng ARE different things.
all other apps either have or not have apng, and people would know instantly about it, no need for fallback.


the discussion is here because mozilla added the apng even with alot of complains against it, trying to move it to a newsgroup or mozillazine is just another way of ignoring it, just like it ignored the past complains. Its frustrating to see the animated gif problem reborn all over again and people still thinking its a good idea!!
i will probably open a new bug about this...

>If the PNG group voted that PNG should kill puppies, would that be ok?

Is PNG group responsible with defining how to kill puppies? no, its responsible with the PNG format. The PNG group, not Mozilla! If mozilla isnt happy with PNG, create a NEW format, leave the old one alone.

the majority of the png group voted no, so until the png group votes yes, mozilla is hijacking the png

> APNG gracefully degrades in non-APNG compatible apps, so there's no need to worry about "lost information".

again, how can a non-apng app know that isnt displaying all the info? if it doesnt know, we have lost information (or else the extra frames would not be needed).

>We're not responsible for what bad designers

you are right, just like MS isnt responsible for all broken web pages and javascript in the web. No only didnt promoted a cleaner web, but also built a severe broken browser. APNG is broken, bad designers will make it show up, just like they did with animated gifs.

Just check all the apng samples with a non-apng viewer/app/browser and will see already that most of then are useless with just the first frame and people will not know they are missing something else.
Comment 134 Vladimir Vukicevic [:vlad] [:vladv] 2008-07-06 21:56:34 PDT
As I said:

(In reply to comment #131)
> Bugzilla is /not/ a place for random advocacy.  If you have issues, feel free
> to take them to the newsgroups or to mozillazine.

Please stop spamming this bug.  Last warning.
Comment 135 Robert Accettura [:raccettura] 2008-07-07 05:56:31 PDT
(In reply to comment #129)

> if i receive a .png file on a non-apng app, how can i know that i'm missing
> something? the aminated gif was a bad hack, apng is also a bad hack, it hijacks
> the png format. apng must differentiate it self from png so people know when
> all info is in there.
> 

I was going to ignore the trolls, but this is worth a comment to kill the misinformation.  

The web is built on this principle:  If a browser doesn't explicitly know about markup it ignores it.  css and html BOTH follow this very principle.  Try using -moz-column-width or -moz-column-gap in an older gecko browser or IE.  Try using text-shadow is IE, word-wrap in gecko.  All browsers ignore what they don't know.  

It's called Graceful Degradation.  It's what keeps the web from breaking thousands of times.

You can read all about this type of stuff by reading the thousands of closed bugzilla bugs, developers.mozilla.org, mozillazine forums, w3c etc. etc.

Stop trolling and start reading.
Comment 136 Max Stepin 2010-01-31 23:26:08 PST
https://wiki.mozilla.org/APNG_Specification#Error_Handling

"when any error is encountered decoders should discard all subsequent frames, stop the animation, and revert to displaying the default image"

This part of the specs is not implemented. 
Currently any errors in acTL, fcTL, fDAT chunks result in png_error().
So Firefox won't display anything, while IE will display the first frame.
Comment 137 Glenn Randers-Pehrson 2010-02-01 05:38:08 PST
@max, please open a new bug for this error.
Comment 138 Max Stepin 2010-02-01 23:59:46 PST
OK, I opened a bug 543674

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