From 1e53adf85ea63e197e7c4392daef71755e3a0d06 Mon Sep 17 00:00:00 2001 From: Jeffrey Parker Date: Wed, 13 Sep 2023 02:26:55 -0500 Subject: [PATCH 1/2] Fix for issue 2504 - ensuring image and individual frame metadata are written out correctly --- .../Formats/Tiff/TiffEncoderCore.cs | 5 + .../Tiff/TiffEncoderEntriesCollector.cs | 114 +++++++++++++++--- .../Formats/Tiff/TiffMetadataTests.cs | 91 ++++++++++++++ 3 files changed, 195 insertions(+), 15 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs b/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs index d7243c6964..b7338ac20a 100644 --- a/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs +++ b/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs @@ -157,6 +157,7 @@ public void Encode(Image image, Stream stream, CancellationToken long ifdMarker = WriteHeader(writer, buffer); Image metadataImage = image; + foreach (ImageFrame frame in image.Frames) { cancellationToken.ThrowIfCancellationRequested(); @@ -235,9 +236,13 @@ private long WriteFrame( if (image != null) { + // Write the metadata for the root image entriesCollector.ProcessMetadata(image, this.skipMetadata); } + // Write the metadata for the frame + entriesCollector.ProcessMetadata(frame, this.skipMetadata); + entriesCollector.ProcessFrameInfo(frame, imageMetadata); entriesCollector.ProcessImageFormat(this); diff --git a/src/ImageSharp/Formats/Tiff/TiffEncoderEntriesCollector.cs b/src/ImageSharp/Formats/Tiff/TiffEncoderEntriesCollector.cs index cf9b4ae213..e7ee0c65bc 100644 --- a/src/ImageSharp/Formats/Tiff/TiffEncoderEntriesCollector.cs +++ b/src/ImageSharp/Formats/Tiff/TiffEncoderEntriesCollector.cs @@ -7,6 +7,7 @@ using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.Metadata.Profiles.Exif; using SixLabors.ImageSharp.Metadata.Profiles.Xmp; +using SixLabors.ImageSharp.PixelFormats; namespace SixLabors.ImageSharp.Formats.Tiff; @@ -19,6 +20,9 @@ internal class TiffEncoderEntriesCollector public void ProcessMetadata(Image image, bool skipMetadata) => new MetadataProcessor(this).Process(image, skipMetadata); + public void ProcessMetadata(ImageFrame frame, bool skipMetadata) + => new MetadataProcessor(this).Process(frame, skipMetadata); + public void ProcessFrameInfo(ImageFrame frame, ImageMetadata imageMetadata) => new FrameInfoProcessor(this).Process(frame, imageMetadata); @@ -56,15 +60,30 @@ public MetadataProcessor(TiffEncoderEntriesCollector collector) public void Process(Image image, bool skipMetadata) { - ImageFrame rootFrame = image.Frames.RootFrame; - ExifProfile rootFrameExifProfile = rootFrame.Metadata.ExifProfile; - XmpProfile rootFrameXmpProfile = rootFrame.Metadata.XmpProfile; + this.ProcessProfiles(image.Metadata, skipMetadata); - this.ProcessProfiles(image.Metadata, skipMetadata, rootFrameExifProfile, rootFrameXmpProfile); + if (!skipMetadata) + { + this.ProcessMetadata(image.Metadata.ExifProfile ?? new ExifProfile()); + } + + if (!this.Collector.Entries.Exists(t => t.Tag == ExifTag.Software)) + { + this.Collector.Add(new ExifString(ExifTagValue.Software) + { + Value = SoftwareValue + }); + } + } + + + public void Process(ImageFrame frame, bool skipMetadata) + { + this.ProcessProfiles(frame.Metadata, skipMetadata); if (!skipMetadata) { - this.ProcessMetadata(rootFrameExifProfile ?? new ExifProfile()); + this.ProcessMetadata(frame.Metadata.ExifProfile ?? new ExifProfile()); } if (!this.Collector.Entries.Exists(t => t.Tag == ExifTag.Software)) @@ -150,16 +169,16 @@ private void ProcessMetadata(ExifProfile exifProfile) } } - private void ProcessProfiles(ImageMetadata imageMetadata, bool skipMetadata, ExifProfile exifProfile, XmpProfile xmpProfile) + private void ProcessProfiles(ImageMetadata imageMetadata, bool skipMetadata) { - if (!skipMetadata && (exifProfile != null && exifProfile.Parts != ExifParts.None)) + if (!skipMetadata && (imageMetadata.ExifProfile != null && imageMetadata.ExifProfile.Parts != ExifParts.None)) { - foreach (IExifValue entry in exifProfile.Values) + foreach (IExifValue entry in imageMetadata.ExifProfile.Values) { if (!this.Collector.Entries.Exists(t => t.Tag == entry.Tag) && entry.GetValue() != null) { ExifParts entryPart = ExifTags.GetPart(entry.Tag); - if (entryPart != ExifParts.None && exifProfile.Parts.HasFlag(entryPart)) + if (entryPart != ExifParts.None && imageMetadata.ExifProfile.Parts.HasFlag(entryPart)) { this.Collector.AddOrReplace(entry.DeepClone()); } @@ -168,7 +187,7 @@ private void ProcessProfiles(ImageMetadata imageMetadata, bool skipMetadata, Exi } else { - exifProfile?.RemoveValue(ExifTag.SubIFDOffset); + imageMetadata.ExifProfile?.RemoveValue(ExifTag.SubIFDOffset); } if (!skipMetadata && imageMetadata.IptcProfile != null) @@ -183,7 +202,7 @@ private void ProcessProfiles(ImageMetadata imageMetadata, bool skipMetadata, Exi } else { - exifProfile?.RemoveValue(ExifTag.IPTC); + imageMetadata.ExifProfile?.RemoveValue(ExifTag.IPTC); } if (imageMetadata.IccProfile != null) @@ -197,21 +216,86 @@ private void ProcessProfiles(ImageMetadata imageMetadata, bool skipMetadata, Exi } else { - exifProfile?.RemoveValue(ExifTag.IccProfile); + imageMetadata.ExifProfile?.RemoveValue(ExifTag.IccProfile); + } + + if (!skipMetadata && imageMetadata.XmpProfile != null) + { + ExifByteArray xmp = new(ExifTagValue.XMP, ExifDataType.Byte) + { + Value = imageMetadata.XmpProfile.Data + }; + + this.Collector.AddOrReplace(xmp); + } + else + { + imageMetadata.ExifProfile?.RemoveValue(ExifTag.XMP); + } + } + + private void ProcessProfiles(ImageFrameMetadata frameMetadata, bool skipMetadata) + { + if (!skipMetadata && (frameMetadata.ExifProfile != null && frameMetadata.ExifProfile.Parts != ExifParts.None)) + { + foreach (IExifValue entry in frameMetadata.ExifProfile.Values) + { + if (!this.Collector.Entries.Exists(t => t.Tag == entry.Tag) && entry.GetValue() != null) + { + ExifParts entryPart = ExifTags.GetPart(entry.Tag); + if (entryPart != ExifParts.None && frameMetadata.ExifProfile.Parts.HasFlag(entryPart)) + { + this.Collector.AddOrReplace(entry.DeepClone()); + } + } + } + } + else + { + frameMetadata.ExifProfile?.RemoveValue(ExifTag.SubIFDOffset); + } + + if (!skipMetadata && frameMetadata.IptcProfile != null) + { + frameMetadata.IptcProfile.UpdateData(); + ExifByteArray iptc = new(ExifTagValue.IPTC, ExifDataType.Byte) + { + Value = frameMetadata.IptcProfile.Data + }; + + this.Collector.AddOrReplace(iptc); + } + else + { + frameMetadata.ExifProfile?.RemoveValue(ExifTag.IPTC); + } + + if (frameMetadata.IccProfile != null) + { + ExifByteArray icc = new(ExifTagValue.IccProfile, ExifDataType.Undefined) + { + Value = frameMetadata.IccProfile.ToByteArray() + }; + + this.Collector.AddOrReplace(icc); + } + else + { + frameMetadata.ExifProfile?.RemoveValue(ExifTag.IccProfile); } - if (!skipMetadata && xmpProfile != null) + if (!skipMetadata && frameMetadata.XmpProfile != null) { ExifByteArray xmp = new(ExifTagValue.XMP, ExifDataType.Byte) { - Value = xmpProfile.Data + Value = frameMetadata.XmpProfile.Data }; this.Collector.AddOrReplace(xmp); } else { - exifProfile?.RemoveValue(ExifTag.XMP); + frameMetadata.ExifProfile?.RemoveValue(ExifTag.XMP); } } } diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs index b671addf95..1225bbc8cb 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs @@ -7,6 +7,7 @@ using SixLabors.ImageSharp.Formats.Tiff.Constants; using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.Metadata.Profiles.Exif; +using SixLabors.ImageSharp.Metadata.Profiles.Icc; using SixLabors.ImageSharp.Metadata.Profiles.Iptc; using SixLabors.ImageSharp.Metadata.Profiles.Xmp; using SixLabors.ImageSharp.PixelFormats; @@ -318,4 +319,94 @@ public void Encode_PreservesMetadata(TestImageProvider provider) Assert.Equal((ushort)TiffPlanarConfiguration.Chunky, encodedImageExifProfile.GetValue(ExifTag.PlanarConfiguration)?.Value); Assert.Equal(exifProfileInput.Values.Count, encodedImageExifProfile.Values.Count); } + + [Theory] + [WithFile(SampleMetadata, PixelTypes.Rgba32)] + public void Encode_PreservesMetadata_IptcAndIcc(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + // Load Tiff image + DecoderOptions options = new() { SkipMetadata = false }; + using Image image = provider.GetImage(TiffDecoder.Instance, options); + + ImageMetadata inputMetaData = image.Metadata; + ImageFrame rootFrameInput = image.Frames.RootFrame; + + IptcProfile iptcProfile = new(); + iptcProfile.SetValue(IptcTag.Name, "Test name"); + rootFrameInput.Metadata.IptcProfile = iptcProfile; + + IccProfileHeader iccProfileHeader = new IccProfileHeader(); + iccProfileHeader.Class = IccProfileClass.ColorSpace; + IccProfile iccProfile = new(); + rootFrameInput.Metadata.IccProfile = iccProfile; + + TiffFrameMetadata frameMetaInput = rootFrameInput.Metadata.GetTiffMetadata(); + XmpProfile xmpProfileInput = rootFrameInput.Metadata.XmpProfile; + ExifProfile exifProfileInput = rootFrameInput.Metadata.ExifProfile; + IptcProfile iptcProfileInput = rootFrameInput.Metadata.IptcProfile; + IccProfile iccProfileInput = rootFrameInput.Metadata.IccProfile; + + Assert.Equal(TiffCompression.Lzw, frameMetaInput.Compression); + Assert.Equal(TiffBitsPerPixel.Bit4, frameMetaInput.BitsPerPixel); + + // Save to Tiff + TiffEncoder tiffEncoder = new() { PhotometricInterpretation = TiffPhotometricInterpretation.Rgb }; + using MemoryStream ms = new(); + image.Save(ms, tiffEncoder); + + // Assert + ms.Position = 0; + using Image encodedImage = Image.Load(ms); + + ImageMetadata encodedImageMetaData = encodedImage.Metadata; + ImageFrame rootFrameEncodedImage = encodedImage.Frames.RootFrame; + TiffFrameMetadata tiffMetaDataEncodedRootFrame = rootFrameEncodedImage.Metadata.GetTiffMetadata(); + ExifProfile encodedImageExifProfile = rootFrameEncodedImage.Metadata.ExifProfile; + XmpProfile encodedImageXmpProfile = rootFrameEncodedImage.Metadata.XmpProfile; + IptcProfile encodedImageIptcProfile = rootFrameEncodedImage.Metadata.IptcProfile; + IccProfile encodedImageIccProfile = rootFrameEncodedImage.Metadata.IccProfile; + + Assert.Equal(TiffBitsPerPixel.Bit4, tiffMetaDataEncodedRootFrame.BitsPerPixel); + Assert.Equal(TiffCompression.Lzw, tiffMetaDataEncodedRootFrame.Compression); + + Assert.Equal(inputMetaData.HorizontalResolution, encodedImageMetaData.HorizontalResolution); + Assert.Equal(inputMetaData.VerticalResolution, encodedImageMetaData.VerticalResolution); + Assert.Equal(inputMetaData.ResolutionUnits, encodedImageMetaData.ResolutionUnits); + + Assert.Equal(rootFrameInput.Width, rootFrameEncodedImage.Width); + Assert.Equal(rootFrameInput.Height, rootFrameEncodedImage.Height); + + PixelResolutionUnit resolutionUnitInput = UnitConverter.ExifProfileToResolutionUnit(exifProfileInput); + PixelResolutionUnit resolutionUnitEncoded = UnitConverter.ExifProfileToResolutionUnit(encodedImageExifProfile); + Assert.Equal(resolutionUnitInput, resolutionUnitEncoded); + Assert.Equal(exifProfileInput.GetValue(ExifTag.XResolution).Value.ToDouble(), encodedImageExifProfile.GetValue(ExifTag.XResolution).Value.ToDouble()); + Assert.Equal(exifProfileInput.GetValue(ExifTag.YResolution).Value.ToDouble(), encodedImageExifProfile.GetValue(ExifTag.YResolution).Value.ToDouble()); + + Assert.NotNull(xmpProfileInput); + Assert.NotNull(encodedImageXmpProfile); + Assert.Equal(xmpProfileInput.Data, encodedImageXmpProfile.Data); + + Assert.NotNull(iptcProfileInput); + Assert.NotNull(encodedImageIptcProfile); + Assert.Equal(iptcProfileInput.Data, encodedImageIptcProfile.Data); + Assert.Equal(iptcProfileInput.GetValues(IptcTag.Name)[0].Value, encodedImageIptcProfile.GetValues(IptcTag.Name)[0].Value); + + Assert.NotNull(iccProfileInput); + Assert.NotNull(encodedImageIccProfile); + Assert.Equal(iccProfileInput.Entries.Length, encodedImageIccProfile.Entries.Length); + Assert.Equal(iccProfileInput.Header.Class, encodedImageIccProfile.Header.Class); + + Assert.Equal(exifProfileInput.GetValue(ExifTag.Software).Value, encodedImageExifProfile.GetValue(ExifTag.Software).Value); + Assert.Equal(exifProfileInput.GetValue(ExifTag.ImageDescription).Value, encodedImageExifProfile.GetValue(ExifTag.ImageDescription).Value); + Assert.Equal(exifProfileInput.GetValue(ExifTag.Make).Value, encodedImageExifProfile.GetValue(ExifTag.Make).Value); + Assert.Equal(exifProfileInput.GetValue(ExifTag.Copyright).Value, encodedImageExifProfile.GetValue(ExifTag.Copyright).Value); + Assert.Equal(exifProfileInput.GetValue(ExifTag.Artist).Value, encodedImageExifProfile.GetValue(ExifTag.Artist).Value); + Assert.Equal(exifProfileInput.GetValue(ExifTag.Orientation).Value, encodedImageExifProfile.GetValue(ExifTag.Orientation).Value); + Assert.Equal(exifProfileInput.GetValue(ExifTag.Model).Value, encodedImageExifProfile.GetValue(ExifTag.Model).Value); + + Assert.Equal((ushort)TiffPlanarConfiguration.Chunky, encodedImageExifProfile.GetValue(ExifTag.PlanarConfiguration)?.Value); + // Adding the IPTC and ICC profiles dynamically increments the number of values in the original EXIF profile by 2 + Assert.Equal(exifProfileInput.Values.Count + 2, encodedImageExifProfile.Values.Count); + } } From bf261f0d5efcb732da1a28a1f767d60344040f6c Mon Sep 17 00:00:00 2001 From: Jeffrey Parker Date: Wed, 13 Sep 2023 23:24:14 -0500 Subject: [PATCH 2/2] Stylecop extra blank line issue fixed. Refactored code to extract common logic. --- .../Tiff/TiffEncoderEntriesCollector.cs | 114 ++++++------------ .../Formats/Tiff/TiffMetadataTests.cs | 4 +- 2 files changed, 39 insertions(+), 79 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/TiffEncoderEntriesCollector.cs b/src/ImageSharp/Formats/Tiff/TiffEncoderEntriesCollector.cs index e7ee0c65bc..c8e28111ec 100644 --- a/src/ImageSharp/Formats/Tiff/TiffEncoderEntriesCollector.cs +++ b/src/ImageSharp/Formats/Tiff/TiffEncoderEntriesCollector.cs @@ -6,8 +6,9 @@ using SixLabors.ImageSharp.Formats.Tiff.Constants; using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.Metadata.Profiles.Exif; +using SixLabors.ImageSharp.Metadata.Profiles.Icc; +using SixLabors.ImageSharp.Metadata.Profiles.Iptc; using SixLabors.ImageSharp.Metadata.Profiles.Xmp; -using SixLabors.ImageSharp.PixelFormats; namespace SixLabors.ImageSharp.Formats.Tiff; @@ -76,7 +77,6 @@ public void Process(Image image, bool skipMetadata) } } - public void Process(ImageFrame frame, bool skipMetadata) { this.ProcessProfiles(frame.Metadata, skipMetadata); @@ -171,79 +171,30 @@ private void ProcessMetadata(ExifProfile exifProfile) private void ProcessProfiles(ImageMetadata imageMetadata, bool skipMetadata) { - if (!skipMetadata && (imageMetadata.ExifProfile != null && imageMetadata.ExifProfile.Parts != ExifParts.None)) - { - foreach (IExifValue entry in imageMetadata.ExifProfile.Values) - { - if (!this.Collector.Entries.Exists(t => t.Tag == entry.Tag) && entry.GetValue() != null) - { - ExifParts entryPart = ExifTags.GetPart(entry.Tag); - if (entryPart != ExifParts.None && imageMetadata.ExifProfile.Parts.HasFlag(entryPart)) - { - this.Collector.AddOrReplace(entry.DeepClone()); - } - } - } - } - else - { - imageMetadata.ExifProfile?.RemoveValue(ExifTag.SubIFDOffset); - } - - if (!skipMetadata && imageMetadata.IptcProfile != null) - { - imageMetadata.IptcProfile.UpdateData(); - ExifByteArray iptc = new(ExifTagValue.IPTC, ExifDataType.Byte) - { - Value = imageMetadata.IptcProfile.Data - }; - - this.Collector.AddOrReplace(iptc); - } - else - { - imageMetadata.ExifProfile?.RemoveValue(ExifTag.IPTC); - } - - if (imageMetadata.IccProfile != null) - { - ExifByteArray icc = new(ExifTagValue.IccProfile, ExifDataType.Undefined) - { - Value = imageMetadata.IccProfile.ToByteArray() - }; - - this.Collector.AddOrReplace(icc); - } - else - { - imageMetadata.ExifProfile?.RemoveValue(ExifTag.IccProfile); - } - - if (!skipMetadata && imageMetadata.XmpProfile != null) - { - ExifByteArray xmp = new(ExifTagValue.XMP, ExifDataType.Byte) - { - Value = imageMetadata.XmpProfile.Data - }; - - this.Collector.AddOrReplace(xmp); - } - else - { - imageMetadata.ExifProfile?.RemoveValue(ExifTag.XMP); - } + this.ProcessExifProfile(skipMetadata, imageMetadata.ExifProfile); + this.ProcessIptcProfile(skipMetadata, imageMetadata.IptcProfile, imageMetadata.ExifProfile); + this.ProcessIccProfile(imageMetadata.IccProfile, imageMetadata.ExifProfile); + this.ProcessXmpProfile(skipMetadata, imageMetadata.XmpProfile, imageMetadata.ExifProfile); } private void ProcessProfiles(ImageFrameMetadata frameMetadata, bool skipMetadata) { - if (!skipMetadata && (frameMetadata.ExifProfile != null && frameMetadata.ExifProfile.Parts != ExifParts.None)) + this.ProcessExifProfile(skipMetadata, frameMetadata.ExifProfile); + this.ProcessIptcProfile(skipMetadata, frameMetadata.IptcProfile, frameMetadata.ExifProfile); + this.ProcessIccProfile(frameMetadata.IccProfile, frameMetadata.ExifProfile); + this.ProcessXmpProfile(skipMetadata, frameMetadata.XmpProfile, frameMetadata.ExifProfile); + } + + private void ProcessExifProfile(bool skipMetadata, ExifProfile exifProfile) + { + if (!skipMetadata && (exifProfile != null && exifProfile.Parts != ExifParts.None)) { - foreach (IExifValue entry in frameMetadata.ExifProfile.Values) + foreach (IExifValue entry in exifProfile.Values) { if (!this.Collector.Entries.Exists(t => t.Tag == entry.Tag) && entry.GetValue() != null) { ExifParts entryPart = ExifTags.GetPart(entry.Tag); - if (entryPart != ExifParts.None && frameMetadata.ExifProfile.Parts.HasFlag(entryPart)) + if (entryPart != ExifParts.None && exifProfile.Parts.HasFlag(entryPart)) { this.Collector.AddOrReplace(entry.DeepClone()); } @@ -252,50 +203,59 @@ private void ProcessProfiles(ImageFrameMetadata frameMetadata, bool skipMetadata } else { - frameMetadata.ExifProfile?.RemoveValue(ExifTag.SubIFDOffset); + exifProfile?.RemoveValue(ExifTag.SubIFDOffset); } + } - if (!skipMetadata && frameMetadata.IptcProfile != null) + private void ProcessIptcProfile(bool skipMetadata, IptcProfile iptcProfile, ExifProfile exifProfile) + { + if (!skipMetadata && iptcProfile != null) { - frameMetadata.IptcProfile.UpdateData(); + iptcProfile.UpdateData(); ExifByteArray iptc = new(ExifTagValue.IPTC, ExifDataType.Byte) { - Value = frameMetadata.IptcProfile.Data + Value = iptcProfile.Data }; this.Collector.AddOrReplace(iptc); } else { - frameMetadata.ExifProfile?.RemoveValue(ExifTag.IPTC); + exifProfile?.RemoveValue(ExifTag.IPTC); } + } - if (frameMetadata.IccProfile != null) + private void ProcessIccProfile(IccProfile iccProfile, ExifProfile exifProfile) + { + if (iccProfile != null) { ExifByteArray icc = new(ExifTagValue.IccProfile, ExifDataType.Undefined) { - Value = frameMetadata.IccProfile.ToByteArray() + Value = iccProfile.ToByteArray() }; this.Collector.AddOrReplace(icc); } else { - frameMetadata.ExifProfile?.RemoveValue(ExifTag.IccProfile); + exifProfile?.RemoveValue(ExifTag.IccProfile); } + } - if (!skipMetadata && frameMetadata.XmpProfile != null) + private void ProcessXmpProfile(bool skipMetadata, XmpProfile xmpProfile, ExifProfile exifProfile) + { + if (!skipMetadata && xmpProfile != null) { ExifByteArray xmp = new(ExifTagValue.XMP, ExifDataType.Byte) { - Value = frameMetadata.XmpProfile.Data + Value = xmpProfile.Data }; this.Collector.AddOrReplace(xmp); } else { - frameMetadata.ExifProfile?.RemoveValue(ExifTag.XMP); + exifProfile?.RemoveValue(ExifTag.XMP); } } } diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs index 1225bbc8cb..e31487cd23 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs @@ -336,8 +336,7 @@ public void Encode_PreservesMetadata_IptcAndIcc(TestImageProvider(TestImageProvider