From 6245666d61cfc2d79981b81806b3d253d9bc8b07 Mon Sep 17 00:00:00 2001 From: LuisAlfredo92 <92luisalfredo@protonmail.com> Date: Thu, 29 Jun 2023 13:39:45 -0600 Subject: [PATCH] Fixing most of things of review Also I fixed a little bug with PreviouslySeenPixels array It's weird, but I don't see any problems with encoding and the tests are ok, I wrote the memory stream to files and they look the same, but the hashes and bytes aren't the same This is very weird --- .../Qoi/{QoiChunkEnum.cs => QoiChunk.cs} | 2 +- src/ImageSharp/Formats/Qoi/QoiDecoderCore.cs | 16 ++++----- src/ImageSharp/Formats/Qoi/QoiEncoder.cs | 18 +++++++++- src/ImageSharp/Formats/Qoi/QoiEncoderCore.cs | 33 ++++++++----------- src/ImageSharp/Formats/Qoi/QoiMetadata.cs | 12 ------- .../Formats/Qoi/QoiEncoderTests.cs | 5 +-- 6 files changed, 43 insertions(+), 43 deletions(-) rename src/ImageSharp/Formats/Qoi/{QoiChunkEnum.cs => QoiChunk.cs} (98%) diff --git a/src/ImageSharp/Formats/Qoi/QoiChunkEnum.cs b/src/ImageSharp/Formats/Qoi/QoiChunk.cs similarity index 98% rename from src/ImageSharp/Formats/Qoi/QoiChunkEnum.cs rename to src/ImageSharp/Formats/Qoi/QoiChunk.cs index 7fb0de8da1..06886b9691 100644 --- a/src/ImageSharp/Formats/Qoi/QoiChunkEnum.cs +++ b/src/ImageSharp/Formats/Qoi/QoiChunk.cs @@ -7,7 +7,7 @@ namespace SixLabors.ImageSharp.Formats.Qoi; /// Enum that contains the operations that encoder and decoder must process, written /// in binary to be easier to compare them in the reference /// -public enum QoiChunkEnum +internal enum QoiChunk { /// /// Indicates that the operation is QOI_OP_RGB where the RGB values are written diff --git a/src/ImageSharp/Formats/Qoi/QoiDecoderCore.cs b/src/ImageSharp/Formats/Qoi/QoiDecoderCore.cs index 844d7bc7a2..db383efd9d 100644 --- a/src/ImageSharp/Formats/Qoi/QoiDecoderCore.cs +++ b/src/ImageSharp/Formats/Qoi/QoiDecoderCore.cs @@ -160,10 +160,10 @@ internal class QoiDecoderCore : IImageDecoderInternals byte[] pixelBytes; Rgba32 readPixel; TPixel pixel = default; - switch ((QoiChunkEnum)operationByte) + switch ((QoiChunk)operationByte) { // Reading one pixel with previous alpha intact - case QoiChunkEnum.QoiOpRgb: + case QoiChunk.QoiOpRgb: pixelBytes = new byte[3]; if (stream.Read(pixelBytes) < 3) { @@ -177,7 +177,7 @@ internal class QoiDecoderCore : IImageDecoderInternals break; // Reading one pixel with new alpha - case QoiChunkEnum.QoiOpRgba: + case QoiChunk.QoiOpRgba: pixelBytes = new byte[4]; if (stream.Read(pixelBytes) < 4) { @@ -191,16 +191,16 @@ internal class QoiDecoderCore : IImageDecoderInternals break; default: - switch ((QoiChunkEnum)(operationByte & 0b11000000)) + switch ((QoiChunk)(operationByte & 0b11000000)) { // Getting one pixel from previously seen pixels - case QoiChunkEnum.QoiOpIndex: + case QoiChunk.QoiOpIndex: readPixel = previouslySeenPixels[operationByte]; pixel.FromRgba32(readPixel); break; // Get one pixel from the difference (-2..1) of the previous pixel - case QoiChunkEnum.QoiOpDiff: + case QoiChunk.QoiOpDiff: byte redDifference = (byte)((operationByte & 0b00110000) >> 4), greenDifference = (byte)((operationByte & 0b00001100) >> 2), blueDifference = (byte)(operationByte & 0b00000011); @@ -217,7 +217,7 @@ internal class QoiDecoderCore : IImageDecoderInternals // Get green difference in 6 bits and red and blue differences // depending on the green one - case QoiChunkEnum.QoiOpLuma: + case QoiChunk.QoiOpLuma: byte diffGreen = (byte)(operationByte & 0b00111111), currentGreen = (byte)((previousPixel.G + (diffGreen - 32)) % 256), nextByte = (byte)stream.ReadByte(), @@ -232,7 +232,7 @@ internal class QoiDecoderCore : IImageDecoderInternals break; // Repeating the previous pixel 1..63 times - case QoiChunkEnum.QoiOpRun: + case QoiChunk.QoiOpRun: byte repetitions = (byte)(operationByte & 0b00111111); if (repetitions is 62 or 63) { diff --git a/src/ImageSharp/Formats/Qoi/QoiEncoder.cs b/src/ImageSharp/Formats/Qoi/QoiEncoder.cs index 699962edb7..50d9cb30f7 100644 --- a/src/ImageSharp/Formats/Qoi/QoiEncoder.cs +++ b/src/ImageSharp/Formats/Qoi/QoiEncoder.cs @@ -1,6 +1,8 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Text; + namespace SixLabors.ImageSharp.Formats.Qoi; /// @@ -8,10 +10,24 @@ namespace SixLabors.ImageSharp.Formats.Qoi; /// public class QoiEncoder : ImageEncoder { + /// + /// Gets the color channels on the image that can be + /// RGB or RGBA. This is purely informative. It doesn't + /// change the way data chunks are encoded. + /// + public QoiChannels? Channels { get; init; } + + /// + /// Gets the color space of the image that can be sRGB with + /// linear alpha or all channels linear. This is purely + /// informative. It doesn't change the way data chunks are encoded. + /// + public QoiColorSpace? ColorSpace { get; init; } + /// protected override void Encode(Image image, Stream stream, CancellationToken cancellationToken) { - QoiEncoderCore encoder = new(); + QoiEncoderCore encoder = new(this); encoder.Encode(image, stream, cancellationToken); } } diff --git a/src/ImageSharp/Formats/Qoi/QoiEncoderCore.cs b/src/ImageSharp/Formats/Qoi/QoiEncoderCore.cs index 2a35713367..a7ec49b146 100644 --- a/src/ImageSharp/Formats/Qoi/QoiEncoderCore.cs +++ b/src/ImageSharp/Formats/Qoi/QoiEncoderCore.cs @@ -12,12 +12,11 @@ namespace SixLabors.ImageSharp.Formats.Qoi; /// public class QoiEncoderCore : IImageEncoderInternals { + private readonly QoiEncoder encoder; /// /// Initializes a new instance of the class. /// - public QoiEncoderCore() - { - } + public QoiEncoderCore(QoiEncoder encoder) => this.encoder = encoder; /// public void Encode(Image image, Stream stream, CancellationToken cancellationToken) @@ -26,23 +25,21 @@ public class QoiEncoderCore : IImageEncoderInternals Guard.NotNull(image, nameof(image)); Guard.NotNull(stream, nameof(stream)); - WriteHeader(image, stream); + this.WriteHeader(image, stream); WritePixels(image, stream); WriteEndOfStream(stream); stream.Flush(); } - private static void WriteHeader(Image image, Stream stream) + private void WriteHeader(Image image, Stream stream) { // Get metadata Span width = stackalloc byte[4]; Span height = stackalloc byte[4]; BinaryPrimitives.WriteUInt32BigEndian(width, (uint)image.Width); BinaryPrimitives.WriteUInt32BigEndian(height, (uint)image.Height); - QoiChannels qoiChannels = image.PixelType.BitsPerPixel == 24 ? QoiChannels.Rgb : QoiChannels.Rgba; - - // I need to check this, how do I check it with the pixel type or metadata of the original image? - const QoiColorSpace qoiColorSpace = QoiColorSpace.SrgbWithLinearAlpha; + QoiChannels qoiChannels = this.encoder.Channels ?? QoiChannels.Rgba; + QoiColorSpace qoiColorSpace = this.encoder.ColorSpace ?? QoiColorSpace.SrgbWithLinearAlpha; // Write header to the stream stream.Write(QoiConstants.Magic); @@ -58,11 +55,9 @@ public class QoiEncoderCore : IImageEncoderInternals // Start image encoding Rgba32[] previouslySeenPixels = new Rgba32[64]; Rgba32 previousPixel = new(0, 0, 0, 255); - int pixelArrayPosition = GetArrayPosition(previousPixel); - previouslySeenPixels[pixelArrayPosition] = previousPixel; - - Buffer2D pixels = image.Frames[0].PixelBuffer; Rgba32 currentRgba32 = default; + Buffer2D pixels = image.Frames[0].PixelBuffer; + for (int i = 0; i < pixels.Height; i++) { for (int j = 0; j < pixels.Width && i < pixels.Height; j++) @@ -105,7 +100,7 @@ public class QoiEncoderCore : IImageEncoderInternals while (currentRgba32.Equals(previousPixel) && repetitions < 62); j--; - stream.WriteByte((byte)((byte)QoiChunkEnum.QoiOpRun | (repetitions - 1))); + stream.WriteByte((byte)((byte)QoiChunk.QoiOpRun | (repetitions - 1))); /* If it's a QOI_OP_RUN, we don't overwrite the previous pixel since * it will be taken and compared on the next iteration @@ -115,7 +110,7 @@ public class QoiEncoderCore : IImageEncoderInternals // else, we check if it exists in the previously seen pixels // If so, we do a QOI_OP_INDEX - pixelArrayPosition = GetArrayPosition(currentRgba32); + int pixelArrayPosition = GetArrayPosition(currentRgba32); if (previouslySeenPixels[pixelArrayPosition].Equals(currentPixel)) { stream.WriteByte((byte)pixelArrayPosition); @@ -140,7 +135,7 @@ public class QoiEncoderCore : IImageEncoderInternals byte dr = (byte)(diffRed + 2), dg = (byte)(diffGreen + 2), db = (byte)(diffBlue + 2), - valueToWrite = (byte)((byte)QoiChunkEnum.QoiOpDiff | (dr << 4) | (dg << 2) | db); + valueToWrite = (byte)((byte)QoiChunk.QoiOpDiff | (dr << 4) | (dg << 2) | db); stream.WriteByte(valueToWrite); } else @@ -156,7 +151,7 @@ public class QoiEncoderCore : IImageEncoderInternals { byte dr_dg = (byte)(diffRedGreen + 8), db_dg = (byte)(diffBlueGreen + 8), - byteToWrite1 = (byte)((byte)QoiChunkEnum.QoiOpLuma | (diffGreen + 32)), + byteToWrite1 = (byte)((byte)QoiChunk.QoiOpLuma | (diffGreen + 32)), byteToWrite2 = (byte)((dr_dg << 4) | db_dg); stream.WriteByte(byteToWrite1); stream.WriteByte(byteToWrite2); @@ -167,7 +162,7 @@ public class QoiEncoderCore : IImageEncoderInternals // If so, we do a QOI_OP_RGB if (currentRgba32.A == previousPixel.A) { - stream.WriteByte((byte)QoiChunkEnum.QoiOpRgb); + stream.WriteByte((byte)QoiChunk.QoiOpRgb); stream.WriteByte(currentRgba32.R); stream.WriteByte(currentRgba32.G); stream.WriteByte(currentRgba32.B); @@ -175,7 +170,7 @@ public class QoiEncoderCore : IImageEncoderInternals else { // else, we do a QOI_OP_RGBA - stream.WriteByte((byte)QoiChunkEnum.QoiOpRgba); + stream.WriteByte((byte)QoiChunk.QoiOpRgba); stream.WriteByte(currentRgba32.R); stream.WriteByte(currentRgba32.G); stream.WriteByte(currentRgba32.B); diff --git a/src/ImageSharp/Formats/Qoi/QoiMetadata.cs b/src/ImageSharp/Formats/Qoi/QoiMetadata.cs index 11ab314a8b..3440dfc1f6 100644 --- a/src/ImageSharp/Formats/Qoi/QoiMetadata.cs +++ b/src/ImageSharp/Formats/Qoi/QoiMetadata.cs @@ -21,22 +21,10 @@ public class QoiMetadata : IDeepCloneable /// The metadata to create an instance from. public QoiMetadata(QoiMetadata other) { - this.Width = other.Width; - this.Height = other.Height; this.Channels = other.Channels; this.ColorSpace = other.ColorSpace; } - /// - /// Gets or sets image width in pixels (BE) - /// - public uint Width { get; set; } - - /// - /// Gets or sets image height in pixels (BE) - /// - public uint Height { get; set; } - /// /// Gets or sets color channels of the image. 3 = RGB, 4 = RGBA. /// diff --git a/tests/ImageSharp.Tests/Formats/Qoi/QoiEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Qoi/QoiEncoderTests.cs index 4d69a1f060..90813da690 100644 --- a/tests/ImageSharp.Tests/Formats/Qoi/QoiEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Qoi/QoiEncoderTests.cs @@ -4,6 +4,7 @@ using SixLabors.ImageSharp.Formats.Qoi; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; +using SixLabors.ImageSharp.Tests.TestUtilities.ReferenceCodecs; namespace SixLabors.ImageSharp.Tests.Formats.Qoi; @@ -24,12 +25,12 @@ public class QoiEncoderTests private static void Encode(TestImageProvider provider) where TPixel : unmanaged, IPixel { - using Image image = provider.GetImage(); + using Image image = provider.GetImage(new MagickReferenceDecoder()); using MemoryStream stream = new(); QoiEncoder encoder = new(); image.Save(stream, encoder); stream.Position = 0; using Image encodedImage = (Image)Image.Load(stream); - ImageComparingUtils.CompareWithReferenceDecoder(provider, encodedImage); + ImageComparer.Exact.CompareImages(image, encodedImage); } }