From 1b4bd0c4e9e46b370f7ab126904e5b38f5e04ed5 Mon Sep 17 00:00:00 2001 From: JimBobSquarePants Date: Tue, 26 Sep 2017 14:01:39 +1000 Subject: [PATCH] JFifMarker now handles parsing logic --- .../Formats/Jpeg/Common/Decoder/JFifMarker.cs | 87 +++++++++++++---- .../Jpeg/GolangPort/OrigJpegDecoderCore.cs | 33 ++----- .../Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs | 22 +---- .../Formats/Jpg/AdobeMarkerTests.cs | 2 +- .../Formats/Jpg/JFifMarkerTests.cs | 95 +++++++++++++++++++ 5 files changed, 177 insertions(+), 62 deletions(-) create mode 100644 tests/ImageSharp.Tests/Formats/Jpg/JFifMarkerTests.cs diff --git a/src/ImageSharp/Formats/Jpeg/Common/Decoder/JFifMarker.cs b/src/ImageSharp/Formats/Jpeg/Common/Decoder/JFifMarker.cs index acaa0eb53f..b4ef7a83d4 100644 --- a/src/ImageSharp/Formats/Jpeg/Common/Decoder/JFifMarker.cs +++ b/src/ImageSharp/Formats/Jpeg/Common/Decoder/JFifMarker.cs @@ -5,6 +5,8 @@ using System; namespace SixLabors.ImageSharp.Formats.Jpeg.Common.Decoder { + using Guard = SixLabors.Guard; + /// /// Provides information about the JFIF marker segment /// TODO: Thumbnail? @@ -12,32 +14,84 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Common.Decoder internal struct JFifMarker : IEquatable { /// - /// The major version + /// Gets the length of an JFIF marker segment. /// - public byte MajorVersion; + public const int Length = 13; /// - /// The minor version + /// Initializes a new instance of the struct. /// - public byte MinorVersion; + /// The major version + /// The minor version + /// The units for the density values + /// The horizontal pixel density + /// The veritcal pixel density + private JFifMarker(byte majorVersion, byte minorVersion, byte densityUnits, short xDensity, short yDensity) + { + Guard.MustBeGreaterThan(xDensity, 0, nameof(xDensity)); + Guard.MustBeGreaterThan(yDensity, 0, nameof(yDensity)); + + this.MajorVersion = majorVersion; + this.MinorVersion = minorVersion; + this.DensityUnits = densityUnits; + this.XDensity = xDensity; + this.YDensity = yDensity; + } /// - /// Units for the following pixel density fields + /// Gets the major version + /// + public byte MajorVersion { get; } + + /// + /// Gets the minor version + /// + public byte MinorVersion { get; } + + /// + /// Gets the units for the following pixel density fields /// 00 : No units; width:height pixel aspect ratio = Ydensity:Xdensity /// 01 : Pixels per inch (2.54 cm) /// 02 : Pixels per centimeter /// - public byte DensityUnits; + public byte DensityUnits { get; } + + /// + /// Gets the horizontal pixel density. Must not be zero. + /// + public short XDensity { get; } /// - /// Horizontal pixel density. Must not be zero. + /// Gets the vertical pixel density. Must not be zero. /// - public short XDensity; + public short YDensity { get; } /// - /// Vertical pixel density. Must not be zero. + /// Converts the specified byte array representation of an JFIF marker to its equivalent and + /// returns a value that indicates whether the conversion succeeded. /// - public short YDensity; + /// The byte array containing metadata to parse + /// The marker to return. + public static bool TryParse(byte[] bytes, out JFifMarker marker) + { + if (ProfileResolver.IsProfile(bytes, ProfileResolver.JFifMarker)) + { + byte majorVersion = bytes[5]; + byte minorVersion = bytes[6]; + byte densityUnits = bytes[7]; + short xDensity = (short)((bytes[8] << 8) | bytes[9]); + short yDensity = (short)((bytes[10] << 8) | bytes[11]); + + if (xDensity > 0 && yDensity > 0) + { + marker = new JFifMarker(majorVersion, minorVersion, densityUnits, xDensity, yDensity); + return true; + } + } + + marker = default(JFifMarker); + return false; + } /// public bool Equals(JFifMarker other) @@ -62,19 +116,14 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Common.Decoder /// public override int GetHashCode() - { - return GetHashCode(this); - } - - private static int GetHashCode(JFifMarker marker) { return HashHelpers.Combine( - marker.MajorVersion.GetHashCode(), + this.MajorVersion.GetHashCode(), HashHelpers.Combine( - marker.MinorVersion.GetHashCode(), + this.MinorVersion.GetHashCode(), HashHelpers.Combine( - marker.DensityUnits.GetHashCode(), - HashHelpers.Combine(marker.XDensity, marker.YDensity)))); + this.DensityUnits.GetHashCode(), + HashHelpers.Combine(this.XDensity, this.YDensity)))); } } } \ No newline at end of file diff --git a/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs index fa890a2a52..e7ffaa9d1a 100644 --- a/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs @@ -48,18 +48,14 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort /// /// Whether the image has a JFIF header + /// It's faster to check this than to use the equality operator on the struct /// private bool isJFif; /// - /// The horizontal resolution gleaned from a JFIF marker if present - /// - private short jFifHorizontalResolution; - - /// - /// The vertical resolution gleaned from a JFIF marker if present + /// Contains information about the JFIF marker /// - private short jFifVerticalResolution; + private JFifMarker jFif; /// /// Whether the image has a EXIF header @@ -77,11 +73,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort /// private AdobeMarker adobe; - /// - /// Contains information about the JFIF marker - /// - private JFifMarker jFif; - /// /// Initializes a new instance of the class. /// @@ -418,10 +409,10 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort this.MetaData.VerticalResolution = verticalValue; } } - else if (this.jFifHorizontalResolution > 0 && this.jFifVerticalResolution > 0) + else if (this.isJFif) { - this.MetaData.HorizontalResolution = this.jFifHorizontalResolution; - this.MetaData.VerticalResolution = this.jFifVerticalResolution; + this.MetaData.HorizontalResolution = this.jFif.XDensity; + this.MetaData.VerticalResolution = this.jFif.YDensity; } } @@ -437,15 +428,11 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort return; } - this.InputProcessor.ReadFull(this.Temp, 0, 13); - remaining -= 13; + const int MarkerLength = JFifMarker.Length; + this.InputProcessor.ReadFull(this.Temp, 0, MarkerLength); + remaining -= MarkerLength; - if (ProfileResolver.IsProfile(this.Temp, ProfileResolver.JFifMarker)) - { - this.isJFif = true; - this.jFifHorizontalResolution = (short)((this.Temp[8] << 8) | this.Temp[9]); - this.jFifVerticalResolution = (short)((this.Temp[10] << 8) | this.Temp[11]); - } + this.isJFif = JFifMarker.TryParse(this.Temp, out this.jFif); if (remaining > 0) { diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs index ad13098bae..211c24d208 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs @@ -410,26 +410,10 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort return; } - this.InputStream.Read(this.temp, 0, 13); - remaining -= 13; + this.InputStream.Read(this.temp, 0, JFifMarker.Length); + remaining -= JFifMarker.Length; - bool isJfif = this.temp[0] == PdfJsJpegConstants.Markers.JFif.J && - this.temp[1] == PdfJsJpegConstants.Markers.JFif.F && - this.temp[2] == PdfJsJpegConstants.Markers.JFif.I && - this.temp[3] == PdfJsJpegConstants.Markers.JFif.F && - this.temp[4] == PdfJsJpegConstants.Markers.JFif.Null; - - if (isJfif) - { - this.jFif = new JFifMarker - { - MajorVersion = this.temp[5], - MinorVersion = this.temp[6], - DensityUnits = this.temp[7], - XDensity = (short)((this.temp[8] << 8) | this.temp[9]), - YDensity = (short)((this.temp[10] << 8) | this.temp[11]) - }; - } + JFifMarker.TryParse(this.temp, out this.jFif); // TODO: thumbnail if (remaining > 0) diff --git a/tests/ImageSharp.Tests/Formats/Jpg/AdobeMarkerTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/AdobeMarkerTests.cs index 18e07b5572..b98a6fe8e7 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/AdobeMarkerTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/AdobeMarkerTests.cs @@ -13,7 +13,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg // Taken from actual test image private readonly byte[] bytes = { 0x41, 0x64, 0x6F, 0x62, 0x65, 0x0, 0x64, 0x0, 0x0, 0x0, 0x0, 0x2 }; - /// Altered components + // Altered components private readonly byte[] bytes2 = { 0x41, 0x64, 0x6F, 0x62, 0x65, 0x0, 0x64, 0x0, 0x0, 0x1, 0x1, 0x1 }; [Fact] diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JFifMarkerTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JFifMarkerTests.cs new file mode 100644 index 0000000000..cc06d5701b --- /dev/null +++ b/tests/ImageSharp.Tests/Formats/Jpg/JFifMarkerTests.cs @@ -0,0 +1,95 @@ +// 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 JFifMarkerTests + { + // Taken from actual test image + private readonly byte[] bytes = { 0x4A, 0x46, 0x49, 0x46, 0x0, 0x1, 0x1, 0x1, 0x0, 0x60, 0x0, 0x60, 0x0 }; + + // Altered components + private readonly byte[] bytes2 = { 0x4A, 0x46, 0x49, 0x46, 0x0, 0x1, 0x1, 0x1, 0x0, 0x48, 0x0, 0x48, 0x0 }; + + // Incorrect density values. Zero is invalid. + private readonly byte[] bytes3 = { 0x4A, 0x46, 0x49, 0x46, 0x0, 0x1, 0x1, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0 }; + + [Fact] + public void MarkerLengthIsCorrect() + { + Assert.Equal(13, JFifMarker.Length); + } + + [Fact] + public void MarkerReturnsCorrectParsedValue() + { + bool isJFif = JFifMarker.TryParse(this.bytes, out var marker); + + Assert.True(isJFif); + Assert.Equal(1, marker.MajorVersion); + Assert.Equal(1, marker.MinorVersion); + Assert.Equal(1, marker.DensityUnits); + Assert.Equal(96, marker.XDensity); + Assert.Equal(96, marker.YDensity); + } + + [Fact] + public void MarkerIgnoresIncorrectValue() + { + bool isJFif = JFifMarker.TryParse(new byte[] { 0, 0, 0, 0 }, out var marker); + + Assert.False(isJFif); + Assert.Equal(default(JFifMarker), marker); + } + + [Fact] + public void MarkerIgnoresCorrectHeaderButInvalidDensities() + { + bool isJFif = JFifMarker.TryParse(this.bytes3, out var marker); + + Assert.False(isJFif); + Assert.Equal(default(JFifMarker), marker); + } + + [Fact] + public void MarkerEqualityIsCorrect() + { + JFifMarker.TryParse(this.bytes, out var marker); + JFifMarker.TryParse(this.bytes, out var marker2); + + Assert.True(marker.Equals(marker2)); + } + + [Fact] + public void MarkerInEqualityIsCorrect() + { + JFifMarker.TryParse(this.bytes, out var marker); + JFifMarker.TryParse(this.bytes2, out var marker2); + + Assert.False(marker.Equals(marker2)); + } + + [Fact] + public void MarkerHashCodeIsReplicable() + { + JFifMarker.TryParse(this.bytes, out var marker); + JFifMarker.TryParse(this.bytes, out var marker2); + + Assert.True(marker.GetHashCode().Equals(marker2.GetHashCode())); + } + + [Fact] + public void MarkerHashCodeIsUnique() + { + JFifMarker.TryParse(this.bytes, out var marker); + JFifMarker.TryParse(this.bytes2, out var marker2); + + Assert.False(marker.GetHashCode().Equals(marker2.GetHashCode())); + } + } +} \ No newline at end of file