Closed Bug 886499 Opened 11 years ago Closed 11 years ago

Update libpng to version 1.5.17

Categories

(Core :: Graphics: ImageLib, enhancement)

Product:
Core
Component:
Graphics: ImageLib
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla25

People

(Reporter: glennrp+bmo, Assigned: glennrp+bmo)

References

Details

Attachments

(1 file, 10 obsolete files)

v10 Update libpng to version 1.5.17
114.05 KB, patch
joe
: review+
Details | Diff | Splinter Review 
Libpng-1.5.17 will be released this week, on June 25.  Libpng-1.6.3 isn't ready yet, so let's upgrade to 1.5.17 for now.  1.5.17 has the same improved ARM support as 1.6.3.
June 27th not 25th.
Depends on: 880847
Depends on: 873001
Blocks: 832390
Need tryserver run
Flags: needinfo?(ryanvm)
Comment on attachment 769408 [details] [diff] [review]
v00 Update libpng to version 1.5.17

Tryserver shows all green.  r?
Attachment #769408 - Flags: review?(joe)
Comment on attachment 769408 [details] [diff] [review]
v00 Update libpng to version 1.5.17

Review of attachment 769408 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/libpng/README
@@ -85,5 @@
>  these at http://www.libpng.org/pub/png/documents/
>  
>  This code is currently being archived at libpng.sf.net in the
> -[DOWNLOAD] area, and on CompuServe, Lib 20 (PNG SUPPORT)
> -at GO GRAPHSUP.  If you can't find it in any of those places,

Hah.
Attachment #769408 - Flags: review?(joe) → review+
Keywords: checkin-needed
heh, I'd already pushed it but forgot to mark the bug.
https://hg.mozilla.org/integration/mozilla-inbound/rev/28298b00b5cb
Keywords: checkin-needed
Backed out for 08:57:50 ERROR - ../../../prebuilt/linux-x86/toolchain/arm-linux-androideabi-4.4.x/bin/../lib/gcc/arm-linux-androideabi/4.4.3/../../../../arm-linux-androideabi/bin/ld: /builds/slave/b2g_m-in_p_dep-000000000000000/build/objdir-gecko/toolkit/library/../../media/libpng/pngrutil.o: in function MOZ_PNG_read_filt_row:../../../gecko/media/libpng/pngrutil.c:3873: error: undefined reference to 'MOZ_PNG_init_filt_func_neon'

https://hg.mozilla.org/integration/mozilla-inbound/rev/8080d41c7c08
Evidently the arm/*.c and arm/*.S are not getting compiled.  The v00 patch doesn't add them to moz.build or Makefile.in.  See bug #832390.  Should be a fairly simple fix.
Added arm/moz.build and revised Makefile.in to compile the ARM-neon-supporting files.  Please submit to try server.
Attachment #769408 - Attachment is obsolete: true
Flags: needinfo?(ryanvm)
Removed DISABLED_CSRCS and DISABLED_SSRCES from Makefile.in.  These are handled by moz.build now.

Need a tryserver run, please.
Attachment #770586 - Attachment is obsolete: true
:(

applying attachment.cgi?id=775979
unable to find 'media/libpng/arm/moz.build' for patching
1 out of 1 hunks FAILED -- saving rejects to file media/libpng/arm/moz.build.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh attachment.cgi?id=775979
Restored the "new file" directive, inadvertently deleted from the v02 patch.  Please try the tryserver again.
Attachment #775979 - Attachment is obsolete: true
Flags: needinfo?(joe)
Flags: needinfo?(ryanvm)
The v03 patch failed to compile the arm .c and .S sources.  Revised moz.build following libjpeg's moz.build. Please submit v04 to the try server.
Flags: needinfo?(joe)
Attachment #776092 - Attachment is obsolete: true
Try *finally* unborked: https://tbpl.mozilla.org/?tree=Try&rev=7d5450284688
Flags: needinfo?(joe)
Unfortunately the patch is borked again.  I forgot that the filenames have to be in alphabetical order in the CSRCS list.
alphabetized filenames in moz.build
Attachment #776142 - Attachment is obsolete: true
Flags: needinfo?(joe)
Added a DIRS directive to moz.build
Attachment #776579 - Attachment is obsolete: true
Flags: needinfo?(joe)
Define PNG_ALIGNED_MEMORY_SUPPORTED in mozpngconf.h, required for ARM support.
Changed line endings in arm/filter_neon.S to CRLF to try to work around Win7 errors.
Attachment #776694 - Attachment is obsolete: true
Flags: needinfo?(joe)
Changing the line endings didn't help.  It seems that the Win7 assembler does not do CPP preprocessing so we get a flood of syntax errors while assembling arm/filter_neon.S. We'll have to eliminate the compilation of arm/* via a configuration flag.
Revised configure.in and moz.build to avoid compiling arm/* sources on non-ARM platforms.
Attachment #777103 - Attachment is obsolete: true
Flags: needinfo?(joe)
v08 try is all read due to python syntax error.  Guessing it was some missing whitespace, added to v09.
Attachment #777211 - Attachment is obsolete: true
Flags: needinfo?(joe)
(In reply to Glenn Randers-Pehrson from comment #26)

> v08 try is all read due to python syntax error.
s/read/red/
This time I tried building it on my  machine too :) https://tbpl.mozilla.org/?tree=Try&rev=1dc69877a666
Flags: needinfo?(joe)
Comment on attachment 777253 [details] [diff] [review]
v09 Update libpng to version 1.5.17

Try server results look good.  Win7 and B2G are now green.   r?
Attachment #777253 - Flags: review?(joe)
Comment on attachment 777253 [details] [diff] [review]
v09 Update libpng to version 1.5.17

Review of attachment 777253 [details] [diff] [review]:
-----------------------------------------------------------------

Happily r=me for the png changes, but the configure.in and moz.build changes need review from a build peer.
Attachment #777253 - Flags: review?(joe)
Attachment #777253 - Flags: review?(gps)
Attachment #777253 - Flags: review+
Comment on attachment 777253 [details] [diff] [review]
v09 Update libpng to version 1.5.17

Review of attachment 777253 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +4176,5 @@
> +if test "$MOZ_NATIVE_PNG" != 1; then
> +    if test "$CPU_ARCH" = "arm" ; then
> +       MOZ_PNG_ARM_NEON=1
> +    fi
> +fi

You can use test -a to combine both tests if you like.

I'm not sure if this entire chunk is behind a conditional. If not, you can move the AC_SUBST to right here.

::: media/libpng/moz.build
@@ +32,5 @@
>  
> +if CONFIG['MOZ_PNG_ARM_NEON']:
> +    DIRS += [
> +        'arm',
> +]

Nit: Need to indent ]. x3 for entire fire.

::: media/libpng/MOZCHANGES
@@ +1,4 @@
>  
>  Changes made to pristine png source by mozilla.org developers.
>  
> +2013/06/29  -- Synced with libpng-1.5.17 (bug #886499).

Might want to update that date!
Attachment #777253 - Flags: review?(gps) → review+
configure.in changed as gps suggested.  Joe & Greg, please transfer your "r" from v09 to v10.  I don't think we need another try server run.
Attachment #777253 - Attachment is obsolete: true
Flags: needinfo?(joe)
Flags: needinfo?(gps)
Keywords: checkin-needed
Comment on attachment 777436 [details] [diff] [review]
v10 Update libpng to version 1.5.17

r=joe, gps (given that this is just implementing the changes requested).
Attachment #777436 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/e9195ed65161
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: