From afa8a1b8b0bc04e0f8091071fa5b1cdd357657e5 Mon Sep 17 00:00:00 2001 From: Ildar Khayrutdinov Date: Thu, 7 Jan 2021 10:53:27 +0300 Subject: [PATCH] Exif reader fixes --- .../Metadata/Profiles/Exif/ExifReader.cs | 114 +++++++++++------- .../Formats/Tiff/TiffMetadataTests.cs | 4 + .../Profiles/Exif/ExifProfileTests.cs | 15 +++ .../Metadata/Profiles/Exif/ExifReaderTests.cs | 1 + .../Exif/ExifTagDescriptionAttributeTests.cs | 1 + .../Metadata/Profiles/Exif/ExifValueTests.cs | 1 + .../Profiles/Exif/Values/ExifValuesTests.cs | 4 +- 7 files changed, 94 insertions(+), 46 deletions(-) diff --git a/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs b/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs index 65a1eeaee..dd4fada78 100644 --- a/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs +++ b/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs @@ -5,6 +5,7 @@ using System; using System.Buffers.Binary; using System.Collections.Generic; using System.Collections.ObjectModel; +using System.Diagnostics; using System.IO; using System.Runtime.CompilerServices; using System.Text; @@ -23,12 +24,17 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Exif private readonly byte[] buf2 = new byte[2]; // used for sequential read big values (actual for multiframe big files) - private readonly SortedList lazyLoaders = new SortedList(); + // todo: different tags can link to the same data (stream offset) - investigate + private readonly SortedList lazyLoaders = new SortedList(new DuplicateKeyComparer()); private bool isBigEndian; private List invalidTags; + private uint exifOffset = 0; + + private uint gpsOffset = 0; + public ExifReader(bool isBigEndian, Stream stream) { this.isBigEndian = isBigEndian; @@ -91,7 +97,6 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Exif } uint ifdOffset = this.ReadUInt32(); - this.AddValues(values, ifdOffset); uint thumbnailOffset = this.ReadUInt32(); @@ -134,43 +139,15 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Exif protected void AddSubIfdValues(List values) { - uint exifOffset = 0; - uint gpsOffset = 0; - foreach (IExifValue value in values) - { - if (value.Tag == ExifTag.SubIFDOffset) - { - exifOffset = ((ExifLong)value).Value; - } - - if (value.Tag == ExifTag.GPSIFDOffset) - { - gpsOffset = ((ExifLong)value).Value; - } - } - - if (exifOffset != 0) - { - this.AddValues(values, exifOffset); - } - - if (gpsOffset != 0) + if (this.exifOffset != 0) { - this.AddValues(values, gpsOffset); + this.AddValues(values, this.exifOffset); } - } - private static bool IsDuplicate(IList values, IExifValue value) - { - foreach (IExifValue val in values) + if (this.gpsOffset != 0) { - if (val == value) - { - return true; - } + this.AddValues(values, this.gpsOffset); } - - return false; } private static TDataType[] ToArray(ExifDataType dataType, ReadOnlySpan data, ConverterMethod converter) @@ -337,7 +314,6 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Exif } uint size = numberOfComponents * ExifDataTypes.GetSize(dataType); - object value = null; if (size > 4) { uint newIndex = this.ConvertToUInt32(this.offsetBuffer); @@ -349,29 +325,60 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Exif return; } + if (this.lazyLoaders.ContainsKey(newIndex)) + { + Debug.WriteLine($"Duplicate offset: tag={tag}, size={size}, offset={newIndex}"); + } + this.lazyLoaders.Add(newIndex, () => { var dataBuffer = new byte[size]; this.Seek(newIndex); if (this.TryReadSpan(dataBuffer)) { - value = this.ConvertValue(dataType, dataBuffer, numberOfComponents); - } + object value = this.ConvertValue(dataType, dataBuffer, numberOfComponents); - if (exifValue.TrySetValue(value) && !IsDuplicate(values, exifValue)) - { - values.Add(exifValue); + this.Add(values, exifValue, value); } }); } else { - value = this.ConvertValue(dataType, this.offsetBuffer, numberOfComponents); + object value = this.ConvertValue(dataType, this.offsetBuffer, numberOfComponents); + + this.Add(values, exifValue, value); + } + } + + private void Add(IList values, IExifValue exif, object value) + { + if (!exif.TrySetValue(value)) + { + return; + } + + foreach (IExifValue val in values) + { + // sometimes duplicates appear, + // can compare val.Tag == exif.Tag + if (val == exif) + { + Debug.WriteLine($"Duplicate Exif tag: tag={exif.Tag}, dataType={exif.DataType}"); + return; + } } - if (exifValue.TrySetValue(value) && !IsDuplicate(values, exifValue)) + if (exif.Tag == ExifTag.SubIFDOffset) + { + this.exifOffset = (uint)value; + } + else if (exif.Tag == ExifTag.GPSIFDOffset) + { + this.gpsOffset = (uint)value; + } + else { - values.Add(exifValue); + values.Add(exif); } } @@ -387,12 +394,10 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Exif if (this.RemainingLength < length) { span = default; - return false; } int readed = this.data.Read(span); - return readed == length; } @@ -408,6 +413,11 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Exif private void GetThumbnail(uint offset) { + if (offset == 0) + { + return; + } + var values = new List(); this.AddValues(values, offset); @@ -528,5 +538,19 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Exif ? BinaryPrimitives.ReadInt16BigEndian(buffer) : BinaryPrimitives.ReadInt16LittleEndian(buffer); } + + /// used for possiblity add a duplicate offsets (but tags don't duplicate). + /// The type of the key. + public class DuplicateKeyComparer : IComparer + where TKey : IComparable + { + public int Compare(TKey x, TKey y) + { + int result = x.CompareTo(y); + + // Handle equality as beeing greater + return (result == 0) ? 1 : result; + } + } } } diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs index 7e0b472c4..ad2d7e291 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs @@ -146,6 +146,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff Assert.Equal(10, image.Metadata.VerticalResolution); TiffFrameMetadata frame = image.Frames.RootFrame.Metadata.GetTiffMetadata(); + Assert.Equal(30, frame.FrameTags.Values.Count); + Assert.Equal(32u, frame.Width); Assert.Equal(32u, frame.Height); Assert.Equal(new ushort[] { 4 }, frame.BitsPerSample); @@ -176,6 +178,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff Assert.Equal(TiffPredictor.None, frame.Predictor); Assert.Null(frame.SampleFormat); Assert.Equal("This is Авторские права", frame.Copyright); + Assert.Equal(4, frame.FrameTags.GetValue(ExifTag.Rating).Value); + Assert.Equal(75, frame.FrameTags.GetValue(ExifTag.RatingPercent).Value); } } diff --git a/tests/ImageSharp.Tests/Metadata/Profiles/Exif/ExifProfileTests.cs b/tests/ImageSharp.Tests/Metadata/Profiles/Exif/ExifProfileTests.cs index 466568bfe..10f6ff9bf 100644 --- a/tests/ImageSharp.Tests/Metadata/Profiles/Exif/ExifProfileTests.cs +++ b/tests/ImageSharp.Tests/Metadata/Profiles/Exif/ExifProfileTests.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; using System.Text; + using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.Metadata.Profiles.Exif; using SixLabors.ImageSharp.PixelFormats; @@ -14,6 +15,7 @@ using Xunit; namespace SixLabors.ImageSharp.Tests { + [Trait("Profile", "Exif")] public class ExifProfileTests { public enum TestImageWriteFormat @@ -201,10 +203,14 @@ namespace SixLabors.ImageSharp.Tests IExifValue latitude = image.Metadata.ExifProfile.GetValue(ExifTag.GPSLatitude); Assert.Equal(expectedLatitude, latitude.Value); + // todo: duplicate tags + Assert.Equal(2, image.Metadata.ExifProfile.Values.Count(v => (ushort)v.Tag == 59932)); + int profileCount = image.Metadata.ExifProfile.Values.Count; image = WriteAndRead(image, imageFormat); Assert.NotNull(image.Metadata.ExifProfile); + Assert.Equal(0, image.Metadata.ExifProfile.Values.Count(v => (ushort)v.Tag == 59932)); // Should be 3 less. // 1 x due to setting of null "ReferenceBlackWhite" value. @@ -363,8 +369,14 @@ namespace SixLabors.ImageSharp.Tests // Force parsing of the profile. Assert.Equal(25, profile.Values.Count); + // todo: duplicate tags (from root container and subIfd) + Assert.Equal(2, profile.Values.Count(v => (ExifTagValue)(ushort)v.Tag == ExifTagValue.DateTime)); + byte[] bytes = profile.ToByteArray(); Assert.Equal(525, bytes.Length); + + var profile2 = new ExifProfile(bytes); + Assert.Equal(25, profile2.Values.Count); } [Theory] @@ -477,6 +489,9 @@ namespace SixLabors.ImageSharp.Tests { Assert.NotNull(profile); + // todo: duplicate tags + Assert.Equal(2, profile.Values.Count(v => (ushort)v.Tag == 59932)); + Assert.Equal(16, profile.Values.Count); foreach (IExifValue value in profile.Values) diff --git a/tests/ImageSharp.Tests/Metadata/Profiles/Exif/ExifReaderTests.cs b/tests/ImageSharp.Tests/Metadata/Profiles/Exif/ExifReaderTests.cs index 401546e5c..4ff37eb6b 100644 --- a/tests/ImageSharp.Tests/Metadata/Profiles/Exif/ExifReaderTests.cs +++ b/tests/ImageSharp.Tests/Metadata/Profiles/Exif/ExifReaderTests.cs @@ -8,6 +8,7 @@ using Xunit; namespace SixLabors.ImageSharp.Tests { + [Trait("Profile", "Exif")] public class ExifReaderTests { [Fact] diff --git a/tests/ImageSharp.Tests/Metadata/Profiles/Exif/ExifTagDescriptionAttributeTests.cs b/tests/ImageSharp.Tests/Metadata/Profiles/Exif/ExifTagDescriptionAttributeTests.cs index 2b00cc5b4..42a3b72a6 100644 --- a/tests/ImageSharp.Tests/Metadata/Profiles/Exif/ExifTagDescriptionAttributeTests.cs +++ b/tests/ImageSharp.Tests/Metadata/Profiles/Exif/ExifTagDescriptionAttributeTests.cs @@ -6,6 +6,7 @@ using Xunit; namespace SixLabors.ImageSharp.Tests { + [Trait("Profile", "Exif")] public class ExifTagDescriptionAttributeTests { [Fact] diff --git a/tests/ImageSharp.Tests/Metadata/Profiles/Exif/ExifValueTests.cs b/tests/ImageSharp.Tests/Metadata/Profiles/Exif/ExifValueTests.cs index 5fe1b51ba..a2c01ea61 100644 --- a/tests/ImageSharp.Tests/Metadata/Profiles/Exif/ExifValueTests.cs +++ b/tests/ImageSharp.Tests/Metadata/Profiles/Exif/ExifValueTests.cs @@ -7,6 +7,7 @@ using Xunit; namespace SixLabors.ImageSharp.Tests { + [Trait("Profile", "Exif")] public class ExifValueTests { private ExifProfile profile; diff --git a/tests/ImageSharp.Tests/Metadata/Profiles/Exif/Values/ExifValuesTests.cs b/tests/ImageSharp.Tests/Metadata/Profiles/Exif/Values/ExifValuesTests.cs index 898c69356..3358b1f97 100644 --- a/tests/ImageSharp.Tests/Metadata/Profiles/Exif/Values/ExifValuesTests.cs +++ b/tests/ImageSharp.Tests/Metadata/Profiles/Exif/Values/ExifValuesTests.cs @@ -6,6 +6,7 @@ using Xunit; namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Exif.Values { + [Trait("Profile", "Exif")] public class ExifValuesTests { public static TheoryData ByteTags => new TheoryData @@ -94,6 +95,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Exif.Values public static TheoryData NumberArrayTags => new TheoryData { { ExifTag.StripOffsets }, + { ExifTag.StripByteCounts }, { ExifTag.TileByteCounts }, { ExifTag.ImageLayer } }; @@ -160,6 +162,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Exif.Values { ExifTag.Orientation }, { ExifTag.SamplesPerPixel }, { ExifTag.PlanarConfiguration }, + { ExifTag.Predictor }, { ExifTag.GrayResponseUnit }, { ExifTag.ResolutionUnit }, { ExifTag.CleanFaxData }, @@ -208,7 +211,6 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Exif.Values { ExifTag.ExtraSamples }, { ExifTag.PageNumber }, { ExifTag.TransferFunction }, - { ExifTag.Predictor }, { ExifTag.HalftoneHints }, { ExifTag.SampleFormat }, { ExifTag.TransferRange },