Merge branch 'js/better-identify' of https://github.com/SixLabors/ImageSharp into js/better-identify

src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs

@@ -474,7 +474,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components
                     if (nextByte != 0)
                     {
 #if DEBUG
-                    Debug.WriteLine($"DecodeScan - Unexpected marker {(c << 8) | nextByte:X} at {stream.Position}");
+                        Debug.WriteLine($"DecodeScan - Unexpected marker {(c << 8) | nextByte:X} at {stream.Position}");
 #endif
 
                         // We've encountered an unexpected marker. Reverse the stream and exit.

tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.MetaData.cs

@@ -0,0 +1,158 @@
+// Copyright (c) Six Labors and contributors.
+// Licensed under the Apache License, Version 2.0.
+
+using System.IO;
+using SixLabors.ImageSharp.Formats;
+using SixLabors.ImageSharp.MetaData.Profiles.Exif;
+using SixLabors.ImageSharp.MetaData.Profiles.Icc;
+using SixLabors.ImageSharp.PixelFormats;
+using Xunit;
+
+// ReSharper disable InconsistentNaming
+namespace SixLabors.ImageSharp.Tests.Formats.Jpg
+{
+    using System.Runtime.CompilerServices;
+
+    using SixLabors.ImageSharp.Formats.Jpeg;
+
+    public partial class JpegDecoderTests
+    {
+        // TODO: A JPEGsnoop & metadata expert should review if the Exif/Icc expectations are correct. 
+        // I'm seeing several entries with Exif-related names in images where we do not decode an exif profile. (- Anton)
+        public static readonly TheoryData<bool, string, int, bool, bool> MetaDataTestData =
+            new TheoryData<bool, string, int, bool, bool>
+                {
+                    { false, TestImages.Jpeg.Progressive.Progress, 24, false, false },
+                    { false, TestImages.Jpeg.Progressive.Fb, 24, false, true },
+                    { false, TestImages.Jpeg.Baseline.Cmyk, 32, false, true },
+                    { false, TestImages.Jpeg.Baseline.Ycck, 32, true, true },
+                    { false, TestImages.Jpeg.Baseline.Jpeg400, 8, false, false },
+                    { false, TestImages.Jpeg.Baseline.Snake, 24, true, true },
+                    { false, TestImages.Jpeg.Baseline.Jpeg420Exif, 24, true, false },
+
+                    { true, TestImages.Jpeg.Progressive.Progress, 24, false, false },
+                    { true, TestImages.Jpeg.Progressive.Fb, 24, false, true },
+                    { true, TestImages.Jpeg.Baseline.Cmyk, 32, false, true },
+                    { true, TestImages.Jpeg.Baseline.Ycck, 32, true, true },
+                    { true, TestImages.Jpeg.Baseline.Jpeg400, 8, false, false },
+                    { true, TestImages.Jpeg.Baseline.Snake, 24, true, true },
+                    { true, TestImages.Jpeg.Baseline.Jpeg420Exif, 24, true, false },
+                };
+
+        [Theory]
+        [MemberData(nameof(MetaDataTestData))]
+        public void MetaDataIsParsedCorrectly_Orig(
+            bool useIdentify,
+            string imagePath,
+            int expectedPixelSize,
+            bool exifProfilePresent,
+            bool iccProfilePresent)
+        {
+            TestMetaDataImpl(
+                useIdentify,
+                OrigJpegDecoder,
+                imagePath,
+                expectedPixelSize,
+                exifProfilePresent,
+                iccProfilePresent);
+        }
+
+        [Theory]
+        [MemberData(nameof(MetaDataTestData))]
+        public void MetaDataIsParsedCorrectly_PdfJs(
+            bool useIdentify,
+            string imagePath,
+            int expectedPixelSize,
+            bool exifProfilePresent,
+            bool iccProfilePresent)
+        {
+            TestMetaDataImpl(
+                useIdentify,
+                PdfJsJpegDecoder,
+                imagePath,
+                expectedPixelSize,
+                exifProfilePresent,
+                iccProfilePresent);
+        }
+
+        private static void TestMetaDataImpl(
+            bool useIdentify,
+            IImageDecoder decoder,
+            string imagePath,
+            int expectedPixelSize,
+            bool exifProfilePresent,
+            bool iccProfilePresent)
+        {
+            var testFile = TestFile.Create(imagePath);
+            using (var stream = new MemoryStream(testFile.Bytes, false))
+            {
+                IImageInfo imageInfo = useIdentify
+                                           ? ((IImageInfoDetector)decoder).Identify(Configuration.Default, stream)
+                                           : decoder.Decode<Rgba32>(Configuration.Default, stream);
+
+                Assert.NotNull(imageInfo);
+                Assert.NotNull(imageInfo.PixelType);
+                
+                if (useIdentify)
+                {
+                    Assert.Equal(expectedPixelSize, imageInfo.PixelType.BitsPerPixel);
+                }
+                else
+                {
+                    // When full Image<TPixel> decoding is performed, BitsPerPixel will match TPixel
+                    int bpp32 = Unsafe.SizeOf<Rgba32>() * 8;
+                    Assert.Equal(bpp32, imageInfo.PixelType.BitsPerPixel);
+                }
+
+                ExifProfile exifProfile = imageInfo.MetaData.ExifProfile;
+
+                if (exifProfilePresent)
+                {
+                    Assert.NotNull(exifProfile);
+                    Assert.NotEmpty(exifProfile.Values);
+                }
+                else
+                {
+                    Assert.Null(exifProfile);
+                }
+
+                IccProfile iccProfile = imageInfo.MetaData.IccProfile;
+
+                if (iccProfilePresent)
+                {
+                    Assert.NotNull(iccProfile);
+                    Assert.NotEmpty(iccProfile.Entries);
+                }
+                else
+                {
+                    Assert.Null(iccProfile);
+                }
+            }
+        }
+        
+        [Theory]
+        [InlineData(false)]
+        [InlineData(true)]
+        public void IgnoreMetaData_ControlsWhetherMetaDataIsParsed(bool ignoreMetaData)
+        {
+            var decoder = new JpegDecoder() { IgnoreMetadata = ignoreMetaData };
+
+            // Snake.jpg has both Exif and ICC profiles defined:
+            var testFile = TestFile.Create(TestImages.Jpeg.Baseline.Snake);
+
+            using (Image<Rgba32> image = testFile.CreateImage(decoder))
+            {
+                if (ignoreMetaData)
+                {
+                    Assert.Null(image.MetaData.ExifProfile);
+                    Assert.Null(image.MetaData.IccProfile);
+                }
+                else
+                {
+                    Assert.NotNull(image.MetaData.ExifProfile);
+                    Assert.NotNull(image.MetaData.IccProfile);
+                }
+            }
+        }
+    }
+}

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

@@ -22,7 +22,7 @@ using Xunit.Abstractions;
 namespace SixLabors.ImageSharp.Tests.Formats.Jpg
 {
     // TODO: Scatter test cases into multiple test classes
-    public class JpegDecoderTests
+    public partial class JpegDecoderTests
     {
         public static string[] BaselineTestJpegs =
         {
@@ -115,9 +115,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg
 
         private ITestOutputHelper Output { get; }
 
-        private static IImageDecoder OrigJpegDecoder => new OrigJpegDecoder();
+        private static OrigJpegDecoder OrigJpegDecoder => new OrigJpegDecoder();
 
-        private static IImageDecoder PdfJsJpegDecoder => new PdfJsJpegDecoder();
+        private static PdfJsJpegDecoder PdfJsJpegDecoder => new PdfJsJpegDecoder();
 
         [Fact]
         public void ParseStream_BasicPropertiesAreCorrect1_PdfJs()
@@ -151,7 +151,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg
             // For 32 bit test enviroments:
             provider.Configuration.MemoryManager = ArrayPoolMemoryManager.CreateWithModeratePooling();
 
-            IImageDecoder decoder = useOldDecoder ? OrigJpegDecoder : PdfJsJpegDecoder;
+            IImageDecoder decoder = useOldDecoder ? (IImageDecoder)OrigJpegDecoder : PdfJsJpegDecoder;
             using (Image<TPixel> image = provider.GetImage(decoder))
             {
                 image.DebugSave(provider);
@@ -406,39 +406,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg
                 Assert.Equal(72, image.MetaData.VerticalResolution);
             }
         }
-
+        
-        [Fact]
-        public void Decode_IgnoreMetadataIsFalse_ExifProfileIsRead()
-        {
-            var decoder = new JpegDecoder()
-            {
-                IgnoreMetadata = false
-            };
-
-            var testFile = TestFile.Create(TestImages.Jpeg.Baseline.Floorplan);
-
-            using (Image<Rgba32> image = testFile.CreateImage(decoder))
-            {
-                Assert.NotNull(image.MetaData.ExifProfile);
-            }
-        }
-
-        [Fact]
-        public void Decode_IgnoreMetadataIsTrue_ExifProfileIgnored()
-        {
-            var options = new JpegDecoder()
-            {
-                IgnoreMetadata = true
-            };
-
-            var testFile = TestFile.Create(TestImages.Jpeg.Baseline.Floorplan);
-
-            using (Image<Rgba32> image = testFile.CreateImage(options))
-            {
-                Assert.Null(image.MetaData.ExifProfile);
-            }
-        }
-
         // DEBUG ONLY!
         // The PDF.js output should be saved by "tests\ImageSharp.Tests\Formats\Jpg\pdfjs\jpeg-converter.htm"
         // into "\tests\Images\ActualOutput\JpegDecoderTests\"
@@ -470,39 +438,5 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg
                 this.Output.WriteLine($"Difference for PORT: {portReport.DifferencePercentageString}");
             }
         }
-
-        [Theory]
-        [InlineData(TestImages.Jpeg.Progressive.Progress, 24)]
-        [InlineData(TestImages.Jpeg.Progressive.Fb, 24)]
-        [InlineData(TestImages.Jpeg.Baseline.Cmyk, 32)]
-        [InlineData(TestImages.Jpeg.Baseline.Ycck, 32)]
-        [InlineData(TestImages.Jpeg.Baseline.Jpeg400, 8)]
-        [InlineData(TestImages.Jpeg.Baseline.Snake, 24)]
-        [InlineData(TestImages.Jpeg.Baseline.Jpeg420Exif, 24)]
-        public void DetectPixelSizeGolang(string imagePath, int expectedPixelSize)
-        {
-            var testFile = TestFile.Create(imagePath);
-            using (var stream = new MemoryStream(testFile.Bytes, false))
-            {
-                Assert.Equal(expectedPixelSize, ((IImageInfoDetector)OrigJpegDecoder).Identify(Configuration.Default, stream)?.PixelType?.BitsPerPixel);
-            }
-        }
-
-        [Theory]
-        [InlineData(TestImages.Jpeg.Progressive.Progress, 24)]
-        [InlineData(TestImages.Jpeg.Progressive.Fb, 24)]
-        [InlineData(TestImages.Jpeg.Baseline.Cmyk, 32)]
-        [InlineData(TestImages.Jpeg.Baseline.Ycck, 32)]
-        [InlineData(TestImages.Jpeg.Baseline.Jpeg400, 8)]
-        [InlineData(TestImages.Jpeg.Baseline.Snake, 24)]
-        [InlineData(TestImages.Jpeg.Baseline.Jpeg420Exif, 24)]
-        public void DetectPixelSizePdfJs(string imagePath, int expectedPixelSize)
-        {
-            var testFile = TestFile.Create(imagePath);
-            using (var stream = new MemoryStream(testFile.Bytes, false))
-            {
-                Assert.Equal(expectedPixelSize, ((IImageInfoDetector)PdfJsJpegDecoder).Identify(Configuration.Default, stream)?.PixelType?.BitsPerPixel);
-            }
-        }
     }
 }