From f057b5f47d7c146a1d0a755ddde86ca35743ef9d Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Tue, 13 Jan 2026 15:36:04 +0100 Subject: [PATCH] Reduce tag_filter to a minimum, implement copy --- src/encoder/mod.rs | 218 ++++++++++++++++++++++++++++++++++---- src/encoder/tag_filter.rs | 49 ++------- 2 files changed, 204 insertions(+), 63 deletions(-) diff --git a/src/encoder/mod.rs b/src/encoder/mod.rs index f62c2b2..9f4ec28 100644 --- a/src/encoder/mod.rs +++ b/src/encoder/mod.rs @@ -2,27 +2,27 @@ pub use tiff_value::*; use std::{ cmp, - io::{self, Seek, Write}, + io::{self, Read, Seek, Write}, marker::PhantomData, mem, num::{NonZeroU64, TryFromIntError}, }; use crate::{ - decoder::{ifd::Entry, IfdDecoder}, + decoder::{ifd::Entry, Decoder}, error::{TiffResult, UsageError}, tags::{ ByteOrder, CompressionMethod, ExtraSamples, IfdPointer, PhotometricInterpretation, - ResolutionUnit, SampleFormat, Tag, Type, ValueBuffer, + PlanarConfiguration, ResolutionUnit, SampleFormat, Tag, Type, ValueBuffer, }, Directory, TiffError, TiffFormatError, }; pub mod colortype; pub mod compression; +mod tag_filter; mod tiff_value; mod writer; -mod tag_filter; use self::colortype::*; use self::compression::Compression as Comp; @@ -438,16 +438,142 @@ impl<'a, W: 'a + Write + Seek, K: TiffKind> DirectoryEncoder<'a, W, K> { } /// Copy metadata from the IFD that a decoder refers to. - pub fn write_metadata_from( + /// + /// You generally want to use [`ImageEncoder::write_metadata_copy_from`] which automatically + /// fills out the [`EncodeMetadataApplicability`] as applicable to the chosen color space and + /// other sample configuration. + pub fn write_metadata_copy_from( &mut self, - mut decoder: IfdDecoder<'_>, + decoder: &mut Decoder, options: EncodeMetadataOptions<'_>, + kind: EncodeMetadataApplicability, ) -> TiffResult { let mut report = EncodeMetadataReport { + copied: 0, + filtered: 0, }; - for tag in decoder.tag_iter() { - let (tag, entry) = tag?; + enum Role { + Ifd, + ExifPrivate, + GpsPrivate, + } + + let ptr = decoder.ifd_pointer().unwrap(); + let mut stack = vec![(ptr, Role::Ifd, usize::MAX)]; + let mut buf = ValueBuffer::empty(Type::UNDEFINED); + + // Directories which we will later write in reverse-pre-order. + let mut pre_order_dirs: Vec<(Directory, usize, Tag)> = vec![]; + + while let Some((ifd, role, parent)) = stack.pop() { + let dir = decoder.read_directory(ifd)?; + let mut created_dir = Directory::empty(); + + let filter = match role { + Role::Ifd => TagFilter::filter_primary, + Role::ExifPrivate => TagFilter::filter_exif_private, + Role::GpsPrivate => TagFilter::filter_gps_private, + }; + + let mut ifd_reader = decoder.read_directory_tags(&dir); + + for (tag, entry) in dir.iter() { + let level = filter(options.tag_filter, tag.to_u16()); + match options + .tag_filter + .should_keep_for_image_writer(level, &kind) + { + tag_filter::Choice::Ok => {} + tag_filter::Choice::Descend(subifd_kind) => { + // Only single pointers can be directories to descend to, avoid an overly + // large read by pulling that value out of the file. + if entry.count() != 1 { + report.filtered = report.filtered.saturating_add(1); + continue; + } + + if ifd_reader.find_tag_buf(tag, &mut buf)?.is_none() { + // Should not happen (we're iterating the dict already), but be robust. + report.filtered = report.filtered.saturating_add(1); + continue; + }; + + let Some(ifd) = buf.as_ifd_pointer() else { + report.filtered = report.filtered.saturating_add(1); + continue; + }; + + let role = match subifd_kind { + tag_filter::SubifdKind::ExifPrivate => Role::ExifPrivate, + tag_filter::SubifdKind::Gps => Role::GpsPrivate, + tag_filter::SubifdKind::Interoperability => { + // Only applicable to compressed images. + report.filtered = report.filtered.saturating_add(1); + continue; + } + }; + + let this_as_parent = pre_order_dirs.len(); + stack.push((ifd, role, this_as_parent)); + } + tag_filter::Choice::Discard => continue, + tag_filter::Choice::Filtered + // Not supported yet. + | tag_filter::Choice::Special(tag_filter::Special::JpegThumbnail) => { + // Not to be copied. + report.filtered = report.filtered.saturating_add(1); + continue; + } + tag_filter::Choice::Special(tag_filter::Special::CompressedExclusive) => { + // We're not writing an App segment for a compressed file, are we? + continue; + } + } + + if ifd_reader.find_tag_buf(tag, &mut buf)?.is_none() { + report.filtered = report.filtered.saturating_add(1); + continue; + } + + // Re-encode the value to the target file. + buf.set_byte_order(ByteOrder::native()); + let entry = self.write_entry_buf(&buf)?; + created_dir.extend([(tag, entry)]); + report.copied = report.copied.saturating_add(1); + } + + let tag_in_parent = match role { + // should be unreachable? For now. But thumbnails would use this so we best be a + // little cautious here. Also thumbnails are weird, they do have image data but we + // would not copy that data itself with these methods. Just eh. + // + // Anyways the tree root will use this tag as a dummy. + Role::Ifd => Tag::SubIfd, + Role::ExifPrivate => Tag::ExifDirectory, + Role::GpsPrivate => Tag::GpsDirectory, + }; + + pre_order_dirs.push((created_dir, parent, tag_in_parent)); + } + + // Now post-order traverse the directory tree. + while let Some((directory, parent, tag)) = pre_order_dirs.pop() { + // The root is the items for the IFD itself. + if pre_order_dirs.is_empty() { + // We do not enter ourselves into a parent directory. + let _ = (parent, tag); + + self.extend_from(&directory); + break; + } + + let offset = Self::write_directory_inner(self.writer, &directory)?; + let offset = K::convert_offset(offset)?; + let entry = self.write_entry(offset)?; + + let (parent_dir, _, _) = &mut pre_order_dirs[parent]; + parent_dir.extend([(tag, entry)]); } Ok(report) @@ -479,17 +605,21 @@ impl<'a, W: 'a + Write + Seek, K: TiffKind> DirectoryEncoder<'a, W, K> { } fn write_directory(&mut self) -> TiffResult { + Self::write_directory_inner(self.writer, &self.directory) + } + + fn write_directory_inner(writer: &mut TiffWriter, directory: &Directory) -> TiffResult { // Start by turning all buffered unwritten values into entries. - let offset = self.writer.offset(); - K::write_entry_count(self.writer, self.directory.len())?; + let offset = writer.offset(); + K::write_entry_count(writer, directory.len())?; let offset_bytes = mem::size_of::(); - for (tag, entry) in self.directory.iter() { - self.writer.write_u16(tag.to_u16())?; - self.writer.write_u16(entry.field_type().to_u16())?; + for (tag, entry) in directory.iter() { + writer.write_u16(tag.to_u16())?; + writer.write_u16(entry.field_type().to_u16())?; let count = K::convert_offset(entry.count())?; - count.write(self.writer)?; - self.writer.write_bytes(&entry.offset()[..offset_bytes])?; + count.write(writer)?; + writer.write_bytes(&entry.offset()[..offset_bytes])?; } Ok(offset) @@ -617,16 +747,46 @@ pub struct EncodeMetadataOptions<'lt> { /// Setting this flag will skip such unknown tags instead of attempting a potentially incorrect /// copy. However the set of copied metadata is then dependent on the `tiff` library. pub tag_filter: &'lt TagFilter, - /// Some tags are not allowed to be recorded or can be semantically validated. The library can - /// skip these tags while copying metadata. For instance, `YCbCrCoefficients` _may_ be - /// recorded for images in `YCbCr` photometric interpretation but _must not_ be recorded for - /// images in any other interpretation (colorspace conversion should have been done during the - /// transformation). - pub skip_invalid_tags: bool, + // TODO: + // Some tags are not allowed to be recorded or can be semantically validated. The library can + // skip these tags while copying metadata. For instance, `YCbCrCoefficients` _may_ be + // recorded for images in `YCbCr` photometric interpretation but _must not_ be recorded for + // images in any other interpretation (colorspace conversion should have been done during the + // transformation). + // pub validate_tags: enum ValidateTags { + // None, + // Skip, + // Error, + // Strict /* Error but also do not allow anything that can not be validated */ + // }, +} + +impl Default for EncodeMetadataOptions<'_> { + fn default() -> Self { + EncodeMetadataOptions { + tag_filter: TagFilter::only_known_tags(), + } + } +} + +/// Describe the kind of image for which metadata is recorded. +/// +/// Metadata that is not applicable to the color space or the layout may be skipped. For instance, +/// subsampling basis position is only relevant on YCbCr / Chroma color photometric interpretation +/// where subsampling is defined. +#[non_exhaustive] +#[derive(Clone, Copy, Default)] +pub struct EncodeMetadataApplicability { + pub photometric: Option, + pub planar: Option, } #[non_exhaustive] pub struct EncodeMetadataReport { + /// Number of tags transferred (recursively). + pub copied: u64, + /// Number of tags encountered that were skipped due to the provided filter. + pub filtered: u64, } /// Type to encode images strip by strip. @@ -869,6 +1029,22 @@ impl<'a, W: 'a + Write + Seek, T: ColorType, K: TiffKind> ImageEncoder<'a, W, T, Ok(()) } + /// Copy metadata from the IFD that a decoder refers to. + pub fn write_metadata_copy_from( + &mut self, + decoder: &mut Decoder, + options: EncodeMetadataOptions<'_>, + ) -> TiffResult { + self.encoder.write_metadata_copy_from( + decoder, + options, + EncodeMetadataApplicability { + photometric: Some(::TIFF_VALUE), + planar: Some(PlanarConfiguration::Chunky), + }, + ) + } + /// Set image resolution pub fn resolution(&mut self, unit: ResolutionUnit, value: Rational) { self.encoder diff --git a/src/encoder/tag_filter.rs b/src/encoder/tag_filter.rs index 23cec3f..5b7ffef 100644 --- a/src/encoder/tag_filter.rs +++ b/src/encoder/tag_filter.rs @@ -24,18 +24,6 @@ impl TagFilter { &INSTANCE } - /// Return a filter that attempts to remove identifying tags. This is stricter than - /// [`Self::only_known_tags`] and excludes many tags except those describing the image data - /// layout. - /// - /// **Note that it comes with *no warranty*.** although this will remove tags with prejudice. - pub fn attempt_at_privacy() -> &'static TagFilter { - static INSTANCE: TagFilter = TagFilter { - filter_level: FilterLevel::Privacy, - }; - &INSTANCE - } - /// FIXME: take a configuration by which to filter out complete metadata kinds. /// FIXME: when we iterate over a whole directory of tags, this iteration is more expensive /// than necessary. Tags within a directory should be ordered. We can thus perform a binary set @@ -168,32 +156,15 @@ impl TagFilter { } } - fn filter_interoperability(&self, tag: u16) -> Level { - match tag { - 0x0001 => Level::Special(Special::CompressedExclusive), - _ => Level::Unknown, - } - } - - pub(crate) fn filter_secondary_exif(&self, tag: u16) -> Level { - // No difference for now. The only tabular difference I could spot is the treatment of - // sample positioning which is mandatory in the primary IFD of YCbCr but optional for all - // following IFDs.. - self.filter_primary_exif(tag) - } - - pub(crate) fn filter_secondary_gps(&self, tag: u16) -> Level { - self.filter_primary_gps(tag) - } - - pub(crate) fn should_keep_on_level( + pub(crate) fn should_keep_for_image_writer( &self, level: Level, target: &super::EncodeMetadataApplicability, ) -> Choice { match level { - Level::Unknown if matches!(self.filter_level, FilterLevel::All) => Choice::Ok, - Level::SampleLayout => Choice::Ok, + Level::Unknown if matches!(self.filter_level, FilterLevel::All) => Choice::Filtered, + // Sample Layout is controlled by the writer (dimensions, color type, …). Do not copy! + Level::SampleLayout => Choice::Discard, Level::Ifd(subifd_kind) => Choice::Descend(subifd_kind), Level::Photometric(photometric) => { match (photometric, target.planar, target.photometric) { @@ -213,15 +184,7 @@ impl TagFilter { _ => Choice::Discard, } } - Level::Value if matches!(self.filter_level, FilterLevel::Privacy) => Choice::Discard, Level::Value => Choice::Ok, - Level::Special(Special::JpegThumbnail) - if matches!(self.filter_level, FilterLevel::Privacy) => - { - // Contains arbitrary unverified other data, so, no, we do not heed backward - // compatibility of a tag that should no longer be in active use if possible. - Choice::Discard - } Level::Special(special) => Choice::Special(special), _ => Choice::Discard, } @@ -231,7 +194,10 @@ impl TagFilter { pub(crate) enum Choice { Ok, Descend(SubifdKind), + /// Tag is inapplicable to be copied. Discard, + /// Tag was chosen not to be copied. + Filtered, Special(Special), } @@ -239,7 +205,6 @@ pub(crate) enum Choice { enum FilterLevel { All, Known, - Privacy, } /// The level of support for a value.