From 291e9e7c1e619722ab57b092370483b7adc21c7a Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 1 Dec 2023 13:25:20 +1000 Subject: [PATCH] Use source length as bounds --- .../PixelOperations/Rgb24.PixelOperations.cs | 5 ++- .../PixelFormats/PixelOperations{TPixel}.cs | 16 +++++++-- .../Formats/Jpg/JpegEncoderTests.cs | 36 +++++++++++++++---- 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/src/ImageSharp/PixelFormats/PixelImplementations/PixelOperations/Rgb24.PixelOperations.cs b/src/ImageSharp/PixelFormats/PixelImplementations/PixelOperations/Rgb24.PixelOperations.cs index 2a8f80ebe0..5473a602f9 100644 --- a/src/ImageSharp/PixelFormats/PixelImplementations/PixelOperations/Rgb24.PixelOperations.cs +++ b/src/ImageSharp/PixelFormats/PixelImplementations/PixelOperations/Rgb24.PixelOperations.cs @@ -40,6 +40,9 @@ internal override void UnpackIntoRgbPlanes( Span greenChannel, Span blueChannel, ReadOnlySpan source) - => SimdUtils.UnpackToRgbPlanes(redChannel, greenChannel, blueChannel, source); + { + GuardUnpackIntoRgbPlanes(redChannel, greenChannel, blueChannel, source); + SimdUtils.UnpackToRgbPlanes(redChannel, greenChannel, blueChannel, source); + } } } diff --git a/src/ImageSharp/PixelFormats/PixelOperations{TPixel}.cs b/src/ImageSharp/PixelFormats/PixelOperations{TPixel}.cs index 9d93d27aca..beebec8283 100644 --- a/src/ImageSharp/PixelFormats/PixelOperations{TPixel}.cs +++ b/src/ImageSharp/PixelFormats/PixelOperations{TPixel}.cs @@ -165,7 +165,7 @@ public virtual void To( } /// - /// Bulk operation that packs 3 seperate RGB channels to . + /// Bulk operation that packs 3 separate RGB channels to . /// The destination must have a padding of 3. /// /// A to the red values. @@ -198,7 +198,7 @@ internal virtual void PackFromRgbPlanes( /// /// Bulk operation that unpacks pixels from - /// into 3 seperate RGB channels. The destination must have a padding of 3. + /// into 3 separate RGB channels. /// /// A to the red values. /// A to the green values. @@ -210,7 +210,9 @@ internal virtual void UnpackIntoRgbPlanes( Span blueChannel, ReadOnlySpan source) { - int count = redChannel.Length; + GuardUnpackIntoRgbPlanes(redChannel, greenChannel, blueChannel, source); + + int count = source.Length; Rgba32 rgba32 = default; @@ -227,6 +229,14 @@ internal virtual void UnpackIntoRgbPlanes( } } + [MethodImpl(InliningOptions.ShortMethod)] + internal static void GuardUnpackIntoRgbPlanes(Span redChannel, Span greenChannel, Span blueChannel, ReadOnlySpan source) + { + Guard.IsTrue(greenChannel.Length == redChannel.Length, nameof(greenChannel), "Channels must be of same size!"); + Guard.IsTrue(blueChannel.Length == redChannel.Length, nameof(blueChannel), "Channels must be of same size!"); + Guard.IsTrue(source.Length <= redChannel.Length, nameof(source), "'source' span should not be bigger than the destination channels!"); + } + [MethodImpl(InliningOptions.ShortMethod)] internal static void GuardPackFromRgbPlanes(ReadOnlySpan greenChannel, ReadOnlySpan blueChannel, Span destination, int count) { diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs index aed84a7d92..5842c8e1a0 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs @@ -3,6 +3,7 @@ using SixLabors.ImageSharp.Formats.Jpeg; using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Tests.TestUtilities; using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; @@ -87,7 +88,7 @@ public void EncodeBaseline_NonInterleavedMode(TestImageProvider { using Image image = provider.GetImage(); - var encoder = new JpegEncoder + JpegEncoder encoder = new() { Quality = quality, ColorType = colorType, @@ -164,8 +165,8 @@ public void EncodeBaseline_WorksWithDiscontiguousBuffers(TestImageProvid [InlineData(JpegEncodingColor.YCbCrRatio444)] public async Task Encode_IsCancellable(JpegEncodingColor colorType) { - var cts = new CancellationTokenSource(); - using var pausedStream = new PausedStream(new MemoryStream()); + CancellationTokenSource cts = new(); + using PausedStream pausedStream = new(new MemoryStream()); pausedStream.OnWaiting(s => { // after some writing @@ -181,14 +182,37 @@ public async Task Encode_IsCancellable(JpegEncodingColor colorType) } }); - using var image = new Image(5000, 5000); + using Image image = new(5000, 5000); await Assert.ThrowsAsync(async () => { - var encoder = new JpegEncoder() { ColorType = colorType }; + JpegEncoder encoder = new() { ColorType = colorType }; await image.SaveAsync(pausedStream, encoder, cts.Token); }); } + // https://github.com/SixLabors/ImageSharp/issues/2595 + [Theory] + [WithFile(TestImages.Jpeg.Baseline.ForestBridgeDifferentComponentsQuality, PixelTypes.Bgra32)] + [WithFile(TestImages.Jpeg.Baseline.ForestBridgeDifferentComponentsQuality, PixelTypes.Rgb24)] + public static void Issue2595(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(); + image.Mutate(x => x.Crop(132, 1606)); + + int[] quality = new int[] { 100, 50 }; + JpegEncodingColor[] colors = new[] { JpegEncodingColor.YCbCrRatio444, JpegEncodingColor.YCbCrRatio420 }; + for (int i = 0; i < quality.Length; i++) + { + int q = quality[i]; + for (int j = 0; j < colors.Length; j++) + { + JpegEncodingColor c = colors[j]; + image.VerifyEncoder(provider, "jpeg", $"{q}-{c}", new JpegEncoder() { Quality = q, ColorType = c }, GetComparer(q, c)); + } + } + } + /// /// Anton's SUPER-SCIENTIFIC tolerance threshold calculation /// @@ -225,7 +249,7 @@ private static void TestJpegEncoderCore(TestImageProvider provid { using Image image = provider.GetImage(); - var encoder = new JpegEncoder + JpegEncoder encoder = new() { Quality = quality, ColorType = colorType