Use AdobeMarker for parsing logic

src/ImageSharp/Formats/Jpeg/Common/Decoder/AdobeMarker.cs

@@ -7,33 +7,80 @@ using System;
 namespace SixLabors.ImageSharp.Formats.Jpeg.Common.Decoder
 {
     /// <summary>
-    /// Provides information about the Adobe marker segment
+    /// Provides information about the Adobe marker segment.
     /// </summary>
+    /// <remarks>See the included 5116.DCT.pdf file in the source for more information.</remarks>
     internal struct AdobeMarker : IEquatable<AdobeMarker>
     {
         /// <summary>
-        /// The DCT Encode Version
+        /// Gets the length of an adobe marker segment.
         /// </summary>
-        public short DCTEncodeVersion;
+        public const int Length = 12;
 
         /// <summary>
-        /// 0x0 : (none)
+        /// Initializes a new instance of the <see cref="AdobeMarker"/> struct.
+        /// </summary>
+        /// <param name="dctEncodeVersion">The DCT encode version</param>
+        /// <param name="app14Flags0">The horizontal downsampling hint used for DCT encoding</param>
+        /// <param name="app14Flags1">The vertical downsampling hint used for DCT encoding</param>
+        /// <param name="colorTransform">The color transform model used</param>
+        private AdobeMarker(short dctEncodeVersion, short app14Flags0, short app14Flags1, byte colorTransform)
+        {
+            this.DCTEncodeVersion = dctEncodeVersion;
+            this.APP14Flags0 = app14Flags0;
+            this.APP14Flags1 = app14Flags1;
+            this.ColorTransform = colorTransform;
+        }
+
+        /// <summary>
+        /// Gets the DCT Encode Version
+        /// </summary>
+        public short DCTEncodeVersion { get; }
+
+        /// <summary>
+        /// Gets the horizontal downsampling hint used for DCT encoding
+        /// 0x0 : (none - Chop)
         /// Bit 15 : Encoded with Blend=1 downsampling
         /// </summary>
-        public short APP14Flags0;
+        public short APP14Flags0 { get; }
 
         /// <summary>
-        /// 0x0 : (none)
+        /// Gets the vertical downsampling hint used for DCT encoding
+        /// 0x0 : (none - Chop)
+        /// Bit 15 : Encoded with Blend=1 downsampling
         /// </summary>
-        public short APP14Flags1;
+        public short APP14Flags1 { get; }
 
         /// <summary>
-        /// Determines the colorspace transform
+        /// Gets the colorspace transform model used
         /// 00 : Unknown (RGB or CMYK)
         /// 01 : YCbCr
         /// 02 : YCCK
         /// </summary>
-        public byte ColorTransform;
+        public byte ColorTransform { get; }
+
+        /// <summary>
+        /// Converts the specified byte array representation of an Adobe marker to its <see cref="AdobeMarker"/> equivalent and
+        /// returns a value that indicates whether the conversion succeeded.
+        /// </summary>
+        /// <param name="bytes">The byte array containing metadata to parse</param>
+        /// <param name="marker">The marker to return.</param>
+        public static bool TryParse(byte[] bytes, out AdobeMarker marker)
+        {
+            if (ProfileResolver.IsProfile(bytes, ProfileResolver.AdobeMarker))
+            {
+                short dctEncodeVersion = (short)((bytes[5] << 8) | bytes[6]);
+                short app14Flags0 = (short)((bytes[7] << 8) | bytes[8]);
+                short app14Flags1 = (short)((bytes[9] << 8) | bytes[10]);
+                byte colorTransform = bytes[11];
+
+                marker = new AdobeMarker(dctEncodeVersion, app14Flags0, app14Flags1, colorTransform);
+                return true;
+            }
+
+            marker = default(AdobeMarker);
+            return false;
+        }
 
         /// <inheritdoc/>
         public bool Equals(AdobeMarker other)
@@ -57,19 +104,14 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Common.Decoder
 
         /// <inheritdoc/>
         public override int GetHashCode()
-        {
-            return GetHashCode(this);
-        }
-
-        private static int GetHashCode(AdobeMarker marker)
         {
             return HashHelpers.Combine(
-                marker.DCTEncodeVersion.GetHashCode(),
+                this.DCTEncodeVersion.GetHashCode(),
                 HashHelpers.Combine(
-                    marker.APP14Flags0.GetHashCode(),
+                    this.APP14Flags0.GetHashCode(),
                     HashHelpers.Combine(
-                        marker.APP14Flags1.GetHashCode(),
+                        this.APP14Flags1.GetHashCode(),
-                        marker.ColorTransform.GetHashCode())));
+                        this.ColorTransform.GetHashCode())));
         }
     }
 }

src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs

@@ -67,14 +67,20 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort
         private bool isExif;
 
         /// <summary>
-        /// Whether the image has an Adobe marker
+        /// Whether the image has an Adobe marker.
+        /// It's faster to check this than to use the equality operator on the struct
         /// </summary>
         private bool isAdobe;
 
         /// <summary>
-        /// The Adobe color transform value for determining what color space the image uses.
+        /// Contains information about the Adobe marker
         /// </summary>
-        private byte adobeColorTransform;
+        private AdobeMarker adobe;
+
+        /// <summary>
+        /// Contains information about the JFIF marker
+        /// </summary>
+        private JFifMarker jFif;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="OrigJpegDecoderCore" /> class.
@@ -515,21 +521,18 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort
         /// <param name="remaining">The remaining bytes in the segment block.</param>
         private void ProcessApp14Marker(int remaining)
         {
-            if (remaining < 12)
+            const int MarkerLength = AdobeMarker.Length;
+            if (remaining < MarkerLength)
             {
                 // Skip the application header length
                 this.InputProcessor.Skip(remaining);
                 return;
             }
 
-            this.InputProcessor.ReadFull(this.Temp, 0, 12);
+            this.InputProcessor.ReadFull(this.Temp, 0, MarkerLength);
-            remaining -= 12;
+            remaining -= MarkerLength;
 
-            if (ProfileResolver.IsProfile(this.Temp, ProfileResolver.AdobeMarker))
+            this.isAdobe = AdobeMarker.TryParse(this.Temp, out this.adobe);
-            {
-                this.isAdobe = true;
-                this.adobeColorTransform = this.Temp[11];
-            }
 
             if (remaining > 0)
             {
@@ -750,19 +753,19 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort
                 case 1:
                     return JpegColorSpace.GrayScale;
                 case 3:
-                    if (!this.isAdobe || this.adobeColorTransform == OrigJpegConstants.Adobe.ColorTransformYCbCr)
+                    if (!this.isAdobe || this.adobe.ColorTransform == OrigJpegConstants.Adobe.ColorTransformYCbCr)
                     {
                         return JpegColorSpace.YCbCr;
                     }
 
-                    if (this.adobeColorTransform == OrigJpegConstants.Adobe.ColorTransformUnknown)
+                    if (this.adobe.ColorTransform == OrigJpegConstants.Adobe.ColorTransformUnknown)
                     {
                         return JpegColorSpace.RGB;
                     }
 
                     break;
                 case 4:
-                    if (this.adobeColorTransform == OrigJpegConstants.Adobe.ColorTransformYcck)
+                    if (this.adobe.ColorTransform == OrigJpegConstants.Adobe.ColorTransformYcck)
                     {
                         return JpegColorSpace.Ycck;
                     }

src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs

@@ -525,32 +525,18 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort
         /// <param name="remaining">The remaining bytes in the segment block.</param>
         private void ProcessApp14Marker(int remaining)
         {
-            if (remaining < 12)
+            const int MarkerLength = AdobeMarker.Length;
+            if (remaining < MarkerLength)
             {
                 // Skip the application header length
                 this.InputStream.Skip(remaining);
                 return;
             }
 
-            this.InputStream.Read(this.temp, 0, 12);
+            this.InputStream.Read(this.temp, 0, MarkerLength);
-            remaining -= 12;
+            remaining -= MarkerLength;
 
-            bool isAdobe = this.temp[0] == PdfJsJpegConstants.Markers.Adobe.A &&
+            AdobeMarker.TryParse(this.temp, out this.adobe);
-                           this.temp[1] == PdfJsJpegConstants.Markers.Adobe.D &&
-                           this.temp[2] == PdfJsJpegConstants.Markers.Adobe.O &&
-                           this.temp[3] == PdfJsJpegConstants.Markers.Adobe.B &&
-                           this.temp[4] == PdfJsJpegConstants.Markers.Adobe.E;
-
-            if (isAdobe)
-            {
-                this.adobe = new AdobeMarker
-                {
-                    DCTEncodeVersion = (short)((this.temp[5] << 8) | this.temp[6]),
-                    APP14Flags0 = (short)((this.temp[7] << 8) | this.temp[8]),
-                    APP14Flags1 = (short)((this.temp[9] << 8) | this.temp[10]),
-                    ColorTransform = this.temp[11]
-                };
-            }
 
             if (remaining > 0)
             {

tests/ImageSharp.Tests/Formats/Jpg/AdobeMarkerTests.cs

@@ -0,0 +1,43 @@
+// Copyright (c) Six Labors and contributors.
+// Licensed under the Apache License, Version 2.0.
+
+namespace SixLabors.ImageSharp.Tests.Formats.Jpg
+{
+    using SixLabors.ImageSharp.Formats.Jpeg.Common.Decoder;
+    using SixLabors.ImageSharp.Formats.Jpeg.GolangPort;
+
+    using Xunit;
+
+    public class AdobeMarkerTests
+    {
+        // Taken from actual test image
+        private readonly byte[] bytes = { 0x41, 0x64, 0x6F, 0x62, 0x65, 0x0, 0x64, 0x0, 0x0, 0x0, 0x0, 0x2 };
+
+        [Fact]
+        public void MarkerLengthIsCorrect()
+        {
+            Assert.Equal(12, AdobeMarker.Length);
+        }
+
+        [Fact]
+        public void MarkerReturnsCorrectParsedValue()
+        {
+            bool isAdobe = AdobeMarker.TryParse(this.bytes, out var marker);
+
+            Assert.True(isAdobe);
+            Assert.Equal(100, marker.DCTEncodeVersion);
+            Assert.Equal(0, marker.APP14Flags0);
+            Assert.Equal(0, marker.APP14Flags1);
+            Assert.Equal(OrigJpegConstants.Adobe.ColorTransformYcck, marker.ColorTransform);
+        }
+
+        [Fact]
+        public void MarkerIgnoresIncorrectValue()
+        {
+            bool isAdobe = AdobeMarker.TryParse(new byte[] { 0, 0, 0, 0 }, out var marker);
+
+            Assert.False(isAdobe);
+            Assert.Equal(default(AdobeMarker), marker);
+        }
+    }
+}