twinside/juicy.pixels

Haskell library to load & save pictures

juicy.pixels's Issues

Segmentation Fault when decoding images

Hello,

I tracked down a segmentation fault (sometimes also "out: internal error: evacuate: strange closure type 34032", "out: internal error: scavenge_one: strange object 54383" or "out: internal error: RELEASE_LOCK: I do not own this lock: rts/Capability.c 431") in my program to the image decoding functions of JuicyPixels. It doesn't seem to be triggered by a specific image, but the crash happens after going through a few dozen to a hundred images, and can be reproduced with some basic code like this:

import qualified Codec.Picture as JP
import System.Directory

images <- getDirectoryContents "./images"
forM_ images $ \fn' -> do
    let fn = "./images/" ++ fn'
    di <- JP.readImage fn
    case di of
        Left err -> putStrLn $ fn ++ ": " ++ err
        Right i -> do putStrLn $ "Loaded " ++ fn

This is with JuicyPixels 3.1.1 from Hackage and the HP 2013.2.0.0 on OS X 10.6.

You can download the following set of images (they're random twitter user avatars) which seems to trigger the crash ~75% of the time for me:

http://www.blitzcode.net/stuff/img.zip

A fix would be very much appreciated, please let me know if there's something I can do to make it easier to narrow down.

Handle 16 bits PNG without depth reduction

Right now PNG 16 bits are automagically converted to 8bit depth. With the introduction of 16bits pixel, add a way to gather them.

Decoding tiff 16bit compressed images doesn't work

This is to reference the issue with decoding YA tiff images.

I played around with it a bit and this is what I have found:

• Problem only seems to affect compressed images. Unpacking an image (tiffcp -c none) seems to make the reader work.
• Playing around with compression options, by changing strip size changes the effects on loaded image (tiffcp -c xx -r xx). I was also able to make an YA8 image disappear (show as completely transparent) by some compression options, but I was unable to make it display striped, as with YA16.

I'm going now. I may get back to it tomorrow.

Bitmap not decoding properly

This may be because my code is wrong, as I'm very much a newbie, but I am finding that readImage is not correctly decoding an RGB8 bitmap file. Specifically, this one.

Is this my fault, or a genuine issue?

(Using 3.1.1, BTW)

A bug in 'packHuffmanTree' that will never arise in production

Good day, gentlemen!

There is a bug in 'packHuffmanTree' (Jpg/DefaultTable.hs:78)
aux (Branch i1 i2@(Leaf _)) idx = do

In a situation, where a tree is Left-biased (e.g. Branch (Branch (Leaf 1) (Leaf 2)) (Leaf 3), one should put an index of the beginning of i1, not of the end of it. Replace ix1 with idx + 2. The bug is reproduced in testPackHuffmanTree.hs (what is the best way to attach a file? Pull request?) Here is the gist.

Anyway, that bug will never arise in production, because buildHuffmanTree always returns Right-biased tree.

P.S. Here is the diff on gist (I think this is too small for a pull request)

TIFF support

TIFF is still used pretty frequently in scientific imaging (e.g. microscopy). It would be nice to have support for it.

Support for animated gif?

I was wondering if this is something that's planned?

Information query

It might be usefull to add an image information query to retrieve

• image size
• bit depth
• component count

without decompressing the image

A boo boo

https://github.com/Twinside/Juicy.Pixels/blob/master/Codec/Picture/Types.hs#L111

Nothing to handle GIF transparency.

My use case is composing a series of images including GIFs, but the issue with this is that JuicyPixels doesn't expose the transparency information that I can see. It would seem like it would've made more sense to have GIF as PixelRGBA8 rather than PixelRGB8.

No conversion support from RGBA8 (true color with alpha) to RGB8 (true color no alpha)

Please add support for ImageRGBA8 -> ImageRGB8. It is trivially implemented:

dropAlpha :: Vector Word8 -> Vector Word8
dropAlpha v = concat [rgb, v'] where
    rgb = take 3 v
    v' = dropAlpha (drop 4 v)

promotePixel :: Pixel8 -> Pixel16 is incorrect

I think the promotePixel function for 8bit to 16bit conversion is a bit off.

Currently it maps:

00 -> 0000
01 -> 0100
..
ff -> ff00

But 0xffff is the biggest 16-bit value. I think it should be

00 -> 0000
01 -> 0101
..
ff -> ffff

Bad PNG transparency loading

The image used in Twinside/Rasterific#15 doesn't good transparency information.

Next Hackage Release

I'm looking forward to the next hackage release of Juicy.Pixels so I created this ticket so we could track what progress towards that. I'll even help out where I can :)

If there is anything blocking the next release, please add it here.

Support for adding metadata when writing PNGs

I'm using your great library in my raytracing playground and by the nature of this I'm producing quite a few renderings on the go.It would be really nice if it was possible to add some metadata when writing out images, like the rendering algorithm / settings used, the time it took and so on. So it would be nice if it was possible to specify the metadata when writing out PNGs:

http://www.w3.org/TR/PNG/#11textinfo

HDR files written by JP can't be reliably displayed by OS X 10.9 preview

HDR images written JP cause all sorts of strange behavior with Maverick's preview application, like:

• Fully black rectangular image flat out crashes Preview (Ok, that one pretty much has to be at least partially bug in Preview)
• An image with an HDR photograph in it shows up black
• An image consisting of 6 different colors shows up with a darkened circular area in the center

I tried loading some of the files with some old C++ HDR loading code I've written. Some work, some don't, others crash the reading code.

This image crashes my (old, perhaps really wrong) reading code and shows up back in Preview:

http://www.blitzcode.net/stuff/scaling_test.hdr

I noticed messing with the exposure in Preview makes the upper half of the image show up, while the lower half seems truly black. Using JP's saveBmpImage instead of saveRadianceImage writes the image out correctly.

This image has six different regions each with a solid color. Loads fine with my HDR loading code, has a strange dark circular spot in the center in Preview:

http://www.blitzcode.net/stuff/scaling_test_2.hdr

Better error reporting

Firstly - thanks for the library.

The suggestion: rather than return Either String DynamicImage it would be better to have a custom data type for errors so that users could more accurately handle exceptions. This would also document what sort of exceptions can be thrown.

For example you could have a new data type

JPError = EncodeError String | DecodeError String | ...

and the signature would then be Either JPError DynamicImage

I can implement this if this kind of patch would be welcomed. If so, some guidance about the kinds of errors you would like to report would be appreciated.

Build Failure

I've attached the build log

https://gist.github.com/xavier83/11f98d538bc5b4e5863a

Loading this JPG causes exception

When I load this image:

I get the exception

*** Exception: .\Data\Vector\Generic\Mutable.hs:730 (update): index out of bounds (2,2)

Bug in `isColorCountBelow` in ColorQuant.hs

In the code below count should only be incremented when a new color is added to the
set, but we are incrementing every time through the loop, which means we almost always return False. This actually causes problems when making GIFs with diagrams since they often have less than 256 colors. A fix is below which i think is fine since Set.size is O(1)

isColorCountBelow :: Int -> Image PixelRGB8 -> (Set.Set PixelRGB8, Bool)
isColorCountBelow maxColorCount img = go 0 0 Set.empty
  where rawData = imageData img
        maxIndex = VS.length rawData

        go !count !idx !allColors
            | count > maxColorCount = (Set.empty, False)
            | idx >= maxIndex - 2 = (allColors, True)
            | otherwise = go (count + 1) (idx + 3)
                        $ Set.insert px allColors
                where px = unsafePixelAt rawData idx 

| otherwise = go ((Set.size allColors) + 1) (idx + 3)
                        $ Set.insert px allColors
                where px = unsafePixelAt rawData idx

Document assumptions about color spaces

Currently, there is no indication of color space at all. For example, when you convert from CMYK to RGB, is the result linear or in sRGB? Does the conversion from RGB to CMYK expect linear or sRGB? When you load an RGB image, does it simply preserve whatever color space the image was in? The list of questions goes on. At minimum, I think the documentation should be clear about these things.

A further enhancement would be to have some control over these things.

add affine transform, etc

especially resizing. i recommend binding into gd/imagemagick (see discussion http://stackoverflow.com/questions/15352306/resize-pngs-in-haskell-on-windows) if possible, but only if these can be easily built on windows (i couldn't figure it out).

if it has to be written from scratch, here's a start:
https://gist.github.com/eflister/5456125

Performance issues in the withImage function

http://thread.gmane.org/gmane.comp.lang.haskell.cafe/109984/focus=110065

"Dunno"

Decoding this image throws a "Dunno" exception

transformers 0.3

hi, could you update JuicyPixels (probably only the cabal file) so that it can use transformers 0.3? mtl 2.1 depends on it, and many users probably have that version already installed.

New Release

Hi.

Is there any way to get you to release a new version of JuicyPixels? I'm using it in a game, and the imageData being a Vector is very important to me. Also, building off trunk just seems wrong when we can have cabal do it for us.

Thanks,

• clark

Png decoding has awfull space leak

Gif support

I created a haskell gif parser and writer a while back: https://bitbucket.org/nudded/gif-parser/

I thought you might be interested. It should be able to read any GIF87a file (and most of the GIF89a files). But the lzw code is there (it is quite hard to cover all the edge cases).

I'm having exams atm, so i will be looking into this after them.

Add better support for 32bits grayscale Tiff

Avoiding double compilation of the library (minor .cabal modification)

Hi!

You can move your executable file in a separate directory and include your library as a dependency of imageTest, so that main.hs will not require to rebuild the whole library (there is perhaps a reason for this, I apologise if so). This requires a minor modification of the .cabal file:

 Executable imageTest
   Default-Language: Haskell2010
+  Hs-Source-Dirs: exec-src
   Main-Is: main.hs

-  Build-depends: base                >= 4       && < 5,
+  Build-depends: JuicyPixels,
+                 base                >= 4       && < 5,
                  bytestring          >= 0.9     && < 0.11,

There is an answer at StackOverflow that explains the trick in more details.

Can't unlong TagSampleFormat in decodeTiff

My issue is either with the error message, or with the library not unlonging TagSampleFormat. I cannot tell from the error message.

I can provide an example file if it helps.

Tiff writer does not specify ExtraSamples tag for alpha channel

According to TIFF6 specification, if there are extra samples (values) in pixels, an ExtraSamples tag is required to specify their meaning.

ExtraSamples
...
The default is no extra samples. This field must be present if there are extra
samples.

I believe it should specify Unassociated alpha data (value 2) i.e. non-premultiplied alpha.

scaleQuantisationMatrix (`div` 0)

scaleQuantisationMatrix in /Codec/Picture/Jpg/DefaultTable.hs will throw a runtime error when called with a quality of 0. Wouldn't it be nicer to wrap the result in a Maybe, returning Nothing when quality = 0, and Just (the normal answer) otherwise?

This function is used by encodeJpegAtQuality, which should be modified to cope with this were scaleQuantisationMatrix to be changed.

Jpeg decoding is dog slow

Pixel1

Hi,

it seems that it would make sense for this library to also support bitmap pictures.

Greetings,
Joachim

monadic image creation and better support for IO/ST polymorphism

I've been using your excellent JuicyPixels package for a ray tracer that I'm writing.

It occurred to me recently that the native JuicyPixels API could do a better job of supporting monadic image creation and possibly it could do a better job of working with IO or ST monads.

Monadic generateImage

Here is a utility function I wrote so that I could construct an image monadicly:
dagit/haray@0863dc1#L0R25

The type is:

withImage :: Int -> Int -> (Int -> Int -> ST s PixelRGB8) -> ST s (Image PixelRGB8)

Compare that with:

generateImage :: forall a. (Pixel a)
              => (Int -> Int -> a)  -- ^ Generating function, with `x` and `y` params.
              -> Int        -- ^ Width in pixels
              -> Int        -- ^ Height in pixels
              -> Image a

Note that I've changed the order of the arguments because I think withImage supports currying/partial application a bit better. On the other hand, I've fixed the pixel type and you'd probably want to generalize that to a if you actually include this in the library. I think there are some places internal to JuicyPixels that could use the monadic version of generateImage. For example, the following places:

• https://github.com/Twinside/Juicy.Pixels/blob/master/Codec/Picture/Bitmap.hs#L232
• https://github.com/Twinside/Juicy.Pixels/blob/master/Codec/Picture/Png.hs#L356

Pretty much anywhere you allocate a MutableImage in ST and then fill it with pixels with a monadic operation :)

PrimMonad

The other thing I want to suggest is related to the use of ST everywhere. Frequently when I use your library I'm already in IO and I need to add stToIO on all the mutable image operations. If you use the PrimMonad from the primitive package, then the operations would be polymorphic over IO and ST. See the mwc-random and vector packages for examples. Basically, when you use it, you write things like:

foo :: PrimMonad m => MutableImage (PrimState m) PixelRGB8

The primitive package introduces a type class PrimMonad which has ST and IO as instances and a type level function, PrimState, that takes either ST or IO and maps them to the corresponding type (IO maps to RealWorld and ST maps to s).

Making this change would make it so that stToIO is no longer needed as those operations are either ST or IO as determined by the application code. The mwc-random and vector packages both use this trick.

If you're interested in these changes, I'd be happy to take a stab at implementing them.

Thanks!

PNG alpha decoding problem

Decoding megaman.png results in ImageRGB8. Decoding megaman2.png results in ImageRGBA8. I think they both have alpha information.

Removing Pixel instance repetition

Hi! First of all, thank you for the library, it's a great help.

I've been using your library, and at one point I needed a Pixel type that held 3 Doubles, since I wanted to keep the precision consistent with the rest of my program. I found that I could create my own custom instances for PixelD (aka Double), as well as PixelRGBD, but I was basically copy-pasting code from inside Codec.Picture.Types . I feel that a lot of this repetition can be removed from within the library, and automatically generalize to a lot of other types, like Double without the user having to do anything.

The suggestion comes down to "subclassing" instances. To give an intuition:

instance (Eq a, Storable a, Num a) => Pixel a where
    type PixelBaseComponent a = a

    -- etc...

There's only one instance required for all existing single-value pixels, both for types used in the library, and those that may be needed by users. Of course, the above code is questionable, as it leads to UndecidableInstances. One solution is a newtype wrapper:

-- a no-cost wrapper to 'tag' types as pixels
newtype Pix a = Pix a
    deriving (Eq)

instance (Eq a, Storable a, Num a) => Pixel (Pix a) where
    type PixelBaseComponent (Pix a) = a

    -- etc... but with more wrapping/unwrapping

This is ideal from an implementation standpoint, but is probably bad from the user's standpoint, as all their single-value pixels come wrapped in a newtype. Backwards compatibility goes out the window.

For multi-value pixels (e.g. RGBF), one can use a fixed-size vector library, that provides the vector size statically. Instances can then be generically created for many of the existing pixel formats. I don't know if there is a de-facto fixed-vector library that everyone goes to, because it seems many libraries recreate this functionality constantly.

A side-effect of the above approach is that vectors of size 1 effectively serve the purpose of a newtype wrapper too. So there are many approaches.

In summary:

• Generic Pixel instance for (Eq a, Storable a, Num a) types.
  • Pros: Very generic, works with current types
  • Cons: UndecidableInstances
• Above but with a newtype wrapper to keep the typesystem happy.
  • Pros: Guaranteed to play nice with the typesystem
  • Cons: Annoying wrapper around single-value pixels
• Generic Pixel instance for fixed-size vectors of single pixels.
  • Pros: More genericity/extensibility
  • Cons: Still have to differentiate pixels that have the same number of components but mean different things. (e.g. PixelRGB8 vs PixelYCbCr8). This could be dealt with newtype wrappers, or introduce the concept of color spaces.

You may have considered these possibilities already, and ruled them out, but I just wanted to pitch in a few suggestions. If you feel this is worthy of an investigation, I could try and experiment with these ideas on a separate branch.

incorrect transparent gif handling

JuicyPixels seems to leave old frames in transparent areas of GIF animations despite the correct interpretation of the disposal field of the GIF specifying otherwise. Compare the JuicyPixels-decoded contents of frames after the first with the result in a viewing program such as a browser.

This image demonstrates the problem.

add support for png compression?

Hey,
I'm noticing that the png encoder doesn't (as yet) do the line based filter type compression.

a) would code that enables support for richer png compression be something you'd welcome into the code base?

(at some point in the coming months, i'm planning on adding some sort of png raster backend for diagrams, and vector graphics are a class of images that png can compress really well, so its a natural choice)

b) since i'll be playing with writing lots of compression related algs over the coming months, theres a small chance I might get around to doing this myself (though no guarantees)

Restrictive lower bound on Cabal

The .cabal file specifies that Cabal >= 1.16 is required. Does JuicyPixels really rely on features that were introduced in 1.16? (I don't see any, just glancing at the .cabal file; I'm guessing 1.10 suffices.) This makes it hard to build JuicyPixels on older platforms that have e.g. Cabal-1.14.

Incorect behaviour of extractComponent

Reported by Joel Bo Svensson

Hi.

Today I tried to add a few image functions to a library of mine and decided to use your juicy pixels
(Which seems great). But I do not get the behaviour I expected from extractComponent. Ill illustrate in code:

loadBMP_DBG fp = 
    withFile fp ReadMode $ \handle -> do 
    bs <- BS.hGetContents handle 
    case decodeBitmap bs of 
        (Left str) -> error str 
        (Right (ImageRGB8 img)) ->
            do 
            let red   = extractComponent 0 img
                green = extractComponent 1 img
                blue  = extractComponent 2 img 
                (Image _ _ vr)    = red 
                (Image _ _ vg)    = green
                (Image _ _ vb)    = blue 
            putStrLn $ show vr
            putStrLn $ show vg 
            putStrLn $ show vb

The picture I use is attached, it is just an image filled with the same color (10,20,30). So I expected
that show vr would yield [10,10,10,...
and vg [20,20,20....
and vb [30,30,30...

instead i get [10,30,20,10,30,20.. for vb
and [20,10,30,20,10,30... for vg
and [30,20,10,30,20,10... for vb

Support interleaved JPEG

Might be on the rise in the future, better support them.

codec error at image save (*** Exception: Prelude.(!!): negative index)

got this error: "*** Exception: Prelude.(!!): negative index" saving this image:
http://0103.static.prezi.com.s3.amazonaws.com/media/4/9/a/7327e1860a1097d550f5eda351acc02c98ef0.jpeg
using transcodeToPng function

transcodeToPng :: FilePath -> FilePath -> IO ()
transcodeToPng pathIn pathOut = do
eitherImg <- readImage pathIn
case eitherImg of
Left _ -> return ()
Right img -> savePngImage pathOut img

Upper bound for binary?

Hi!

I'm interested to know if the current upper bound on the binary dependency (currently 0.7.2) can be raised to make this library compile with other packages that depend on newer versions of binary.

Thanks!

Failure reading gif

Juicy pixels fails reading the following gif file (found in the wild on the web): http://commonmeasure.org/~jkr/rId98.gif

Prelude> import Codec.Picture
Prelude Codec.Picture> Left msg <- readImage "rId98.gif" 
Prelude Codec.Picture> putStrLn msg
Cannot load file
Jpeg Invalid marker used
PNG Invalid PNG file, signature broken
Bitmap Invalid Bitmap magic identifier
GIF Unrecognized gif block 32
HDR Invalid radiance file signature
Tiff Invalid endian tag value

Prelude Codec.Picture> 

If we convert the problematic gif to a gif with ImageMagick (convert rId98.gif foo.gif), JP reads it fine.

Handle BMP palettes

It would be great if Juicy Pixels can read and write BMP images that uses palettes.

Thank you.

Color component order is inconsistent YCbCr between comments and code

Description of PixelYCbCr type states that the color components are placed in Y Cr Cb order, which seems to reverse Cb and Cr as opposed to other pixel types that put them in the same order as listed in their name.
Looking at some of the code, the color components seem to be used as YCbCr in that order, not YCrCb as stated in documentation.

index bug in inverse discrete cosine transform

iclip is declared like this:

iclip :: UArray Int Int16
iclip = listArray (-512, 512) [ val i| i <- [(-512) .. 511]]
    where val i | i < (-256) = -256
                | i > 255     = 255
                | otherwise   = i

Two bugs:

• bounds should be listArray (-512, 511)
• Specifying the range this way means that you should now be able to index directly into this array instead of adding 512 in the definition of (!!!). I bet this didn't come up in testing due to the use of unsafe index function in the definition of (!!!).

I'm currently doing a refactor to use Data.Vector.Storable instead of Arrays. I've fixed this in my branch, would you like me to submit a patch for this separate from my refactor to use Data.Vector? (I don't know if you are interested in my Data.Vector work).

DynamicImage to DynamicImage map function

The library already has a dynamicMap function but it just goes to some arbitary a. I think adding a map function from DynamicImage to DynamicImage may be helpful. Example of the function:

dynMap :: (forall pixel . (Pixel pixel) => Image pixel -> Image pixel)            
           -> DynamicImage -> DynamicImage                                        
dynMap f (ImageY8    i) = ImageY8 (f i)                                           
dynMap f (ImageY16   i) = ImageY16 (f i)                                          
dynMap f (ImageYF    i) = ImageYF (f i)                                           
dynMap f (ImageYA8   i) = ImageYA8 (f i)                                          
dynMap f (ImageYA16  i) = ImageYA16 (f i)                                         
dynMap f (ImageRGB8  i) = ImageRGB8 (f i)                                         
dynMap f (ImageRGB16 i) = ImageRGB16 (f i)                                        
dynMap f (ImageRGBF  i) = ImageRGBF (f i)                                         
dynMap f (ImageRGBA8 i) = ImageRGBA8 (f i)                                        
dynMap f (ImageRGBA16 i) = ImageRGBA16 (f i)                                      
dynMap f (ImageYCbCr8 i) = ImageYCbCr8 (f i)                                      
dynMap f (ImageCMYK8 i) = ImageCMYK8 (f i)                                        
dynMap f (ImageCMYK16 i) = ImageCMYK16 (f i)

use case

dynSquare :: DynamicImage -> DynamicImage                                         
dynSquare = dynMap squareImage                                                    

squareImage :: Pixel a => Image a -> Image a                                      
squareImage img = generateImage (\x y -> pixelAt img x y) edge edge               
   where edge = min (imageWidth img) (imageHeight img) 

This way I can make a n by n image easily without changing or even dealing with the pixel type.