pngload_source: IDAT stream error when using sourceImage.ThumbnailImage()

[majora2007]

I switched up some of my code that creates a thumbnail, to instead first create an Image from Image.NewFromStream(stream) so that I can get some width/height to identify how to scale the image beforehand. However, after this, some images are now throwing an exception:

NetVips.VipsException: unable to call VipsForeignSaveSpngFile
pngload_source: IDAT stream error

I can't find much information about this error nor do I see anything wrong with the code. It works for most images (but not from PDF streams).

The only thing that MIGHT be the issue is that this stream I'm using is a RecyclableMemoryStream, whereas my other image flows that pass through this code use Streams from archives or raw images.

Any insight is appreciated.

NetVips: 2.4.1
NetVips.Native: 8.15.2

Here's the code:

public string WriteCoverThumbnail(Stream stream, string fileName, string outputDirectory, EncodeFormat encodeFormat, CoverImageSize size = CoverImageSize.Default)
    {
        var (targetWidth, targetHeight) = size.GetDimensions();
        if (stream.CanSeek) stream.Position = 0;
        using var sourceImage = Image.NewFromStream(stream);
        if (stream.CanSeek) stream.Position = 0;

        using var thumbnail = sourceImage.ThumbnailImage(targetWidth, targetHeight,
            size: GetSizeForDimensions(sourceImage, targetWidth, targetHeight),
            crop: GetCropForDimensions(sourceImage, targetWidth, targetHeight));

        var filename = fileName + encodeFormat.GetExtension();
        _directoryService.ExistOrCreate(outputDirectory);
        try
        {
            _directoryService.FileSystem.File.Delete(_directoryService.FileSystem.Path.Join(outputDirectory, filename));
        } catch (Exception) {/* Swallow exception */}

        thumbnail.WriteToFile(_directoryService.FileSystem.Path.Join(outputDirectory, filename));
        return filename;
    }
    public static Enums.Size GetSizeForDimensions(Image image, int targetWidth, int targetHeight)
    {
        try
        {
            if (WillScaleWell(image, targetWidth, targetHeight) || IsLikelyWideImage(image.Width, image.Height))
            {
                return Enums.Size.Force;
            }
        }
        catch (Exception)
        {
            /* Swallow */
        }

        return Enums.Size.Both;
    }

    public static Enums.Interesting? GetCropForDimensions(Image image, int targetWidth, int targetHeight)
    {
        try
        {
            if (WillScaleWell(image, targetWidth, targetHeight) || IsLikelyWideImage(image.Width, image.Height))
            {
                return null;
            }
        } catch (Exception)
        {
            /* Swallow */
            return null;
        }

        return Enums.Interesting.Attention;
    }

[kleisauke]

Are you able to provide a complete, standalone code sample that allows someone else to reproduce this issue?

Note that I'd avoid ThumbnailImage, if possible. It can't do any of the shrink-on-load tricks, so it's much, much slower.

[majora2007]

Just threw something up, but having trouble reproducing, so will need to augment with RecycleMemoryStream.

Instead of ThumbnailImage, what would you recommend instead?

[kleisauke]

Instead of ThumbnailImage, what would you recommend instead?

I recommend using the static Image.Thumbnail* functions, which accept a file, stream, or buffer as input, see:
#97 (comment)

Additionally, you can combine these functions with the various Image.NewFrom* functions to determine the necessary scale. Since libvips operates lazily, it only reads the image header. Any pixel decoding occurs much later, only when the pixel values are actually needed.

[majora2007]

Here you go. It wasn't the memory stream at all.

NetVipsPngLoadIssue.zip

Let me try and refactor my code. I had no idea I was doing it poorly this whole time.

[kleisauke]

Thanks, I was able to reproduce this. Here's a simpler reproducer:

using var stream = File.OpenRead("images/PNG_transparency_demonstration_1.png");
using var image = Image.NewFromStream(stream, access: Enums.Access.Sequential); // Header phase

// Rewind the stream
stream.Position = 0;

// libvips is "lazy" and will not process pixels
// until you write to an output file, buffer or memory
_ = image.CopyMemory(); // Load phase

The issue arises because the libspng loader in libvips reads a few bytes during the header phase and assumes the stream remains at that position. If you rewind the stream during the header and load phases, the wrong bytes are read, causing an exception to be thrown.

A simple fix is to ensure the stream is not rewound during these two stages. For example:

@@ -11,7 +11,6 @@ string? WriteCoverThumbnail(Stream stream, string fileName, string outputDirecto
     var targetHeight = 455;
     if (stream.CanSeek) stream.Position = 0;
     using var sourceImage = Image.NewFromStream(stream);
-    if (stream.CanSeek) stream.Position = 0;
 
     var scalingSize = GetSizeForDimensions(sourceImage, targetWidth, targetHeight);
     var scalingCrop = GetCropForDimensions(sourceImage, targetWidth, targetHeight);

@jcupitt Do you think this might be worth fixing in libvips? For example, we could do the following:

--- a/libvips/foreign/spngload.c
+++ b/libvips/foreign/spngload.c
@@ -581,7 +581,8 @@ vips_foreign_load_png_load(VipsForeignLoad *load)
 	enum spng_decode_flags flags;
 	int error;
 
-	if (vips_source_decode(png->source))
+	if (vips_source_seek(png->source, 0, SEEK_CUR) < 0 ||
+		vips_source_decode(png->source))
 		return -1;
 
 	/* Decode transparency, if available.

(untested)

i.e. that will ensure the stream is at the correct position during the ->load phase.

[jcupitt]

Hello @majora2007 and @kleisauke,

Yes, seeking a stream behind libvips's back will cause terrible problems. I think most loaders will break if you do this, won't they?

I'm not sure I understand the use case here. Why do you need to seek a stream that libvips is still reading from?

*nix lets you do eg.:

int new_fd = dup(fd);
VipsImage *image = vips_image_new_from_fd(new_fd);

Now the file descriptor is private to libvips, has its own read position, and no one can change it. Does C# let you dup streams? Maybe something like that could help?

[majora2007]

I actually didn't need to rewind the stream after the fact, I coded that as a safety precaution, as I've hit this type of error many times with streams. I have updated my code with Kleis' suggestion and will be testing this week to confirm that removing the rewinding works.

[majora2007]

I actually didn't need to rewind the stream after the fact, I coded that as a safety precaution, as I've hit this type of error many times with streams. I have updated my code with Kleis' suggestion and it worked fine. Thanks again for the support and this great library.