Bug 696654 - PCL/PXL fails to build with system-shared libjpeg
Summary: PCL/PXL fails to build with system-shared libjpeg
Status: RESOLVED FIXED
Alias: None
Product: Ghostscript
Classification: Unclassified
Component: Build Process (show other bugs)
Version: master
Hardware: PC Linux
: P4 normal
Assignee: Default assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-14 12:44 UTC by Jonas Smedegaard
Modified: 2023-06-02 16:31 UTC (History)
4 users (show)

See Also:
Customer:
Word Size: ---


Attachments
Make jpeg structure usage compatible with jpeg 6b. (3.34 KB, patch)
2016-10-08 20:11 UTC, Hin-Tak Leung
Details | Diff
first part of CLJ3600, not dependent on libjpeg (34.70 KB, patch)
2023-06-01 22:50 UTC, Hin-Tak Leung
Details | Diff
2nd part of CLJ3600 emulation, libpeg dependent. reference only, do not apply (19.20 KB, patch)
2023-06-01 22:54 UTC, Hin-Tak Leung
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonas Smedegaard 2016-03-14 12:44:25 UTC
When attempting to build GhostPDL for Debian, which uses system-shared libjpeg(-turbp) instead of the embedded code copy, the build fails:

gcc  -DHAVE_MKSTEMP -DHAVE_FILE64 -DHAVE_FSEEKO -DHAVE_MKSTEMP64 -DHAVE_FONTCONFIG -DHAVE_LIBIDN -DHAVE_SETLOCALE -DHAVE_SSE2  -DHAVE_BSWAP32 -DHAVE_BYTESWAP_H -DHAVE_
STRERROR -DHAVE_ISNAN -DHAVE_ISINF -DHAVE_PREAD_PWRITE=1 -DGS_RECURSIVE_MUTEXATTR=PTHREAD_MUTEX_RECURSIVE -fPIC  -O2 -fPIC -Wdate-time -D_FORTIFY_SOURCE=2  -Wall -Wstr
ict-prototypes -Wundef -Wmissing-declarations -Wmissing-prototypes -Wwrite-strings -Wno-strict-aliasing -Werror=declaration-after-statement -fno-builtin -fno-common -W
error=return-type -DHAVE_STDINT_H=1 -DHAVE_DIRENT_H=1 -DHAVE_SYS_DIR_H=1 -DHAVE_SYS_TIME_H=1 -DHAVE_SYS_TIMES_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_LIBDL=1 -DGX_COLOR_INDEX_T
YPE="unsigned long long" -D__USE_UNIX98=1  -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wall -DUSE_LIBPAPER -I/usr/include/x86_64-linux-gnu -DGS_D
EVS_SHARED -DGS_DEVS_SHARED_DIR=\"/usr/lib/ghostscript/9.19\"  -I./pcl/pxl -I./soobj -I./pcl/pcl -I./pcl/pl -I./base -I./soobj -c ./pcl/pxl/pxvendor.c -o ./soobj/pxven
dor.o
In file included from ./base/sdct.h:23:0,
                 from ./pcl/pxl/pxvendor.c:35:
./base/setjmp_.h:33:5: warning: variably modified 'stuff' at file scope
     unsigned char stuff[sizeof(jmp_buf) + gsfix_jmp_buf_align];
     ^
./pcl/pxl/pxvendor.c: In function 'fastforward_jpeg_stream_state':
./pcl/pxl/pxvendor.c:288:10: error: 'struct jpeg_decompress_struct' has no member named 'is_baseline'
     cinfo->is_baseline = 1;
          ^
./pcl/pxl/pxvendor.c:317:17: error: 'jpeg_component_info {aka struct <anonymous>}' has no member named 'DCT_h_scaled_size'
          compptr->DCT_h_scaled_size = 8, \
                 ^
./pcl/pxl/pxvendor.c:327:5: note: in expansion of macro 'SET_COMP'
     SET_COMP(0, 1, 1, 1, 0, 0, 0); /* not default! */
     ^
./pcl/pxl/pxvendor.c:318:17: error: 'jpeg_component_info {aka struct <anonymous>}' has no member named 'DCT_v_scaled_size'
          compptr->DCT_v_scaled_size = 8, \
                 ^
./pcl/pxl/pxvendor.c:327:5: note: in expansion of macro 'SET_COMP'
     SET_COMP(0, 1, 1, 1, 0, 0, 0); /* not default! */
     ^
./pcl/pxl/pxvendor.c:317:40: warning: left-hand operand of comma expression has no effect [-Wunused-value]
          compptr->DCT_h_scaled_size = 8, \
                                        ^
./pcl/pxl/pxvendor.c:327:5: note: in expansion of macro 'SET_COMP'
     SET_COMP(0, 1, 1, 1, 0, 0, 0); /* not default! */
     ^
./pcl/pxl/pxvendor.c:318:40: warning: left-hand operand of comma expression has no effect [-Wunused-value]
          compptr->DCT_v_scaled_size = 8, \
                                        ^
./pcl/pxl/pxvendor.c:327:5: note: in expansion of macro 'SET_COMP'
     SET_COMP(0, 1, 1, 1, 0, 0, 0); /* not default! */
     ^
./pcl/pxl/pxvendor.c:317:17: error: 'jpeg_component_info {aka struct <anonymous>}' has no member named 'DCT_h_scaled_size'
          compptr->DCT_h_scaled_size = 8, \
                 ^
./pcl/pxl/pxvendor.c:329:2: note: in expansion of macro 'SET_COMP'
  SET_COMP(1, 2, 1, 1, 1, 1, 1);
  ^
./pcl/pxl/pxvendor.c:318:17: error: 'jpeg_component_info {aka struct <anonymous>}' has no member named 'DCT_v_scaled_size'
          compptr->DCT_v_scaled_size = 8, \
                 ^
./pcl/pxl/pxvendor.c:329:2: note: in expansion of macro 'SET_COMP'
  SET_COMP(1, 2, 1, 1, 1, 1, 1);
  ^
./pcl/pxl/pxvendor.c:317:40: warning: left-hand operand of comma expression has no effect [-Wunused-value]
          compptr->DCT_h_scaled_size = 8, \
                                        ^
./pcl/pxl/pxvendor.c:329:2: note: in expansion of macro 'SET_COMP'
  SET_COMP(1, 2, 1, 1, 1, 1, 1);
  ^
./pcl/pxl/pxvendor.c:318:40: warning: left-hand operand of comma expression has no effect [-Wunused-value]
          compptr->DCT_v_scaled_size = 8, \
                                        ^
./pcl/pxl/pxvendor.c:329:2: note: in expansion of macro 'SET_COMP'
  SET_COMP(1, 2, 1, 1, 1, 1, 1);
  ^
./pcl/pxl/pxvendor.c:317:17: error: 'jpeg_component_info {aka struct <anonymous>}' has no member named 'DCT_h_scaled_size'
          compptr->DCT_h_scaled_size = 8, \
                 ^
./pcl/pxl/pxvendor.c:330:2: note: in expansion of macro 'SET_COMP'
  SET_COMP(2, 3, 1, 1, 1, 1, 1);
  ^
./pcl/pxl/pxvendor.c:318:17: error: 'jpeg_component_info {aka struct <anonymous>}' has no member named 'DCT_v_scaled_size'
          compptr->DCT_v_scaled_size = 8, \
                 ^
./pcl/pxl/pxvendor.c:330:2: note: in expansion of macro 'SET_COMP'
  SET_COMP(2, 3, 1, 1, 1, 1, 1);
  ^
./pcl/pxl/pxvendor.c:317:40: warning: left-hand operand of comma expression has no effect [-Wunused-value]
          compptr->DCT_h_scaled_size = 8, \
                                        ^
./pcl/pxl/pxvendor.c:330:2: note: in expansion of macro 'SET_COMP'
  SET_COMP(2, 3, 1, 1, 1, 1, 1);
  ^
./pcl/pxl/pxvendor.c:318:40: warning: left-hand operand of comma expression has no effect [-Wunused-value]
          compptr->DCT_v_scaled_size = 8, \
                                        ^
./pcl/pxl/pxvendor.c:330:2: note: in expansion of macro 'SET_COMP'
  SET_COMP(2, 3, 1, 1, 1, 1, 1);
  ^
./pcl/pxl/pxvendor.c:361:10: error: 'struct jpeg_decompress_struct' has no member named 'block_size'
     cinfo->block_size = DCTSIZE;
          ^
./pcl/pxl/pxvendor.c:363:10: error: 'struct jpeg_decompress_struct' has no member named 'natural_order'
     cinfo->natural_order = jpeg_natural_order;
          ^
./pcl/pxl/pxvendor.c:364:10: error: 'struct jpeg_decompress_struct' has no member named 'lim_Se'
     cinfo->lim_Se = DCTSIZE2 - 1;
          ^
./pcl/pxl/pxvendor.c:365:10: error: 'struct jpeg_decompress_struct' has no member named 'min_DCT_h_scaled_size'
     cinfo->min_DCT_h_scaled_size = 8;
          ^
./pcl/pxl/pxvendor.c:366:10: error: 'struct jpeg_decompress_struct' has no member named 'min_DCT_v_scaled_size'
     cinfo->min_DCT_v_scaled_size = 8;
          ^
./pcl/pxl/pxvendor.c:375:5: warning: implicit declaration of function 'jpeg_core_output_dimensions' [-Wimplicit-function-declaration]
     jpeg_core_output_dimensions(cinfo);
     ^
pcl/pxl/pxl.mak:212: recipe for target 'soobj/pxvendor.o' failed
make[3]: *** [soobj/pxvendor.o] Error 1
make[3]: Leaving directory '/build/ghostscript-9.19rc1~dfsg'
base/unix-dll.mak:151: recipe for target 'so-only-subtarget' failed
make[2]: *** [so-only-subtarget] Error 2
make[2]: Leaving directory '/build/ghostscript-9.19rc1~dfsg'
base/unix-dll.mak:124: recipe for target 'so' failed
make[1]: *** [so] Error 2
Comment 1 Hin-Tak Leung 2016-03-14 15:56:32 UTC
What version of libjpeg-turbo? It looks like it is also heavily customized by debian?
Comment 2 Hin-Tak Leung 2016-03-14 17:01:34 UTC
https://github.com/libjpeg-turbo/libjpeg-turbo/blob/8fc75ab3daaa839d63111febc9f06f2e8c9e87b4/jpeglib.h

It looks like cinfo->is_baseline need jpeg8 compat mode enabled in libjpeg-turbo, at least. You may need to contact debian on that, to correct the flaw in the system library.
Comment 3 Jonas Smedegaard 2016-03-14 17:20:35 UTC
jpeg-turbo 1.4.2. Debian package has only one patch compared to upstream, which seems secific to MIPS architecture (,y experienced failure was on AMD64).

Yes, it looks like Debian jpeg-turbo package is compiled without "--with-jpeg8", and likely deliberate: That flag is documented to break backwards compatibility with libjpeg 6, which is how jpeg-turbo is currently used in Debian.

I will try link against libjpeg8 instead (which is discouraged in general in Debian: if possible jpeg-turbo should be used).

Perhaps autoconf should test the libjpeg in use and disable PCL with a descriptive warning if not compatible with libjpeg8.
Comment 4 Jonas Smedegaard 2016-03-14 17:42:58 UTC
Ah, too bad: I cannot make ghostscript link against libjpeg8 in Debian, because only one libjpeg development header package can be installed at a time, and ghostscript needs libcupsimage too which pulls in libjpeg(-turbo).

So if Ghostscript cannot link against libjpeg6 then it seems we canot have GhostPCL in Debian currently :-(
Comment 5 Hin-Tak Leung 2016-03-14 18:28:19 UTC
You could try modifying that particular part of pcl/pxl/pxvendor.c to be in line with
https://github.com/libjpeg-turbo/libjpeg-turbo/blob/8fc75ab3daaa839d63111febc9f06f2e8c9e87b4/jpeglib.h

if JPEG_LIB_VERSION >= 80

etc.

I believe that code is not sensitive to the jpeg lib version per se. It just needs to initialize the jpeg structure in very specific and unusual way. (I wrote that code, btw, although HP generates the unusual data that it tries to read). It should work. You can test that your modification is correct by testing against the test files it was written for - attachment 9945 [details] in bug 693661 .
Comment 6 Chris Liddell (chrisl) 2016-03-15 01:57:40 UTC
We probably need to fully disable that flavour of PXL if the libjpeg is too old.
Comment 7 Hin-Tak Leung 2016-03-15 04:30:45 UTC
(In reply to Chris Liddell (chrisl) from comment #6)
> We probably need to fully disable that flavour of PXL if the libjpeg is too
> old.

It is never too old - it should work with libJPEG 6b, which is as old as reasonably old... Those printers are sold this side of millennium, libJPEG 6b released before that.
Comment 8 Chris Liddell (chrisl) 2016-03-15 04:35:38 UTC
(In reply to Hin-Tak Leung from comment #7)
> (In reply to Chris Liddell (chrisl) from comment #6)
> > We probably need to fully disable that flavour of PXL if the libjpeg is too
> > old.
> 
> It is never too old - it should work with libJPEG 6b, which is as old as
> reasonably old... Those printers are sold this side of millennium, libJPEG
> 6b released before that.


Still, my preference would be to see the code changed as per the comment: have a "dummy" JFIF header to feed into the jpeg decoder, before the actual image data. Further, I'd like to see the code changed to use the JPEG decoder stream API in the graphics library, so we avoid this nonsense in the future.
Comment 9 Chris Liddell (chrisl) 2016-03-15 04:36:29 UTC
(In reply to Jonas Smedegaard from comment #4)
> Ah, too bad: I cannot make ghostscript link against libjpeg8 in Debian,
> because only one libjpeg development header package can be installed at a
> time, and ghostscript needs libcupsimage too which pulls in libjpeg(-turbo).
> 
> So if Ghostscript cannot link against libjpeg6 then it seems we canot have
> GhostPCL in Debian currently :-(

To be *very* clear, this is not Ghostscript, but GhostPCL - Ghostscript, AFAIK, works fine.
Comment 10 Henry Stiles 2016-03-15 09:17:59 UTC
(In reply to Chris Liddell (chrisl) from comment #8)
> (In reply to Hin-Tak Leung from comment #7)
> > (In reply to Chris Liddell (chrisl) from comment #6)
> > > We probably need to fully disable that flavour of PXL if the libjpeg is too
> > > old.
> > 
> > It is never too old - it should work with libJPEG 6b, which is as old as
> > reasonably old... Those printers are sold this side of millennium, libJPEG
> > 6b released before that.
> 
> 
> Still, my preference would be to see the code changed as per the comment:
> have a "dummy" JFIF header to feed into the jpeg decoder, before the actual
> image data. Further, I'd like to see the code changed to use the JPEG
> decoder stream API in the graphics library, so we avoid this nonsense in the
> future.


Hello Hin-Tak

This does seem like a bug against the original implementation and we'd like to have it fixed as Chris suggests.
Comment 11 Hin-Tak Leung 2016-10-08 20:11:02 UTC
Created attachment 13005 [details]
Make jpeg structure usage compatible with jpeg 6b.

This patch enables building against jpeg 6b.

I guess it is up to Chris whether this should go into the upcoming release. The patch is a no-op with the in-tree libjpeg v9, and fairly obviously so. I guess there are people who really would like to build against an older system libjpeg so this might be important.
Comment 12 Henry Stiles 2018-02-12 10:29:44 UTC
Hello Hin-Tak, we do need this fixed properly or we will pull it out.  The code cannot use jpeg private data.  It must use the JPEG API or the GS DCT filters (sdct.h).  The PXL image code uses the latter.
Comment 13 Hin-Tak Leung 2018-02-12 14:36:54 UTC
(In reply to Henry Stiles from comment #12)
> Hello Hin-Tak, we do need this fixed properly or we will pull it out.  The
> code cannot use jpeg private data.  It must use the JPEG API or the GS DCT
> filters (sdct.h).  The PXL image code uses the latter.

The problem is really that the driver code (the HP side) uses a customized libJPEG - with a customized HUV transform -, so it is not entirely straight forward to use an uncustomized libjpeg, and restricting to public API to decode it. Injecting customized jpeg header (instead of playing with libjpeg's internal structure) is one way I think of which *might* be cleaner. Hindsight is a great thing. :-(.

The attached patch, however, should remedy the problem a bit under a better answer can be found, if at all.
Comment 14 Henry Stiles 2018-02-13 06:20:17 UTC
I'm inclined to remove the code unless you can show evidence of use.  I've no outside feedback since its implementation suggesting it's not used.
Comment 15 Hin-Tak Leung 2018-02-13 06:52:43 UTC
(In reply to Henry Stiles from comment #14)
> I'm inclined to remove the code unless you can show evidence of use.  I've
> no outside feedback since its implementation suggesting it's not used.

okay, that's fair. I'd suggest you remove it and close this as LATER. When I come up with something I'll re-open and attach the new version. How about that?
Comment 16 Henry Stiles 2018-02-15 09:53:36 UTC
commit 6a8c1d6be02fc7020ab4cf5cfbecf91790f7dc10
Comment 17 Hin-Tak Leung 2023-06-01 22:50:13 UTC
Created attachment 24372 [details]
first part of CLJ3600, not dependent on libjpeg

Turn out that, this part alone, works on all the user-supplied test file,
and most (8/14) of the test files I made for Bug 693661 too. 6 of the test
files I made, requires the 2nd libjpeg-dependent part.
Comment 18 Hin-Tak Leung 2023-06-01 22:54:20 UTC
Created attachment 24373 [details]
2nd part of CLJ3600 emulation, libpeg dependent. reference only, do not apply

This part actually does 

#include "jpeglib_.h"

and etc, and the questionable part of this issue.

Turn out that 6 of the test files I made specially for Bug 693661
(and none from other users) requires this code path. So probably
this part alone can be left out.

DO NOT APPLY!!! Included here just for completeness.
Comment 19 Hin-Tak Leung 2023-06-01 23:08:23 UTC
I had another idea lately, and remember the CLJ3600 have two print modes, a loss-less modified mode 10-like one and a lossy bastardized libjpeg mode. I had implemented both. But actually everybody else submitted files uses the mode 10-like mode, and all the bastardized libjpeg mode test files were made by me.

For reference: the windows driver calls the former "best", and have two "lower/draft" quality modes. The Linux driver only provides the
"lower/draft" quality mode.

So it looks like that all the other users (except me) uses windows, and leave the windows driver at the default "best" settings.

Thus applying only the first part, non-libpeg dependent, should make most people happy.

The 2nd part is only included here for completeness of CLJ3600 implementation. DO NOT APPLY THE 2nd part.

DO NOT APPLY THE 2nd PART. First part is enough for most purposes.
Comment 20 Ken Sharp 2023-06-02 08:01:55 UTC
Thanks for the update and code, but we don't really like the idea of having a half-functional implementation (only supporting one print mode).

Given that we've had no further feedback on this we don't want to go further with it.
Comment 21 Hin-Tak Leung 2023-06-02 16:31:08 UTC
Fair enough. I thought the first half does the most popular print mode (and all the test files submitted by everybody else from me) so is perhaps worth having. Anyway, the patches are here for those who want it. They have been adjusted to build against current, and tested okay against the old test files. (It has been a little effort to fix the bit-rot due to changes elsewhere)