Skip to content

Commit 6245666

Browse files
committed
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
1 parent aed9acd commit 6245666

File tree

6 files changed

+43
-43
lines changed

6 files changed

+43
-43
lines changed

src/ImageSharp/Formats/Qoi/QoiChunkEnum.cs renamed to src/ImageSharp/Formats/Qoi/QoiChunk.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace SixLabors.ImageSharp.Formats.Qoi;
77
/// Enum that contains the operations that encoder and decoder must process, written
88
/// in binary to be easier to compare them in the reference
99
/// </summary>
10-
public enum QoiChunkEnum
10+
internal enum QoiChunk
1111
{
1212
/// <summary>
1313
/// Indicates that the operation is QOI_OP_RGB where the RGB values are written

src/ImageSharp/Formats/Qoi/QoiDecoderCore.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,10 @@ private void ProcessPixels<TPixel>(Stream stream, Buffer2D<TPixel> pixels)
160160
byte[] pixelBytes;
161161
Rgba32 readPixel;
162162
TPixel pixel = default;
163-
switch ((QoiChunkEnum)operationByte)
163+
switch ((QoiChunk)operationByte)
164164
{
165165
// Reading one pixel with previous alpha intact
166-
case QoiChunkEnum.QoiOpRgb:
166+
case QoiChunk.QoiOpRgb:
167167
pixelBytes = new byte[3];
168168
if (stream.Read(pixelBytes) < 3)
169169
{
@@ -177,7 +177,7 @@ private void ProcessPixels<TPixel>(Stream stream, Buffer2D<TPixel> pixels)
177177
break;
178178

179179
// Reading one pixel with new alpha
180-
case QoiChunkEnum.QoiOpRgba:
180+
case QoiChunk.QoiOpRgba:
181181
pixelBytes = new byte[4];
182182
if (stream.Read(pixelBytes) < 4)
183183
{
@@ -191,16 +191,16 @@ private void ProcessPixels<TPixel>(Stream stream, Buffer2D<TPixel> pixels)
191191
break;
192192

193193
default:
194-
switch ((QoiChunkEnum)(operationByte & 0b11000000))
194+
switch ((QoiChunk)(operationByte & 0b11000000))
195195
{
196196
// Getting one pixel from previously seen pixels
197-
case QoiChunkEnum.QoiOpIndex:
197+
case QoiChunk.QoiOpIndex:
198198
readPixel = previouslySeenPixels[operationByte];
199199
pixel.FromRgba32(readPixel);
200200
break;
201201

202202
// Get one pixel from the difference (-2..1) of the previous pixel
203-
case QoiChunkEnum.QoiOpDiff:
203+
case QoiChunk.QoiOpDiff:
204204
byte redDifference = (byte)((operationByte & 0b00110000) >> 4),
205205
greenDifference = (byte)((operationByte & 0b00001100) >> 2),
206206
blueDifference = (byte)(operationByte & 0b00000011);
@@ -217,7 +217,7 @@ private void ProcessPixels<TPixel>(Stream stream, Buffer2D<TPixel> pixels)
217217

218218
// Get green difference in 6 bits and red and blue differences
219219
// depending on the green one
220-
case QoiChunkEnum.QoiOpLuma:
220+
case QoiChunk.QoiOpLuma:
221221
byte diffGreen = (byte)(operationByte & 0b00111111),
222222
currentGreen = (byte)((previousPixel.G + (diffGreen - 32)) % 256),
223223
nextByte = (byte)stream.ReadByte(),
@@ -232,7 +232,7 @@ private void ProcessPixels<TPixel>(Stream stream, Buffer2D<TPixel> pixels)
232232
break;
233233

234234
// Repeating the previous pixel 1..63 times
235-
case QoiChunkEnum.QoiOpRun:
235+
case QoiChunk.QoiOpRun:
236236
byte repetitions = (byte)(operationByte & 0b00111111);
237237
if (repetitions is 62 or 63)
238238
{
Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,33 @@
11
// Copyright (c) Six Labors.
22
// Licensed under the Six Labors Split License.
33

4+
using System.Text;
5+
46
namespace SixLabors.ImageSharp.Formats.Qoi;
57

68
/// <summary>
79
/// Image encoder for writing an image to a stream as a QOI image
810
/// </summary>
911
public class QoiEncoder : ImageEncoder
1012
{
13+
/// <summary>
14+
/// Gets the color channels on the image that can be
15+
/// RGB or RGBA. This is purely informative. It doesn't
16+
/// change the way data chunks are encoded.
17+
/// </summary>
18+
public QoiChannels? Channels { get; init; }
19+
20+
/// <summary>
21+
/// Gets the color space of the image that can be sRGB with
22+
/// linear alpha or all channels linear. This is purely
23+
/// informative. It doesn't change the way data chunks are encoded.
24+
/// </summary>
25+
public QoiColorSpace? ColorSpace { get; init; }
26+
1127
/// <inheritdoc />
1228
protected override void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken cancellationToken)
1329
{
14-
QoiEncoderCore encoder = new();
30+
QoiEncoderCore encoder = new(this);
1531
encoder.Encode(image, stream, cancellationToken);
1632
}
1733
}

src/ImageSharp/Formats/Qoi/QoiEncoderCore.cs

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,11 @@ namespace SixLabors.ImageSharp.Formats.Qoi;
1212
/// </summary>
1313
public class QoiEncoderCore : IImageEncoderInternals
1414
{
15+
private readonly QoiEncoder encoder;
1516
/// <summary>
1617
/// Initializes a new instance of the <see cref="QoiEncoderCore"/> class.
1718
/// </summary>
18-
public QoiEncoderCore()
19-
{
20-
}
19+
public QoiEncoderCore(QoiEncoder encoder) => this.encoder = encoder;
2120

2221
/// <inheritdoc />
2322
public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken cancellationToken)
@@ -26,23 +25,21 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
2625
Guard.NotNull(image, nameof(image));
2726
Guard.NotNull(stream, nameof(stream));
2827

29-
WriteHeader(image, stream);
28+
this.WriteHeader(image, stream);
3029
WritePixels(image, stream);
3130
WriteEndOfStream(stream);
3231
stream.Flush();
3332
}
3433

35-
private static void WriteHeader(Image image, Stream stream)
34+
private void WriteHeader(Image image, Stream stream)
3635
{
3736
// Get metadata
3837
Span<byte> width = stackalloc byte[4];
3938
Span<byte> height = stackalloc byte[4];
4039
BinaryPrimitives.WriteUInt32BigEndian(width, (uint)image.Width);
4140
BinaryPrimitives.WriteUInt32BigEndian(height, (uint)image.Height);
42-
QoiChannels qoiChannels = image.PixelType.BitsPerPixel == 24 ? QoiChannels.Rgb : QoiChannels.Rgba;
43-
44-
// I need to check this, how do I check it with the pixel type or metadata of the original image?
45-
const QoiColorSpace qoiColorSpace = QoiColorSpace.SrgbWithLinearAlpha;
41+
QoiChannels qoiChannels = this.encoder.Channels ?? QoiChannels.Rgba;
42+
QoiColorSpace qoiColorSpace = this.encoder.ColorSpace ?? QoiColorSpace.SrgbWithLinearAlpha;
4643

4744
// Write header to the stream
4845
stream.Write(QoiConstants.Magic);
@@ -58,11 +55,9 @@ private static void WritePixels<TPixel>(Image<TPixel> image, Stream stream)
5855
// Start image encoding
5956
Rgba32[] previouslySeenPixels = new Rgba32[64];
6057
Rgba32 previousPixel = new(0, 0, 0, 255);
61-
int pixelArrayPosition = GetArrayPosition(previousPixel);
62-
previouslySeenPixels[pixelArrayPosition] = previousPixel;
63-
64-
Buffer2D<TPixel> pixels = image.Frames[0].PixelBuffer;
6558
Rgba32 currentRgba32 = default;
59+
Buffer2D<TPixel> pixels = image.Frames[0].PixelBuffer;
60+
6661
for (int i = 0; i < pixels.Height; i++)
6762
{
6863
for (int j = 0; j < pixels.Width && i < pixels.Height; j++)
@@ -105,7 +100,7 @@ private static void WritePixels<TPixel>(Image<TPixel> image, Stream stream)
105100
while (currentRgba32.Equals(previousPixel) && repetitions < 62);
106101

107102
j--;
108-
stream.WriteByte((byte)((byte)QoiChunkEnum.QoiOpRun | (repetitions - 1)));
103+
stream.WriteByte((byte)((byte)QoiChunk.QoiOpRun | (repetitions - 1)));
109104

110105
/* If it's a QOI_OP_RUN, we don't overwrite the previous pixel since
111106
* it will be taken and compared on the next iteration
@@ -115,7 +110,7 @@ private static void WritePixels<TPixel>(Image<TPixel> image, Stream stream)
115110

116111
// else, we check if it exists in the previously seen pixels
117112
// If so, we do a QOI_OP_INDEX
118-
pixelArrayPosition = GetArrayPosition(currentRgba32);
113+
int pixelArrayPosition = GetArrayPosition(currentRgba32);
119114
if (previouslySeenPixels[pixelArrayPosition].Equals(currentPixel))
120115
{
121116
stream.WriteByte((byte)pixelArrayPosition);
@@ -140,7 +135,7 @@ diffBlue is > -3 and < 2 &&
140135
byte dr = (byte)(diffRed + 2),
141136
dg = (byte)(diffGreen + 2),
142137
db = (byte)(diffBlue + 2),
143-
valueToWrite = (byte)((byte)QoiChunkEnum.QoiOpDiff | (dr << 4) | (dg << 2) | db);
138+
valueToWrite = (byte)((byte)QoiChunk.QoiOpDiff | (dr << 4) | (dg << 2) | db);
144139
stream.WriteByte(valueToWrite);
145140
}
146141
else
@@ -156,7 +151,7 @@ diffBlueGreen is > -9 and < 8 &&
156151
{
157152
byte dr_dg = (byte)(diffRedGreen + 8),
158153
db_dg = (byte)(diffBlueGreen + 8),
159-
byteToWrite1 = (byte)((byte)QoiChunkEnum.QoiOpLuma | (diffGreen + 32)),
154+
byteToWrite1 = (byte)((byte)QoiChunk.QoiOpLuma | (diffGreen + 32)),
160155
byteToWrite2 = (byte)((dr_dg << 4) | db_dg);
161156
stream.WriteByte(byteToWrite1);
162157
stream.WriteByte(byteToWrite2);
@@ -167,15 +162,15 @@ diffBlueGreen is > -9 and < 8 &&
167162
// If so, we do a QOI_OP_RGB
168163
if (currentRgba32.A == previousPixel.A)
169164
{
170-
stream.WriteByte((byte)QoiChunkEnum.QoiOpRgb);
165+
stream.WriteByte((byte)QoiChunk.QoiOpRgb);
171166
stream.WriteByte(currentRgba32.R);
172167
stream.WriteByte(currentRgba32.G);
173168
stream.WriteByte(currentRgba32.B);
174169
}
175170
else
176171
{
177172
// else, we do a QOI_OP_RGBA
178-
stream.WriteByte((byte)QoiChunkEnum.QoiOpRgba);
173+
stream.WriteByte((byte)QoiChunk.QoiOpRgba);
179174
stream.WriteByte(currentRgba32.R);
180175
stream.WriteByte(currentRgba32.G);
181176
stream.WriteByte(currentRgba32.B);

src/ImageSharp/Formats/Qoi/QoiMetadata.cs

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,22 +21,10 @@ public QoiMetadata()
2121
/// <param name="other">The metadata to create an instance from.</param>
2222
public QoiMetadata(QoiMetadata other)
2323
{
24-
this.Width = other.Width;
25-
this.Height = other.Height;
2624
this.Channels = other.Channels;
2725
this.ColorSpace = other.ColorSpace;
2826
}
2927

30-
/// <summary>
31-
/// Gets or sets image width in pixels (BE)
32-
/// </summary>
33-
public uint Width { get; set; }
34-
35-
/// <summary>
36-
/// Gets or sets image height in pixels (BE)
37-
/// </summary>
38-
public uint Height { get; set; }
39-
4028
/// <summary>
4129
/// Gets or sets color channels of the image. 3 = RGB, 4 = RGBA.
4230
/// </summary>

tests/ImageSharp.Tests/Formats/Qoi/QoiEncoderTests.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using SixLabors.ImageSharp.Formats.Qoi;
55
using SixLabors.ImageSharp.PixelFormats;
66
using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison;
7+
using SixLabors.ImageSharp.Tests.TestUtilities.ReferenceCodecs;
78

89
namespace SixLabors.ImageSharp.Tests.Formats.Qoi;
910

@@ -24,12 +25,12 @@ public class QoiEncoderTests
2425
private static void Encode<TPixel>(TestImageProvider<TPixel> provider)
2526
where TPixel : unmanaged, IPixel<TPixel>
2627
{
27-
using Image<TPixel> image = provider.GetImage();
28+
using Image<TPixel> image = provider.GetImage(new MagickReferenceDecoder());
2829
using MemoryStream stream = new();
2930
QoiEncoder encoder = new();
3031
image.Save(stream, encoder);
3132
stream.Position = 0;
3233
using Image<TPixel> encodedImage = (Image<TPixel>)Image.Load(stream);
33-
ImageComparingUtils.CompareWithReferenceDecoder(provider, encodedImage);
34+
ImageComparer.Exact.CompareImages(image, encodedImage);
3435
}
3536
}

0 commit comments

Comments
 (0)