117983 - libpr0n has only minimal support for ppm images

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Mark Wilkinson]

libpr0n contains a simple image decoder for one particular type of ppm image
(type P6; 24-bit images in raw format), but doesn't support the companion bitmap
and greymap formats or the (admitedly wasteful) plain format. It is also very
picky about the format of the image header (for example, it won't handle comments).

If the code is intended to be useful then it should handle the full range of
pbm, pgm and ppm image formats correctly.

I have some code that I've been working on to improve things; I'll attach it
shortly.

Comment 1

[Mark Wilkinson]

Attachment: pbm, pgm, ppm support v1

Here's the code I've been working on. It supports both plain and raw pbm, pgm
and ppm files (well, it works for my test cases, anyway!)

Things I still want to do:

1. There's a performance anomaly: mozilla seems to create lots of new threads
when using memcpy to move larger chunks of data from raw PPM images, but only
when I'm running it under gdb.

2. Tidy up: factor the main chunks into separate private functions.

3. Either implement support for PGM and PPM files with maximum values other
than 255, or fail on them. I've never seen these files other than in the Unix
man pages, so there aren't many test cases.

Comment 2

[Mark Wilkinson]

The X-Face support I'm working on uses raw pbm as the intermediate format for
the dynamic images it generates.

Comment 3

[Terri Preston]

Mark, would you attach your testcases or let me know where I can find them?  
Thanks :-)

Comment 4

[Mark Wilkinson]

Attachment: test case images

Here are some test cases that I've used. The plain PPM files are too large to
upload, so I've uploaded JPEG versions along with a GNU make Makefile which
will generate the various PBM, PGM and PPM versions. There's also a set of HTML
files to load the various images in each format, because mozilla won't open PPM
files directly at the moment (it only renders them if they are in an IMG
element or similar).

I need to come up with some smaller images with more variation in use of
whitespace and comments to verify that the parser functions correctly.

Comment 5

[Mark Wilkinson]

Forgot to mention: the Makefile was written using netpbm-9.14 as shipped with
Red Hat Linux 7.2. It won't work with earlier versions of the pbm tools.

Comment 6

[Henrik Gemal]

http://gemal.dk/browserspy/images.html tests the image format support of your
browser including PBM, PGM and PPM files

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

this really belongs to imagelib

Comment 8

[Stuart Parmenter]

taking bug

Comment 9

[Stuart Parmenter]

Attachment: updated 1.0 patch

Comment 10

[Stuart Parmenter]

Attachment: Original patch reapplied to newer files

just an updated version since the original patch didn't want to apply to the
current 1.0 branch or the trunk nicely.

Comment 11

[Stuart Parmenter]

Comment on attachment 95033 [details] [diff] [review]
Original patch reapplied to newer files

r=pavlov on Mark's patch

Comment 12

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 95033 [details] [diff] [review]
Original patch reapplied to newer files

sr=[e-mail]

Comment 13

[Stuart Parmenter]

checked in to the trunk

Comment 14

[Adam D. Moss]

I have to wonder why we care about supporting PNM fully; it sounds
rather like bloat to me.

I assume that the previous PPM support was for a very specific
internal use just as PBM support for XFace headers would be.
BMP support is (regrettably) in because Internet Explorer supports
them and some FrontPage-using marching morons think that raw 24bit
images are just swell as a web-embedded image format.

However, no-one embeds PNM files.  No-one should want to.  Really.
PBM support I'll grant as justifyable as internally useful for XFace,
but I am reserved about supporting the sweeping gamut of PNM oddness
(entirely because of code size  concerns since I don't think that anyone
who knows what PNM files are is going to inline them in web pages;
if mozilla grows by 1K then I can't complain, but there are people
out there in other bugs who work very hard to shrink footprint by
just 10K or 20K, and then features like this fill the resulting
vacuum).

Comment 15

[Stuart Parmenter]

i'm using it for a few specific things i'm working on.  if embedders don't want
the decoder then don't ship the library.