Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

refactor PngDecoder to SpecializedImageDecoder<PngDecoderOptions> #1

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions src/ImageSharp/Formats/Png/PngDecoder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace SixLabors.ImageSharp.Formats.Png;
/// <summary>
/// Decoder for generating an image out of a png encoded stream.
/// </summary>
public sealed class PngDecoder : ImageDecoder
public sealed class PngDecoder : SpecializedImageDecoder<PngDecoderOptions>
{
private PngDecoder()
{
Expand All @@ -25,31 +25,31 @@ protected override ImageInfo Identify(DecoderOptions options, Stream stream, Can
Guard.NotNull(options, nameof(options));
Guard.NotNull(stream, nameof(stream));

return new PngDecoderCore(options).Identify(options.Configuration, stream, cancellationToken);
return new PngDecoderCore(new PngDecoderOptions() { GeneralOptions = options }).Identify(options.Configuration, stream, cancellationToken);
}

/// <inheritdoc/>
protected override Image<TPixel> Decode<TPixel>(DecoderOptions options, Stream stream, CancellationToken cancellationToken)
protected override Image<TPixel> Decode<TPixel>(PngDecoderOptions options, Stream stream, CancellationToken cancellationToken)
{
Guard.NotNull(options, nameof(options));
Guard.NotNull(stream, nameof(stream));

PngDecoderCore decoder = new(options);
Image<TPixel> image = decoder.Decode<TPixel>(options.Configuration, stream, cancellationToken);
Image<TPixel> image = decoder.Decode<TPixel>(options.GeneralOptions.Configuration, stream, cancellationToken);

ScaleToTargetSize(options, image);
ScaleToTargetSize(options.GeneralOptions, image);

return image;
}

/// <inheritdoc/>
protected override Image Decode(DecoderOptions options, Stream stream, CancellationToken cancellationToken)
protected override Image Decode(PngDecoderOptions options, Stream stream, CancellationToken cancellationToken)
{
Guard.NotNull(options, nameof(options));
Guard.NotNull(stream, nameof(stream));

PngDecoderCore decoder = new(options, true);
ImageInfo info = decoder.Identify(options.Configuration, stream, cancellationToken);
ImageInfo info = decoder.Identify(options.GeneralOptions.Configuration, stream, cancellationToken);
stream.Position = 0;

PngMetadata meta = info.Metadata.GetPngMetadata();
Expand Down Expand Up @@ -99,4 +99,7 @@ protected override Image Decode(DecoderOptions options, Stream stream, Cancellat
return this.Decode<Rgba32>(options, stream, cancellationToken);
}
}

/// <inheritdoc/>
protected override PngDecoderOptions CreateDefaultSpecializedOptions(DecoderOptions options) => new PngDecoderOptions() { GeneralOptions = options };
}
32 changes: 21 additions & 11 deletions src/ImageSharp/Formats/Png/PngDecoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -113,27 +113,34 @@ internal sealed class PngDecoderCore : IImageDecoderInternals
/// </summary>
private PngChunk? nextChunk;

/// <summary>
/// If true, ADLER32 checksum in the IDAT chunk as well as the chunk CRCs will be ignored.
/// </summary>
private bool ignoreCrcErrors;

/// <summary>
/// Initializes a new instance of the <see cref="PngDecoderCore"/> class.
/// </summary>
/// <param name="options">The decoder options.</param>
public PngDecoderCore(DecoderOptions options)
public PngDecoderCore(PngDecoderOptions options)
{
this.Options = options;
this.configuration = options.Configuration;
this.maxFrames = options.MaxFrames;
this.skipMetadata = options.SkipMetadata;
this.Options = options.GeneralOptions;
this.configuration = options.GeneralOptions.Configuration;
this.maxFrames = options.GeneralOptions.MaxFrames;
this.skipMetadata = options.GeneralOptions.SkipMetadata;
this.memoryAllocator = this.configuration.MemoryAllocator;
this.ignoreCrcErrors = options.IgnoreCrcCheck;
}

internal PngDecoderCore(DecoderOptions options, bool colorMetadataOnly)
internal PngDecoderCore(PngDecoderOptions options, bool colorMetadataOnly)
{
this.Options = options;
this.Options = options.GeneralOptions;
this.colorMetadataOnly = colorMetadataOnly;
this.maxFrames = options.MaxFrames;
this.maxFrames = options.GeneralOptions.MaxFrames;
this.skipMetadata = true;
this.configuration = options.Configuration;
this.configuration = options.GeneralOptions.Configuration;
this.memoryAllocator = this.configuration.MemoryAllocator;
this.ignoreCrcErrors = options.IgnoreCrcCheck;
}

/// <inheritdoc/>
Expand Down Expand Up @@ -1789,8 +1796,11 @@ private bool TryReadChunk(Span<byte> buffer, out PngChunk chunk)
type: type,
data: this.ReadChunkData(length));

this.ValidateChunk(chunk, buffer);

if (!this.ignoreCrcErrors)
{
this.ValidateChunk(chunk, buffer);
}

// Restore the stream position for IDAT and fdAT chunks, because it will be decoded later and
// was only read to verifying the CRC is correct.
if (type is PngChunkType.Data or PngChunkType.FrameData)
Expand Down
19 changes: 19 additions & 0 deletions src/ImageSharp/Formats/Png/PngDecoderOptions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright (c) Six Labors.
// Licensed under the Six Labors Split License.

namespace SixLabors.ImageSharp.Formats.Png;

/// <summary>
/// Configuration options for decoding png images.
/// </summary>
public sealed class PngDecoderOptions : ISpecializedDecoderOptions
{
/// <inheritdoc/>
public DecoderOptions GeneralOptions { get; init; } = new DecoderOptions();

/// <summary>
/// If true, ADLER32 checksum in the IDAT chunk as well as the chunk CRCs will be ignored.
/// Similar to PNG_CRC_QUIET_USE in libpng.
/// </summary>
public bool IgnoreCrcCheck { get; init; }
}
2 changes: 1 addition & 1 deletion tests/ImageSharp.Tests/Formats/ImageFormatManagerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public void IfAutoLoadWellKnownFormatsIsTrueAllFormatsAreLoaded()
Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType<PngDecoder>().Count());
Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType<BmpDecoder>().Count());
Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType<JpegDecoder>().Count());
Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType<BmpDecoder>().Count());
Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType<GifDecoder>().Count());
Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType<TgaDecoder>().Count());
Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType<TiffDecoder>().Count());
Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType<WebpDecoder>().Count());
Expand Down
11 changes: 11 additions & 0 deletions tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,17 @@ public void Decode_InvalidDataChunkCrc_ThrowsException<TPixel>(TestImageProvider
Assert.Contains("CRC Error. PNG IDAT chunk is corrupt!", ex.Message);
}

[Theory]
[WithFile(TestImages.Png.Bad.WrongCrcDataChunk, PixelTypes.Rgba32)]
public void Decode_InvalidDataChunkCrc_IgnoreCrcErrors<TPixel>(TestImageProvider<TPixel> provider)
where TPixel : unmanaged, IPixel<TPixel>
{
using Image<TPixel> image = provider.GetImage(PngDecoder.Instance, new PngDecoderOptions() { IgnoreCrcCheck = true });

image.DebugSave(provider);
image.CompareToOriginal(provider, new MagickReferenceDecoder(false));
}

// https://github.com/SixLabors/ImageSharp/issues/1014
[Theory]
[WithFileCollection(nameof(TestImagesIssue1014), PixelTypes.Rgba32)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,17 @@ protected override Image<TPixel> Decode<TPixel>(DecoderOptions options, Stream s
{
IgnoreFileSize = !this.validate
};
PngReadDefines pngReadDefines = new()
{
IgnoreCrc = !this.validate
};

MagickReadSettings settings = new()
{
FrameCount = (int)options.MaxFrames
};
settings.SetDefines(bmpReadDefines);
settings.SetDefines(pngReadDefines);

using MagickImageCollection magickImageCollection = new(stream, settings);
List<ImageFrame<TPixel>> framesList = new();
Expand Down