Bug 693661

The print job makes use of vendor-unique pcl xl extension specific to the hp color laserjet 3500, 3550, 3600 models. Adding support to those extensions is possible but not trivial.

We'll make this bountiable.

Fwiw, the current pxldis.py script can completely parse the attachment. But the CLJ 3500, 3550, 3600 shares very little with the other HP CLJ printers, the task is probably closer to adding a new (but much  much simplier) frontend as xps/svg.

(In reply to comment #3)
> Fwiw, the current pxldis.py script can completely parse the attachment. But
> the CLJ 3500, 3550, 3600 shares very little with the other HP CLJ printers,
> the task is probably closer to adding a new (but much  much simplier)
> frontend as xps/svg.

I can do the parsing in the XL interpreter butI have no idea what format these raster images are.  Do you know Hin-Tak?

I'll continue working on this, turned off bountiable, if anyone was involved with this problem please contact me and we'll work something out, relevant commit:

commit 76f9208424f41e4177cda4a468af1b0bb6a5e166
Author: Henry Stiles <henry.stiles@artifex.com>
Date:   Wed Mar 27 16:34:39 2013 -0600

    Preliminary parsing code for the PXL VendorUnique operator.

    Some lower end HP color laser printers use an undocumented diminutive
    PXL-like language based on the VendorUnique operator.  This commit
    fleshes out enough of the parsing to get through the PXL in the test
    file attached to 693661 without a syntax error.  We've not done
    anything to implement VendorUnique commands beyond parsing.

Hin Tak do you want to do this on an hourly consulting basis? I can see you have done most of the "leg work" already.  The parsing is all pretty much done and you can dispatch from pxVendorUnique:pxsessio.c, the parser will have already filled in all the parameters, as of the change in Comment #5.

(In reply to comment #6)
> Hin Tak do you want to do this on an hourly consulting basis? I can see you
> have done most of the "leg work" already.  The parsing is all pretty much
> done and you can dispatch from pxVendorUnique:pxsessio.c, the parser will
> have already filled in all the parameters, as of the change in Comment #5.

I have had some thought on and off about how much effort is required, and just did a bit more thinking. There are two substantially different variants of the raster image (just as there are three in PXL, which are all different, although one is easy to do) and also B/W variants of those, so it means writing a small raster image decompression library; and make up 3 other test files on the way. That's just to claim full support (to the extent I am aware) of the CLJ 3500, 3550, 3600 usage of the VendorUnique operators.

OTOH, I believe at least one other commercial competitors of ghostpcl (one of those pcl{xxxx} out there) claims specifically to support that.

Anyway, I think I would say 3 weeks on implementing those, and another week to work on the parsing code to bounce unknown usage of the VendorUnique back into emit an error as it should; so that makes it 4 weeks in total, and I'd be happy to do that at the hourly rate plus 10% (adjusting for inflation) as 3 summers ago. Are you and Miles okay with that?

Oh, and I meant 4 full weeks, although it would need to spread out a bit due to other commitments & traveling.

Yes Hin Tak that will be fine.

Ok. I'll post some additional test files at some stage, but otherwise will just keep my head down for a few weeks until I have some code to show.

Created attachment 9912 [details]
12 test files, tar gz'ed

12 test files, tar gz'ed . Different compression modes, and gray (those with m in the name) vs color, all should show the tiger when processed correctly.

tiger10hm.pxl
tiger10h.pxl
tiger11hm.pxl
tiger11h.pxl
tiger20hm.pxl
tiger20h.pxl
tiger20rm.pxl
tiger20r.pxl
tiger21hm.pxl
tiger21h.pxl
tiger26hm.pxl
tiger26h.pxl

As of today, my patch is about 1500 lines. c.f. pxl/pximage.c which contains most of the pxl image compression code, is just over 1000. Somewhat larger than pxl/pximage.c is roughly what I expect, as I wrote in the beginning.

Attachment 9321 [details] shows the word "Test" but partly of wrong shade and have tears; 
tiger10hm.pxl, tiger10h.pxl, tiger11hm.pxl, tiger21hm.pxl, tiger26hm.pxl are in similar status; the rest works perfectly.

Attachment 4406 [details] (another file of this type in the past) still don't work yet; I assume when I get tiger10h.pxl working, it will.

I suspect tiger10hm.pxl is wrong (it is generated by hpijs, so it is 'official', but causes a real printer to eject a blank page). This may need to be ignored. Waiting for confirmation from hpijs people.

In summary, I have two little problems to fix before attaching the patch - 11hm, 21hm, 26hm are the same issue; 4406 and 9321 and 10h are possibly another.

If the original reporter has a reliable way of generating these, I'd appreciate two additional test files, (1) one with some actual color - e.g. change one of the letters of 'Test' to non-black, (2) explicitly request b/w mode (it is a driver setting).

The newer windows CLJ3600 driver/firmware seems to be somewhat different from its older siblings 3500/3550.

PageTech's PCL reader can process every file mentioned in comment 11 (all the tiger* as well as attachment 4406 [details]) but shows blank on attachment 9321 [details].

At this point, my code can process attachment 9321 [details] correctly and everything else, other than tiger11hm.pxl, tiger21hm.pxl, tiger26hm.pxl. (one and same problem - the b/w version of one of the compression modes).

I can also confirm tiger10hm.pxl and also tiger20r.pxl (possibly also tiger20rm.pxl) are somewhat wrong - they are valid data, but the tiger is reversed in tiger10hm.pxl, and color around the noise/iris in tiger20r.pxl . Those are issues with the generator/driver, not the decoder, as PageTech's shows them the same way independently. I'll take those issues up with hpijs's driver people.

Created attachment 9916 [details]
patch for HP CLJ 3500/3550/3600 emulation

This patch implements HP CLJ 3500/3550/3600 emulation.

$ git diff --numstat HEAD..
0       1       gs/base/gdevpxat.h
0       1       gs/base/gdevpxen.h
4       8       pl/plmain.c
2       10      pxl/pxl.mak
4       3       pxl/pxptable.c
1       35      pxl/pxsessio.c
0       7       pxl/pxstate.h
0       1097    pxl/pxvendor.c
0       21      pxl/pxvendor.h
2       3       tools/pxldis.py

Note that this patch adds two new files, pxl/pxvendor.{c,h}. The rest of the changes are small and mostly obvious. Included are
- an unrelated change of pl/plmain.c to not repeat repeated error messages,
- a somewhat related and minor bug fix to px_attribute_names in pxl/pxptable.c reported elsewhere which affects debug messages,
- some vaguely related stubs in pxl/pxsessio.c and tools/pxldis.py for HP 1011/1012/1015 emulation.

Assigning back to support for testing, etc.

The patch should have no effect what so ever on any existing test file; it should simply enable attachment 9321 [details], attachment 4406 [details], and all the tiger*.pxl in attachment 9912 [details] to work, when they don't work before.

Suggesting adding most of that 14 files to the test suite. Out of those,
 
attachment 4406 [details] covers similar code paths and better than tiger10h.pxl, tiger20h.pxl, tiger20r.pxl .

tiger10hm.pxl covers similar code paths as tiger20hm.pxl and tiger20rm.pxl .

tiger11hm.pxl and tiger21hm.pxl covers similar paths.

tiger11h.pxl and tiger21h.pxl covers similar paths.

So 7 files should cover it all. Note tiger10hm.pxl shows the tiger in reverse video - that's a regression of recent hpijs and I'll take it up with the relevant people; likewise the irises & area around the nose in tiger20r.pxl is a flaw of the generator. Also, due to the lossy nature of jpeg compression/decompression (and rounding errors in HP's custom color look-up), the tiger{X}{Y}??.pxl, where Y is 1 or 6, may not be color-accurate when compared to the original tiger.eps. 

My windows dev env isn't working at the moment, but I'll address any MSVC build issue if there is any.

Thanks Hin-Tak, I should get to testing it next week.

Having largely working on the debug build ('make pcl-debug') for obvious reasons, I tried the release build ('make pcl') and half of the files returns InternalOverflow in the JR3BeginImage - specifically, those with a jpeg qtable (even if not used, like 9931 & 4406, and the tigerX1??.pxl and tigerX6??.pcl) fail, while those without (tigerX0??.pxl's) continue to work in release build.

This seems to suggest tag_dispatch_8000() does not work correctly in release build; (in release build, the code never reaches tag_dispatch_generic() which is 8001).

I tried something like this - debug build continues to work, release build continues to fail. (this code change should only have effect if data is not available at larger pieces than 728, which isn't the case.) - oh, the magic number should be 768, not 728.
---------------
@@ -911,13 +910,9 @@ static int
 tag_dispatch_8000(px_args_t * par, px_state_t * pxs)
 {
     px_vendor_state_t *v_state = pxs->vendor_state;
+    int i = 192 - v_state->qcount;
 
-    if (par->source.available < 728)
-       return pxNeedData;
-    int i = 192;
-
-    v_state->qcount = 0;
-    while (i > 0) {
+    while ((i > 0) && (par->source.available >= 4)) {
        v_state->qvalues[v_state->qcount++] =
            uint32at(par->source.data, pxs->data_source_big_endian);
        par->source.data += 4;
@@ -925,8 +920,11 @@ tag_dispatch_8000(px_args_t * par, px_state_t * pxs)
        par->source.position += 4;
        i--;
     }
-    v_state->state = vu_blank;
-    return 0;
+    if (i == 0) {
+       v_state->state = vu_blank;
+       return 0;
+    } else
+       return pxNeedData;
 }
 static int
 tag_dispatch_generic(px_args_t * par, px_state_t * pxs)
@@ -1045,7 +1051,6 @@ pxJR3BeginImage(px_args_t * par, px_state_t * pxs)
        if (len) {
            if (par->source.available == 0) {
                pxs->vendor_state->qcount = 0;
-               return pxNeedData;
            }
 
            code = vu_tag_dispatch(par, pxs);
----------------

debug build also seems to differ quite substantially in pxl/pxparse.c - that might be subtly important? Anyway, I'll have another go next week. Meanwhile please just note that debug build works and release build doesn't. (on linux - I'll need to try windows at some point).

Created attachment 9920 [details]
windows msvc 9 related fixes and some.

Some code arrangements to make MSVC 9 happy (mostly C99-style variable declarations in the middle of a block is not allowed, and must happy in the beginning), some re-indentation; no functional changes. Just for reference to be quickly obsoleted by an integrated updated patch.

Created attachment 9921 [details]
updated patch, including arrangements to make MSVC happy.

updated patch

Created attachment 9922 [details]
windows debug build binary, zip'ed

windows debug build binary, zip'ed

managed to build for windows - the functional difference between debug build & release build is similar to those on linux: debug build works correctly, release build fails on a specific half.

Unlike on linux (12MB vs 32MB, almost 3 times), the windows binaries only differ by 3% (9.9MB vs 10.2MB).

(In reply to comment #21)
> Created attachment 9922 [details]
> 
> Unlike on linux (12MB vs 32MB, almost 3 times), the windows binaries only
> differ by 3% (9.9MB vs 10.2MB).

That's (partly) because the Linux debug binary contains the extra symbols for macro expansion in gdb - that adds a large chunk to the size of the debug exe.

Created attachment 9923 [details]
updated patch, to make release uild work

It turns out that the parser code's behavior in debug build differs from release build by streaming in much smaller chunks. In the previous version of the code, I was relying on the fact that the operator is being called multiple times (at least once with incomplete embedded data). So it failed with release build where the whole embedded data is available in one go.

The important part of the update is simply this:
---------------------------------
--- a/pxl/pxvendor.c
+++ b/pxl/pxvendor.c
@@ -1062,7 +1062,7 @@ pxJR3BeginImage(px_args_t * par, px_state_t * pxs)
            code = vu_tag_dispatch(par, pxs);
            /* Multiple payloads, reset to read 2nd */
            if (code == 0 && par->source.position != len)
-               code = pxNeedData;
+               code = vu_tag_dispatch(par, pxs);
        }
     }
     return code;
---------------------------------

I also made some further minor changes, switching the debug code status from "|" (experimental) to "w" (compression decoder) or "I" (interpreter) where appropriate; also found some 12-year-old code in plmain.c which says "|", surely they are out of experimental by now. It would just make life easier for people who are writing exp code, not having to read 12-year-old ones.

So here is the updated changelog:

This patch adds two new files, pxl/pxvendor.{c,h} which implement HP CLJ 3500/3550/3600 emulation, plus small associated changes in existing files. Included and somewhat less related are:

- an unrelated change of pl/plmain.c to not repeat repeated error messages,
- switching some 12-year-old debug code from '|' (experimental) to '|' (interpreter), to make genuine exp code devel easier.
- a somewhat related and minor bug fix to px_attribute_names in pxl/pxptable.c reported elsewhere which affects debug messages,
- some vaguely related stubs in pxl/pxsessio.c and tools/pxldis.py for HP 1011/1012/1015 emulation.

Created attachment 9924 [details]
msvc release build binary, zip'ed

msvc release-build binary, zip'ed - built with VC9.

By release-build, I just meant DEBUG=0 (i.e. just plain "nmake -f pcl6_msvc.mak", without a following DEBUG=1 as was the last one). This is built from patching HEAD, not from patching last release.

FYI, attachment 4406 [details] was filed in Bug 690083

Created attachment 9945 [details]
all the test files in one bundle, tar gz'ed

All the test files, 16 of them, in one convenient bundle.

attachment-4406.pxl
attachment-9321.pxl
tiger10hm-c.pxl
tiger10h.pxl
tiger11hm.pxl
tiger11h.pxl
tiger20hm.pxl
tiger20h.pxl
tiger20r-c.pxl
tiger20rm.pxl
tiger20um.pxl
tiger20u.pxl
tiger21hm.pxl
tiger21h.pxl
tiger26hm.pxl
tiger26h.pxl

This add the two attachments as well as two new ones, tiger20u*, and also replace the two flawed ones tiger20r, tiger10hm, mentioned earlier with the "corrected" equivalents.

attachment*, tiger1*, tiger26* (8 of these) should be for complete functional coverage.

As far as I see there is little else I need to do now.

The patch posted was AFAIC functionally alright, but technically quite ugly in a few places. (search for "TODO"). Mainly there are three areas which could be improved:

- the jpeg-related code. It is not very future-proof and very much tied to specific's of libjpeg's internal. If I had started again, I would rather have chosen to go via feeding a made-up header instead of what it does. (that would be about 1k of "magic" data). Unfortunately the format is that of headerless jpeg with a custom color transform, so it is ugly whichever approach one takes.

- the mode10 variant code can probably be simplified/re-factored, together with adding mode 10 compression to the pcl 5 code (bug 694082), when that happens.

- it is testing for colorspace way too often.

OTOH, this code path is unlikely to be in big demand, so this should do for the time being.

This can be closed. The patch was committed.

commit 37b9962406c99564f6ac16e37540a45cfae5d557
Author: Hin-Tak Leung <hintak@ghostscript.com>
Date:   Sun Jul 14 23:30:50 2013 +0000

    Bug 693661 - implement HP CLJ 3500/3550/3600 emulation
    
    The actual subject of the bug report is
    "pcl6 interpreter exited with error code -995".
    
    This patch adds two new files, pxl/pxvendor.{c,h} which implement
    HP CLJ 3500/3550/3600 emulation, plus small associated changes
    in existing files.
    
    Included and somewhat less related are:
    
    - a small change of pl/plmain.c to not repeat repeated error messages,
    - switching some 12-year-old debug code from '|' (experimental)
        to '|' (interpreter), to make genuine exp code devel easier.
    - a minor bug fix to px_attribute_names in
        pxl/pxptable.c reported elsewhere which affects debug messages,
    - some stubs in pxl/pxsessio.c and tools/pxldis.py
        for HP 1011/1012/1015 emulation.
    
    Where/what those are should be fairly obvious.
    
    No cluster differences.

Closing, see comment #28.