Merge pull request #1779 from SixLabors/js/jpeg-encoder-color-type

Ensure jpeg encoder is immutable.
SixLabors · Oct 19, 2021 · 07d50d0 · 07d50d0
2 parents 424d4f9 + d5cea15
commit 07d50d0
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 28 deletions.
diff --git a/src/ImageSharp/Formats/Jpeg/JpegEncoder.cs b/src/ImageSharp/Formats/Jpeg/JpegEncoder.cs
@@ -29,7 +29,6 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream)
         where TPixel : unmanaged, IPixel<TPixel>
         {
             var encoder = new JpegEncoderCore(this);
-            this.InitializeColorType(image);
             encoder.Encode(image, stream);
         }
 
@@ -45,31 +44,7 @@ public Task EncodeAsync<TPixel>(Image<TPixel> image, Stream stream, Cancellation
             where TPixel : unmanaged, IPixel<TPixel>
         {
             var encoder = new JpegEncoderCore(this);
-            this.InitializeColorType(image);
             return encoder.EncodeAsync(image, stream, cancellationToken);
         }
-
-        /// <summary>
-        /// If ColorType was not set, set it based on the given image.
-        /// </summary>
-        private void InitializeColorType<TPixel>(Image<TPixel> image)
-            where TPixel : unmanaged, IPixel<TPixel>
-        {
-            // 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
@@ -86,10 +86,10 @@ public void Encode<TPixel>(Image<TPixel> 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 = GetFallbackColorType(image);
             }
 
             // Compute number of components based on color type in options.
@@ -156,6 +156,39 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
             stream.Flush();
         }
 
+        /// <summary>
+        /// 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 <see langword="null"/> defering the field assignment
+        /// to <see cref="InitQuantizationTables(int, JpegMetadata, out Block8x8F, out Block8x8F)"/>.
+        /// </summary>
+        private static JpegColorType? GetFallbackColorType<TPixel>(Image<TPixel> image)
+            where TPixel : unmanaged, IPixel<TPixel>
+        {
+            // First inspect the image metadata.
+            JpegColorType? colorType = null;
+            JpegMetadata metadata = image.Metadata.GetJpegMetadata();
+            if (IsSupportedColorType(metadata.ColorType))
+            {
+                return metadata.ColorType;
+            }
+
+            // Secondly, inspect the pixel type.
+            // TODO: PixelTypeInfo should contain a component count!
+            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;
+        }
+
         /// <summary>
         /// Returns true, if the color type is supported by the encoder.
         /// </summary>