Add support for the new animated PNG spec to Mozilla.

The spec can be found at http://www.vlad1.com/~vladimir/projects/apng/AnimatedPNG.html

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.

Please do the decoder changes for cairo gfx in a seperate bug.

Yeah, those changes weren't supposed to be part of this patch. Please ignore them..

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.

(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.

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.

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?

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

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).

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.

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.

> 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

> 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?

(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

> 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

If it would become a MNG profile, would it remain backwards compatible? If not, I think it shouldn't even be considered.

>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.

(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.

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.

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

>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.

brendan: have you followed the apng qt interactions?

(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).

> 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

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

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

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...

(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).

(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?

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

(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.

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 #33, I posted a rebuttal in the spec bug (where further discussion on that topic definataly belongs!)

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>.

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"

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...

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.

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

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)

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.

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.

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...

(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.

Related: bug 48570

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.

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?

(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.

(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 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.

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.

Re my comment #52 s/should/must/

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.'

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.

yes, that's what i meant

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.

#define PNG_HAVE_fCTL 0x8000 should be #define PNG_HAVE_fCTL 0x8000L

You haven't changed the chunk names to have lower-case second letters.

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

> - everything from imgContainerGIF has been copied to imgContainer. once i'm > done there will be no imgContainerGIF heh. so this undoes bug 77497 :-)

I feel myself being erased :)

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

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).

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 on attachment 247757 [details] [diff] [review] updated patch i think this is good to go

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).

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)?

(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.

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. :)

(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.

Um, cough. Don't break --enable-system-png.

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.

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.

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?

oh sorry, i see what you're talking about. will do some research.

(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.

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

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

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.

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.

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.

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.

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.

> 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.

(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 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

--> Andrew, who's done all but the first patch here.

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.

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.

+ 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 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.

(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.

(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*

hex flags replaced with #defines

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?

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?

(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.

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.

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.

Where is "the PNG group should allow APNG to use public chunks" bug button?

I suggest we keep this bug open and put patches here until we hit APNG 1.0...

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 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.

this splits out dispose_op and blend_op from render_op

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 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.

attaching the patch I checked in

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.

Please add a line to mozilla/modules/libimg/png/MOZCHANGES to indicate that APNG support was added on 20 March 2007.

(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.

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.

AC_CHECK_LIB(png, png_get_acTl, , should be AC_CHECK_LIB(png, png_get_acTL, , in configure.in

- 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.

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.

+++ 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?

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.

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.

Could this bug have caused bug 376446? (Note: the 'various small fixes' from comment 121 haven't changed bug 376446)

This one can be closed, no?

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.

"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

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.

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.

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.

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.

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.

(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.

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.

>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.

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.

(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.

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.

@max, please open a new bug for this error.

OK, I opened a bug 543674