Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

PngEncoder ignores FilterMethod #2771

Closed
4 tasks done
melnikov77 opened this issue Jul 13, 2024 · 2 comments
Closed
4 tasks done

PngEncoder ignores FilterMethod #2771

melnikov77 opened this issue Jul 13, 2024 · 2 comments

Comments

@melnikov77
Copy link

melnikov77 commented Jul 13, 2024

Prerequisites

  • I have written a descriptive issue title
  • I have verified that I am running the latest version of ImageSharp
  • I have verified if the problem exist in both DEBUG and RELEASE mode
  • I have searched open and closed issues to ensure it has not already been reported

ImageSharp version

3.1.4

Other ImageSharp packages and versions

NA

Environment (Operating system, version and so on)

NA

.NET Framework version

net8

Description

When a new PngEncoder is created with custom FilterMethod it always produces the same file size,
i.e. the FilterMethod is being ignored.

Example:

var encoder = new PngEncoder
{
    FilterMethod = PngFilterMethod.Paeth,
};

I found only one usage of this property here

if (!encoder.FilterMethod.HasValue)
{
// Specification recommends default filter method None for paletted images and Paeth for others.
this.filterMethod = this.colorType is PngColorType.Palette ? PngFilterMethod.None : PngFilterMethod.Paeth;
}

i.e. filterMethod is assigned only if encoder.FilterMethod is null,
otherwise, it is always 0 (PngFilterMethod.None).

Steps to Reproduce

    SixLabors.ImageSharp.Image img = null!;
    
    var paethFilterStream = new MemoryStream();
    img.Save(paethFilterStream, new PngEncoder
    {
        FilterMethod = PngFilterMethod.Paeth,
    });

    var noneFilterStream = new MemoryStream();
    img.Save(noneFilterStream , new PngEncoder
    {
        FilterMethod = PngFilterMethod.None,
    });

    Assert.NotEquals(paethFilterStream.Length, noneFilterStream.Length);

Images

No response

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Jul 16, 2024

@melnikov77 The filter method is used in the release/3.1.x branch here.

private void FilterPixelBytes(ref Span<byte> filter, ref Span<byte> attempt)
{
switch (this.filterMethod)
{
case PngFilterMethod.None:
NoneFilter.Encode(this.currentScanline.GetSpan(), filter);
break;
case PngFilterMethod.Sub:
SubFilter.Encode(this.currentScanline.GetSpan(), filter, this.bytesPerPixel, out int _);
break;
case PngFilterMethod.Up:
UpFilter.Encode(this.currentScanline.GetSpan(), this.previousScanline.GetSpan(), filter, out int _);
break;
case PngFilterMethod.Average:
AverageFilter.Encode(this.currentScanline.GetSpan(), this.previousScanline.GetSpan(), filter, (uint)this.bytesPerPixel, out int _);
break;
case PngFilterMethod.Paeth:
PaethFilter.Encode(this.currentScanline.GetSpan(), this.previousScanline.GetSpan(), filter, this.bytesPerPixel, out int _);
break;
default:
this.ApplyOptimalFilteredScanline(ref filter, ref attempt);
break;
}
}

Your test confused me given you are use null! to define the source image.

SixLabors.ImageSharp.Image img = null!;

However, you are correct! We are not assigning the correct filter on encode, your finding actually uncovers a bug in our SIMD accelerated filtering methods where we suffered from an unsigned integer overflow defect!

Many thanks for raising this.

I've opened a PR #2775 with the fix.

@JimBobSquarePants
Copy link
Member

Fixed with 3.1.5

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants