(In reply to comment #163)
> (2) RGB_{RED|GREEN|BLUE} macro conflicts on OS/2 (OS/2? Really?): This seems
> like an innocuous enough patch, although I'd rather have it in jmorecfg.h
> instead of jpeglib.h. I'd also like to know more of the back story as to why
> OS/2 is defining those macros. Is this a generic problem that anyone building
> libjpeg-turbo on OS/2 would encounter? My lingering doubt here is that libjpeg
> supports OS/2, so I wonder why they haven't encountered this problem on that
> platform. Is Mozilla including some header file that causes those macros to be
> defined?
I'd be fine with dropping this altogether. If OS/2 still needs it, we can always add it back.
>
> (3) bitscan instructions: I tested the proposed patch to make jcdctmgr.c use
> GCC 3.4+ compiler intrinsics to replace flss() with bitscan instructions, but I
> cannot observe any measurable speedup from this (tested several different image
> types and platforms, as well as 32-bit and 64-bit.) If it were even 2% faster,
> I would agree to include it, but a performance patch that causes no appreciable
> increase in performance does nothing but needlessly obfuscate the code.
It's probably worth adding a comment to flss saying that it's not performance critical enough to justify using compiler intrinsics.
> (4) The tables in jccolor.c are part of the optimized RGB-to-grayscale
> conversion routines. These are pre-computed R-to-luminance, G-to-luminance,
> and B-to-luminance conversion values. The problem with this approach is that
> it loses a bit of accuracy due to round-off, so I recently checked in code to
> the SVN trunk (1.2 working version) that uses new SIMD routines to perform
> RGB-to-grayscale conversion. This is faster and eliminates the round-off
> issues, as well as the need for the tables in jccolor.c and the #ifdef in
> rgb_gray_convert().
(In reply to comment #163) GREEN|BLUE} macro conflicts on OS/2 (OS/2? Really?): This seems
> (2) RGB_{RED|
> like an innocuous enough patch, although I'd rather have it in jmorecfg.h
> instead of jpeglib.h. I'd also like to know more of the back story as to why
> OS/2 is defining those macros. Is this a generic problem that anyone building
> libjpeg-turbo on OS/2 would encounter? My lingering doubt here is that libjpeg
> supports OS/2, so I wonder why they haven't encountered this problem on that
> platform. Is Mozilla including some header file that causes those macros to be
> defined?
I'd be fine with dropping this altogether. If OS/2 still needs it, we can always add it back.
>
> (3) bitscan instructions: I tested the proposed patch to make jcdctmgr.c use
> GCC 3.4+ compiler intrinsics to replace flss() with bitscan instructions, but I
> cannot observe any measurable speedup from this (tested several different image
> types and platforms, as well as 32-bit and 64-bit.) If it were even 2% faster,
> I would agree to include it, but a performance patch that causes no appreciable
> increase in performance does nothing but needlessly obfuscate the code.
It's probably worth adding a comment to flss saying that it's not performance critical enough to justify using compiler intrinsics.
> (4) The tables in jccolor.c are part of the optimized RGB-to-grayscale
> conversion routines. These are pre-computed R-to-luminance, G-to-luminance,
> and B-to-luminance conversion values. The problem with this approach is that
> it loses a bit of accuracy due to round-off, so I recently checked in code to
> the SVN trunk (1.2 working version) that uses new SIMD routines to perform
> RGB-to-grayscale conversion. This is faster and eliminates the round-off
> issues, as well as the need for the tables in jccolor.c and the #ifdef in
> rgb_gray_convert().
That sounds like a good solution to me.