libpng 1.2.21 update broke a unit test

RESOLVED FIXED in mozilla1.9beta2

Status

()

Core
ImageLib
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Dolske, Assigned: Dolske)

Tracking

Trunk
mozilla1.9beta2
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

11 years ago
The update to 1.2.21 caused the modules/libpr0n/test/unit/test_encoder_png.js unit test to fail... The encoded APNG looked and worked properly, but the bits were different.

pngcheck on the expected output is:

File: png1-ok.png (0 bytes)
  chunk IHDR at offset 0x0000c, length 13
    3 x 3 image, 24-bit RGB, non-interlaced
  chunk acTL at offset 0x00025, length 8
    unknown private, ancillary, unsafe-to-copy chunk
  chunk fcTL at offset 0x00039, length 26
    unknown private, ancillary, unsafe-to-copy chunk
  chunk IDAT at offset 0x0005f, length 16
    zlib:  deflated, 256-byte window, default compression
    zlib line filters (0 none, 1 sub, 2 up, 3 avg, 4 paeth):
      1 2 2 (3 out of 3)
  chunk fcTL at offset 0x0007b, length 26
    unknown private, ancillary, unsafe-to-copy chunk
  chunk fdAT at offset 0x000a1, length 22
    unknown private, ancillary, unsafe-to-copy chunk
  chunk fcTL at offset 0x000c3, length 26
    unknown private, ancillary, unsafe-to-copy chunk
  chunk fdAT at offset 0x000e9, length 27
    unknown private, ancillary, unsafe-to-copy chunk
  chunk IEND at offset 0x00110, length 0
No errors detected in png1-ok.png (-937.0% compression).


pngcheck on the new output is:

File: png1-bad.png (0 bytes)
  chunk IHDR at offset 0x0000c, length 13
    3 x 3 image, 24-bit RGB, non-interlaced
  chunk acTL at offset 0x00025, length 8
    unknown private, ancillary, unsafe-to-copy chunk
  chunk fcTL at offset 0x00039, length 26
    unknown private, ancillary, unsafe-to-copy chunk
  chunk IDAT at offset 0x0005f, length 16
    zlib:  deflated, 256-byte window, default compression
    zlib line filters (0 none, 1 sub, 2 up, 3 avg, 4 paeth):
      0 0 0 (3 out of 3)
  chunk fcTL at offset 0x0007b, length 26
    unknown private, ancillary, unsafe-to-copy chunk
  chunk fdAT at offset 0x000a1, length 19
    unknown private, ancillary, unsafe-to-copy chunk
  chunk fcTL at offset 0x000c0, length 26
    unknown private, ancillary, unsafe-to-copy chunk
  chunk fdAT at offset 0x000e6, length 19
    unknown private, ancillary, unsafe-to-copy chunk
  chunk IEND at offset 0x00105, length 0
No errors detected in png1-bad.png (-896.3% compression).


Note that in the original image, the IDAT line filters are set to 1/2/2, but in the new output they're 0/0/0. The fDAT checks also changed in size. [They're basically IDATs, so I'd suspect the same thing happened, pngcheck just doesn't know how to handle them.]

The libpng update seems to have added |#define PNG_NO_WRITE_FILTER| to modules/libimg/png/mozpngconf.h, so it looks like this is probably expected if I'm understanding that right.

Our nsIImageEncoder interface doesn't support specifying row filters, so this seems fine... I do wonder how the 1/2/2 values were getting set before -- were those the values, or were we picking up initialized data?

Glenn, if this all seems normal and expected I'll just update the unit tests for the new expected values.
(Assignee)

Comment 1

11 years ago
Created attachment 284568 [details] [diff] [review]
Patch to temporarily disable tests.
Assignee: nobody → dolske
Status: NEW → ASSIGNED
(Assignee)

Comment 2

11 years ago
Checking in modules/libpr0n/test/Makefile.in;
/cvsroot/mozilla/modules/libpr0n/test/Makefile.in,v  <--  Makefile.in
new revision: 1.2; previous revision: 1.1

I'll back this out when checking in a test fix (assuming that's what we do).
(Assignee)

Comment 3

11 years ago
(In reply to comment #0)

> Our nsIImageEncoder interface doesn't support specifying row filters, so this
> seems fine... I do wonder how the 1/2/2 values were getting set before -- were
> those the values, or were we picking up initialized data?

Err, let me try making sense this time:

I do wonder how the 1/2/2 values were getting set before -- were those the default values, or were we picking up uninitialized data?

Comment 4

11 years ago
That is correct.  The embedded libpng-1.2.21 writes all 0 filters while
the previous version would compute filters that could be anything 0-4.
The change was made to save footprint size, and because the usual PNG being
written is a screen dump for which the 0 filter is almost always the best
one anyhow.  Notice that the file has smaller fDATs now.

Please update the unit tests.  Note, however, that you will get a different
result when the system library is used instead of the embedded one.
(Assignee)

Comment 5

11 years ago
Created attachment 288170 [details] [diff] [review]
Patch for review, v.1

This reenables the tests, and uses new expected data per discussion above. [I verified that the expected data is rendered correctly.]

I noticed that the tests complained about trailing commands in strict mode, so I went ahead and removed the unneeded commas.
Attachment #284568 - Attachment is obsolete: true
Attachment #288170 - Flags: review?(pavlov)

Updated

11 years ago
Attachment #288170 - Flags: review?(pavlov) → review+
(Assignee)

Comment 6

11 years ago
Checking in modules/libpr0n/test/Makefile.in;
/cvsroot/mozilla/modules/libpr0n/test/Makefile.in,v  <--  Makefile.in
new revision: 1.3; previous revision: 1.2
done
Checking in modules/libpr0n/test/unit/test_encoder_apng.js;
/cvsroot/mozilla/modules/libpr0n/test/unit/test_encoder_apng.js,v  <--  test_encoder_apng.js
new revision: 1.2; previous revision: 1.1
done
Checking in modules/libpr0n/test/unit/test_encoder_png.js;
/cvsroot/mozilla/modules/libpr0n/test/unit/test_encoder_png.js,v  <--  test_encoder_png.js
new revision: 1.2; previous revision: 1.1
Flags: in-testsuite+
Target Milestone: --- → mozilla1.9 M10
(Assignee)

Comment 7

11 years ago
Hmm. Forgot to mark this as FIXED.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.