slyrus / opticl

An image processing library for Common Lisp
Other
182 stars 35 forks source link

Missing type declaration for jpeg pixel array #12

Closed varjagg closed 8 years ago

varjagg commented 8 years ago

In line 71 of jpeg.lisp should be

(let ((jpeg-array (make-array (* height width +ncomp-rgb+) :element-type '(unsigned-byte 8))))
varjagg commented 8 years ago

seems to botch up encoding otherwise

slyrus commented 8 years ago

thanks for the fix but do you have some code that exercises this we could add to the tests?

slyrus commented 8 years ago

And should this fix be applied to the other make-array calls in write-jpeg-stream? I'm curious why I didn't run into this. Is it some sort of default encoding thing?

varjagg commented 8 years ago

a straight recode like

(opticl:write-image-file "/tmp/test.jpg" (opticl:read-image-file "1.jpg"))

would fail producing mangled image, while direct invocation of cl-jpeg encoder/decoder would not. Not sure how to put that into automated testcase.

It's quite possible that things worked properly with early versions of cl-jpeg. But after a number of bug reports and performance improvement requests much of the library was revamped in its type declarations and assumptions. As the majority of users employ decode part typically, the encoder bugs get caught last..

And yes, it's a good idea to keep all incoming arrays as 8 bit uint elements. I'm in process of extending cl-jpeg to handle reuse of buffers and standard jpeg buffer allocation function which might simplify things, but it'll take some time until it gets through QL pipeline.

slyrus commented 8 years ago

Should be fixed now!