From 54f733eaa36235225e212334039348790cd96bd0 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Sat, 12 Jul 2025 18:31:08 +0200 Subject: [PATCH 1/3] Allow non-chained directories in writer This is just the implementation details. In the DirectoryEncoder we did not differentiate between different directories that should go into the main image chain and other directories. This is now possible by storing the prior offset as an explicit Option. Additionally, the whole encoder did not allow any intermediate data between its last directory and new directories on the chain. The offset of the relevant `next` field was not stored explicitly but derived from the current stream position at the moment of starting the directory. This would not allow any padding data or data written outside the returned `DirectoryEncoder` object. This relaxed both requirements by turning the offsets into attributes. --- src/encoder/mod.rs | 77 ++++++++++++++++++++++++++++++------------- src/encoder/writer.rs | 4 +++ 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/src/encoder/mod.rs b/src/encoder/mod.rs index 47c62e6..61ace70 100644 --- a/src/encoder/mod.rs +++ b/src/encoder/mod.rs @@ -6,7 +6,7 @@ use std::{ io::{self, Seek, Write}, marker::PhantomData, mem, - num::TryFromIntError, + num::{NonZeroU64, TryFromIntError}, }; use crate::{ @@ -107,6 +107,8 @@ pub struct TiffEncoder { kind: PhantomData, predictor: Predictor, compression: Compression, + /// The offset of the last main image directory's `next` field. + last_ifd_chain: NonZeroU64, } /// Constructor functions to create standard Tiff files. @@ -135,16 +137,19 @@ impl TiffEncoder { impl TiffEncoder { /// Creates a new Tiff or BigTiff encoder, inferred from the return type. pub fn new_generic(writer: W) -> TiffResult { - let mut encoder = TiffEncoder { - writer: TiffWriter::new(writer), + let mut writer = TiffWriter::new(writer); + K::write_header(&mut writer)?; + + let last_ifd_chain = NonZeroU64::new(writer.previous_ifd_pointer::()) + .expect("Header is at a non-zero offset"); + + Ok(TiffEncoder { + writer, kind: PhantomData, predictor: Predictor::None, compression: Compression::Uncompressed, - }; - - K::write_header(&mut encoder.writer)?; - - Ok(encoder) + last_ifd_chain, + }) } /// Set the predictor to use @@ -165,8 +170,8 @@ impl TiffEncoder { } /// Create a [`DirectoryEncoder`] to encode an ifd directory. - pub fn new_directory(&mut self) -> TiffResult> { - DirectoryEncoder::new(&mut self.writer) + pub fn new_directory(&mut self) -> TiffResult> { + Self::chain_directory(&mut self.writer, &mut self.last_ifd_chain) } /// Create an [`ImageEncoder`] to encode an image one slice at a time. @@ -175,7 +180,7 @@ impl TiffEncoder { width: u32, height: u32, ) -> TiffResult> { - let encoder = DirectoryEncoder::new(&mut self.writer)?; + let encoder = Self::chain_directory(&mut self.writer, &mut self.last_ifd_chain)?; ImageEncoder::new(encoder, width, height, self.compression, self.predictor) } @@ -189,11 +194,19 @@ impl TiffEncoder { where [C::Inner]: TiffValue, { - let encoder = DirectoryEncoder::new(&mut self.writer)?; + let encoder = Self::chain_directory(&mut self.writer, &mut self.last_ifd_chain)?; let image: ImageEncoder = ImageEncoder::new(encoder, width, height, self.compression, self.predictor)?; image.write_data(data) } + + fn chain_directory<'lt>( + writer: &'lt mut TiffWriter, + last_ifd_chain: &'lt mut NonZeroU64, + ) -> TiffResult> { + let last_ifd = *last_ifd_chain; + DirectoryEncoder::new(writer, Some(last_ifd), Some(last_ifd_chain)) + } } /// Low level interface to encode ifd directories. @@ -202,22 +215,30 @@ impl TiffEncoder { /// Encoding can silently fail while this is dropping. pub struct DirectoryEncoder<'a, W: 'a + Write + Seek, K: TiffKind> { writer: &'a mut TiffWriter, - dropped: bool, + /// The position of the previous directory's `next` field, if any. + chained_ifd_pos: Option, + /// An output to write the `next` field offset on completion. + write_chain: Option<&'a mut NonZeroU64>, // We use BTreeMap to make sure tags are written in correct order - ifd_pointer_pos: u64, ifd: BTreeMap>, + dropped: bool, } impl<'a, W: 'a + Write + Seek, K: TiffKind> DirectoryEncoder<'a, W, K> { - fn new(writer: &'a mut TiffWriter) -> TiffResult { - // the previous word is the IFD offset position - let ifd_pointer_pos = writer.offset() - mem::size_of::() as u64; + /// Construct a directory writer appending to data, assuming the writer is currently positioned + /// immediately after a previously written IFD to which to append. + fn new( + writer: &'a mut TiffWriter, + chained_ifd_pos: Option, + chain_into: Option<&'a mut NonZeroU64>, + ) -> TiffResult { writer.pad_word_boundary()?; // TODO: Do we need to adjust this for BigTiff? Ok(DirectoryEncoder { writer, - dropped: false, - ifd_pointer_pos, + chained_ifd_pos, + write_chain: chain_into, ifd: BTreeMap::new(), + dropped: false, }) } @@ -300,13 +321,23 @@ impl<'a, W: 'a + Write + Seek, K: TiffKind> DirectoryEncoder<'a, W, K> { fn finish_internal(&mut self) -> TiffResult<()> { let ifd_pointer = self.write_directory()?; - let curr_pos = self.writer.offset(); - self.writer.goto_offset(self.ifd_pointer_pos)?; - K::write_offset(self.writer, ifd_pointer)?; - self.writer.goto_offset(curr_pos)?; + if let Some(prior) = self.chained_ifd_pos { + let curr_pos = self.writer.offset(); + + self.writer.goto_offset(prior.get())?; + K::write_offset(self.writer, ifd_pointer)?; + + self.writer.goto_offset(curr_pos)?; + } + K::write_offset(self.writer, 0)?; + if let Some(prior) = self.write_chain.take() { + *prior = NonZeroU64::new(self.writer.previous_ifd_pointer::()) + .expect("IFD chain field is at a non-zero offset"); + } + self.dropped = true; Ok(()) diff --git a/src/encoder/writer.rs b/src/encoder/writer.rs index 46ead93..f45038b 100644 --- a/src/encoder/writer.rs +++ b/src/encoder/writer.rs @@ -69,6 +69,10 @@ impl TiffWriter { self.offset } + pub(crate) fn previous_ifd_pointer(&self) -> u64 { + self.offset() - core::mem::size_of::() as u64 + } + pub fn last_written(&self) -> u64 { self.byte_count } From 525bb2b9b11605d81b636b1730d9bdc50c623bc9 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Sun, 13 Jul 2025 01:11:36 +0200 Subject: [PATCH 2/3] Simplify encoder writing of dictionary --- src/decoder/ifd.rs | 4 ++ src/encoder/mod.rs | 98 +++++++++++++++++++++++++++++----------------- 2 files changed, 65 insertions(+), 37 deletions(-) diff --git a/src/decoder/ifd.rs b/src/decoder/ifd.rs index e0aa7e2..ce9c8e1 100644 --- a/src/decoder/ifd.rs +++ b/src/decoder/ifd.rs @@ -371,6 +371,10 @@ impl Entry { self.count } + pub(crate) fn offset(&self) -> &[u8] { + &self.offset + } + /// Returns a mem_reader for the offset/value field fn r(&self, byte_order: ByteOrder) -> EndianReader>> { EndianReader::new(io::Cursor::new(self.offset.to_vec()), byte_order) diff --git a/src/encoder/mod.rs b/src/encoder/mod.rs index 61ace70..735b0c9 100644 --- a/src/encoder/mod.rs +++ b/src/encoder/mod.rs @@ -10,9 +10,10 @@ use std::{ }; use crate::{ + decoder::ifd::Entry, error::{TiffResult, UsageError}, - tags::{CompressionMethod, ResolutionUnit, SampleFormat, Tag}, - TiffError, TiffFormatError, + tags::{CompressionMethod, ResolutionUnit, SampleFormat, Tag, Type}, + Directory, TiffError, TiffFormatError, }; pub mod colortype; @@ -220,6 +221,7 @@ pub struct DirectoryEncoder<'a, W: 'a + Write + Seek, K: TiffKind> { /// An output to write the `next` field offset on completion. write_chain: Option<&'a mut NonZeroU64>, // We use BTreeMap to make sure tags are written in correct order + directory: Directory, ifd: BTreeMap>, dropped: bool, } @@ -237,6 +239,7 @@ impl<'a, W: 'a + Write + Seek, K: TiffKind> DirectoryEncoder<'a, W, K> { writer, chained_ifd_pos, write_chain: chain_into, + directory: Directory::empty(), ifd: BTreeMap::new(), dropped: false, }) @@ -253,7 +256,7 @@ impl<'a, W: 'a + Write + Seek, K: TiffKind> DirectoryEncoder<'a, W, K> { self.ifd.insert( tag.to_u16(), DirectoryEntry { - data_type: ::FIELD_TYPE.to_u16(), + data_type: ::FIELD_TYPE, count: value.count().try_into()?, data: bytes, }, @@ -263,48 +266,69 @@ impl<'a, W: 'a + Write + Seek, K: TiffKind> DirectoryEncoder<'a, W, K> { } fn write_directory(&mut self) -> TiffResult { - // Start by writing out all values - for &mut DirectoryEntry { - data: ref mut bytes, - .. - } in self.ifd.values_mut() - { - let data_bytes = mem::size_of::(); - - if bytes.len() > data_bytes { - let offset = self.writer.offset(); - self.writer.write_bytes(bytes)?; - *bytes = vec![0; data_bytes]; - let mut writer = TiffWriter::new(bytes as &mut [u8]); - K::write_offset(&mut writer, offset)?; - } else { - while bytes.len() < data_bytes { - bytes.push(0); - } - } + // Start by turning all buffered unwritten values into entries. + for (&tag, value) in &self.ifd { + let entry = Self::write_value(&mut self.writer, value)?; + self.directory.extend([(Tag::Unknown(tag), entry)]); } let offset = self.writer.offset(); + K::write_entry_count(self.writer, self.directory.len())?; - K::write_entry_count(self.writer, self.ifd.len())?; - for ( - tag, - DirectoryEntry { - data_type: field_type, - count, - data: offset, - }, - ) in self.ifd.iter() - { - self.writer.write_u16(*tag)?; - self.writer.write_u16(*field_type)?; - (*count).write(self.writer)?; - self.writer.write_bytes(offset)?; + 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())?; + let count = K::convert_offset(entry.count())?; + count.write(self.writer)?; + self.writer.write_bytes(&entry.offset()[..offset_bytes])?; } Ok(offset) } + fn write_value( + writer: &mut TiffWriter, + value: &DirectoryEntry, + ) -> TiffResult { + let &DirectoryEntry { + data: ref bytes, + ref count, + data_type, + } = value; + + let in_entry_bytes = mem::size_of::(); + let mut offset_bytes = [0; 8]; + + if bytes.len() > in_entry_bytes { + let offset = writer.offset(); + writer.write_bytes(&bytes)?; + + let offset = K::convert_offset(offset)?; + offset_bytes[..offset.bytes()].copy_from_slice(&offset.data()); + } else { + // Note: we have indicated our native byte order in the header, hence this + // corresponds to our byte order no matter the value type. + offset_bytes[..bytes.len()].copy_from_slice(&bytes); + } + + // Undoing some hidden type. Offset is either u32 or u64. Due to the trait API being public + // and some oversight, we can not clone the `count: K::OffsetType` and thus not convert it. + // Instead, write it to a buffer... + let mut count_bytes = [0; 8]; + // Nominally Cow but we only expect `Cow::Borrowed`. + count_bytes[..count.bytes()].copy_from_slice(&count.data()); + + Ok(if in_entry_bytes == 4 { + let count = u32::from_ne_bytes(count_bytes[..4].try_into().unwrap()); + Entry::new(data_type, count, offset_bytes[..4].try_into().unwrap()) + } else { + debug_assert_eq!(in_entry_bytes, 8); + let count = u64::from_ne_bytes(count_bytes); + Entry::new_u64(data_type, count, offset_bytes) + }) + } + /// Write some data to the tiff file, the offset of the data is returned. /// /// This could be used to write tiff strips. @@ -653,7 +677,7 @@ impl<'a, W: Write + Seek, C: ColorType, K: TiffKind> Drop for ImageEncoder<'a, W } struct DirectoryEntry { - data_type: u16, + data_type: Type, count: S, data: Vec, } From f6e2aeffe57a3ad7592e3eeaf86d890c5229a4c1 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Sun, 13 Jul 2025 03:06:40 +0200 Subject: [PATCH 3/3] Encode values immediately This currently avoids buffering the collection, but not buffering them individually where we still format it into a byte vector. The main drive here is making the DirectoryEncoder less dependent on the values. That is, it should be concerned with the Entry representation of values only. A few decoders expect all values to strictly follow the image directory and the data layout of such files should be controllable by the user. This could be achieved by encoding dummy entries which are later corrected once the actual item is known by seeking back and writing the directory anew. By no longer assuming values to be available on finishing a directory we also prepare an interface that encodes an existing directory. --- src/encoder/mod.rs | 24 +++++++++++------------- tests/encode_images.rs | 4 ++-- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/encoder/mod.rs b/src/encoder/mod.rs index 735b0c9..a6661c3 100644 --- a/src/encoder/mod.rs +++ b/src/encoder/mod.rs @@ -2,7 +2,6 @@ pub use tiff_value::*; use std::{ cmp, - collections::BTreeMap, io::{self, Seek, Write}, marker::PhantomData, mem, @@ -220,9 +219,9 @@ pub struct DirectoryEncoder<'a, W: 'a + Write + Seek, K: TiffKind> { chained_ifd_pos: Option, /// An output to write the `next` field offset on completion. write_chain: Option<&'a mut NonZeroU64>, + kind: PhantomData, // We use BTreeMap to make sure tags are written in correct order directory: Directory, - ifd: BTreeMap>, dropped: bool, } @@ -235,43 +234,42 @@ impl<'a, W: 'a + Write + Seek, K: TiffKind> DirectoryEncoder<'a, W, K> { chain_into: Option<&'a mut NonZeroU64>, ) -> TiffResult { writer.pad_word_boundary()?; // TODO: Do we need to adjust this for BigTiff? - Ok(DirectoryEncoder { + Ok(Self { writer, chained_ifd_pos, write_chain: chain_into, + kind: PhantomData, directory: Directory::empty(), - ifd: BTreeMap::new(), dropped: false, }) } /// Write a single ifd tag. pub fn write_tag(&mut self, tag: Tag, value: T) -> TiffResult<()> { + // Encodes the value if necessary. In the current bytes all integers are taken as native + // endian and thus transparent but this keeps the interface generic. let mut bytes = Vec::with_capacity(value.bytes()); { let mut writer = TiffWriter::new(&mut bytes); value.write(&mut writer)?; } - self.ifd.insert( - tag.to_u16(), - DirectoryEntry { + let entry = Self::write_value( + &mut self.writer, + &DirectoryEntry { data_type: ::FIELD_TYPE, count: value.count().try_into()?, data: bytes, }, - ); + )?; + + self.directory.extend([(tag, entry)]); Ok(()) } fn write_directory(&mut self) -> TiffResult { // Start by turning all buffered unwritten values into entries. - for (&tag, value) in &self.ifd { - let entry = Self::write_value(&mut self.writer, value)?; - self.directory.extend([(Tag::Unknown(tag), entry)]); - } - let offset = self.writer.offset(); K::write_entry_count(self.writer, self.directory.len())?; diff --git a/tests/encode_images.rs b/tests/encode_images.rs index 2f17d7d..fb71ca5 100644 --- a/tests/encode_images.rs +++ b/tests/encode_images.rs @@ -57,7 +57,7 @@ fn encode_decode() { ), (Tag::Compression, ifd::Value::Short(1)), (Tag::PhotometricInterpretation, ifd::Value::Short(2)), - (Tag::StripOffsets, ifd::Value::Unsigned(8)), + (Tag::StripOffsets, ifd::Value::Unsigned(47)), (Tag::SamplesPerPixel, ifd::Value::Short(3)), (Tag::RowsPerStrip, ifd::Value::Unsigned(3334)), (Tag::StripByteCounts, ifd::Value::Unsigned(30000)), @@ -137,7 +137,7 @@ fn encode_decode_big() { ), (Tag::Compression, ifd::Value::Short(1)), (Tag::PhotometricInterpretation, ifd::Value::Short(2)), - (Tag::StripOffsets, ifd::Value::UnsignedBig(16)), + (Tag::StripOffsets, ifd::Value::UnsignedBig(27)), (Tag::SamplesPerPixel, ifd::Value::Short(3)), (Tag::RowsPerStrip, ifd::Value::Unsigned(3334)), (Tag::StripByteCounts, ifd::Value::UnsignedBig(30000)),