Closed j-p-sequeira closed 6 years ago
Hi José,
Thanks for reporting!
Fir the first case, I will need some sample files to further investigate. Feel free to send them in a separate email.
For the second case, the PCX file has a color mode that isn't fully supported (yet) . If you add a IIOWarningListener
to the reader, you will see a warning:
Unsupported color mode: 0, colors may look incorrect
If you have time to investigate this further, feel free! :-)
Best regards,
-- Harald K
Well, I did not know about the IIOWarningListener
, but I tried it and you are indeed right! I get that warning every single time I try to read an image that will get all black. The strange thing is that I have lots of Black and White PCX images, and the ones that display fine do not trigger the warning... I wonder why some are fine and some are not. I compared images that are fine with problematic ones and they are both read into a Java Image with ColorSpace.TYPE_RGB
with pixel size = 1
(bitmap). What I have managed to check is that every PCX I save in bitmap mode (black and white) with Photoshop Elements 2.0 will have the problem. Maybe there was a bug in Photoshop? I do not have any application here to open PCX besides Photoshop Elements 2.0 to check if other applications can display the image properly or not.
Also, I wonder what would happen if you hacked it a bit and treated color mode: 0
the same as 1
... for example, just see what happens if in PCXHeader.java you added a second line:
header.paletteInfo = imageInput.readUnsignedShort(); // 1 == Color/BW, 2 == Gray
header.paletteInfo = header.paletteInfo == 0 ? 1 : header.paletteInfo;
About the ArrayIndexOutOfBoundsException
, you will be receiving an email shortly ; )
Hi José,
I believe I have fixed the issue in the sample file you sent.
For the other issue, I don't see what treating color mode 0 same as 1 would help..? It would just remove the warning, but the image still looks just the same (all black). Which in my mind is worse than the current behavior.
-- Harald K
That's great, thank you very much! :)
About the color mode, I don't know why but after a quick glance at the code in the PCXReader, I thought that when it was 0 the execution would follow a different if path. Please pay no mind xP
By the way is there even any specification for the PCX color mode 0?
I've done some investigation (and you probably already did this), but this is what I found:
1) I took a working black and white PCX and saved a copy with Photoshop (which results in an all black PCX). Comparing the headers, I got this (2nd one is from the problematic image):
PCXHeader{version=0, compression=1, bitsPerPixel=1, width=9476, height=32513, hdpi=24576, vdpi=24576, channels=1, bytesPerLine=26112, paletteInfo=256, hScreenSize=0, vScreenSize=0, palette=[0, 0, 0, -1, -1, -1, -116, 94, 1, 0, 0, 0, 65, 28, -108, 94, -96, 92, -93, 94, 88, -91, 52, 11, 1, 122, 18, 0, -88, -42, -114, 94, -16, 46, 21, 11, 32, 105, -116, 25, 40, -73, 52, 11, 8, 0, 0, 0]}
PCXHeader{version=5, compression=1, bitsPerPixel=1, width=9476, height=32513, hdpi=24576, vdpi=24576, channels=1, bytesPerLine=26112, paletteInfo=256, hScreenSize=0, vScreenSize=0, palette=[15, 15, 15, 14, 14, 14, 13, 13, 13, 12, 12, 12, 11, 11, 11, 10, 10, 10, 9, 9, 9, 8, 8, 8, 7, 7, 7, 6, 6, 6, 5, 5, 5, 4, 4, 4, 3, 3, 3, 2, 2, 2, 1, 1, 1, 0, 0, 0]}
2) I did a printscreen of the all black image displayed on my image viewer into an editor and found out that the all black is not really all black. The colors are there: what should be white background is actually dark gray 14,14,14. And what should be black is dark gray 15,15,15.
Thus the problem is the palette when the color mode is black and white. Maybe you could hack the palette when you get the black and white images? Question is, is the white background always the 2nd color? And are bitmap ("black and white") images really always black and white, or can they be for example, red and blue? If there are no guarantees, the warning should remain even if the plugin hacks the palette (which I think it should, since it would be correct in most cases).
(What I don't get is where is the color mode in all this, since both have paletteInfo=256 and the only other change is the version...)
Hmm...
That is very strange. Could you send me the version 0 image? Also, paletteInfo = 256
, what does that mean? The value can only be 1 or 2 according to the spec (ie. no, there's no specification of color mode 0 or anything else, like 256).
The colors are there: what should be white background is actually dark gray 14,14,14. And what should be black is dark gray 15,15,15.
As you see from the palette, the values are as they are read from the palette (palette=[0, 0, 0, -1, -1, -1
means black and white, palette=[15, 15, 15, 14, 14, 14
are just two very dark grays).
Maybe you could hack the palette when you get the black and white images? Question is, is the white background always the 2nd color? And are bitmap ("black and white") images really always black and white, or can they be for example, red and blue?
The problem is, yes, they can actually be read and blue... (at least I have sample files like that). Then the question becomes, how do we know it's supposed to be black and white, when the palette clearly says something else?
PS: I have a similar problem trying to decode the CGA images in my sample files. How do we known when to switch into CGA-mode, instead of the supplied palette?
-- Harald K
Yes, that 256 value got me wondering as well, maybe I did something wrong? But I'll send you both pictures by email (since they belong to a customer).
But I think I finally found a website that explains PCX in a bit more detail: http://www.fysnet.net/pcxfile.htm
In particular, it says this:
Bts/pxl # planes Interpretation
---------------------------------------------------------------------------
1 1 monochrome
1 2 4 colors
1 3 8 colors
1 4 16 colors
2 1 4 colors using CGA header palette*
2 4 16 colors
4 1 16 colors using EGA header palette*
8 1 256 colors using palette at EOF
8 3 16.7mil colors
*If version 5, use palette at EOF, not palette in header
So, it seems CGA is to be used when you have a 4-color image with 2 bits per pixel in a single plane (1 channel?). However, if version is 5 (and that's our problematic images) the palette in the header is to be ignored.
I hope this helps you. And thanks a lot for maintaining the PCX plugin (I know the format is pretty much obsolete, but I still use a lot of PCX files for work).
Thanks,
I've seen (and read) that file, but I still don't understand.
How do I know if the 1 bit (monochrome) data should really be monochrome and not using the color in the palette? The above table just says monochrome, but I have test data that uses palette color. Please see the lena9.pcx
test file in the repository to see what I mean. This sample file currently displays as intended.
How do I know if I should use the CGA mode? I'm not talking about the palette in the header, but the CGA mode, which encodes one of 6 (or 8?) possible palettes + a background color into the first 4 bits of byte 0 and first 3 bits of byte 3 of the "palette" data. Please see the test data in the repository prefixed with CGA_
to understand what I mean. These samples does currently not display as intended (and cga-pcx.txt
is clearly in error).
All the sample files I have mentioned above are version 5, but does not contain a palette at EOF.
Best regards,
-- Harald K
Oh, I see what you mean, what a mess!
About the file lena9.pcx, it is a nice example of a bitmap that is not actually black and white (I had never seen one but suspected they might exist). Meew (my viewer using TwelveMonkeys plugins) displays it with a somewhat yellow-ish and red-ish colors. And I know that is correct because I get the same result with Legitronic (the labeling software I use). But the funny thing is that Photoshop Elements 2.0 incorrectly shows it as black and white. Urgh... I am starting to think this problem might be a Photoshop bug...
About the CGA images, both Legitronic and Photoshop Elements 2.0 display them incorrectly (white background and black foreground).
Oh and I also found this with a similar table: http://www.fileformat.info/format/pcx/egff.htm
So shouldn't the code become something like this in PCXHeader.java
:
// Add another field:
private byte[] paletteEOF;
public IndexColorModel getEGAPalette() {
if (channels == 1 && bitsPerPixel == 2) {
// This seems to be CGA for sure!!
return CGAColorModel.create(palette, bitsPerPixel);
}
if (channels == 1 && bitsPerPixel == 1) {
// We do not know if CGA or monochrome...
// I think it is better to assume monochrome.
// Let it continue...
}
int bits = channels * bitsPerPixel;
if (channels == 1 && version == 5) {
// Should use palette at EOF.
return new IndexColorModel(bits, Math.min(16, 1 << bits), paletteEOF, 0, false);
} else {
return new IndexColorModel(bits, Math.min(16, 1 << bits), palette, 0, false);
}
}
// And then while reading the header:
//...
header.hScreenSize = imageInput.readUnsignedShort();
header.vScreenSize = imageInput.readUnsignedShort();
if (header.channels == 1 && header.version == 5) {
// Read a 256-color palette of RGB values at the end of the file and is 768 bytes in
// length. So to find the palette, find the end of the file, and go back 768 bytes. The
// byte just before the palette has the value 10 (0Ah)
// The 256 color scheme (EOF palette scheme)
// Each piece of data (each pixel) takes one byte to represent the offset of the RGB color
// scheme to use. For example: If the pixel value is 0 (zero), then go to the front of the
// palette and use the first 3 bytes as the RGB values (respectively). If the pixel value is
// 1, then use the fourth, fifth, and sixth positions as the RGB values (respectively). etc.
byte[] paletteEOF = new byte[768];
imageInput.seek(imageInput.length() - 768);
imageInput.readFully(paletteEOF); // 256 RGB triplets
header.paletteEOF = paletteEOF;
}
imageInput.seek(PCX.HEADER_SIZE);
//...
I'm proposing the change so that the code is more readable, but please note that the code is completely untested and I have not that many experience in color models and and decoding files (and maybe you should see it as pseudo-code as I don't know if it's this easy to convert the EOF palette to a Java IndexColorModel)...
PS: Also, I'm not having that much free time to spend these days, I may not be able to reply in time and much less help more than this (if I had been helpful at all).
There's already code (in the PCXImageReader
) to read the 256 color palette at the end of the file. It's not in the PCXHeader
class, as the palette is not really in the header... You should also check the byte directly preceeding the (possible) palette. If this "magic" value isn't 12 (0x0C
), you shouldn't try to read this into a palette (it is just the end of the RLE data). In most cases, you'll find it's not there anyway...
Also, the header.getEGAPalette()
method isn't used, in the cases we expect a full 256 color (VGA) palette. But we might want to use the VGA palette instead of the EGA palette in some cases, if it is there.
Do you have one or more sample files that has 2, 4/ or 6 colors, and a real palette at EOF?
Anyway, I will push some changes later today, that fixes some of the issues.
PS: If you want to submit changes to the library, please do so by creating a PR. This will make sure the code compiles and passes all the tests before it is integrated (or not, in which case it will be rejected ;-) ).
-- Harald K
Another question, before you go... :-)
Do you have more Photoshop-created B/W sample images? I need some that I can use as test cases.
I'm thinking maybe we can detect the Photoshop palette case, using a heuristic. If the values are always 15, 14, 13, ..., 0, then we can ignore it. But it's only good if that is always the case with images written by Photoshop.
Less than ideal, but when the spec is broken and Adobe (as usual) do their own thing.. :-P
-- Harald K
Ah, sorry, I should have read everything more carefully. And I am quite new to github as well, so I still don't know how to do the things properly.
And now that you've mentioned and I tested some more, the problem may not be only with photoshop and in version 5 files. There are other problematic images (but they are all in monochrome color and they all display properly in photoshop and legitronic):
PCXHeader{version=3, compression=1, bitsPerPixel=1, width=26369, height=23809, hdpi=0, vdpi=0, channels=1, bytesPerLine=3328, paletteInfo=0, hScreenSize=0, vScreenSize=0, palette=[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]}
And these are indeed all black.
I think if you want to do a detection of this issue, the best way is getting a bit stupid and check if the first 2 colors in the palette are too close (or equal) to one another, like getting the sum of the absolute difference values between the two reds, two greens and blues. If too close (say, like less than 30 units apart), force the first color in the palette to be white and the second color to be black (but who knows if it's always in this order, we may have cases of inverted colors, but still better for the user to see an inverted image than seeing everything in the same color).
I'm sending you a few example images by email then.
With the latest changes, all your images are read fine.
Also see this StackOverflow post, if we're lucky we get some useful feedback. :-)
Closing this issue for now.
-- Harald K
I'd just like to report a couple of problems I found while reading PCX images (with version 3.3.2).
First is a problem where some images fail to load with an ArrayIndexOutOfBoundsException. Here is the stack (index varies depending on the image):
At this point, it seems to only happen in some (not all) images using 4bits Index Color (16 colors).
The second issue is with PCX images (again, only in some images, not all) that are in "Black and White" mode. The "white background" becomes black, resulting a totally black image.
I'll provide an image example for the second issue: CAV76A.zip
For the first issue, the images I have belong to a customer and I'd rather not put them on the net without authorization, but if needed, I can send it via email.
I'd love it if you could find a fix for these : )
Cheers and keep up with the excellent work!