Adding QOI support #2446

LuisAlfredo92 · 2023-05-04T16:44:07Z

Prerequisites

I have written a descriptive pull-request title
I have verified that there are no overlapping pull-requests open
I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
I have provided test coverage for my change (where applicable)

Description

First of all, I'm sorry for being so late to add this, I've been extremely busy :(
Also I took some time to see the code to understand it and do it with the same structure

Then, I've add some files to work with qoi, at this moment it only identifies qoi images and returns the ImageInfo, but I want to make sure that I'm doing this correctly before doing the rest of functions. I've followed the Stylecop rules with Visual Studio Intellisense.

The test images were taken from the official website in the Test images in QOI and PNG format link.

This is about #2403

Could you review it please?

I'm starting to need help because I don't understand the project structure Everything I added was taken from the specification and structured similarly to PNG and BMP

I'm going to do some tests before the pull request and add all the functions

Now it identifies qoi images successfully

CLAassistant · 2023-05-04T16:44:12Z

All committers have signed the CLA.

tests/ImageSharp.Benchmarks/Program.cs

tests/ImageSharp.Benchmarks/Codecs/Qoi/IdentifyQoi.cs

antonfirsov

Since you already implemented Identify, your next step is probably to make sure it actually works. Here's an example test, you want an equivalent for Qoi:

ImageSharp/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs

Lines 338 to 355 in 7d0bbbd

    
           [Theory] 
        
           [InlineData(TestImages.Png.Bpp1, 1)] 
        
           [InlineData(TestImages.Png.Gray4Bpp, 4)] 
        
           [InlineData(TestImages.Png.Palette8Bpp, 8)] 
        
           [InlineData(TestImages.Png.Pd, 24)] 
        
           [InlineData(TestImages.Png.Blur, 32)] 
        
           [InlineData(TestImages.Png.Rgb48Bpp, 48)] 
        
           [InlineData(TestImages.Png.Rgb48BppInterlaced, 48)] 
        
           public void Identify(string imagePath, int expectedPixelSize) 
        
           { 
        
               TestFile testFile = TestFile.Create(imagePath); 
        
               using MemoryStream stream = new(testFile.Bytes, false); 
        
               ImageInfo imageInfo = Image.Identify(stream); 
        
               Assert.NotNull(imageInfo); 
        
               Assert.Equal(expectedPixelSize, imageInfo.PixelType.BitsPerPixel); 
        
           }

(I recommend to validate image size and stuff like QoiChannels and QoiColorSpace there.)

To make sure the format is recognized by Image.Identify and Image.Load and triggers your decoder, you also need to implement and register a "configuration module":

ImageSharp/src/ImageSharp/Configuration.cs

Lines 217 to 225 in 7d0bbbd

    
           internal static Configuration CreateDefaultInstance() => new( 
        
                   new PngConfigurationModule(), 
        
                   new JpegConfigurationModule(), 
        
                   new GifConfigurationModule(), 
        
                   new BmpConfigurationModule(), 
        
                   new PbmConfigurationModule(), 
        
                   new TgaConfigurationModule(), 
        
                   new TiffConfigurationModule(), 
        
                   new WebpConfigurationModule());

src/ImageSharp/Formats/Qoi/QoiDecoderCore.cs

tests/ImageSharp.Benchmarks/Codecs/Qoi/IdentifyQoi.cs

antonfirsov · 2023-05-05T11:58:49Z

There are some some StyleCop complaints breaking the build since we use TreatWarningsAsErrors:
https://github.com/SixLabors/ImageSharp/actions/runs/4885169437/jobs/8735504561?pr=2446

When you will move on to the actual Decode implementation, you want to base your decoder tests on this:

ImageSharp/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs

Lines 81 to 99 in 7d0bbbd

    
           [Theory] 
        
           [WithFile(BlackAndWhitePlain, PixelTypes.L8, "pbm")] 
        
           [WithFile(BlackAndWhiteBinary, PixelTypes.L8, "pbm")] 
        
           [WithFile(GrayscalePlain, PixelTypes.L8, "pgm")] 
        
           [WithFile(GrayscalePlainNormalized, PixelTypes.L8, "pgm")] 
        
           [WithFile(GrayscaleBinary, PixelTypes.L8, "pgm")] 
        
           [WithFile(GrayscaleBinaryWide, PixelTypes.L16, "pgm")] 
        
           [WithFile(RgbPlain, PixelTypes.Rgb24, "ppm")] 
        
           [WithFile(RgbPlainNormalized, PixelTypes.Rgb24, "ppm")] 
        
           [WithFile(RgbBinary, PixelTypes.Rgb24, "ppm")] 
        
           public void DecodeReferenceImage<TPixel>(TestImageProvider<TPixel> provider, string extension) 
        
               where TPixel : unmanaged, IPixel<TPixel> 
        
           { 
        
               using Image<TPixel> image = provider.GetImage(); 
        
               image.DebugSave(provider, extension: extension); 
        
               bool isGrayscale = extension is "pgm" or "pbm"; 
        
               image.CompareToReferenceOutput(provider, grayscale: isGrayscale); 
        
           }

First just make sure visually that DebugSave is saving the right thing. CompareToReferenceOutput would need your reference png to be in the right directory with the right name, we can explain that later.

src/ImageSharp/Formats/Qoi/QoiDecoderCore.cs

LuisAlfredo92 · 2023-05-05T16:15:02Z

Thanks for the reviewing, I'll correct everything and tell when I start working on the other functions 👍

src/ImageSharp/Formats/Qoi/QoiConstants.cs

src/ImageSharp/Formats/Qoi/QoiHeader.cs

-Adding validation for size, channels and colorspace -Refactoring to throw general exceptions -Creating tests -Using other better data types

LuisAlfredo92 · 2023-06-12T19:38:43Z

Hello again, I can finally work in this again 🎉

I've made the changes you told me, however, I didn't find how to run the tests, so I only added the configurations but didn't test them

Soon I'll try to add the Decode and Encode options

antonfirsov · 2023-06-13T00:36:41Z

I didn't find how to run the tests.

You can use dotnet test from the command line:

 dotnet test -f net6.0 --filter FullyQualifiedName~QoiDecoderTests

Or if you are using Visual Studio, they should appear in the Test Explorer window.

-I had several errors about header size comparison and comparing byte arrays, but I've fixed them and tested the Identify function and it works perfect!

LuisAlfredo92 · 2023-06-20T16:07:35Z

There are some some StyleCop complaints breaking the build since we use TreatWarningsAsErrors: https://github.com/SixLabors/ImageSharp/actions/runs/4885169437/jobs/8735504561?pr=2446

When you will move on to the actual Decode implementation, you want to base your decoder tests on this:

ImageSharp/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs

Lines 81 to 99 in 7d0bbbd

[Theory]

[WithFile(BlackAndWhitePlain, PixelTypes.L8, "pbm")]

[WithFile(BlackAndWhiteBinary, PixelTypes.L8, "pbm")]

[WithFile(GrayscalePlain, PixelTypes.L8, "pgm")]

[WithFile(GrayscalePlainNormalized, PixelTypes.L8, "pgm")]

[WithFile(GrayscaleBinary, PixelTypes.L8, "pgm")]

[WithFile(GrayscaleBinaryWide, PixelTypes.L16, "pgm")]

[WithFile(RgbPlain, PixelTypes.Rgb24, "ppm")]

[WithFile(RgbPlainNormalized, PixelTypes.Rgb24, "ppm")]

[WithFile(RgbBinary, PixelTypes.Rgb24, "ppm")]

public void DecodeReferenceImage<TPixel>(TestImageProvider<TPixel> provider, string extension)

where TPixel : unmanaged, IPixel<TPixel>

{

using Image<TPixel> image = provider.GetImage();

image.DebugSave(provider, extension: extension);

bool isGrayscale = extension is "pgm" or "pbm";

image.CompareToReferenceOutput(provider, grayscale: isGrayscale);

}

First just make sure visually that DebugSave is saving the right thing. CompareToReferenceOutput would need your reference png to be in the right directory with the right name, we can explain that later.

Hello again, I need some help with the tests

When I try to test the Qoi decoder, when it tries to get the extension, qoi image format never appears:

However, during debugging, it actually is correctly added:

So I don't know if I must add something else on the code or tests

Just in case, my qoi configuration module adds only a decoder and format detector

EDIT
I created the encoder class just to add it to the configuration (although it doesn't do anything yet) but still doesn't work

LuisAlfredo92 · 2023-06-21T13:44:00Z

Nevermind, I found where the configuration is done

JimBobSquarePants · 2023-06-23T11:21:57Z

Nevermind, I found where the configuration is done

Sorry, didn't see this. Glad you figured it out. Any other questions please let me know!

I need to check phoboslab/qoi#258 because there's a bug with the decoder

See phoboslab/qoi#258 and previous commit

LuisAlfredo92 · 2023-06-25T03:55:03Z

Hello!
I've finished the decoder and it passes all the tests 🎉 I've also taken into consideration the edge case and now it works perfectly, however, I'm not sure if I followed correctly the standards of the project, could you check it please?

I'll be working on the encoder now

-Also adding Decode method without TPixel value -Adding stream end check to decoder (we must discuss if it's necesarry or not) -formating general code

LuisAlfredo92 · 2023-06-28T23:01:27Z

Hello again
I've finished encoder, decoder and extensions, and all of them have passed all the tests, but we should discuss some things about the encoder (if it's correct to allow file to start with a QOI_OP_RUN) and decoder (throw an exception when the end of stream doesn't end with seven zero-bytes and one byte with value 1). I wrote some comments if you want to check

I'd like to ask for a review to see if I'm not doing something wrong

Changing formats from 8 to 9

Also adding Modulo64 and 256 to Numerics and optimizing a little bit Encoder

src/ImageSharp/Formats/Qoi/QoiDecoderCore.cs

LuisAlfredo92 · 2023-06-30T19:03:04Z

Okay, I've done everything you've asked

Is there anything else I should do?

antonfirsov

I see some optimization opportunities in QoiEncoder too, but focusing on functionality first.

src/ImageSharp/Formats/Qoi/QoiDecoderCore.cs

src/ImageSharp/Formats/Qoi/QoiEncoderCore.cs

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

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

I'm not sure the encoder is right

antonfirsov

Almost there! A few more suggestions for perf.

src/ImageSharp/Formats/Qoi/QoiEncoderCore.cs

tests/ImageSharp.Benchmarks/Codecs/Qoi/IdentifyQoi.cs

src/ImageSharp/Formats/Qoi/QoiEncoderCore.cs

Also removing benchmark test and fixing a typo in PngEncoderCore

LuisAlfredo92 · 2023-07-03T23:33:31Z

Changes done
I'm pretty excited to see these changes merged into the main project 😄

src/ImageSharp/Formats/ImageExtensions.Save.cs

src/ImageSharp/Formats/Qoi/QoiEncoderCore.cs

LuisAlfredo92 · 2023-07-05T02:52:59Z

Everything's done 🎉

LuisAlfredo92 · 2023-07-05T17:41:58Z

Is there anything else I could do?

src/ImageSharp/Formats/Qoi/QoiMetadata.cs

antonfirsov

Looks good to merge now. Thanks a lot for the nice work, and really appreciate the patience for the long review!

LuisAlfredo92 · 2023-07-06T00:10:32Z

Thanks to you all for reviewing the changes so constantly and I'm sorry it took so long, I posted the idea when I was about to get back to school and I was able to do it until the semester finished xD

JimBobSquarePants · 2023-07-06T02:03:10Z

Adding an entirely new image format is no small task! Really well done and thanks so much for your efforts!

LuisAlfredo92 · 2023-07-06T02:37:53Z

When you publish the 3.1.0 version, could I add ImageSharp it to the phoboslab/qoi repository README? Maybe on tools or implementations

antonfirsov · 2023-07-06T18:48:04Z

Yeah I also wanted to call out that we need to get into that hall of fame after the release.

LuisAlfredo92 added 5 commits April 20, 2023 12:43

Adding QOI metadata and constants

9a07ee9

I'm starting to need help because I don't understand the project structure Everything I added was taken from the specification and structured similarly to PNG and BMP

Merge branch 'main' of https://github.com/LuisAlfredo92/ImageSharp

6784acf

Adding qoi identification

c79dd77

I'm going to do some tests before the pull request and add all the functions

Adding tests and fixing bugs

60d8763

Now it identifies qoi images successfully

Merge branch 'SixLabors:main' into main

43bf27a

LuisAlfredo92 commented May 5, 2023

View reviewed changes

tests/ImageSharp.Benchmarks/Program.cs Outdated Show resolved Hide resolved

LuisAlfredo92 commented May 5, 2023

View reviewed changes

tests/ImageSharp.Benchmarks/Codecs/Qoi/IdentifyQoi.cs Outdated Show resolved Hide resolved

antonfirsov reviewed May 5, 2023

View reviewed changes

src/ImageSharp/Formats/Qoi/QoiDecoderCore.cs Outdated Show resolved Hide resolved

src/ImageSharp/Formats/Qoi/QoiDecoderCore.cs Outdated Show resolved Hide resolved

tests/ImageSharp.Benchmarks/Codecs/Qoi/IdentifyQoi.cs Outdated Show resolved Hide resolved

antonfirsov reviewed May 5, 2023

View reviewed changes

src/ImageSharp/Formats/Qoi/QoiDecoderCore.cs Show resolved Hide resolved

gfoidl reviewed May 7, 2023

View reviewed changes

LuisAlfredo92 added 2 commits June 12, 2023 13:05

Making changes from #2446 review

24e5ce6

-Adding validation for size, channels and colorspace -Refactoring to throw general exceptions -Creating tests -Using other better data types

Merge branch 'main' of https://github.com/LuisAlfredo92/ImageSharp

8b9e0a9

Fixing Qoi detector

a1342bf

-I had several errors about header size comparison and comparing byte arrays, but I've fixed them and tested the Identify function and it works perfect!

LuisAlfredo92 added 2 commits June 24, 2023 18:07

Implementing qoi decoder

bca998d

I need to check phoboslab/qoi#258 because there's a bug with the decoder

Fixing edge case

a87f781

See phoboslab/qoi#258 and previous commit

LuisAlfredo92 added 2 commits June 28, 2023 16:30

Finishing qoi encoder

07e6597

-Also adding Decode method without TPixel value -Adding stream end check to decoder (we must discuss if it's necesarry or not) -formating general code

Merge branch 'main' into main

4a494c7

LuisAlfredo92 marked this pull request as ready for review June 28, 2023 22:34

Adding extensions was easier than I though

c92a4ec

Fixing configuration tests

8729fed

Changing formats from 8 to 9

antonfirsov mentioned this pull request Jun 29, 2023

Add QOI to .gitattributes SixLabors/SharedInfrastructure#39

Merged

4 tasks

Optimizing Decoder

02212bd

Also adding Modulo64 and 256 to Numerics and optimizing a little bit Encoder

antonfirsov reviewed Jun 30, 2023

View reviewed changes

src/ImageSharp/Formats/Qoi/QoiDecoderCore.cs Outdated Show resolved Hide resolved

src/ImageSharp/Formats/Qoi/QoiDecoderCore.cs Outdated Show resolved Hide resolved

LuisAlfredo92 added 3 commits June 29, 2023 20:06

Fixing encoder optimizations

88de0a6

Fixing and optimizing decoder and encoder

383aa22

Fixing and making easier to read encoder

bf9f1f3

Fixing StyleCop

f7b4f49

antonfirsov reviewed Jul 3, 2023

View reviewed changes

LuisAlfredo92 added 2 commits July 3, 2023 11:40

Fixing declarations

3f1fe69

Adding Channels and ColorSpace validations to De-/Encoder

ef78f98

I'm not sure the encoder is right

antonfirsov reviewed Jul 3, 2023

View reviewed changes

src/ImageSharp/Formats/Qoi/QoiEncoderCore.cs Outdated Show resolved Hide resolved

tests/ImageSharp.Benchmarks/Codecs/Qoi/IdentifyQoi.cs Outdated Show resolved Hide resolved

src/ImageSharp/Formats/Qoi/QoiEncoderCore.cs Outdated Show resolved Hide resolved

Optimizing avoiding casts to byte

dea1ee8

Also removing benchmark test and fixing a typo in PngEncoderCore

antonfirsov reviewed Jul 5, 2023

View reviewed changes

src/ImageSharp/Formats/ImageExtensions.Save.cs Show resolved Hide resolved

antonfirsov reviewed Jul 5, 2023

View reviewed changes

src/ImageSharp/Formats/Qoi/QoiEncoderCore.cs Outdated Show resolved Hide resolved

src/ImageSharp/Formats/Qoi/QoiEncoderCore.cs Show resolved Hide resolved

Fixing extensions, class access and StyleCop

472bc04

JimBobSquarePants added this to the v3.1.0 milestone Jul 5, 2023

Merge branch 'main' into main

1069a6d

antonfirsov reviewed Jul 6, 2023

View reviewed changes

src/ImageSharp/Formats/Qoi/QoiMetadata.cs Outdated Show resolved Hide resolved

private QoiMetadata(QoiMetadata other)

eb5c6a7

antonfirsov approved these changes Jul 6, 2023

View reviewed changes

Merge branch 'main' into main

59c4a6a

JimBobSquarePants merged commit 900e9d0 into SixLabors:main Jul 6, 2023
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding QOI support #2446

Adding QOI support #2446

LuisAlfredo92 commented May 4, 2023 •

edited

CLAassistant commented May 4, 2023 •

edited

antonfirsov left a comment •

edited

antonfirsov commented May 5, 2023

LuisAlfredo92 commented May 5, 2023

LuisAlfredo92 commented Jun 12, 2023

antonfirsov commented Jun 13, 2023

LuisAlfredo92 commented Jun 20, 2023 •

edited

LuisAlfredo92 commented Jun 21, 2023

JimBobSquarePants commented Jun 23, 2023

LuisAlfredo92 commented Jun 25, 2023

LuisAlfredo92 commented Jun 28, 2023

LuisAlfredo92 commented Jun 30, 2023

antonfirsov left a comment •

edited

antonfirsov left a comment

LuisAlfredo92 commented Jul 3, 2023 •

edited

LuisAlfredo92 commented Jul 5, 2023

LuisAlfredo92 commented Jul 5, 2023

antonfirsov left a comment •

edited

LuisAlfredo92 commented Jul 6, 2023 •

edited

JimBobSquarePants commented Jul 6, 2023

LuisAlfredo92 commented Jul 6, 2023

antonfirsov commented Jul 6, 2023

	[Theory]
	[InlineData(TestImages.Png.Bpp1, 1)]
	[InlineData(TestImages.Png.Gray4Bpp, 4)]
	[InlineData(TestImages.Png.Palette8Bpp, 8)]
	[InlineData(TestImages.Png.Pd, 24)]
	[InlineData(TestImages.Png.Blur, 32)]
	[InlineData(TestImages.Png.Rgb48Bpp, 48)]
	[InlineData(TestImages.Png.Rgb48BppInterlaced, 48)]
	public void Identify(string imagePath, int expectedPixelSize)
	{
	TestFile testFile = TestFile.Create(imagePath);
	using MemoryStream stream = new(testFile.Bytes, false);

	ImageInfo imageInfo = Image.Identify(stream);

	Assert.NotNull(imageInfo);
	Assert.Equal(expectedPixelSize, imageInfo.PixelType.BitsPerPixel);
	}

	internal static Configuration CreateDefaultInstance() => new(
	new PngConfigurationModule(),
	new JpegConfigurationModule(),
	new GifConfigurationModule(),
	new BmpConfigurationModule(),
	new PbmConfigurationModule(),
	new TgaConfigurationModule(),
	new TiffConfigurationModule(),
	new WebpConfigurationModule());

Adding QOI support #2446

Adding QOI support #2446

Conversation

LuisAlfredo92 commented May 4, 2023 • edited

Prerequisites

Description

CLAassistant commented May 4, 2023 • edited

antonfirsov left a comment • edited

Choose a reason for hiding this comment

antonfirsov commented May 5, 2023

LuisAlfredo92 commented May 5, 2023

LuisAlfredo92 commented Jun 12, 2023

antonfirsov commented Jun 13, 2023

LuisAlfredo92 commented Jun 20, 2023 • edited

LuisAlfredo92 commented Jun 21, 2023

JimBobSquarePants commented Jun 23, 2023

LuisAlfredo92 commented Jun 25, 2023

LuisAlfredo92 commented Jun 28, 2023

LuisAlfredo92 commented Jun 30, 2023

antonfirsov left a comment • edited

Choose a reason for hiding this comment

antonfirsov left a comment

Choose a reason for hiding this comment

LuisAlfredo92 commented Jul 3, 2023 • edited

LuisAlfredo92 commented Jul 5, 2023

LuisAlfredo92 commented Jul 5, 2023

antonfirsov left a comment • edited

Choose a reason for hiding this comment

LuisAlfredo92 commented Jul 6, 2023 • edited

JimBobSquarePants commented Jul 6, 2023

LuisAlfredo92 commented Jul 6, 2023

antonfirsov commented Jul 6, 2023

LuisAlfredo92 commented May 4, 2023 •

edited

CLAassistant commented May 4, 2023 •

edited

antonfirsov left a comment •

edited

LuisAlfredo92 commented Jun 20, 2023 •

edited

antonfirsov left a comment •

edited

LuisAlfredo92 commented Jul 3, 2023 •

edited

antonfirsov left a comment •

edited

LuisAlfredo92 commented Jul 6, 2023 •

edited