The PNG spec says "truecolor."
Also use "uncompressed RGB" rather than "uncompressed true color" when
referring to TARGA images, to conform to that spec.
fc324109d5 checked for unsigned int
overflow, but jsize is always a signed int, even when using a 64-bit
JVM. (This reflects the fact that array sizes in Java are always signed
ints.) Thus, array sizes should be checked against INT_MAX, not
UINT_MAX.
In almost all cases, this is a belt-and-suspenders measure. With the
exception of TJCompressor.loadSourceImage(), the array size checks in
question are checking a Java array, which can never have more than
INT_MAX elements. Thus, even if the previous unsigned int overflow
checks failed to catch signed int overflow, the error would have been
caught in a different place. With TJCompressor.loadSourceImage(), it
was possible to load a packed-pixel image large enough that arraySize
overflowed a signed int and caused the NewByteArray() or NewShortArray()
JNI methods to crash the JVM. However, this would have required a
multi-gigabyte packed-pixel image.
This commit also improves the array size checks in
TJCompressor.loadSourceImage() and TJDecompressor.saveImage() so that
they take into account row alignment/pitch.
Fixes#877
In the decompressor, this does nothing except fix a -Wswitch-default
warning with Clang, because cinfo.Ss (the predictor selection value) is
already range-checked in start_pass_lossless().
In the compressor, this fixes an oversight whereby a calling application
could manually set cinfo.Ss to an invalid value after calling
jpeg_enable_lossless() and before calling jpeg_start_compress(), thus
causing an incorrect (all white) lossless JPEG image to be silently
generated.
Since the default JPEG colorspace depends on the source pixel format and
selected subsampling level, it is useful to allow applications to
restore the default behavior if they have previously specified a JPEG
colorspace using TJPARAM_COLORSPACE.
Closes#852
Note that this changes the behavior of tj3GetICCProfile() in the
following ways:
- It can now be called with a compression instance, thus allowing the
caller to retrieve an ICC profile (or the size thereof) extracted from
a PNG image by tj3LoadImage*().
- The ICC profile extracted from a JPEG image can now be retrieved
multiple times. If I recall, I didn't allow this in TurboJPEG 3.1
because there were already two buffer copies, one in
jpeg_read_icc_profile() and another in the JNI wrapper. That is no
longer relevant, since the TurboJPEG Java API is now a separate
project.
These changes should be backward-compatible. Applications that treat
tj3GetICCProfile() as a mailbox rather than a newsstand will still work.
The changes were necessary in order to:
- allow decompression programs to both extract the ICC profile from a
JPEG image and embed it into a PNG output image, and
- allow compression programs to retrieve the ICC profile size from a PNG
input image (which is important when computing the worst-case JPEG
destination buffer size.)
Closes#645
If TJPARAM_SAVEMARKERS was set to 2 or 4, TJXOPT_COPYNONE was not
specified, an ICC profile was extracted from the source image, and
another ICC profile was associated with the TurboJPEG instance using
tj3SetICCProfile(), tj3Transform() embedded both profiles in the
destination image. This was mainly problematic because it might have
caused the destination image to exceed the worst-case size reported by
tj3TransformBufSize().
jpegtran would actually embed both ICC profiles as well, except that it
removes ICC profiles from the marker copying option if -icc is
specified. The documented TurboJPEG API behavior is for TJXOPT_COPYNONE
to take precedence over TJPARAM_SAVEMARKERS and for TJPARAM_SAVEMARKERS
to take precedence over the associated ICC profile. It would've been
easier to let the associated ICC profile take precedence, but the intent
of the TurboJPEG 3.1 API design was to allow either the associated or
extracted ICC profile to be selected on a per-transform basis using
TJXOPT_COPYNONE. Basically, if TJPARAM_SAVEMARKERS is 2 or 4 and
TJXOPT_COPYNONE is not specified, that means "always copy the ICC
profile from the source image, even if it doesn't have one."
tj3TransformBufSize() assumes that behavior.
Most RVV implementations issue instructions based on LMUL (the vector
length multiplier), even if VL (the vector length) is less than VLMAX
(the maximum number of elements that will fit in the register.) Thus,
even though our DCT/IDCT implementations always operate on 8 elements at
a time (VL=8), we can improve performance with RVV implementations that
have 256-bit or wider registers (VLEN>=256) by explicitly using
half-register (LMUL=1/2) intrinsics.
Note that LMUL=1/2 with 64-bit elements (SEW=64) isn't allowed, so the
8x8 transpose algorithm has to temporarily use full-register intrinsics.
If a pre-allocated JPEG destination buffer (either new or reused) was
passed to tj3Compress*() or tj3Transform(), then jpeg_mem_dest_tj()
failed to set dest->newbuffer to the value of the buffer pointer. If
the buffer was not large enough to hold the destination image, then a
new buffer was allocated, but because dest->newbuffer was unset, the old
buffer was not freed.
If pre-allocated JPEG destination buffers (either new or reused) were
passed to tj3Transform() and multiple lossless transform operations were
performed, then the value of dest->newbuffer carried over from the first
to the second transform operation. If the second destination buffer was
not large enough to hold the second destination image, then
dest->newbuffer (which pointed to the first destination buffer) was
freed. That could have caused the caller to read from freed memory or
to trigger a double free if it attempted to free the first destination
buffer, as the API documentation instructs.
The compress and transform fuzzers now test for both issues.
- Optimize range limiting in the IDCT algorithms.
- Implement an in-register matrix transpose in the IDCT algorithms
rather than using strided stores/loads to/from memory.
- Avoid non-contiguous memory accesses in the fancy upsampling
algorithms.
If a malformed drop image contained a quantization table with a value of
0, and if that value differed from the same value in the input image's
quantization tables, then 0 was passed as the second (b) argument to
largest_common_denominator() in transupp.c. That led to undefined
behavior when the function attempted to compute a % b.
This commit fixes that issue by implementing the accepted definition of
gcd(x, 0) = |x|. It guards against a similar issue in dequant_comp(),
although the second issue can never occur because
largest_common_denominator() never returns 0.
... which could cause compute_reciprocal() to divide by 0. (Minimally,
that is undefined behavior, and on some platforms it will trigger a
floating point exception.)
The most direct way to trigger the issue is by manually modifying the
exposed quantization table values in jpeg_compress_struct just before
calling jpeg_start_compress(). However, referring to #862, a more
indirect way involves using jpeg_read_header() to read a malformed JPEG
header containing quantization tables with values of 0, calling
jpeg_copy_critical_parameters() to transfer those quantization tables
into a compressor instance, and calling jpeg_start_compress().
In both cases, the calling program is abusing the API:
- Applications are supposed to call jpeg_add_quant_table() to pass
custom quantization tables to the compressor, and
jpeg_add_quant_table() clamps all values < 1.
- jpeg_copy_critical_parameters() is generally only used with
transcoding. Using it with jpeg_start_compress() is only valid when
the application intends to reuse quantization tables from a
tables-only datastream, in which case the application must carefully
control the datastream (as opposed to reading arbitrary data) in order
to generate a valid image.
Fixes#862
Based on:
d48f1ff2fb137fe9ef726d2f276fd0462954dc51d5ac20d3ef6c68dcaad4cdb255c60facb51a1d8db5b660b629
Improvements relative to the aforementioned implementation:
- ~145% faster RGB-to-YCbCr color conversion
~156% faster grayscale-to-YCbCr color conversion
~94% faster h2v1 downsampling
~167% faster h2v2 downsampling
~159% faster h2v2 fancy upsampling
~202% faster h2v1 plain upsampling
~290% faster h2v2 plain upsampling
~64% faster YCbCr-to-RGB color conversion
~54% faster h2v1 merged upsampling
~59% faster h2v2 merged upsampling
This was mostly achieved by:
* reducing non-contiguous memory accesses (including replacing strided
loads/stores with segmented loads/stores),
* using RVV instructions to generate a bias vector for downsampling,
rather than using a clunky dynamically-allocated array and C code,
and
* restructuring the algorithms (usually based on my experience with
other SIMD module implementations) to improve throughput and reduce
register pressure.
The DCT algorithms are unfortunately suboptimal at the moment, because
they use strided loads/stores to transpose the 8x8 coefficient matrix.
I tried using segmented loads/stores, as well as implementing an
in-register transpose with *vrgather*(). The former did not improve
performance significantly, and the latter was slower. The new Zvzip
RVV extension should make it possible to implement a more efficient
in-register transpose, which may open up opportunities for further
optimization. (The color conversion algorithms may benefit from that
as well.)
- ~10-26% (average 18%) better overall RGB-to-baseline compression
performance, and ~1-14% (average 8%) better overall baseline-to-RGB
decompression performance. (The forward and inverse DCT algorithms
are still suboptimal, and they dominate overall performance.)
- Improved build-time detection of compiler support for RVV intrinsics
- Support for run-time RVV detection on *BSD (hypothetically, at least--
I can only verify that the code compiles properly)
- Cleaned up run-time RVV detection on Linux
- Cleaned up SIMD dispatcher integration
- Descriptive code comments
- Corrected and added copyright attributions
- Code formatting tweaks
Closes#620Closes#837
This fixes what was essentially an unexpected-gain-of-function issue.
tjDecompress2() works properly within the scope of functionality that
the TurboJPEG 2.x API claims to support. However, with the introduction
of the TurboJPEG 3 API, the TurboJPEG 2.x API was re-implemented as a
wrapper for the TurboJPEG 3 API. Thus, the TurboJPEG 2.x API gained the
undocumented ability to decompress lossless JPEG images, but it still
lacks the ability to detect such images. If a calling program that uses
the TurboJPEG 2.x API, handles arbitrary JPEG images, and links with
libjpeg-turbo 3.0+ allocated a packed-pixel destination buffer based on
the assumptation that the destination image would be scaled down, then
the caller's buffer would be overrun if it attempted to use
tjDecompress2() to decompress a lossless JPEG image.
Given the fact that this issue has not been reported in the three years
since the TurboJPEG 3 API was released, it is likely that the number of
affected programs is zero.
Referring to #835, there were long-standing bugs in some of the modules,
which necessitated disabling those modules. Over the past four months,
I have tried and failed to find any parties interested in fixing those
bugs (including the parties who contributed the DSPr2 extensions.) I
interpret that as a strong sign that no one in the community cares about
the DSPr2 extensions anymore, but let's see if anyone complains before
3.2 beta1.
See ChangeLog.md for additional justifications, but suffice it to say
that:
- MIPS was a dead architecture in 2021.
- In 2026, it's shuffled off its mortal coil, run down the curtain, and
joined the bleedin' choir invisible.
If the data precision of the PBMPLUS file does not match the target data
precision, then the grayscale or RGB samples are rescaled to the target
data precision. Thus, we need to pass (1 << cinfo->data_precision) - 1
to rgb_to_cmyk() instead of maxval. This commit also modifies
TJUnitTest so that it validates the correctness of upconversion and
downconversion in the PPM reader.
Fixes#841
If loading a 2-to-8-bit image, unset srcBuf12 and srcBuf16. If loading
a 9-to-12-bit image, unset srcBuf8 and srcBuf16. If loading a
13-to-16-bit image, unset srcBuf8 and srcBuf12. Otherwise,
TJCompressor.getSourceBuf() will return srcBuf8 or srcBuf12, in order,
if any previous invocation of TJCompressor.loadSourceImage() set them,
irrespective of the buffer that was set by the most recent invocation.
This probably helps with garbage collection as well, since it signals to
the GC that the unused buffers are really unused.
This is not a security vulnerability, since applications that pass such
values to the Java API would fail regardless, and such a bug would never
make it into the wild. By contrast, a security vulnerability arises
from applications that work correctly with most input data sets but
trigger a library failure, such as a buffer overrun, with one or more
specific input data sets. As with most imaging APIs, the libjpeg-turbo
APIs rely upon the calling application to pass appropriately-sized
buffers and appropriate size/dimension arguments. The failure to do so
is no more the fault of libjpeg-turbo than calling
'buf = malloc(1); buf[2] = 0;' is the fault of the C library.
Buffer size checking is a bonus feature of the Java API that isn't (and
can't be) provided by the C API, so this commit merely hardens the bonus
feature against API abuse, in keeping with the Java paradigm of throwing
an exception rather than crashing due to a caller-imposed buffer
overrun.
- If TJPARAM_LOSSLESS was set, then tj3EncodeYUV*8() called
jpeg_enable_lossless() (via setCompDefaults()), which caused the
underlying libjpeg API to silently disable subsampling and color
conversion. This led to three issues:
1. Attempting to encode RGB pixels produced incorrect YCbCr or
grayscale components, since color conversion did not occur. The
same issue occurred if TJPARAM_COLORSPACE was explicitly set to
TJCS_RGB.
2. Attempting to encode RGB pixels into a grayscale plane caused
tj3EncodeYUVPlanes8() to overflow the caller's destination pointer
array if the array was not big enough to accommodate three
pointers. If called from tj3EncodeYUV8(), tj3EncodeYUVPlanes8()
did not overflow the caller's destination pointer array, but a
segfault occurred when it attempted to copy to the Cb and Cr
pointers, which were NULL. The same issue occurred if
TJPARAM_COLORSPACE was explicitly set to anything other than
TJCS_GRAY.
3. Attempting to encode RGB pixels into subsampled YUV planes caused
tj3EncodeYUV*8() to overflow the caller's buffer(s) if the
buffer(s) were not big enough to accommodate 4:4:4 (non-subsampled)
YUV planes. That would have been the case if the caller allocated
its buffer(s) based on the return value of tj3YUVBufSize() or
tj3YUVPlaneSize(). The same issue occurs if TJPARAM_SUBSAMP is
explicitly set to TJSAMP_444.
tj3EncodeYUV*8() now ignores TJPARAM_LOSSLESS and TJPARAM_COLORSPACE.
- If TJPARAM_LOSSLESS was set, then attempting to compress a grayscale
plane into a JPEG image caused tj3CompressFromYUVPlanes8() to overflow
the caller's source pointer array if the array was not big enough to
accommodate three pointers. If called from tj3CompressFromYUV8(),
tj3CompressFromYUVPlanes8() did not overflow the caller's source
pointer array, but a segfault occurred when it attempted to copy from
the Cb and Cr pointers, which were NULL. This was similar to Issue 2
above. The same issue occurred if TJPARAM_COLORSPACE was explicitly
set to anything other than TJCS_GRAY.
tj3CompressFromYUV*8() now throws an error if TJPARAM_LOSSLESS is set,
and it now ignores TJPARAM_COLORSPACE.
These issues did not pose a security risk, since security exploits
involve supported workflows that function normally except when supplied
with malformed input data. It is documented that colorspace conversion,
chrominance subsampling, and compression from planar YUV images are
unavailable when TJPARAM_LOSSLESS is set. When TJPARAM_LOSSLESS is set,
the library effectively sets TJPARAM_SUBSAMP to TJSAMP_444 and
TJPARAM_COLORSPACE to TJCS_RGB, TJCS_GRAY, or TJCS_CMYK, depending on
the pixel format of the source image. That behavior is strongly implied
by the documentation of TJPARAM_LOSSLESS, although the documentation
isn't specific about whether TJPARAM_LOSSLESS applies to
tj3EncodeYUV*8(). In any case, setting TJPARAM_LOSSLESS before calling
tj3CompressFromYUV*8() was never a supported or functional workflow, and
setting TJPARAM_LOSSLESS before calling tj3EncodeYUV*8() was never a
functional workflow. Thus, there should be no applications "in the
wild" that use either workflow. Such applications would crash every
time they attempted to encode to or compress from a YUV image. In other
words, setting TJPARAM_LOSSLESS or TJPARAM_COLORSPACE required the
caller to understand the ramifications of the loss of color conversion
and/or subsampling, and failing to do so was essentially API abuse
(albeit subtle API abuse, hence the desire to make the behavior more
intuitive.)
This commit also removes no-op code introduced by
6da05150ef. Since setCompDefaults()
returns after calling jpeg_enable_lossless(), modifying the subsampling
level locally had no effect. The libjpeg API already silently disables
subsampling in jinit_c_master_control() if lossless compression is
enabled, so it was not necessary for setCompDefaults() to handle that.
Fixes#839
The libjpeg in-memory destination manager has always re-allocated the
JPEG destination buffer if the specified buffer pointer is NULL or the
specified buffer size is 0. TurboJPEG's destination manager inherited
that behavior. Because of fe80ec2275,
TurboJPEG's destination manager tries to reuse the most recent
destination buffer if the same buffer pointer is specified. (The
purpose of that is to enable repeated invocations of tj*Compress*() or
tj*Transform() to automatically grow the destination buffer, as needed,
with no intervention from the calling program.) However, because of the
inherited code, TurboJPEG's destination manager also reallocated the
destination buffer if the specified buffer size was 0. Thus, passing a
previously-used JPEG destination buffer pointer to tj*Compress*() or
tj*Transform() while specifying a destination buffer size of 0 confused
the destination manager. It reallocated the destination buffer to 4096
bytes but reported the old destination buffer size to the libjpeg API.
This caused a buffer overrun if the old destination buffer size was
larger than 4096 bytes.
The documentation for tj*Compress*() is contradictory on this matter.
It states that the JPEG destination buffer size must be specified if the
destination buffer pointer is non-NULL. However, it also states that,
if the destination buffer is reused, the specified destination buffer
size is ignored. The documentation for tj*Transform() does not specify
the function's behavior if the destination buffer is reused. Thus, the
behavior of the API is at best undefined if a calling application
attempts to reuse a destination buffer while specifying a destination
buffer size of 0. If that ever worked, it only worked in libjpeg-turbo
1.3.x and prior.
This issue was exposed only through API abuse, and calling applications
that abused the API in that manner would not have worked for the last 11
years. Thus, the issue did not represent a security threat. This
commit merely hardens the API against such abuse, by modifying
TurboJPEG's destination manager so that it refuses to re-allocate the
JPEG destination buffer if the buffer pointer is reused and the
specified buffer size is 0. That is consistent with the most permissive
interpretation of the TurboJPEG API documentation. (The API already
ignored the destination buffer size if the destination buffer pointer
was reused and the specified buffer size was non-zero. It makes sense
for it to do likewise if the specified buffer size is 0.) This commit
also modifies TJUnitTest so that it verifies whether the API is hardened
against the aforementioned abuse.
- Use a new preprocessor macro (SIMD_ARCHITECTURE), set by the build
system, to ensure that the same implementation of the SIMD extensions
is selected at configure time and compile time. This eliminates any
dependence on compiler-specific and CPU-specific preprocessor macros.
- Where possible, consolidate all CPU-specific SIMD dispatcher functions
into a common source file (simd/jsimd.c) with CPU-specific code paths
selected by the aforementioned SIMD_ARCHITECTURE preprocessor macro.
- Implement common wrappers for the SIMD module functions that take
different arguments than their C counterparts. (Some SIMD modules do
that because it is difficult to interface with jpeg_*compress_struct
in assembly language.) For such modules, store the SIMD module
function pointers in the opaque jpeg_color_converter,
jpeg_downsampler, jpeg_entropy_encoder, jpeg_inverse_dct,
jpeg_upsampler, and jpeg_color_deconverter structures.
- Eliminate unnecessary wrappers for the SIMD module functions that take
the same arguments as their C counterparts. The dispatchers for those
modules now reuse the existing function pointers in the opaque
my_fdct_controller and phuff_entropy_encoder structures.
- Streamline the implementation of both the C and SIMD extended RGB
color conversion functions. The source and destination colorspaces
are locked in when jpeg_start_*compress() is called, so we can choose
an extended RGB color conversion function at that time. It isn't
necessary to wait until color conversion actually occurs.
- Initialize the SIMD extensions on a per-instance basis rather than a
per-thread basis. This eliminates any dependence on thread-local
storage in the libjpeg API library. (Referring to #834, TLS can be
problematic with older Windows versions.) The TurboJPEG API library
still relies on TLS when throwing errors from global functions, but
that is a less critical feature than the SIMD extensions.
This also makes the initialization of the SIMD extensions more
intuitive, predictable, and controllable from the point of view of
calling programs. (For AArch32, MIPS, and PowerPC architectures,
initializing the SIMD extensions on Linux might require reading
/proc/cpuinfo, which likely incurs a bit of overhead. Thus, it's good
to know exactly when that will occur.)
- Implement architecture-specific jpeg_simd_cpu_support() functions for
all non-x86 architectures, and move non-x86 CPU feature detection code
into the new functions.
- When using GCC or Clang, mark all SIMD dispatcher and module functions
with __attribute__((visibility("hidden"))) so that they will not be
exposed in the symbol table of shared libraries. In practice, the
version script/mapfile already accomplishes this on ELF platforms,
and Windows hides global symbols by default. Thus, this primarily
benefits macOS.
- Set the default value of the SIMD support variable to the bitwise
inverse of all possible bitmask values. This makes it impossible to
mistake an uninitialized value for an initialized/supported value.
- Eliminate the JSIMD_FORCEAVX2 environment variable. It had the effect
of enabling only the AVX2 SIMD modules if AVX2 instructions were
supported. That isn't particularly useful, since not all modules have
an AVX2 implementation.
- Enable SSE SIMD modules with JSIMD_FORCESSE2. This makes the behavior
of JSIMD_FORCESSE2 consistent with that of JSIMD_FORCESSE and
JSIMD_FORCE3DNOW, i.e. "Emulate the behavior of the library when
running on a CPU that supports only this instruction set and older
ones."
- Introduce a new test program (simdcoverage) that reports the SIMD
modules that will be used. This facilitates diagnosing dispatcher
issues.
- Clean up SIMD #includes; ensure that all SIMD headers include their
own dependencies.
- Fix various strict compiler warnings in the SIMD code.
- Eliminate unnecessary preprocessor checks in SIMD dispatcher
functions. (For instance, DCTSIZE will never not be 8.)
- In the non-x86 SIMD extensions, remove references to libjpeg/SIMD and
jsimdext.inc and replace them with the complete zlib License header.
jsimdext.inc only exists in the x86 SIMD extensions, as does any code
derived from libjpeg/SIMD.
