From db51bd749557fc7ed2f3677b106e066b834f7a8b Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 25 Sep 2022 19:52:58 +0200 Subject: [PATCH 01/18] Add support for decoding tiff images with old jpeg compression (#2238) --- .../Formats/Jpeg/JpegDecoderCore.cs | 2 +- .../Decompressors/JpegTiffCompression.cs | 100 ++++++++++++------ .../Decompressors/OldJpegTiffCompression.cs | 30 ++++++ .../Decompressors/WebpTiffCompression.cs | 4 +- .../Compression/TiffDecoderCompressionType.cs | 5 + .../Compression/TiffDecompressorsFactory.cs | 9 +- src/ImageSharp/Formats/Tiff/README.md | 2 +- .../Formats/Tiff/TiffDecoderCore.cs | 17 ++- .../Formats/Tiff/TiffDecoderOptionsParser.cs | 20 ++++ 9 files changed, 145 insertions(+), 44 deletions(-) create mode 100644 src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 11a9bc557..149aad07b 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -275,7 +275,7 @@ internal sealed class JpegDecoderCore : IRawJpegData, IImageDecoderInternals // Get the marker length. int markerContentByteSize = this.ReadUint16(stream) - 2; - // Check whether stream actually has enought bytes to read + // Check whether the stream actually has enough bytes to read // markerContentByteSize is always positive so we cast // to uint to avoid sign extension if (stream.RemainingBytes < (uint)markerContentByteSize) diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs index d67a61355..37ed0845f 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs @@ -14,7 +14,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Compression.Decompressors; /// /// Class to handle cases where TIFF image data is compressed as a jpeg stream. /// -internal sealed class JpegTiffCompression : TiffBaseDecompressor +internal class JpegTiffCompression : TiffBaseDecompressor { private readonly JpegDecoderOptions options; @@ -25,17 +25,17 @@ internal sealed class JpegTiffCompression : TiffBaseDecompressor /// /// Initializes a new instance of the class. /// - /// The specialized jpeg decoder options. /// The memoryAllocator to use for buffer allocations. /// The image width. /// The bits per pixel. + /// The specialized jpeg decoder options. /// The JPEG tables containing the quantization and/or Huffman tables. /// The photometric interpretation. public JpegTiffCompression( - JpegDecoderOptions options, MemoryAllocator memoryAllocator, int width, int bitsPerPixel, + JpegDecoderOptions options, byte[] jpegTables, TiffPhotometricInterpretation photometricInterpretation) : base(memoryAllocator, width, bitsPerPixel) @@ -45,51 +45,85 @@ internal sealed class JpegTiffCompression : TiffBaseDecompressor this.photometricInterpretation = photometricInterpretation; } + /// + /// Initializes a new instance of the class. + /// + /// The memoryAllocator to use for buffer allocations. + /// The image width. + /// The bits per pixel. + /// The specialized jpeg decoder options. + /// The photometric interpretation. + public JpegTiffCompression( + MemoryAllocator memoryAllocator, + int width, + int bitsPerPixel, + JpegDecoderOptions options, + TiffPhotometricInterpretation photometricInterpretation) + : base(memoryAllocator, width, bitsPerPixel) + { + this.options = options; + this.photometricInterpretation = photometricInterpretation; + } + /// protected override void Decompress(BufferedReadStream stream, int byteCount, int stripHeight, Span buffer, CancellationToken cancellationToken) { if (this.jpegTables != null) { - using var jpegDecoder = new JpegDecoderCore(this.options); - Configuration configuration = this.options.GeneralOptions.Configuration; - switch (this.photometricInterpretation) + this.DecodeJpegData(stream, buffer, true, cancellationToken); + } + else + { + using Image image = Image.Load(stream); + CopyImageBytesToBuffer(buffer, image.Frames.RootFrame.PixelBuffer); + } + } + + protected void DecodeJpegData(BufferedReadStream stream, Span buffer, bool loadTables, CancellationToken cancellationToken) + { + using JpegDecoderCore jpegDecoder = new(this.options); + Configuration configuration = this.options.GeneralOptions.Configuration; + switch (this.photometricInterpretation) + { + case TiffPhotometricInterpretation.BlackIsZero: + case TiffPhotometricInterpretation.WhiteIsZero: { - case TiffPhotometricInterpretation.BlackIsZero: - case TiffPhotometricInterpretation.WhiteIsZero: + using SpectralConverter spectralConverterGray = new GrayJpegSpectralConverter(configuration); + HuffmanScanDecoder scanDecoderGray = new(stream, spectralConverterGray, cancellationToken); + + if (loadTables) { - using SpectralConverter spectralConverterGray = new GrayJpegSpectralConverter(configuration); - var scanDecoderGray = new HuffmanScanDecoder(stream, spectralConverterGray, cancellationToken); jpegDecoder.LoadTables(this.jpegTables, scanDecoderGray); - jpegDecoder.ParseStream(stream, spectralConverterGray, cancellationToken); - - using Buffer2D decompressedBuffer = spectralConverterGray.GetPixelBuffer(cancellationToken); - CopyImageBytesToBuffer(buffer, decompressedBuffer); - break; } - case TiffPhotometricInterpretation.YCbCr: - case TiffPhotometricInterpretation.Rgb: + jpegDecoder.ParseStream(stream, spectralConverterGray, cancellationToken); + + using Buffer2D decompressedBuffer = spectralConverterGray.GetPixelBuffer(cancellationToken); + CopyImageBytesToBuffer(buffer, decompressedBuffer); + break; + } + + case TiffPhotometricInterpretation.YCbCr: + case TiffPhotometricInterpretation.Rgb: + { + using SpectralConverter spectralConverter = new TiffJpegSpectralConverter(configuration, this.photometricInterpretation); + HuffmanScanDecoder scanDecoder = new(stream, spectralConverter, cancellationToken); + + if (loadTables) { - using SpectralConverter spectralConverter = - new TiffJpegSpectralConverter(configuration, this.photometricInterpretation); - var scanDecoder = new HuffmanScanDecoder(stream, spectralConverter, cancellationToken); jpegDecoder.LoadTables(this.jpegTables, scanDecoder); - jpegDecoder.ParseStream(stream, spectralConverter, cancellationToken); - - using Buffer2D decompressedBuffer = spectralConverter.GetPixelBuffer(cancellationToken); - CopyImageBytesToBuffer(buffer, decompressedBuffer); - break; } - default: - TiffThrowHelper.ThrowNotSupported($"Jpeg compressed tiff with photometric interpretation {this.photometricInterpretation} is not supported"); - break; + jpegDecoder.ParseStream(stream, spectralConverter, cancellationToken); + + using Buffer2D decompressedBuffer = spectralConverter.GetPixelBuffer(cancellationToken); + CopyImageBytesToBuffer(buffer, decompressedBuffer); + break; } - } - else - { - using var image = Image.Load(stream); - CopyImageBytesToBuffer(buffer, image.Frames.RootFrame.PixelBuffer); + + default: + TiffThrowHelper.ThrowNotSupported($"Jpeg compressed tiff with photometric interpretation {this.photometricInterpretation} is not supported"); + break; } } diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs new file mode 100644 index 000000000..c9fa9e925 --- /dev/null +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs @@ -0,0 +1,30 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using SixLabors.ImageSharp.Formats.Jpeg; +using SixLabors.ImageSharp.Formats.Tiff.Constants; +using SixLabors.ImageSharp.IO; +using SixLabors.ImageSharp.Memory; + +namespace SixLabors.ImageSharp.Formats.Tiff.Compression.Decompressors; + +internal sealed class OldJpegTiffCompression : JpegTiffCompression +{ + private readonly uint startOfImageMarker; + + public OldJpegTiffCompression( + MemoryAllocator memoryAllocator, + int width, + int bitsPerPixel, + JpegDecoderOptions options, + uint startOfImageMarker, + TiffPhotometricInterpretation photometricInterpretation) + : base(memoryAllocator, width, bitsPerPixel, options, photometricInterpretation) => this.startOfImageMarker = startOfImageMarker; + + protected override void Decompress(BufferedReadStream stream, int byteCount, int stripHeight, Span buffer, CancellationToken cancellationToken) + { + stream.Position = this.startOfImageMarker; + + this.DecodeJpegData(stream, buffer, false, cancellationToken); + } +} diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/WebpTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/WebpTiffCompression.cs index 4e1c9c2f8..347f09522 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/WebpTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/WebpTiffCompression.cs @@ -20,12 +20,12 @@ internal class WebpTiffCompression : TiffBaseDecompressor /// /// Initializes a new instance of the class. /// - /// The general decoder options. /// The memory allocator. /// The width of the image. /// The bits per pixel. + /// The general decoder options. /// The predictor. - public WebpTiffCompression(DecoderOptions options, MemoryAllocator memoryAllocator, int width, int bitsPerPixel, TiffPredictor predictor = TiffPredictor.None) + public WebpTiffCompression(MemoryAllocator memoryAllocator, int width, int bitsPerPixel, DecoderOptions options, TiffPredictor predictor = TiffPredictor.None) : base(memoryAllocator, width, bitsPerPixel, predictor) => this.options = options; diff --git a/src/ImageSharp/Formats/Tiff/Compression/TiffDecoderCompressionType.cs b/src/ImageSharp/Formats/Tiff/Compression/TiffDecoderCompressionType.cs index 34f0ed2db..70b9fec07 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/TiffDecoderCompressionType.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/TiffDecoderCompressionType.cs @@ -52,4 +52,9 @@ internal enum TiffDecoderCompressionType /// The image data is compressed as a WEBP stream. /// Webp = 8, + + /// + /// The image data is compressed as a OldJPEG compressed stream. + /// + OldJpeg = 9, } diff --git a/src/ImageSharp/Formats/Tiff/Compression/TiffDecompressorsFactory.cs b/src/ImageSharp/Formats/Tiff/Compression/TiffDecompressorsFactory.cs index e09b93c02..7b1f67b12 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/TiffDecompressorsFactory.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/TiffDecompressorsFactory.cs @@ -21,6 +21,7 @@ internal static class TiffDecompressorsFactory TiffPredictor predictor, FaxCompressionOptions faxOptions, byte[] jpegTables, + uint oldJpegStartOfImageMarker, TiffFillOrder fillOrder, ByteOrder byteOrder) { @@ -58,11 +59,15 @@ internal static class TiffDecompressorsFactory case TiffDecoderCompressionType.Jpeg: DebugGuard.IsTrue(predictor == TiffPredictor.None, "Predictor should only be used with lzw or deflate compression"); - return new JpegTiffCompression(new() { GeneralOptions = options }, allocator, width, bitsPerPixel, jpegTables, photometricInterpretation); + return new JpegTiffCompression(allocator, width, bitsPerPixel, new() { GeneralOptions = options }, jpegTables, photometricInterpretation); + + case TiffDecoderCompressionType.OldJpeg: + DebugGuard.IsTrue(predictor == TiffPredictor.None, "Predictor should only be used with lzw or deflate compression"); + return new OldJpegTiffCompression(allocator, width, bitsPerPixel, new() { GeneralOptions = options }, oldJpegStartOfImageMarker, photometricInterpretation); case TiffDecoderCompressionType.Webp: DebugGuard.IsTrue(predictor == TiffPredictor.None, "Predictor should only be used with lzw or deflate compression"); - return new WebpTiffCompression(options, allocator, width, bitsPerPixel); + return new WebpTiffCompression(allocator, width, bitsPerPixel, options); default: throw TiffThrowHelper.NotSupportedDecompressor(nameof(method)); diff --git a/src/ImageSharp/Formats/Tiff/README.md b/src/ImageSharp/Formats/Tiff/README.md index 00d46c415..d64e3339c 100644 --- a/src/ImageSharp/Formats/Tiff/README.md +++ b/src/ImageSharp/Formats/Tiff/README.md @@ -38,7 +38,7 @@ |CcittGroup3Fax | Y | Y | | |CcittGroup4Fax | Y | Y | | |Lzw | Y | Y | Based on ImageSharp GIF LZW implementation - this code could be modified to be (i) shared, or (ii) optimised for each case. | -|Old Jpeg | | | We should not even try to support this. | +|Old Jpeg | | Y | | |Jpeg (Technote 2) | Y | Y | | |Deflate (Technote 2) | Y | Y | Based on PNG Deflate. | |Old Deflate (Technote 2) | | Y | | diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs index 60bce2174..59d8b6ecb 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs @@ -126,6 +126,11 @@ internal class TiffDecoderCore : IImageDecoderInternals /// public byte[] JpegTables { get; set; } + /// + /// Gets or sets the start of image marker for old Jpeg compression. + /// + public uint? OldJpegCompressionStartOfImageMarker { get; set; } + /// /// Gets or sets the planar configuration type to use when decoding the image. /// @@ -232,7 +237,7 @@ internal class TiffDecoderCore : IImageDecoderInternals private ImageFrame DecodeFrame(ExifProfile tags, CancellationToken cancellationToken) where TPixel : unmanaged, IPixel { - var imageFrameMetaData = new ImageFrameMetadata(); + ImageFrameMetadata imageFrameMetaData = new(); if (!this.skipMetadata) { imageFrameMetaData.ExifProfile = tags; @@ -245,12 +250,12 @@ internal class TiffDecoderCore : IImageDecoderInternals int width = GetImageWidth(tags); int height = GetImageHeight(tags); - var frame = new ImageFrame(this.configuration, width, height, imageFrameMetaData); + ImageFrame frame = new(this.configuration, width, height, imageFrameMetaData); int rowsPerStrip = tags.GetValue(ExifTag.RowsPerStrip) != null ? (int)tags.GetValue(ExifTag.RowsPerStrip).Value : TiffConstants.RowsPerStripInfinity; - var stripOffsetsArray = (Array)tags.GetValueInternal(ExifTag.StripOffsets).GetValue(); - var stripByteCountsArray = (Array)tags.GetValueInternal(ExifTag.StripByteCounts).GetValue(); + Array stripOffsetsArray = (Array)tags.GetValueInternal(ExifTag.StripOffsets).GetValue(); + Array stripByteCountsArray = (Array)tags.GetValueInternal(ExifTag.StripByteCounts).GetValue(); using IMemoryOwner stripOffsetsMemory = this.ConvertNumbers(stripOffsetsArray, out Span stripOffsets); using IMemoryOwner stripByteCountsMemory = this.ConvertNumbers(stripByteCountsArray, out Span stripByteCounts); @@ -358,7 +363,7 @@ internal class TiffDecoderCore : IImageDecoderInternals Buffer2D pixels = frame.PixelBuffer; - var stripBuffers = new IMemoryOwner[stripsPerPixel]; + IMemoryOwner[] stripBuffers = new IMemoryOwner[stripsPerPixel]; try { @@ -379,6 +384,7 @@ internal class TiffDecoderCore : IImageDecoderInternals this.Predictor, this.FaxCompressionOptions, this.JpegTables, + this.OldJpegCompressionStartOfImageMarker.GetValueOrDefault(), this.FillOrder, this.byteOrder); @@ -460,6 +466,7 @@ internal class TiffDecoderCore : IImageDecoderInternals this.Predictor, this.FaxCompressionOptions, this.JpegTables, + this.OldJpegCompressionStartOfImageMarker.GetValueOrDefault(), this.FillOrder, this.byteOrder); diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs index 7593840bb..2198811e5 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -99,6 +99,7 @@ internal static class TiffDecoderOptionsParser options.YcbcrSubSampling = exifProfile.GetValue(ExifTag.YCbCrSubsampling)?.Value; options.FillOrder = fillOrder; options.JpegTables = exifProfile.GetValue(ExifTag.JPEGTables)?.Value; + options.OldJpegCompressionStartOfImageMarker = exifProfile.GetValue(ExifTag.JPEGInterchangeFormat)?.Value; options.ParseColorType(exifProfile); options.ParseCompression(frameMetadata.Compression, exifProfile); @@ -473,6 +474,25 @@ internal static class TiffDecoderOptionsParser break; } + case TiffCompression.OldJpeg: + { + options.CompressionType = TiffDecoderCompressionType.OldJpeg; + + if (!options.OldJpegCompressionStartOfImageMarker.HasValue) + { + TiffThrowHelper.ThrowNotSupported("Missing SOI marker offset for tiff with old jpeg compression"); + } + + if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr) + { + // Note: Setting PhotometricInterpretation and color type to RGB here, since the jpeg decoder will handle the conversion of the pixel data. + options.PhotometricInterpretation = TiffPhotometricInterpretation.Rgb; + options.ColorType = TiffColorType.Rgb; + } + + break; + } + case TiffCompression.Jpeg: { options.CompressionType = TiffDecoderCompressionType.Jpeg; From e6d1c94a6a0c5989b7fe0f41e1b3c23ae879f4b2 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Mon, 26 Sep 2022 21:42:40 +0200 Subject: [PATCH 02/18] Always assume PhotometricInterpretation to be YCbCr, if the compression is old jpeg --- .../Decompressors/JpegCompressionUtils.cs | 35 +++++++++++ .../Decompressors/JpegTiffCompression.cs | 49 +++------------- .../Decompressors/OldJpegTiffCompression.cs | 58 ++++++++++++++++++- .../TiffOldJpegSpectralConverter{TPixel}.cs | 45 ++++++++++++++ .../Formats/Tiff/TiffDecoderOptionsParser.cs | 12 ++-- 5 files changed, 147 insertions(+), 52 deletions(-) create mode 100644 src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegCompressionUtils.cs create mode 100644 src/ImageSharp/Formats/Tiff/Compression/Decompressors/TiffOldJpegSpectralConverter{TPixel}.cs diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegCompressionUtils.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegCompressionUtils.cs new file mode 100644 index 000000000..a52c7e529 --- /dev/null +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegCompressionUtils.cs @@ -0,0 +1,35 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using System.Runtime.InteropServices; +using SixLabors.ImageSharp.Memory; +using SixLabors.ImageSharp.PixelFormats; + +namespace SixLabors.ImageSharp.Formats.Tiff.Compression.Decompressors; + +internal static class JpegCompressionUtils +{ + public static void CopyImageBytesToBuffer(Span buffer, Buffer2D pixelBuffer) + { + int offset = 0; + for (int y = 0; y < pixelBuffer.Height; y++) + { + Span pixelRowSpan = pixelBuffer.DangerousGetRowSpan(y); + Span rgbBytes = MemoryMarshal.AsBytes(pixelRowSpan); + rgbBytes.CopyTo(buffer[offset..]); + offset += rgbBytes.Length; + } + } + + public static void CopyImageBytesToBuffer(Span buffer, Buffer2D pixelBuffer) + { + int offset = 0; + for (int y = 0; y < pixelBuffer.Height; y++) + { + Span pixelRowSpan = pixelBuffer.DangerousGetRowSpan(y); + Span rgbBytes = MemoryMarshal.AsBytes(pixelRowSpan); + rgbBytes.CopyTo(buffer[offset..]); + offset += rgbBytes.Length; + } + } +} diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs index 37ed0845f..818bb3d6d 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs @@ -1,7 +1,6 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -using System.Runtime.InteropServices; using SixLabors.ImageSharp.Formats.Jpeg; using SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder; using SixLabors.ImageSharp.Formats.Tiff.Constants; @@ -14,7 +13,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Compression.Decompressors; /// /// Class to handle cases where TIFF image data is compressed as a jpeg stream. /// -internal class JpegTiffCompression : TiffBaseDecompressor +internal sealed class JpegTiffCompression : TiffBaseDecompressor { private readonly JpegDecoderOptions options; @@ -70,16 +69,16 @@ internal class JpegTiffCompression : TiffBaseDecompressor { if (this.jpegTables != null) { - this.DecodeJpegData(stream, buffer, true, cancellationToken); + this.DecodeJpegData(stream, buffer, cancellationToken); } else { using Image image = Image.Load(stream); - CopyImageBytesToBuffer(buffer, image.Frames.RootFrame.PixelBuffer); + JpegCompressionUtils.CopyImageBytesToBuffer(buffer, image.Frames.RootFrame.PixelBuffer); } } - protected void DecodeJpegData(BufferedReadStream stream, Span buffer, bool loadTables, CancellationToken cancellationToken) + private void DecodeJpegData(BufferedReadStream stream, Span buffer, CancellationToken cancellationToken) { using JpegDecoderCore jpegDecoder = new(this.options); Configuration configuration = this.options.GeneralOptions.Configuration; @@ -91,15 +90,11 @@ internal class JpegTiffCompression : TiffBaseDecompressor using SpectralConverter spectralConverterGray = new GrayJpegSpectralConverter(configuration); HuffmanScanDecoder scanDecoderGray = new(stream, spectralConverterGray, cancellationToken); - if (loadTables) - { - jpegDecoder.LoadTables(this.jpegTables, scanDecoderGray); - } - + jpegDecoder.LoadTables(this.jpegTables, scanDecoderGray); jpegDecoder.ParseStream(stream, spectralConverterGray, cancellationToken); using Buffer2D decompressedBuffer = spectralConverterGray.GetPixelBuffer(cancellationToken); - CopyImageBytesToBuffer(buffer, decompressedBuffer); + JpegCompressionUtils.CopyImageBytesToBuffer(buffer, decompressedBuffer); break; } @@ -109,15 +104,11 @@ internal class JpegTiffCompression : TiffBaseDecompressor using SpectralConverter spectralConverter = new TiffJpegSpectralConverter(configuration, this.photometricInterpretation); HuffmanScanDecoder scanDecoder = new(stream, spectralConverter, cancellationToken); - if (loadTables) - { - jpegDecoder.LoadTables(this.jpegTables, scanDecoder); - } - + jpegDecoder.LoadTables(this.jpegTables, scanDecoder); jpegDecoder.ParseStream(stream, spectralConverter, cancellationToken); using Buffer2D decompressedBuffer = spectralConverter.GetPixelBuffer(cancellationToken); - CopyImageBytesToBuffer(buffer, decompressedBuffer); + JpegCompressionUtils.CopyImageBytesToBuffer(buffer, decompressedBuffer); break; } @@ -127,30 +118,6 @@ internal class JpegTiffCompression : TiffBaseDecompressor } } - private static void CopyImageBytesToBuffer(Span buffer, Buffer2D pixelBuffer) - { - int offset = 0; - for (int y = 0; y < pixelBuffer.Height; y++) - { - Span pixelRowSpan = pixelBuffer.DangerousGetRowSpan(y); - Span rgbBytes = MemoryMarshal.AsBytes(pixelRowSpan); - rgbBytes.CopyTo(buffer[offset..]); - offset += rgbBytes.Length; - } - } - - private static void CopyImageBytesToBuffer(Span buffer, Buffer2D pixelBuffer) - { - int offset = 0; - for (int y = 0; y < pixelBuffer.Height; y++) - { - Span pixelRowSpan = pixelBuffer.DangerousGetRowSpan(y); - Span rgbBytes = MemoryMarshal.AsBytes(pixelRowSpan); - rgbBytes.CopyTo(buffer[offset..]); - offset += rgbBytes.Length; - } - } - /// protected override void Dispose(bool disposing) { diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs index c9fa9e925..e6ddf9570 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs @@ -2,16 +2,22 @@ // Licensed under the Six Labors Split License. using SixLabors.ImageSharp.Formats.Jpeg; +using SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder; using SixLabors.ImageSharp.Formats.Tiff.Constants; using SixLabors.ImageSharp.IO; using SixLabors.ImageSharp.Memory; +using SixLabors.ImageSharp.PixelFormats; namespace SixLabors.ImageSharp.Formats.Tiff.Compression.Decompressors; -internal sealed class OldJpegTiffCompression : JpegTiffCompression +internal sealed class OldJpegTiffCompression : TiffBaseDecompressor { + private readonly JpegDecoderOptions options; + private readonly uint startOfImageMarker; + private readonly TiffPhotometricInterpretation photometricInterpretation; + public OldJpegTiffCompression( MemoryAllocator memoryAllocator, int width, @@ -19,12 +25,58 @@ internal sealed class OldJpegTiffCompression : JpegTiffCompression JpegDecoderOptions options, uint startOfImageMarker, TiffPhotometricInterpretation photometricInterpretation) - : base(memoryAllocator, width, bitsPerPixel, options, photometricInterpretation) => this.startOfImageMarker = startOfImageMarker; + : base(memoryAllocator, width, bitsPerPixel) + { + this.options = options; + this.startOfImageMarker = startOfImageMarker; + this.photometricInterpretation = photometricInterpretation; + } protected override void Decompress(BufferedReadStream stream, int byteCount, int stripHeight, Span buffer, CancellationToken cancellationToken) { stream.Position = this.startOfImageMarker; - this.DecodeJpegData(stream, buffer, false, cancellationToken); + this.DecodeJpegData(stream, buffer, cancellationToken); + } + + private void DecodeJpegData(BufferedReadStream stream, Span buffer, CancellationToken cancellationToken) + { + using JpegDecoderCore jpegDecoder = new(this.options); + Configuration configuration = this.options.GeneralOptions.Configuration; + switch (this.photometricInterpretation) + { + case TiffPhotometricInterpretation.BlackIsZero: + case TiffPhotometricInterpretation.WhiteIsZero: + { + using SpectralConverter spectralConverterGray = new GrayJpegSpectralConverter(configuration); + + jpegDecoder.ParseStream(stream, spectralConverterGray, cancellationToken); + + using Buffer2D decompressedBuffer = spectralConverterGray.GetPixelBuffer(cancellationToken); + JpegCompressionUtils.CopyImageBytesToBuffer(buffer, decompressedBuffer); + break; + } + + case TiffPhotometricInterpretation.YCbCr: + case TiffPhotometricInterpretation.Rgb: + { + using SpectralConverter spectralConverter = new TiffOldJpegSpectralConverter(configuration, this.photometricInterpretation); + + jpegDecoder.ParseStream(stream, spectralConverter, cancellationToken); + + using Buffer2D decompressedBuffer = spectralConverter.GetPixelBuffer(cancellationToken); + JpegCompressionUtils.CopyImageBytesToBuffer(buffer, decompressedBuffer); + break; + } + + default: + TiffThrowHelper.ThrowNotSupported($"Jpeg compressed tiff with photometric interpretation {this.photometricInterpretation} is not supported"); + break; + } + } + + /// + protected override void Dispose(bool disposing) + { } } diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/TiffOldJpegSpectralConverter{TPixel}.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/TiffOldJpegSpectralConverter{TPixel}.cs new file mode 100644 index 000000000..457c8d79c --- /dev/null +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/TiffOldJpegSpectralConverter{TPixel}.cs @@ -0,0 +1,45 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using SixLabors.ImageSharp.Formats.Jpeg.Components; +using SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder; +using SixLabors.ImageSharp.Formats.Tiff.Constants; +using SixLabors.ImageSharp.PixelFormats; + +namespace SixLabors.ImageSharp.Formats.Tiff.Compression.Decompressors; + +/// +/// Spectral converter for YCbCr TIFF's which use the OldJPEG compression. +/// The jpeg data should be always treated as YCbCr color space. +/// +/// The type of the pixel. +internal sealed class TiffOldJpegSpectralConverter : SpectralConverter + where TPixel : unmanaged, IPixel +{ + private readonly TiffPhotometricInterpretation photometricInterpretation; + + /// + /// Initializes a new instance of the class. + /// + /// The configuration. + /// Tiff photometric interpretation. + public TiffOldJpegSpectralConverter(Configuration configuration, TiffPhotometricInterpretation photometricInterpretation) + : base(configuration) + => this.photometricInterpretation = photometricInterpretation; + + /// + protected override JpegColorConverterBase GetColorConverter(JpegFrame frame, IRawJpegData jpegData) + { + JpegColorSpace colorSpace = GetJpegColorSpaceFromPhotometricInterpretation(this.photometricInterpretation); + return JpegColorConverterBase.GetConverter(colorSpace, frame.Precision); + } + + private static JpegColorSpace GetJpegColorSpaceFromPhotometricInterpretation(TiffPhotometricInterpretation interpretation) + => interpretation switch + { + // Like libtiff: Always treat the pixel data as YCbCr when the data is compressed with old jpeg compression. + TiffPhotometricInterpretation.Rgb => JpegColorSpace.YCbCr, + TiffPhotometricInterpretation.YCbCr => JpegColorSpace.YCbCr, + _ => throw new InvalidImageContentException($"Invalid tiff photometric interpretation for jpeg encoding: {interpretation}"), + }; +} diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs index 2198811e5..85cabdc49 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -37,7 +37,7 @@ internal static class TiffDecoderOptionsParser TiffThrowHelper.ThrowNotSupported("ExtraSamples is only supported with one extra sample for alpha data."); } - var extraSamplesType = (TiffExtraSampleType)extraSamples[0]; + TiffExtraSampleType extraSamplesType = (TiffExtraSampleType)extraSamples[0]; options.ExtraSamplesType = extraSamplesType; if (extraSamplesType is not (TiffExtraSampleType.UnassociatedAlphaData or TiffExtraSampleType.AssociatedAlphaData)) { @@ -478,18 +478,14 @@ internal static class TiffDecoderOptionsParser { options.CompressionType = TiffDecoderCompressionType.OldJpeg; + // Like libtiff: always assume PhotometricInterpretation to be YCbCr, if the compression is old jpeg. + options.PhotometricInterpretation = TiffPhotometricInterpretation.YCbCr; + if (!options.OldJpegCompressionStartOfImageMarker.HasValue) { TiffThrowHelper.ThrowNotSupported("Missing SOI marker offset for tiff with old jpeg compression"); } - if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr) - { - // Note: Setting PhotometricInterpretation and color type to RGB here, since the jpeg decoder will handle the conversion of the pixel data. - options.PhotometricInterpretation = TiffPhotometricInterpretation.Rgb; - options.ColorType = TiffColorType.Rgb; - } - break; } From 019dae00ad3d7a166fd558099b7e020ead802910 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Mon, 26 Sep 2022 21:54:27 +0200 Subject: [PATCH 03/18] Add test for old jpeg compression --- tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs | 5 +++++ tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Tiff/OldJpegCompression.tiff | 3 +++ 3 files changed, 9 insertions(+) create mode 100644 tests/Images/Input/Tiff/OldJpegCompression.tiff diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index bcfd759a0..e38ce3c95 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -664,6 +664,11 @@ public class TiffDecoderTests : TiffDecoderBaseTester public void TiffDecoder_CanDecode_JpegCompressed(TestImageProvider provider) where TPixel : unmanaged, IPixel => TestTiffDecoder(provider, useExactComparer: false); + [Theory] + [WithFile(RgbOldJpegCompressed, PixelTypes.Rgba32)] + public void TiffDecoder_CanDecode_OldJpegCompressed(TestImageProvider provider) + where TPixel : unmanaged, IPixel => TestTiffDecoder(provider, useExactComparer: false); + [Theory] [WithFile(WebpCompressed, PixelTypes.Rgba32)] public void TiffDecoder_CanDecode_WebpCompressed(TestImageProvider provider) diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 676d460e5..53f60bff5 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -795,6 +795,7 @@ public static class TestImages public const string RgbDeflateMultistrip = "Tiff/rgb_deflate_multistrip.tiff"; public const string RgbJpegCompressed = "Tiff/rgb_jpegcompression.tiff"; public const string RgbJpegCompressed2 = "Tiff/twain-rgb-jpeg-with-bogus-ycbcr-subsampling.tiff"; + public const string RgbOldJpegCompressed = "Tiff/OldJpegCompression.tiff"; public const string RgbWithStripsJpegCompressed = "Tiff/rgb_jpegcompressed_stripped.tiff"; public const string RgbJpegCompressedNoJpegTable = "Tiff/rgb_jpegcompressed_nojpegtable.tiff"; public const string RgbLzwPredictor = "Tiff/rgb_lzw_predictor.tiff"; diff --git a/tests/Images/Input/Tiff/OldJpegCompression.tiff b/tests/Images/Input/Tiff/OldJpegCompression.tiff new file mode 100644 index 000000000..f58af0bf0 --- /dev/null +++ b/tests/Images/Input/Tiff/OldJpegCompression.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:0a5a02fb888a47ed51aa6d72122f9d1996311e943be62bef4829bc1aa9f457e8 +size 214023 From 08f90632294b781d84edf8ddcc5faac533590a18 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 28 Sep 2022 17:14:16 +0200 Subject: [PATCH 04/18] Set PhotometricInterpretation to RGB, let jpeg decoder handle color conversion --- src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs | 5 +++-- tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs | 1 + tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Tiff/YCbCrOldJpegCompressed.tiff | 3 +++ 4 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 tests/Images/Input/Tiff/YCbCrOldJpegCompressed.tiff diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs index 85cabdc49..f8fd87000 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -478,8 +478,9 @@ internal static class TiffDecoderOptionsParser { options.CompressionType = TiffDecoderCompressionType.OldJpeg; - // Like libtiff: always assume PhotometricInterpretation to be YCbCr, if the compression is old jpeg. - options.PhotometricInterpretation = TiffPhotometricInterpretation.YCbCr; + // Note: Setting PhotometricInterpretation and color type to RGB here, since the jpeg decoder will handle the conversion of the pixel data. + options.PhotometricInterpretation = TiffPhotometricInterpretation.Rgb; + options.ColorType = TiffColorType.Rgb; if (!options.OldJpegCompressionStartOfImageMarker.HasValue) { diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index e38ce3c95..06a9627b9 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -666,6 +666,7 @@ public class TiffDecoderTests : TiffDecoderBaseTester [Theory] [WithFile(RgbOldJpegCompressed, PixelTypes.Rgba32)] + [WithFile(YCbCrOldJpegCompressed, PixelTypes.Rgba32)] public void TiffDecoder_CanDecode_OldJpegCompressed(TestImageProvider provider) where TPixel : unmanaged, IPixel => TestTiffDecoder(provider, useExactComparer: false); diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 53f60bff5..9d017d506 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -796,6 +796,7 @@ public static class TestImages public const string RgbJpegCompressed = "Tiff/rgb_jpegcompression.tiff"; public const string RgbJpegCompressed2 = "Tiff/twain-rgb-jpeg-with-bogus-ycbcr-subsampling.tiff"; public const string RgbOldJpegCompressed = "Tiff/OldJpegCompression.tiff"; + public const string YCbCrOldJpegCompressed = "Tiff/YCbCrOldJpegCompressed.tiff"; public const string RgbWithStripsJpegCompressed = "Tiff/rgb_jpegcompressed_stripped.tiff"; public const string RgbJpegCompressedNoJpegTable = "Tiff/rgb_jpegcompressed_nojpegtable.tiff"; public const string RgbLzwPredictor = "Tiff/rgb_lzw_predictor.tiff"; diff --git a/tests/Images/Input/Tiff/YCbCrOldJpegCompressed.tiff b/tests/Images/Input/Tiff/YCbCrOldJpegCompressed.tiff new file mode 100644 index 000000000..26eff36d5 --- /dev/null +++ b/tests/Images/Input/Tiff/YCbCrOldJpegCompressed.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:b6527e69a3cce3e6d425c35695319711ad36d939f934902faa47f6ee1f1c7e08 +size 10076700 From be1ee582c5744a2c72099e818c8c8fc3918b84d2 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 28 Sep 2022 17:17:38 +0200 Subject: [PATCH 05/18] Only compare first frame in old jpeg compression test --- .../Formats/Tiff/TiffDecoderTests.cs | 15 ++++++++++++++- .../TestUtilities/TestImageExtensions.cs | 13 +++++++------ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index 06a9627b9..74b699c4b 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -668,7 +668,20 @@ public class TiffDecoderTests : TiffDecoderBaseTester [WithFile(RgbOldJpegCompressed, PixelTypes.Rgba32)] [WithFile(YCbCrOldJpegCompressed, PixelTypes.Rgba32)] public void TiffDecoder_CanDecode_OldJpegCompressed(TestImageProvider provider) - where TPixel : unmanaged, IPixel => TestTiffDecoder(provider, useExactComparer: false); + where TPixel : unmanaged, IPixel + { + DecoderOptions decoderOptions = new() + { + MaxFrames = 1 + }; + using Image image = provider.GetImage(TiffDecoder, decoderOptions); + image.DebugSave(provider); + image.CompareToOriginal( + provider, + ImageComparer.Tolerant(0.001f), + ReferenceDecoder, + decoderOptions); + } [Theory] [WithFile(WebpCompressed, PixelTypes.Rgba32)] diff --git a/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs b/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs index 5032f8de5..f15f9abcc 100644 --- a/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs +++ b/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs @@ -518,7 +518,8 @@ public static class TestImageExtensions this Image image, ITestImageProvider provider, ImageComparer comparer, - IImageDecoder referenceDecoder = null) + IImageDecoder referenceDecoder = null, + DecoderOptions referenceDecoderOptions = null) where TPixel : unmanaged, IPixel { string path = TestImageProvider.GetFilePathOrNull(provider); @@ -527,12 +528,12 @@ public static class TestImageExtensions throw new InvalidOperationException("CompareToOriginal() works only with file providers!"); } - var testFile = TestFile.Create(path); + TestFile testFile = TestFile.Create(path); referenceDecoder ??= TestEnvironment.GetReferenceDecoder(path); - using var stream = new MemoryStream(testFile.Bytes); - using (Image original = referenceDecoder.Decode(DecoderOptions.Default, stream, default)) + using MemoryStream stream = new(testFile.Bytes); + using (Image original = referenceDecoder.Decode(referenceDecoderOptions ?? DecoderOptions.Default, stream, default)) { comparer.VerifySimilarity(original, image); } @@ -553,11 +554,11 @@ public static class TestImageExtensions throw new InvalidOperationException("CompareToOriginal() works only with file providers!"); } - var testFile = TestFile.Create(path); + TestFile testFile = TestFile.Create(path); referenceDecoder ??= TestEnvironment.GetReferenceDecoder(path); - using var stream = new MemoryStream(testFile.Bytes); + using MemoryStream stream = new(testFile.Bytes); using (Image original = referenceDecoder.Decode(DecoderOptions.Default, stream, default)) { comparer.VerifySimilarity(original, image); From a4aa6b5c6e28c8897173225e96d68433c78646fb Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 28 Sep 2022 17:54:44 +0200 Subject: [PATCH 06/18] Set FrameCount from the decoding options --- .../TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs b/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs index 57d5a8af4..3392d6814 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs @@ -34,6 +34,7 @@ public class MagickReferenceDecoder : IImageDecoder }; var settings = new MagickReadSettings(); + settings.FrameCount = (int)options.MaxFrames; settings.SetDefines(bmpReadDefines); using var magickImageCollection = new MagickImageCollection(stream, settings); From f171184c0c716eabfc0b48113bcb7ddea2e6f280 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 28 Sep 2022 19:53:03 +0200 Subject: [PATCH 07/18] Override PhotometricInterpretation to RGB only in case of YCbCr, for black and white it should stay as it is --- .../Formats/Tiff/TiffDecoderOptionsParser.cs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs index f8fd87000..deaeb2deb 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -476,17 +476,20 @@ internal static class TiffDecoderOptionsParser case TiffCompression.OldJpeg: { - options.CompressionType = TiffDecoderCompressionType.OldJpeg; - - // Note: Setting PhotometricInterpretation and color type to RGB here, since the jpeg decoder will handle the conversion of the pixel data. - options.PhotometricInterpretation = TiffPhotometricInterpretation.Rgb; - options.ColorType = TiffColorType.Rgb; - if (!options.OldJpegCompressionStartOfImageMarker.HasValue) { TiffThrowHelper.ThrowNotSupported("Missing SOI marker offset for tiff with old jpeg compression"); } + options.CompressionType = TiffDecoderCompressionType.OldJpeg; + + if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr) + { + // Note: Setting PhotometricInterpretation and color type to RGB here, since the jpeg decoder will handle the conversion of the pixel data. + options.PhotometricInterpretation = TiffPhotometricInterpretation.Rgb; + options.ColorType = TiffColorType.Rgb; + } + break; } From e066b839cedec41bcc6fc2188efd78bc797c5cc6 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 29 Sep 2022 17:21:39 +0200 Subject: [PATCH 08/18] Fill read buffer in constructor and when IsInReadBuffer() is false --- src/ImageSharp/IO/BufferedReadStream.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/IO/BufferedReadStream.cs b/src/ImageSharp/IO/BufferedReadStream.cs index 7e233c965..39d294ae5 100644 --- a/src/ImageSharp/IO/BufferedReadStream.cs +++ b/src/ImageSharp/IO/BufferedReadStream.cs @@ -49,7 +49,6 @@ internal sealed class BufferedReadStream : Stream this.BaseStream = stream; this.Length = stream.Length; - this.Position = (int)stream.Position; this.BufferSize = configuration.StreamProcessingBufferSize; this.maxBufferIndex = this.BufferSize - 1; this.readBuffer = ArrayPool.Shared.Rent(this.BufferSize); @@ -59,8 +58,8 @@ internal sealed class BufferedReadStream : Stream this.pinnedReadBuffer = (byte*)this.readBufferHandle.Pointer; } - // This triggers a full read on first attempt. - this.readBufferIndex = this.BufferSize; + this.Position = (int)stream.Position; + this.FillReadBuffer(); } /// @@ -99,6 +98,7 @@ internal sealed class BufferedReadStream : Stream this.BaseStream.Seek(value, SeekOrigin.Begin); this.readerPosition = value; this.readBufferIndex = this.BufferSize; + this.FillReadBuffer(); } } } From 67b1693814720160780323705298917879cb0116 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 29 Sep 2022 17:22:18 +0200 Subject: [PATCH 09/18] Add test for issue when setting the stream position twice --- .../IO/BufferedReadStreamTests.cs | 38 ++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs b/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs index 6a9c34486..619003a64 100644 --- a/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs +++ b/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.IO; using SixLabors.ImageSharp.IO; namespace SixLabors.ImageSharp.Tests.IO; @@ -9,13 +10,10 @@ public class BufferedReadStreamTests { private readonly Configuration configuration; - public BufferedReadStreamTests() - { - this.configuration = Configuration.CreateDefaultInstance(); - } + public BufferedReadStreamTests() => this.configuration = Configuration.CreateDefaultInstance(); public static readonly TheoryData BufferSizes = - new TheoryData() + new() { 1, 2, 4, 8, 16, 97, 503, @@ -352,6 +350,31 @@ public class BufferedReadStreamTests } } + [Fact] + public void BufferedStreamSettingPositionWorksMultipleTimes() + { + // This test replicates an issue, which was caused by setting the stream position multiple times. + // This was then lead to the read buffer not being filled correctly. + int bufferSize = 16; + this.configuration.StreamProcessingBufferSize = bufferSize; + using MemoryStream stream = this.CreateTestStream(bufferSize * 2); + byte[] expected = stream.ToArray(); + using var reader = new BufferedReadStream(this.configuration, stream); + + // Read more then fits into the buffer. + for (int i = 0; i < 20; i++) + { + reader.ReadByte(); + } + + // Set the Position twice. + reader.Position = 10; + reader.Position = 3; + + int actual = reader.ReadByte(); + Assert.Equal(expected[3], actual); + } + private MemoryStream CreateTestStream(int length) { var buffer = new byte[length]; @@ -370,9 +393,6 @@ public class BufferedReadStreamTests { } - public override int Read(byte[] buffer, int offset, int count) - { - return base.Read(buffer, offset, 1); - } + public override int Read(byte[] buffer, int offset, int count) => base.Read(buffer, offset, 1); } } From d1bdedd3659788bd3407013d0c14a64cfb7bf9e0 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 29 Sep 2022 20:29:52 +0200 Subject: [PATCH 10/18] Set stream position to strip offset + stripOffsetBytes after decompressing jpeg data --- .../Tiff/Compression/Decompressors/OldJpegTiffCompression.cs | 5 +++++ tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs | 2 ++ tests/ImageSharp.Tests/TestImages.cs | 2 ++ tests/Images/Input/Tiff/OldJpegCompression2.tiff | 3 +++ tests/Images/Input/Tiff/OldJpegCompression3.tiff | 3 +++ 5 files changed, 15 insertions(+) create mode 100644 tests/Images/Input/Tiff/OldJpegCompression2.tiff create mode 100644 tests/Images/Input/Tiff/OldJpegCompression3.tiff diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs index e6ddf9570..e57035a5a 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs @@ -34,9 +34,14 @@ internal sealed class OldJpegTiffCompression : TiffBaseDecompressor protected override void Decompress(BufferedReadStream stream, int byteCount, int stripHeight, Span buffer, CancellationToken cancellationToken) { + long stripOffset = stream.Position; stream.Position = this.startOfImageMarker; this.DecodeJpegData(stream, buffer, cancellationToken); + + // Setting the stream position to the expected position. + // This is a workaround for some images having set the stripBytesCount not equal to the compressed jpeg data. + stream.Position = stripOffset + byteCount; } private void DecodeJpegData(BufferedReadStream stream, Span buffer, CancellationToken cancellationToken) diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index 74b699c4b..461e817dc 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -666,6 +666,8 @@ public class TiffDecoderTests : TiffDecoderBaseTester [Theory] [WithFile(RgbOldJpegCompressed, PixelTypes.Rgba32)] + [WithFile(RgbOldJpegCompressed2, PixelTypes.Rgba32)] + [WithFile(RgbOldJpegCompressed3, PixelTypes.Rgba32)] [WithFile(YCbCrOldJpegCompressed, PixelTypes.Rgba32)] public void TiffDecoder_CanDecode_OldJpegCompressed(TestImageProvider provider) where TPixel : unmanaged, IPixel diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 9d017d506..de94f2b2f 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -796,6 +796,8 @@ public static class TestImages public const string RgbJpegCompressed = "Tiff/rgb_jpegcompression.tiff"; public const string RgbJpegCompressed2 = "Tiff/twain-rgb-jpeg-with-bogus-ycbcr-subsampling.tiff"; public const string RgbOldJpegCompressed = "Tiff/OldJpegCompression.tiff"; + public const string RgbOldJpegCompressed2 = "Tiff/OldJpegCompression2.tiff"; + public const string RgbOldJpegCompressed3 = "Tiff/OldJpegCompression3.tiff"; public const string YCbCrOldJpegCompressed = "Tiff/YCbCrOldJpegCompressed.tiff"; public const string RgbWithStripsJpegCompressed = "Tiff/rgb_jpegcompressed_stripped.tiff"; public const string RgbJpegCompressedNoJpegTable = "Tiff/rgb_jpegcompressed_nojpegtable.tiff"; diff --git a/tests/Images/Input/Tiff/OldJpegCompression2.tiff b/tests/Images/Input/Tiff/OldJpegCompression2.tiff new file mode 100644 index 000000000..1df25125d --- /dev/null +++ b/tests/Images/Input/Tiff/OldJpegCompression2.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:e8f722bc322b9a7621d1d858bec36ea78d31b85221f314a2b9872d3af91d600a +size 5052 diff --git a/tests/Images/Input/Tiff/OldJpegCompression3.tiff b/tests/Images/Input/Tiff/OldJpegCompression3.tiff new file mode 100644 index 000000000..38dce9d50 --- /dev/null +++ b/tests/Images/Input/Tiff/OldJpegCompression3.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:4b2b73290a40c49a4a29e51ad53dadea65c1ceafc6a5384430f7092d613830db +size 1390339 From 81c3ad3da5328350349216f249a5fe76ac974caf Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Fri, 30 Sep 2022 19:49:54 +0200 Subject: [PATCH 11/18] Revert "Fill read buffer in constructor and when IsInReadBuffer() is false" This reverts commit e066b839. --- src/ImageSharp/IO/BufferedReadStream.cs | 6 +-- .../IO/BufferedReadStreamTests.cs | 38 +++++-------------- 2 files changed, 12 insertions(+), 32 deletions(-) diff --git a/src/ImageSharp/IO/BufferedReadStream.cs b/src/ImageSharp/IO/BufferedReadStream.cs index 39d294ae5..7e233c965 100644 --- a/src/ImageSharp/IO/BufferedReadStream.cs +++ b/src/ImageSharp/IO/BufferedReadStream.cs @@ -49,6 +49,7 @@ internal sealed class BufferedReadStream : Stream this.BaseStream = stream; this.Length = stream.Length; + this.Position = (int)stream.Position; this.BufferSize = configuration.StreamProcessingBufferSize; this.maxBufferIndex = this.BufferSize - 1; this.readBuffer = ArrayPool.Shared.Rent(this.BufferSize); @@ -58,8 +59,8 @@ internal sealed class BufferedReadStream : Stream this.pinnedReadBuffer = (byte*)this.readBufferHandle.Pointer; } - this.Position = (int)stream.Position; - this.FillReadBuffer(); + // This triggers a full read on first attempt. + this.readBufferIndex = this.BufferSize; } /// @@ -98,7 +99,6 @@ internal sealed class BufferedReadStream : Stream this.BaseStream.Seek(value, SeekOrigin.Begin); this.readerPosition = value; this.readBufferIndex = this.BufferSize; - this.FillReadBuffer(); } } } diff --git a/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs b/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs index 619003a64..6a9c34486 100644 --- a/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs +++ b/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs @@ -1,7 +1,6 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -using System.IO; using SixLabors.ImageSharp.IO; namespace SixLabors.ImageSharp.Tests.IO; @@ -10,10 +9,13 @@ public class BufferedReadStreamTests { private readonly Configuration configuration; - public BufferedReadStreamTests() => this.configuration = Configuration.CreateDefaultInstance(); + public BufferedReadStreamTests() + { + this.configuration = Configuration.CreateDefaultInstance(); + } public static readonly TheoryData BufferSizes = - new() + new TheoryData() { 1, 2, 4, 8, 16, 97, 503, @@ -350,31 +352,6 @@ public class BufferedReadStreamTests } } - [Fact] - public void BufferedStreamSettingPositionWorksMultipleTimes() - { - // This test replicates an issue, which was caused by setting the stream position multiple times. - // This was then lead to the read buffer not being filled correctly. - int bufferSize = 16; - this.configuration.StreamProcessingBufferSize = bufferSize; - using MemoryStream stream = this.CreateTestStream(bufferSize * 2); - byte[] expected = stream.ToArray(); - using var reader = new BufferedReadStream(this.configuration, stream); - - // Read more then fits into the buffer. - for (int i = 0; i < 20; i++) - { - reader.ReadByte(); - } - - // Set the Position twice. - reader.Position = 10; - reader.Position = 3; - - int actual = reader.ReadByte(); - Assert.Equal(expected[3], actual); - } - private MemoryStream CreateTestStream(int length) { var buffer = new byte[length]; @@ -393,6 +370,9 @@ public class BufferedReadStreamTests { } - public override int Read(byte[] buffer, int offset, int count) => base.Read(buffer, offset, 1); + public override int Read(byte[] buffer, int offset, int count) + { + return base.Read(buffer, offset, 1); + } } } From 8593ebfbd6e3d579b89a6a359d1034f62950a1d5 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 12 Oct 2022 13:29:05 +0200 Subject: [PATCH 12/18] Add old jpeg compressed tiff test with bi-color --- tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs | 1 + tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Tiff/OldJpegCompressionGray.tiff | 3 +++ 3 files changed, 5 insertions(+) create mode 100644 tests/Images/Input/Tiff/OldJpegCompressionGray.tiff diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index 461e817dc..73c5c00bc 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -668,6 +668,7 @@ public class TiffDecoderTests : TiffDecoderBaseTester [WithFile(RgbOldJpegCompressed, PixelTypes.Rgba32)] [WithFile(RgbOldJpegCompressed2, PixelTypes.Rgba32)] [WithFile(RgbOldJpegCompressed3, PixelTypes.Rgba32)] + [WithFile(RgbOldJpegCompressedGray, PixelTypes.Rgba32)] [WithFile(YCbCrOldJpegCompressed, PixelTypes.Rgba32)] public void TiffDecoder_CanDecode_OldJpegCompressed(TestImageProvider provider) where TPixel : unmanaged, IPixel diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 816d6ea01..b7ecd5516 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -799,6 +799,7 @@ public static class TestImages public const string RgbOldJpegCompressed = "Tiff/OldJpegCompression.tiff"; public const string RgbOldJpegCompressed2 = "Tiff/OldJpegCompression2.tiff"; public const string RgbOldJpegCompressed3 = "Tiff/OldJpegCompression3.tiff"; + public const string RgbOldJpegCompressedGray = "Tiff/OldJpegCompressionGray.tiff"; public const string YCbCrOldJpegCompressed = "Tiff/YCbCrOldJpegCompressed.tiff"; public const string RgbWithStripsJpegCompressed = "Tiff/rgb_jpegcompressed_stripped.tiff"; public const string RgbJpegCompressedNoJpegTable = "Tiff/rgb_jpegcompressed_nojpegtable.tiff"; diff --git a/tests/Images/Input/Tiff/OldJpegCompressionGray.tiff b/tests/Images/Input/Tiff/OldJpegCompressionGray.tiff new file mode 100644 index 000000000..23a697a11 --- /dev/null +++ b/tests/Images/Input/Tiff/OldJpegCompressionGray.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:790662b0fc48d5604e80a9689eb28cfa231be3eadc33f34c9bf43c211cce5c16 +size 440613 From f23fdf27efce98aa3e0daccbe596b761f1297bb2 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 12 Oct 2022 13:33:58 +0200 Subject: [PATCH 13/18] Throw NotSupported exception with OldJpeg+Planar config --- src/ImageSharp/Formats/Tiff/README.md | 48 +++++++++---------- .../Formats/Tiff/TiffDecoderOptionsParser.cs | 5 ++ 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/README.md b/src/ImageSharp/Formats/Tiff/README.md index d64e3339c..0ce467e3f 100644 --- a/src/ImageSharp/Formats/Tiff/README.md +++ b/src/ImageSharp/Formats/Tiff/README.md @@ -30,34 +30,34 @@ ### Compression Formats -| |Encoder|Decoder|Comments | -|---------------------------|:-----:|:-----:|--------------------------| -|None | Y | Y | | -|Ccitt1D | Y | Y | | -|PackBits | Y | Y | | -|CcittGroup3Fax | Y | Y | | -|CcittGroup4Fax | Y | Y | | +| |Encoder|Decoder|Comments | +|---------------------------|:-----:|:-----:|-----------------------------------| +|None | Y | Y | | +|Ccitt1D | Y | Y | | +|PackBits | Y | Y | | +|CcittGroup3Fax | Y | Y | | +|CcittGroup4Fax | Y | Y | | |Lzw | Y | Y | Based on ImageSharp GIF LZW implementation - this code could be modified to be (i) shared, or (ii) optimised for each case. | -|Old Jpeg | | Y | | -|Jpeg (Technote 2) | Y | Y | | -|Deflate (Technote 2) | Y | Y | Based on PNG Deflate. | -|Old Deflate (Technote 2) | | Y | | -|Webp | | Y | | +|Old Jpeg | | Y | Only with chunky configuration. | +|Jpeg (Technote 2) | Y | Y | | +|Deflate (Technote 2) | Y | Y | Based on PNG Deflate. | +|Old Deflate (Technote 2) | | Y | | +|Webp | | Y | | ### Photometric Interpretation Formats -| |Encoder|Decoder|Comments | -|---------------------------|:-----:|:-----:|--------------------------| -|WhiteIsZero | Y | Y | General + 1/4/8-bit optimised implementations | -|BlackIsZero | Y | Y | General + 1/4/8-bit optimised implementations | -|Rgb (Chunky) | Y | Y | General + Rgb888 optimised implementation | -|Rgb (Planar) | | Y | General implementation only | -|PaletteColor | Y | Y | General implementation only | -|TransparencyMask | | | | -|Separated (TIFF Extension) | | | | -|YCbCr (TIFF Extension) | | Y | | -|CieLab (TIFF Extension) | | Y | | -|IccLab (TechNote 1) | | | | +| |Encoder|Decoder|Comments | +|---------------------------|:-----:|:-----:|-------------------------------------------| +|WhiteIsZero | Y | Y | General + 1/4/8-bit optimised implementations. | +|BlackIsZero | Y | Y | General + 1/4/8-bit optimised implementations. | +|Rgb (Chunky) | Y | Y | General + Rgb888 optimised implementation.| +|Rgb (Planar) | | Y | General implementation only. | +|PaletteColor | Y | Y | General implementation only. | +|TransparencyMask | | | | +|Separated (TIFF Extension) | | | | +|YCbCr (TIFF Extension) | | Y | | +|CieLab (TIFF Extension) | | Y | | +|IccLab (TechNote 1) | | | | ### Baseline TIFF Tags diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs index deaeb2deb..648b4a093 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -481,6 +481,11 @@ internal static class TiffDecoderOptionsParser TiffThrowHelper.ThrowNotSupported("Missing SOI marker offset for tiff with old jpeg compression"); } + if (options.PlanarConfiguration is TiffPlanarConfiguration.Planar) + { + TiffThrowHelper.ThrowNotSupported("Old Jpeg compression is not supported with planar configuration"); + } + options.CompressionType = TiffDecoderCompressionType.OldJpeg; if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr) From 8664ff9e6ca203a12c62e15b6c2471b3bf1b6fae Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 13 Oct 2022 10:03:59 +1000 Subject: [PATCH 14/18] Use PixelOperations for buffer copying. --- .../Decoder/SpectralConverter{TPixel}.cs | 19 +++++++++---------- .../Decompressors/JpegCompressionUtils.cs | 16 +++++++--------- .../Decompressors/JpegTiffCompression.cs | 9 +++++---- .../Decompressors/OldJpegTiffCompression.cs | 4 ++-- 4 files changed, 23 insertions(+), 25 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs index 8240a7458..b5780bf53 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs @@ -21,11 +21,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder; internal class SpectralConverter : SpectralConverter, IDisposable where TPixel : unmanaged, IPixel { - /// - /// instance associated with current - /// decoding routine. - /// - private readonly Configuration configuration; private JpegFrame frame; @@ -81,11 +76,15 @@ internal class SpectralConverter : SpectralConverter, IDisposable /// Optional target size for decoded image. public SpectralConverter(Configuration configuration, Size? targetSize = null) { - this.configuration = configuration; - + this.Configuration = configuration; this.targetSize = targetSize; } + /// + /// Gets the configuration instance associated with current decoding routine. + /// + public Configuration Configuration { get; } + /// /// Gets converted pixel buffer. /// @@ -177,7 +176,7 @@ internal class SpectralConverter : SpectralConverter, IDisposable { DebugGuard.IsTrue(this.colorConverter == null, "SpectralConverter.PrepareForDecoding() must be called once."); - MemoryAllocator allocator = this.configuration.MemoryAllocator; + MemoryAllocator allocator = this.Configuration.MemoryAllocator; // color converter from RGB to TPixel JpegColorConverterBase converter = this.GetColorConverter(this.frame, this.jpegData); @@ -196,7 +195,7 @@ internal class SpectralConverter : SpectralConverter, IDisposable this.pixelBuffer = allocator.Allocate2D( pixelSize.Width, pixelSize.Height, - this.configuration.PreferContiguousImageBuffers); + this.Configuration.PreferContiguousImageBuffers); this.paddedProxyPixelRow = allocator.Allocate(pixelSize.Width + 3); // component processors from spectral to RGB @@ -225,7 +224,7 @@ internal class SpectralConverter : SpectralConverter, IDisposable protected ComponentProcessor[] CreateComponentProcessors(JpegFrame frame, IRawJpegData jpegData, int blockPixelSize, Size processorBufferSize) { - MemoryAllocator allocator = this.configuration.MemoryAllocator; + MemoryAllocator allocator = this.Configuration.MemoryAllocator; var componentProcessors = new ComponentProcessor[frame.Components.Length]; for (int i = 0; i < componentProcessors.Length; i++) { diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegCompressionUtils.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegCompressionUtils.cs index a52c7e529..4577a2492 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegCompressionUtils.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegCompressionUtils.cs @@ -1,7 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -using System.Runtime.InteropServices; +using System.Runtime.CompilerServices; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.PixelFormats; @@ -9,27 +9,25 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Compression.Decompressors; internal static class JpegCompressionUtils { - public static void CopyImageBytesToBuffer(Span buffer, Buffer2D pixelBuffer) + public static void CopyImageBytesToBuffer(Configuration configuration, Span buffer, Buffer2D pixelBuffer) { int offset = 0; for (int y = 0; y < pixelBuffer.Height; y++) { Span pixelRowSpan = pixelBuffer.DangerousGetRowSpan(y); - Span rgbBytes = MemoryMarshal.AsBytes(pixelRowSpan); - rgbBytes.CopyTo(buffer[offset..]); - offset += rgbBytes.Length; + PixelOperations.Instance.ToRgb24Bytes(configuration, pixelRowSpan, buffer[offset..], pixelRowSpan.Length); + offset += Unsafe.SizeOf() * pixelRowSpan.Length; } } - public static void CopyImageBytesToBuffer(Span buffer, Buffer2D pixelBuffer) + public static void CopyImageBytesToBuffer(Configuration configuration, Span buffer, Buffer2D pixelBuffer) { int offset = 0; for (int y = 0; y < pixelBuffer.Height; y++) { Span pixelRowSpan = pixelBuffer.DangerousGetRowSpan(y); - Span rgbBytes = MemoryMarshal.AsBytes(pixelRowSpan); - rgbBytes.CopyTo(buffer[offset..]); - offset += rgbBytes.Length; + PixelOperations.Instance.ToL8Bytes(configuration, pixelRowSpan, buffer[offset..], pixelRowSpan.Length); + offset += Unsafe.SizeOf() * pixelRowSpan.Length; } } } diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs index 818bb3d6d..2cbe3eaec 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.Formats.Jpeg; using SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder; using SixLabors.ImageSharp.Formats.Tiff.Constants; @@ -73,8 +74,8 @@ internal sealed class JpegTiffCompression : TiffBaseDecompressor } else { - using Image image = Image.Load(stream); - JpegCompressionUtils.CopyImageBytesToBuffer(buffer, image.Frames.RootFrame.PixelBuffer); + using Image image = Image.Load(this.options.GeneralOptions, stream); + JpegCompressionUtils.CopyImageBytesToBuffer(this.options.GeneralOptions.Configuration, buffer, image.Frames.RootFrame.PixelBuffer); } } @@ -94,7 +95,7 @@ internal sealed class JpegTiffCompression : TiffBaseDecompressor jpegDecoder.ParseStream(stream, spectralConverterGray, cancellationToken); using Buffer2D decompressedBuffer = spectralConverterGray.GetPixelBuffer(cancellationToken); - JpegCompressionUtils.CopyImageBytesToBuffer(buffer, decompressedBuffer); + JpegCompressionUtils.CopyImageBytesToBuffer(spectralConverterGray.Configuration, buffer, decompressedBuffer); break; } @@ -108,7 +109,7 @@ internal sealed class JpegTiffCompression : TiffBaseDecompressor jpegDecoder.ParseStream(stream, spectralConverter, cancellationToken); using Buffer2D decompressedBuffer = spectralConverter.GetPixelBuffer(cancellationToken); - JpegCompressionUtils.CopyImageBytesToBuffer(buffer, decompressedBuffer); + JpegCompressionUtils.CopyImageBytesToBuffer(spectralConverter.Configuration, buffer, decompressedBuffer); break; } diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs index e57035a5a..978c93cb0 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs @@ -58,7 +58,7 @@ internal sealed class OldJpegTiffCompression : TiffBaseDecompressor jpegDecoder.ParseStream(stream, spectralConverterGray, cancellationToken); using Buffer2D decompressedBuffer = spectralConverterGray.GetPixelBuffer(cancellationToken); - JpegCompressionUtils.CopyImageBytesToBuffer(buffer, decompressedBuffer); + JpegCompressionUtils.CopyImageBytesToBuffer(spectralConverterGray.Configuration, buffer, decompressedBuffer); break; } @@ -70,7 +70,7 @@ internal sealed class OldJpegTiffCompression : TiffBaseDecompressor jpegDecoder.ParseStream(stream, spectralConverter, cancellationToken); using Buffer2D decompressedBuffer = spectralConverter.GetPixelBuffer(cancellationToken); - JpegCompressionUtils.CopyImageBytesToBuffer(buffer, decompressedBuffer); + JpegCompressionUtils.CopyImageBytesToBuffer(spectralConverter.Configuration, buffer, decompressedBuffer); break; } From a6dd6e3efbe2bfd2b6a3b90f778d5cb270f55b0d Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 13 Oct 2022 10:06:51 +1000 Subject: [PATCH 15/18] Revert constructor parameter ordering --- .../Tiff/Compression/Decompressors/JpegTiffCompression.cs | 4 ++-- .../Formats/Tiff/Compression/TiffDecompressorsFactory.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs index 2cbe3eaec..16747e1f3 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs @@ -25,17 +25,17 @@ internal sealed class JpegTiffCompression : TiffBaseDecompressor /// /// Initializes a new instance of the class. /// + /// The specialized jpeg decoder options. /// The memoryAllocator to use for buffer allocations. /// The image width. /// The bits per pixel. - /// The specialized jpeg decoder options. /// The JPEG tables containing the quantization and/or Huffman tables. /// The photometric interpretation. public JpegTiffCompression( + JpegDecoderOptions options, MemoryAllocator memoryAllocator, int width, int bitsPerPixel, - JpegDecoderOptions options, byte[] jpegTables, TiffPhotometricInterpretation photometricInterpretation) : base(memoryAllocator, width, bitsPerPixel) diff --git a/src/ImageSharp/Formats/Tiff/Compression/TiffDecompressorsFactory.cs b/src/ImageSharp/Formats/Tiff/Compression/TiffDecompressorsFactory.cs index 7b1f67b12..c4f73862a 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/TiffDecompressorsFactory.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/TiffDecompressorsFactory.cs @@ -59,7 +59,7 @@ internal static class TiffDecompressorsFactory case TiffDecoderCompressionType.Jpeg: DebugGuard.IsTrue(predictor == TiffPredictor.None, "Predictor should only be used with lzw or deflate compression"); - return new JpegTiffCompression(allocator, width, bitsPerPixel, new() { GeneralOptions = options }, jpegTables, photometricInterpretation); + return new JpegTiffCompression(new() { GeneralOptions = options }, allocator, width, bitsPerPixel, jpegTables, photometricInterpretation); case TiffDecoderCompressionType.OldJpeg: DebugGuard.IsTrue(predictor == TiffPredictor.None, "Predictor should only be used with lzw or deflate compression"); From f0038646fff10ae487db0535a7da5f70aeb0c561 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 13 Oct 2022 10:08:29 +1000 Subject: [PATCH 16/18] Remove unused constructor --- .../Decompressors/JpegTiffCompression.cs | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs index 16747e1f3..82b26232a 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs @@ -1,7 +1,6 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.Formats.Jpeg; using SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder; using SixLabors.ImageSharp.Formats.Tiff.Constants; @@ -45,26 +44,6 @@ internal sealed class JpegTiffCompression : TiffBaseDecompressor this.photometricInterpretation = photometricInterpretation; } - /// - /// Initializes a new instance of the class. - /// - /// The memoryAllocator to use for buffer allocations. - /// The image width. - /// The bits per pixel. - /// The specialized jpeg decoder options. - /// The photometric interpretation. - public JpegTiffCompression( - MemoryAllocator memoryAllocator, - int width, - int bitsPerPixel, - JpegDecoderOptions options, - TiffPhotometricInterpretation photometricInterpretation) - : base(memoryAllocator, width, bitsPerPixel) - { - this.options = options; - this.photometricInterpretation = photometricInterpretation; - } - /// protected override void Decompress(BufferedReadStream stream, int byteCount, int stripHeight, Span buffer, CancellationToken cancellationToken) { From 984327a004e263b554b16c35bb97c424950b7475 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 13 Oct 2022 10:11:22 +1000 Subject: [PATCH 17/18] Move options to first parameter position --- .../Tiff/Compression/Decompressors/OldJpegTiffCompression.cs | 2 +- .../Tiff/Compression/Decompressors/WebpTiffCompression.cs | 4 ++-- .../Formats/Tiff/Compression/TiffDecompressorsFactory.cs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs index 978c93cb0..b58183ff6 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/OldJpegTiffCompression.cs @@ -19,10 +19,10 @@ internal sealed class OldJpegTiffCompression : TiffBaseDecompressor private readonly TiffPhotometricInterpretation photometricInterpretation; public OldJpegTiffCompression( + JpegDecoderOptions options, MemoryAllocator memoryAllocator, int width, int bitsPerPixel, - JpegDecoderOptions options, uint startOfImageMarker, TiffPhotometricInterpretation photometricInterpretation) : base(memoryAllocator, width, bitsPerPixel) diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/WebpTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/WebpTiffCompression.cs index 347f09522..4e1c9c2f8 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/WebpTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/WebpTiffCompression.cs @@ -20,12 +20,12 @@ internal class WebpTiffCompression : TiffBaseDecompressor /// /// Initializes a new instance of the class. /// + /// The general decoder options. /// The memory allocator. /// The width of the image. /// The bits per pixel. - /// The general decoder options. /// The predictor. - public WebpTiffCompression(MemoryAllocator memoryAllocator, int width, int bitsPerPixel, DecoderOptions options, TiffPredictor predictor = TiffPredictor.None) + public WebpTiffCompression(DecoderOptions options, MemoryAllocator memoryAllocator, int width, int bitsPerPixel, TiffPredictor predictor = TiffPredictor.None) : base(memoryAllocator, width, bitsPerPixel, predictor) => this.options = options; diff --git a/src/ImageSharp/Formats/Tiff/Compression/TiffDecompressorsFactory.cs b/src/ImageSharp/Formats/Tiff/Compression/TiffDecompressorsFactory.cs index c4f73862a..b9a1f3155 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/TiffDecompressorsFactory.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/TiffDecompressorsFactory.cs @@ -63,11 +63,11 @@ internal static class TiffDecompressorsFactory case TiffDecoderCompressionType.OldJpeg: DebugGuard.IsTrue(predictor == TiffPredictor.None, "Predictor should only be used with lzw or deflate compression"); - return new OldJpegTiffCompression(allocator, width, bitsPerPixel, new() { GeneralOptions = options }, oldJpegStartOfImageMarker, photometricInterpretation); + return new OldJpegTiffCompression(new() { GeneralOptions = options }, allocator, width, bitsPerPixel, oldJpegStartOfImageMarker, photometricInterpretation); case TiffDecoderCompressionType.Webp: DebugGuard.IsTrue(predictor == TiffPredictor.None, "Predictor should only be used with lzw or deflate compression"); - return new WebpTiffCompression(allocator, width, bitsPerPixel, options); + return new WebpTiffCompression(options, allocator, width, bitsPerPixel); default: throw TiffThrowHelper.NotSupportedDecompressor(nameof(method)); From af3eb7dd1f4a1004b437f3066e87917dd2459a72 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 13 Oct 2022 10:44:50 +1000 Subject: [PATCH 18/18] Update SpectralConverter{TPixel}.cs --- .../Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs index b5780bf53..510635cbb 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs @@ -21,7 +21,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder; internal class SpectralConverter : SpectralConverter, IDisposable where TPixel : unmanaged, IPixel { - private JpegFrame frame; private IRawJpegData jpegData; @@ -133,7 +132,7 @@ internal class SpectralConverter : SpectralConverter, IDisposable { int y = yy - this.pixelRowCounter; - var values = new JpegColorConverterBase.ComponentValues(this.componentProcessors, y); + JpegColorConverterBase.ComponentValues values = new(this.componentProcessors, y); this.colorConverter.ConvertToRgbInplace(values); values = values.Slice(0, width); // slice away Jpeg padding @@ -202,7 +201,7 @@ internal class SpectralConverter : SpectralConverter, IDisposable int bufferWidth = majorBlockWidth * blockPixelSize; int batchSize = converter.ElementsPerBatch; int batchRemainder = bufferWidth & (batchSize - 1); - var postProcessorBufferSize = new Size(bufferWidth + (batchSize - batchRemainder), this.pixelRowsPerStep); + Size postProcessorBufferSize = new(bufferWidth + (batchSize - batchRemainder), this.pixelRowsPerStep); this.componentProcessors = this.CreateComponentProcessors(this.frame, this.jpegData, blockPixelSize, postProcessorBufferSize); // single 'stride' rgba32 buffer for conversion between spectral and TPixel @@ -225,7 +224,7 @@ internal class SpectralConverter : SpectralConverter, IDisposable protected ComponentProcessor[] CreateComponentProcessors(JpegFrame frame, IRawJpegData jpegData, int blockPixelSize, Size processorBufferSize) { MemoryAllocator allocator = this.Configuration.MemoryAllocator; - var componentProcessors = new ComponentProcessor[frame.Components.Length]; + ComponentProcessor[] componentProcessors = new ComponentProcessor[frame.Components.Length]; for (int i = 0; i < componentProcessors.Length; i++) { componentProcessors[i] = blockPixelSize switch