1856630 - Update libjpeg-turbo to version 3.0.3

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, task)

Description

[Ryan VanderMeulen [:RyanVM]]

Looks like there's some perf improvements which could be interesting.

https://github.com/libjpeg-turbo/libjpeg-turbo/releases/tag/2.1.91
https://github.com/libjpeg-turbo/libjpeg-turbo/releases/tag/3.0.0

Comment 1

[Ryan VanderMeulen [:RyanVM]]

https://github.com/libjpeg-turbo/libjpeg-turbo/releases/tag/3.0.1

Significant changes relative to 3.0.0:

• The x86-64 SIMD functions now use a standard stack frame, prologue, and epilogue so that debuggers and profilers can reliably capture backtraces from within the functions.
• Fixed two minor issues in the interblock smoothing algorithm that caused mathematical (but not necessarily perceptible) edge block errors when decompressing progressive JPEG images exactly two MCU blocks in width or that use vertical chrominance subsampling.
• Fixed a regression introduced by 3.0 beta2[6] that, in rare cases, caused the C Huffman encoder (which is not used by default on x86 and Arm CPUs) to generate incorrect results if the Neon SIMD extensions were explicitly disabled at build time (by setting the WITH_SIMD CMake variable to 0) in an AArch64 build of libjpeg-turbo.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

https://github.com/libjpeg-turbo/libjpeg-turbo/releases/tag/3.0.2

Significant changes relative to 3.0.1:

• Fixed a signed integer overflow in the tj3CompressFromYUV8(), tj3DecodeYUV8(), tj3DecompressToYUV8(), and tj3EncodeYUV8() functions, detected by the Clang and GCC undefined behavior sanitizers, that could be triggered by setting the align parameter to an unreasonably large value. This issue did not pose a security threat, but removing the warning made it easier to detect actual security issues, should they arise in the future.
• Introduced a new parameter (TJPARAM_MAXMEMORY in the TurboJPEG C API and TJ.PARAM_MAXMEMORY in the TurboJPEG Java API) and a corresponding TJBench option (-maxmemory) for specifying the maximum amount of memory (in megabytes) that will be allocated for intermediate buffers, which are used with progressive JPEG compression and decompression, optimized baseline entropy coding, lossless JPEG compression, and lossless transformation. The new parameter and option serve the same purpose as the max_memory_to_use field in the jpeg_memory_mgr struct in the libjpeg API, the JPEGMEM environment variable, and the cjpeg/djpeg/jpegtran -maxmemory option.
• Introduced a new parameter (TJPARAM_MAXPIXELS in the TurboJPEG C API and TJ.PARAM_MAXPIXELS in the TurboJPEG Java API) and a corresponding TJBench option (-maxpixels) for specifying the maximum number of pixels that the decompression, lossless transformation, and packed-pixel image loading functions/methods will process.
• Fixed an error ("Unsupported color conversion request") that occurred when attempting to decompress a 3-component lossless JPEG image without an Adobe APP14 marker. The decompressor now assumes that a 3-component lossless JPEG image without an Adobe APP14 marker uses the RGB colorspace if its component IDs are 1, 2, and 3.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

I have a patch for this which appears to compile but then die during linking with a whole lot of undefined hidden symbol errors along the lines of:

ld.lld: error: undefined hidden symbol: j12init_d_main_controller
>>> referenced by jdmaster.c:719 (/media/libjpeg/jdmaster.c:719)
>>>               /obj-x86_64-pc-linux-gnu/toolkit/library/build/../../../media/libjpeg/jdmaster.o:(jinit_master_decompress)

https://hg.mozilla.org/try/rev/fd38802aa4658b96e23a6ea382f0c06a89de1717

Comment 4

[Timothy Nikkel (:tnikkel)]

RyanVM asked me to look into this. Recording what I found.

The files seem to need to be compiled multiple times with BITS_IN_JSAMPLE set to different values each time

https://github.com/libjpeg-turbo/libjpeg-turbo/blob/2dfe6c0fe9e18671105e94f7cbf044d4a1d157e6/sharedlib/CMakeLists.txt#L86

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://github.com/libjpeg-turbo/libjpeg-turbo/releases/tag/3.0.3

Significant changes relative to 3.0.2:

• Fixed an issue in the build system, introduced in 3.0.2, that caused all libjpeg-turbo components to depend on the Visual C++ run-time DLL when built with Visual C++ and CMake 3.15 or later, regardless of value of the WITH_CRT_DLL CMake variable.
• The x86-64 SIMD extensions now include support for Intel Control-flow Enforcement Technology (CET), which is enabled automatically if CET is enabled in the C compiler.
• Fixed a regression introduced by 3.0 beta2[6] that made it impossible for calling applications to supply custom Huffman tables when generating 12-bit-per-component lossy JPEG images using the libjpeg API.
• Fixed a segfault that occurred when attempting to use the jpegtran -drop option with a specially-crafted malformed input image or drop image (specifically an image in which all of the scans contain fewer components than the number of components specified in the Start Of Frame segment.)

Comment 6

[Ryan VanderMeulen [:RyanVM]]

(In reply to Timothy Nikkel (:tnikkel) from comment #4)

RyanVM asked me to look into this. Recording what I found.

The files seem to need to be compiled multiple times with BITS_IN_JSAMPLE set to different values each time

https://github.com/libjpeg-turbo/libjpeg-turbo/blob/2dfe6c0fe9e18671105e94f7cbf044d4a1d157e6/sharedlib/CMakeLists.txt#L86

Documenting some discussion with glandium, our build system doesn't allow the same file to be compiled multiple times from the same moz.build file, so the only real option would be to create a different moz.build for each configuration in a different directory where the different BITS_IN_JSAMPLE value can be passed in. That sounds...fun.

Comment 7

[Timothy Nikkel (:tnikkel)]

What about creating a file like

#define BITS_IN_JSAMPLE 8

#include filetocompile.cpp

#define BITS_IN_JSAMPLE 10

#include filetocompile.cpp

And just compiling that, would that work?

Comment 8

[Ryan VanderMeulen [:RyanVM]]

I guess if we wanted to try going that route, this looks like the relevant set of source files we'd need to handle:

set(JPEG16_SOURCES jcapistd.c jccolor.c jcdiffct.c jclossls.c jcmainct.c
  jcprepct.c jcsample.c jdapistd.c jdcolor.c jddiffct.c jdlossls.c jdmainct.c
  jdpostct.c jdsample.c jutils.c)
set(JPEG12_SOURCES ${JPEG16_SOURCES} jccoefct.c jcdctmgr.c jdcoefct.c
  jddctmgr.c jdmerge.c jfdctfst.c jfdctint.c jidctflt.c jidctfst.c jidctint.c
  jidctred.c jquant1.c jquant2.c)
set(JPEG_SOURCES ${JPEG12_SOURCES} jcapimin.c jchuff.c jcicc.c jcinit.c
  jclhuff.c jcmarker.c jcmaster.c jcomapi.c jcparam.c jcphuff.c jctrans.c
  jdapimin.c jdatadst.c jdatasrc.c jdhuff.c jdicc.c jdinput.c jdlhuff.c
  jdmarker.c jdmaster.c jdphuff.c jdtrans.c jerror.c jfdctflt.c jmemmgr.c
  jmemnobs.c jpeg_nbits.c)

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Using separate moz.build files actually turned out to be pretty straightforward. I think that'll be a pretty workable solution here.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1856630 - Update libjpeg-turbo to version 3.0.3.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Green builds: https://treeherder.mozilla.org/jobs?repo=try&revision=5067ba93afc568dd42ee560294cf63bef606f626
Green tests: https://treeherder.mozilla.org/jobs?repo=try&revision=d80a415925802bec5da56aaa74e3b7ea357d5c4d / https://treeherder.mozilla.org/jobs?repo=try&revision=95b16726565ab8b4ea223ce29ccb12798bf86444

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2ed1cc5bdd50
Update libjpeg-turbo to version 3.0.3. r=tnikkel

Comment 13

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/2ed1cc5bdd50

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Per discussion with tnikkel, we're going to revert this for now until it's had more bake time.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Backed out for 128.0b4.
https://hg.mozilla.org/releases/mozilla-beta/rev/e908e03cb152c1a58bf5f03fde456a1fa9956eb7