From 88bb74f39ff9c79a72e1befb5e89e9f6674449ce Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 10 Oct 2021 18:21:47 +1100 Subject: [PATCH 1/2] Fix #1712 --- src/ImageSharp/Formats/Jpeg/JpegEncoder.cs | 25 ----------- .../Formats/Jpeg/JpegEncoderCore.cs | 42 +++++++++++++++++-- 2 files changed, 39 insertions(+), 28 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegEncoder.cs b/src/ImageSharp/Formats/Jpeg/JpegEncoder.cs index 6f116f4fb3..fc6e3189fc 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegEncoder.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegEncoder.cs @@ -29,7 +29,6 @@ public void Encode(Image image, Stream stream) where TPixel : unmanaged, IPixel { var encoder = new JpegEncoderCore(this); - this.InitializeColorType(image); encoder.Encode(image, stream); } @@ -45,31 +44,7 @@ public Task EncodeAsync(Image image, Stream stream, Cancellation where TPixel : unmanaged, IPixel { var encoder = new JpegEncoderCore(this); - this.InitializeColorType(image); return encoder.EncodeAsync(image, stream, cancellationToken); } - - /// - /// If ColorType was not set, set it based on the given image. - /// - private void InitializeColorType(Image image) - where TPixel : unmanaged, IPixel - { - // First inspect the image metadata. - if (this.ColorType == null) - { - JpegMetadata metadata = image.Metadata.GetJpegMetadata(); - this.ColorType = metadata.ColorType; - } - - // Secondly, inspect the pixel type. - if (this.ColorType == null) - { - bool isGrayscale = - typeof(TPixel) == typeof(L8) || typeof(TPixel) == typeof(L16) || - typeof(TPixel) == typeof(La16) || typeof(TPixel) == typeof(La32); - this.ColorType = isGrayscale ? JpegColorType.Luminance : JpegColorType.YCbCrRatio420; - } - } } } diff --git a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs index 6ff8876672..b39dc13158 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs @@ -86,10 +86,10 @@ public void Encode(Image image, Stream stream, CancellationToken ImageMetadata metadata = image.Metadata; JpegMetadata jpegMetadata = metadata.GetJpegMetadata(); - // If the color type was not specified by the user, preserve the color type of the input image, if it's a supported color type. - if (!this.colorType.HasValue && IsSupportedColorType(jpegMetadata.ColorType)) + // If the color type was not specified by the user, preserve the color type of the input image. + if (!this.colorType.HasValue) { - this.colorType = jpegMetadata.ColorType; + this.colorType = SetFallbackColorType(image); } // Compute number of components based on color type in options. @@ -156,6 +156,42 @@ public void Encode(Image image, Stream stream, CancellationToken stream.Flush(); } + /// + /// If color type was not set, set it based on the given image. + /// Note, if there is no metadata and the image has multiple components this method + /// returns defering the field assignment + /// to . + /// + private static JpegColorType? SetFallbackColorType(Image image) + where TPixel : unmanaged, IPixel + { + // First inspect the image metadata. + JpegColorType? colorType = null; + JpegMetadata metadata = image.Metadata.GetJpegMetadata(); + if (IsSupportedColorType(metadata.ColorType)) + { + colorType = metadata.ColorType; + } + + // Secondly, inspect the pixel type. + // TODO: PixelTypeInfo should contain a component count! + if (colorType is null) + { + bool isGrayscale = + typeof(TPixel) == typeof(L8) || typeof(TPixel) == typeof(L16) || + typeof(TPixel) == typeof(La16) || typeof(TPixel) == typeof(La32); + + // We don't set multi-component color types here since we can set it based upon + // the quality in InitQuantizationTables. + if (isGrayscale) + { + colorType = JpegColorType.Luminance; + } + } + + return colorType; + } + /// /// Returns true, if the color type is supported by the encoder. /// From d5cea156afb7ddf1431e5203d2df80311e7b9588 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 18 Oct 2021 22:22:41 +1100 Subject: [PATCH 2/2] Feedback fixes --- .../Formats/Jpeg/JpegEncoderCore.cs | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs index b39dc13158..d9d42e0614 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs @@ -89,7 +89,7 @@ public void Encode(Image image, Stream stream, CancellationToken // If the color type was not specified by the user, preserve the color type of the input image. if (!this.colorType.HasValue) { - this.colorType = SetFallbackColorType(image); + this.colorType = GetFallbackColorType(image); } // Compute number of components based on color type in options. @@ -162,7 +162,7 @@ public void Encode(Image image, Stream stream, CancellationToken /// returns defering the field assignment /// to . /// - private static JpegColorType? SetFallbackColorType(Image image) + private static JpegColorType? GetFallbackColorType(Image image) where TPixel : unmanaged, IPixel { // First inspect the image metadata. @@ -170,23 +170,20 @@ public void Encode(Image image, Stream stream, CancellationToken JpegMetadata metadata = image.Metadata.GetJpegMetadata(); if (IsSupportedColorType(metadata.ColorType)) { - colorType = metadata.ColorType; + return metadata.ColorType; } // Secondly, inspect the pixel type. // TODO: PixelTypeInfo should contain a component count! - if (colorType is null) - { - bool isGrayscale = - typeof(TPixel) == typeof(L8) || typeof(TPixel) == typeof(L16) || - typeof(TPixel) == typeof(La16) || typeof(TPixel) == typeof(La32); + bool isGrayscale = + typeof(TPixel) == typeof(L8) || typeof(TPixel) == typeof(L16) || + typeof(TPixel) == typeof(La16) || typeof(TPixel) == typeof(La32); - // We don't set multi-component color types here since we can set it based upon - // the quality in InitQuantizationTables. - if (isGrayscale) - { - colorType = JpegColorType.Luminance; - } + // We don't set multi-component color types here since we can set it based upon + // the quality in InitQuantizationTables. + if (isGrayscale) + { + colorType = JpegColorType.Luminance; } return colorType;