-
-
Notifications
You must be signed in to change notification settings - Fork 855
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Vectorize Jpeg Encoder Color Conversion #1508
Vectorize Jpeg Encoder Color Conversion #1508
Conversation
|
src/ImageSharp/Formats/Jpeg/Components/Encoder/YCbCrForwardConverter{TPixel}.cs
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #1508 +/- ##
==========================================
+ Coverage 83.52% 83.53% +0.01%
==========================================
Files 741 742 +1
Lines 32672 32732 +60
Branches 3662 3665 +3
==========================================
+ Hits 27289 27344 +55
- Misses 4669 4672 +3
- Partials 714 716 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tkp1n Thanks for having a crack at this! Very much appreciated! The deinterleaving code had me beat the other week when I had a look so I was hoping someone smarter than me would have a go.
I'm gonna leave this to the brains trust to properly review because I'm still very much a SIMD noob but I've added a few comments.
@antonfirsov @saucecontrol If you have ideas and time please comment. I would have though we could have sped up the operation a bit more than the current (welcome) improvement.
src/ImageSharp/Formats/Jpeg/Components/Encoder/RgbToYCbCrConverterVectorized.cs
Outdated
Show resolved
Hide resolved
src/ImageSharp/Formats/Jpeg/Components/Encoder/RgbToYCbCrConverterVectorized.cs
Outdated
Show resolved
Hide resolved
src/ImageSharp/Formats/Jpeg/Components/Encoder/RgbToYCbCrConverterVectorized.cs
Outdated
Show resolved
Hide resolved
src/ImageSharp/Formats/Jpeg/Components/Encoder/RgbToYCbCrConverterVectorized.cs
Outdated
Show resolved
Hide resolved
src/ImageSharp/Formats/Jpeg/Components/Encoder/RgbToYCbCrConverterLut.cs
Show resolved
Hide resolved
Rather than having two methods with a round-trip through memory, I'd try to get this all done in one step. The best option would actually be to do the conversion fixed-point if AVX2 is available, but I assume that's a much larger change if the encoder is already taking the YCbCr input in floating point. This version could be simplified using something like this:
That's untested but should get you going in the right direction. You'll need to allow for 8 bytes of overrun on the last read, or write it such that the 8th iteration reads the last 32 bytes and uses an alternate permute mask to shuffle the 12-byte chunks into position. |
I'm super curious how much the algorithm in #1508 (comment) could give us, but if there's no time to implement it, we can merge the PR as is after changing the In any case let's not close #1476 with the PR, there is definitely space left for improvements by rearranging the pipeline steps within the |
@antonfirsov I'm already on it.. Seems to reduce times roughly from 170ns to 80ns 🚀 |
I’ll do a final pass tonight but this is looking great! 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
…rsion Vectorize Jpeg Encoder Color Conversion
Prerequisites
Description
Touches #1476 with a vectorized implementation of the RGB -> YCbCr conversions.
It is worth noting, that the following benchmarks don't show the entire picture. The current lookup table converter uses 3kB of lookup tables that pollute the cache and likely negatively impact subsequent operations in the encoding chain.
Benchmark 🚀