Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

APNG error handling #360

Closed
svgeesus opened this issue Oct 30, 2023 · 5 comments
Closed

APNG error handling #360

svgeesus opened this issue Oct 30, 2023 · 5 comments

Comments

@svgeesus
Copy link
Contributor

(This is an issue for PNG in general, in terms of testing with known-invalid PNG images; but I noticed it in particular with invalid APNG)

On the one hand, PNG says (my emphasis):

PNG decoders should detect errors as early as possible, recover from errors whenever possible, and fail gracefully otherwise. The error handling philosophy is described in detail in 13.1 Error handling.

On the other hand PNG also says:

APNG is designed to allow incremental display of frames before the entire datastream has been read. This implies that some errors may not be detected until partway through the animation. It is strongly recommended that when any error is encountered decoders should discard all subsequent frames, stop the animation, and revert to displaying the static image. A decoder which detects an error before the animation has started should display the static image. An error message may be displayed to the user if appropriate.

Decoders shall treat out-of-order APNG chunks as an error.

The wording is verbatim from the Mozilla APNG spec.

Looking at the invalid APNG tests, Mozilla does not do the "stop animation, revert to static" in most cases while Chrome does. But also they vary, in some cases the image does not load and in other cases the static image is displayed. The spec is loose enough here that the correct behavior is not always clear.

@svgeesus
Copy link
Contributor Author

There is a related Bugzilla bug:

@svgeesus
Copy link
Contributor Author

svgeesus commented Jan 5, 2024

Related, PNG spec says:

acTL must be before PLTE and IDAT. One fcTL may occur before IDAT; all others shall be after IDAT

This is tested by wpt/png/apng/fcTL-acTL-ordering.html which is passed by Chrome, Edge and Firefox but failed by Safari.

The test file has the following structure:

chris@SuperNomad:/mnt/c/Users/chris/Documents/GitHub/mywpt/png/apng/support$ pngcheck -vv -f 024.png
File: 024.png (508 bytes)
  chunk IHDR at offset 0x0000c, length 13
    128 x 64 image, 32-bit RGB+alpha, non-interlaced
  chunk fcTL at offset 0x00025, length 26
    unknown private, ancillary, unsafe-to-copy chunk
  chunk acTL at offset 0x0004b, length 8
    unknown private, ancillary, unsafe-to-copy chunk
  chunk IDAT at offset 0x0005f, length 147
    zlib: deflated, 32K window, default compression
  chunk fcTL at offset 0x000fe, length 26
    unknown private, ancillary, unsafe-to-copy chunk
  chunk fdAT at offset 0x00124, length 196
    unknown private, ancillary, unsafe-to-copy chunk
  chunk IEND at offset 0x001f4, length 0
No errors detected in 024.png (7 chunks, 98.4% compression).

so there is an fcTL before acTL and Safari error-corrects it.

I don't know what Safari does with acTL deeper into the animation (like after IDAT or after the second fcTL).

@svgeesus
Copy link
Contributor Author

svgeesus commented Jan 9, 2024

From the 8 Jan 2024 PNG WG meeting:

People in the meeting agreed that the current APNG error handling text is okay (and we don’t need to revise it).

@svgeesus
Copy link
Contributor Author

svgeesus commented Jan 9, 2024

I will port over the APNG error handling tests for WPT

@svgeesus
Copy link
Contributor Author

Closing since we resolved that no spec changes are needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant