Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1756,6 +1756,21 @@ foreach(libtype ${TEST_LIBTYPES})
set_tests_properties(example-${sample_bits}bit-${libtype}-decompress-cmp
PROPERTIES DEPENDS example-${sample_bits}bit-${libtype}-decompress)

# Regression test for issue #444: overshoot deringing + SIMD DCT overflow.
# An 8x8 half-black/half-white image triggers maximum overshoot, which
# overflows the 16-bit column pass butterfly in the SIMD forward DCT.
# Without the fix, the block's brightness is completely inverted.
# NOTE: no -revert flag, so mozjpeg extensions (deringing) are active.
if(sample_bits EQUAL 8)
add_test(NAME ${cjpeg}-${libtype}-issue444
COMMAND cjpeg${suffix} -dct int -quality 25 -sample 1x1
-outfile ${testout}_issue444.jpg ${TESTIMAGES}/issue444.ppm)
add_bittest(${djpeg} issue444 "-dct;int;-ppm"
${testout}_issue444.ppm ${testout}_issue444.jpg
ca2f28ac51471eba0e0d30bc7492901c
${cjpeg}-${libtype}-issue444)
endif()

endforeach()

endforeach()
Expand Down
4 changes: 3 additions & 1 deletion jcdctmgr.c
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,9 @@ preprocess_deringing(DCTELEM *data, const JQUANT_TBL *quantization_table)
return;
}

/* Too much overshoot is not good: increased amplitude will cost bits, and the cost is proportional to quantization (here using DC quant as a rough guide). */
/* Too much overshoot is not good: increased amplitude will cost bits, and
the cost is proportional to quantization (here using DC quant as a rough
guide). */
maxovershoot = maxsample + MIN(MIN(31, 2*quantization_table->quantval[0]), (maxsample * size - sum) / maxsample_count);

n = 0;
Expand Down
8 changes: 6 additions & 2 deletions simd/arm/jfdctint-neon.c
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,12 @@ void jsimd_fdct_islow_neon(DCTELEM *data)
tmp11 = vaddq_s16(tmp1, tmp2);
tmp12 = vsubq_s16(tmp1, tmp2);

row0 = vrshrq_n_s16(vaddq_s16(tmp10, tmp11), PASS1_BITS);
row4 = vrshrq_n_s16(vsubq_s16(tmp10, tmp11), PASS1_BITS);
/* Use saturating arithmetic for the final butterfly to prevent 16-bit
* overflow when overshoot deringing produces large uniform column values.
* See https://github.com/mozilla/mozjpeg/issues/444
*/
row0 = vrshrq_n_s16(vqaddq_s16(tmp10, tmp11), PASS1_BITS);
row4 = vrshrq_n_s16(vqsubq_s16(tmp10, tmp11), PASS1_BITS);

tmp12_add_tmp13 = vaddq_s16(tmp12, tmp13);
z1_l = vmull_lane_s16(vget_low_s16(tmp12_add_tmp13), consts.val[0], 2);
Expand Down
2 changes: 1 addition & 1 deletion simd/i386/jfdctint-avx2.asm
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ F_3_072 equ DESCALE(3299298341, 30 - CONST_BITS) ; FIX(3.072711026)

vperm2i128 %7, %1, %1, 0x01 ; %7=tmp11_10
vpsignw %1, %1, [GOTOFF(ebx, PW_1_NEG1)] ; %1=tmp10_neg11
vpaddw %7, %7, %1 ; %7=(tmp10+tmp11)_(tmp10-tmp11)
vpaddsw %7, %7, %1 ; %7=(tmp10+tmp11)_(tmp10-tmp11)
%if %9 == 1
vpsllw %1, %7, PASS1_BITS ; %1=data0_4
%else
Expand Down
4 changes: 2 additions & 2 deletions simd/i386/jfdctint-mmx.asm
Original file line number Diff line number Diff line change
Expand Up @@ -438,8 +438,8 @@ EXTN(jsimd_fdct_islow_mmx):
psubw mm6, mm4 ; mm6=tmp12

movq mm7, mm5
paddw mm5, mm0 ; mm5=tmp10+tmp11
psubw mm7, mm0 ; mm7=tmp10-tmp11
paddsw mm5, mm0 ; mm5=tmp10+tmp11
psubsw mm7, mm0 ; mm7=tmp10-tmp11

paddw mm5, [GOTOFF(ebx,PW_DESCALE_P2X)]
paddw mm7, [GOTOFF(ebx,PW_DESCALE_P2X)]
Comment on lines 444 to 445
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial "lost" fix cdb6c34 changed these ones to paddsw as well.

Expand Down
4 changes: 2 additions & 2 deletions simd/i386/jfdctint-sse2.asm
Original file line number Diff line number Diff line change
Expand Up @@ -457,8 +457,8 @@ EXTN(jsimd_fdct_islow_sse2):
psubw xmm6, xmm4 ; xmm6=tmp12

movdqa xmm5, xmm7
paddw xmm7, xmm2 ; xmm7=tmp10+tmp11
psubw xmm5, xmm2 ; xmm5=tmp10-tmp11
paddsw xmm7, xmm2 ; xmm7=tmp10+tmp11
psubsw xmm5, xmm2 ; xmm5=tmp10-tmp11

paddw xmm7, [GOTOFF(ebx,PW_DESCALE_P2X)]
paddw xmm5, [GOTOFF(ebx,PW_DESCALE_P2X)]
Comment on lines 463 to 464
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial "lost" fix cdb6c34 changed these ones to paddsw as well.

Expand Down
7 changes: 5 additions & 2 deletions simd/mips64/jfdctint-mmi.c
Original file line number Diff line number Diff line change
Expand Up @@ -356,8 +356,11 @@ static uint64_t const_value[] = {
tmp11 = _mm_add_pi16(tmp1, tmp2); /* tmp11=tmp1+tmp2 */ \
tmp12 = _mm_sub_pi16(tmp1, tmp2); /* tmp12=tmp1-tmp2 */ \
\
out0 = _mm_add_pi16(tmp10, tmp11); /* out0=tmp10+tmp11 */ \
out4 = _mm_sub_pi16(tmp10, tmp11); /* out4=tmp10-tmp11 */ \
/* Use saturating add/sub to prevent 16-bit overflow when overshoot \
* deringing produces large uniform column values (issue #444). \
*/ \
out0 = _mm_adds_pi16(tmp10, tmp11); /* out0=tmp10+tmp11 */ \
out4 = _mm_subs_pi16(tmp10, tmp11); /* out4=tmp10-tmp11 */ \
\
out0 = _mm_add_pi16(out0, PW_DESCALE_P2X); \
out4 = _mm_add_pi16(out4, PW_DESCALE_P2X); \
Expand Down
7 changes: 5 additions & 2 deletions simd/powerpc/jfdctint-altivec.c
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,13 @@
tmp11 = vec_add(tmp1, tmp2); \
tmp12 = vec_sub(tmp1, tmp2); \
\
out0 = vec_add(tmp10, tmp11); \
/* Use saturating add/sub to prevent 16-bit overflow when overshoot \
* deringing produces large uniform column values (issue #444). \
*/ \
out0 = vec_adds(tmp10, tmp11); \
out0 = vec_add(out0, pw_descale_p2x); \
out0 = vec_sra(out0, pass1_bits); \
out4 = vec_sub(tmp10, tmp11); \
out4 = vec_subs(tmp10, tmp11); \
out4 = vec_add(out4, pw_descale_p2x); \
out4 = vec_sra(out4, pass1_bits); \
\
Expand Down
2 changes: 1 addition & 1 deletion simd/x86_64/jfdctint-avx2.asm
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ F_3_072 equ DESCALE(3299298341, 30 - CONST_BITS) ; FIX(3.072711026)

vperm2i128 %7, %1, %1, 0x01 ; %7=tmp11_10
vpsignw %1, %1, [rel PW_1_NEG1] ; %1=tmp10_neg11
vpaddw %7, %7, %1 ; %7=(tmp10+tmp11)_(tmp10-tmp11)
vpaddsw %7, %7, %1 ; %7=(tmp10+tmp11)_(tmp10-tmp11)
%if %9 == 1
vpsllw %1, %7, PASS1_BITS ; %1=data0_4
%else
Expand Down
4 changes: 2 additions & 2 deletions simd/x86_64/jfdctint-sse2.asm
Original file line number Diff line number Diff line change
Expand Up @@ -449,8 +449,8 @@ EXTN(jsimd_fdct_islow_sse2):
psubw xmm6, xmm4 ; xmm6=tmp12

movdqa xmm5, xmm7
paddw xmm7, xmm2 ; xmm7=tmp10+tmp11
psubw xmm5, xmm2 ; xmm5=tmp10-tmp11
paddsw xmm7, xmm2 ; xmm7=tmp10+tmp11
psubsw xmm5, xmm2 ; xmm5=tmp10-tmp11

paddw xmm7, [rel PW_DESCALE_P2X]
paddw xmm5, [rel PW_DESCALE_P2X]
Comment on lines 455 to 456
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial "lost" fix cdb6c34 changed these ones to paddsw as well.

Expand Down
Binary file added testimages/issue444.ppm
Binary file not shown.