Png's are encoding wrong after 1.0.0-alpha-000047

[KLuuKer]

png's are encoded incorrectly on version v1.0.0-alpha-000053 and higher (including 57)

last know good version is v1.0.0-alpha-000052
so i think something went wrong in this commit f814dc3

k so after version v1.0.0-alpha-000053 the error's get really obvious (weird colored blocks)
but before that some images are still encoded incorrectly (getting chopped off) and smaller resize actions turning into black images

currently checking how far back this starts occuring

[KLuuKer]

the actual last known good version is 1.0.0-alpha-000047
so the real problem starts on version v1.0.0-alpha-000048
which would be this commit 52cddd3

i have a large batch of images and on version 48 and up some of them are not getting processed all the way
for example it would stop encoding part way trough the image
and images i make into small thumbnails end up getting completely black

[JimBobSquarePants]

@KLuuKer Thanks for all the extra info that was really useful! Turns out I'd moved a line inside the using statement for the deflater stream. 1.0.0-alpha-000062 work fine now for me.

52cddd3#diff-c61aac0b8f9e954e0dae0ff548a8b063R603

[KLuuKer]

bad news everyone.... :(

seems it's not completely fixed (although its allot better now)

there is a small amount of images that have the same "it just stop encoding parts" or weird block parts or other encodings issues

i only get some issues for some resolutions tough, so maybe it's not the pngencoder but some other resizing stuff

i will check\try some more stuff later, but i have todo some other stuff first

using (var thisIsMyOutputData = new MemoryStream())
{
    // yes its a normal memorystream and not something weird
    ImageResizer.Resize(img.Blob, thisIsMyOutputData, true, 300, 300)
}

internal static void Resize(byte[] image, Stream outputStream, bool resize, int newWidth, int newHeight)
        {
            using (var stream = new MemoryStream(image, false))
            {
                Image<Color, uint> img = new Image(stream);
                {
                    if (resize && (img.Width > newWidth || img.Height > newHeight))
                    {
                        if (img.Width > img.Height)
                        {
                            newHeight = (int)(img.Height / (img.Width / (double)newWidth));
                        }
                        else
                        {
                            newWidth = (int)(img.Width / (img.Height / (double)newHeight));
                        }

                        img = img.Resize(newWidth, newHeight, new Lanczos3Resampler(), true);
                    }

                    img.ExifProfile = null; // i have no idea what you are talking about....

                    if (img.Properties != null && img.Properties.Count > 0)
                    {
                        img.Properties.Clear();
                    }

                    var encoder = new PngEncoder()
                    {
                        Quality = int.MaxValue,
                        CompressionLevel = 9,
                    };

                    encoder.Encode(img, outputStream);
                }
            }
        }

[dlemstra]

@KLuuKer Making a small sidestep here... Why are you not setting the ExifProfile to null to remove the profile? I do think I still need to add a RemoveInvalidTags() and a Clear() method to the class.

[KLuuKer]

@dlemstra wouldn't it be better if it didn't get loaded in the first place? :)
Didn't change anything tough, still have the same issue.

[KLuuKer]

the perpetrator in question (this is a jpeg, yes i know i shouldn't turn it into a png but it should still get converted correctly)

even when skipping the resize it is not correct

some other sizes

[KLuuKer]

hmmmm, i think all of the images that are encoded incorrectly are jpeg files

[JimBobSquarePants]

Odd @KLuuKer... I'm not seeing this in my tests.

Here's the image encoded as a png

And here is it resized as a png using the Lancsoz3 sampler

BTW that whole code could be written in a much more concise manner...

// Resizing with one dimensions will automatically preserve the aspect ratio.
// Png doesn't preserve EXIF data so you don't need to remove it.
new Image(stream)
.Resize(newWidth, 0, new Lanczos3Resampler(), true)
.SaveAsPng(output);

@dlemstra Could you also dl the image and test on your machine? I want to make sure there is nothing odd going on.

[KLuuKer]

@JimBobSquarePants even when using your code, i get errors

  internal static void Resize(byte[] image, Stream outputStream, bool resize, int newWidth, int newHeight)
        {
            using (var stream = new MemoryStream(image, false))
            {
                new Image(stream)
                    .Resize(newWidth, 0, new Lanczos3Resampler(), true)
                    .SaveAsPng(outputStream);
            }
        }

i must admit I'm running this with multiple images simultaneous about 80 different webrequest (our product index page), so this could be a concurrency issue

[KLuuKer]

i made sure all image's are the same, and they have all different kinds of encoding issues

image = File.ReadAllBytes("d:\\temp\\resize-me.jpg");

this is what i get when i have muliple encodings running, some are incorrect in different ways, some incorrect in the same way, some are just fine

[JimBobSquarePants]

Ah... now that is interesting. Everything here should be threadsafe but maybe it isn't.

And it's definitely fine before the commit at the top of the thread? And only happens when encoding png files?

If so, The only real change I have made to the png encoder is using is an ArrayPool<T> to reduce allocation but that is supposed to be threadsafe also.

[KLuuKer]

we can ignore the resize code, i commented that out and its still wrong, gonna try outputting jpegs

jpges output just fine (with version 64)

[JimBobSquarePants]

Ok... Can you do me a huge favour please and also test gif and bitmap. I want to narrow it down to the png encoder.

[KLuuKer]

version 47 png output is definitely fine

version 64 bmp output is fine (but damn they are huge :)
version 64 gifs have such great pallet support

[JimBobSquarePants]

Thanks for all that... Yeah bmps are uncompressed, they'll be massive.
The quantizer is pretty good for gifs, I need to optimise it yet though, far too slow.

The encoder it is then. I'll knock up a parallel test and keep at it.

[KLuuKer]

my simple console app i made just now (same issue)

Parallel.For(1, 50, (i) =>
            {
                var image = File.ReadAllBytes("resize-me.jpg");
                using (var outputStream = new FileStream("output\\" + i + ".png", FileMode.Create))
                {
                    using (var stream = new MemoryStream(image, false))
                    {
                        new Image(stream)
                            .SaveAsPng(outputStream);
                    }
                }
            });

[KLuuKer]

so i tested 47-51 with the code above and they are fine
but on verion 52 (commit bdb53d8) i get this (the top is wrong for all but the last image in the for loop)

[JimBobSquarePants]

Give that push a go please. I was able to recreate the issue with a test and fix it. Bonus was a 20% memory usage reduction.

[KLuuKer]

had to wait for the package to get pushed :)
so with the butterfly image in the 50 parallel console app test it works fine,
but when i run the entire product page against it i still get some encoding errors

are you aware of ArrayPool.Shared documentation?

It maintains arrays of multiple sizes, and may hand back a larger array than was actually requested, but will never hand back a smaller array than was requested.

[JimBobSquarePants]

Sorry, didn't mean to close that.

Yeah, I've read that, if you look I'm only ever reading the bytesPerScanline value length from each buffer. I was just doing some debugging in parallel experimenting with something else though and sometimes that value wasn't correct.

I can't for the life of me see how that would happen though, all the variables are private instance ones.

It's after 1am here so I won't be able to do anything more tonight I'm afraid. I'll have another look when I'm more fresh.

[KLuuKer]

Go take your well deserved sleep, you have a great project I'm leeching off.
I just wont update production :)

[JimBobSquarePants]

Once more unto the breach, dear friends, once more...

[KLuuKer]

Seems to work fine for me

o wait you already did 😄

tested with 1.0.0-alpha-000075