Skip to content

Commit db82209

Browse files
feefladderfeefladder
committed
moved extra_tags to its own file; added first tests
1 parent a7c6a7d commit db82209

File tree

6 files changed

+224
-83
lines changed

6 files changed

+224
-83
lines changed

‎.pre-commit-config.yaml

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# See https://pre-commit.com for more information
2+
# See https://pre-commit.com/hooks.html for more hooks
3+
4+
# Default to Python 3
5+
default_language_version:
6+
python: python3
7+
8+
# Optionally both commit and push
9+
default_stages: [pre-commit]
10+
11+
repos:
12+
- repo: https://github.com/pre-commit/pre-commit-hooks
13+
rev: v5.0.0
14+
hooks:
15+
- id: trailing-whitespace
16+
- id: end-of-file-fixer
17+
- id: check-added-large-files
18+
args: ["--maxkb=500"]
19+
20+
- repo: https://github.com/astral-sh/ruff-pre-commit
21+
rev: v0.11.0
22+
hooks:
23+
- id: ruff
24+
args: ["--fix"]
25+
- id: ruff-format
26+
27+
- repo: local
28+
hooks:
29+
- id: rust-fmt
30+
name: cargo fmt
31+
entry: cargo fmt --all -- --check
32+
language: system
33+
types: [rust]
34+
stages: [pre-push]
35+
36+
- id: rust-clippy
37+
name: cargo clippy
38+
entry: cargo clippy --all --all-features --tests -- -D warnings
39+
language: system
40+
types: [rust]
41+
stages: [pre-push]
42+
pass_filenames: false
43+
44+
- id: rust-test
45+
name: cargo test
46+
entry: cargo test --all --all-features
47+
language: system
48+
types: [rust]
49+
stages: [pre-push]
50+
pass_filenames: false

‎src/ifd.rs

Lines changed: 3 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
1-
use std::collections::{HashMap, HashSet};
1+
use std::collections::HashMap;
22
use std::fmt::Debug;
33
use std::ops::Range;
4-
use std::sync::Arc;
54

65
use bytes::Bytes;
76
use num_enum::TryFromPrimitive;
87

98
use crate::error::{AsyncTiffError, AsyncTiffResult};
109
use crate::geo::{GeoKeyDirectory, GeoKeyTag};
10+
use crate::metadata::ExtraTagsRegistry;
1111
use crate::predictor::PredictorInfo;
1212
use crate::reader::{AsyncFileReader, Endianness};
1313
use crate::tiff::tags::{
@@ -19,82 +19,6 @@ use crate::tile::Tile;
1919

2020
const DOCUMENT_NAME: u16 = 269;
2121

22-
/// Trait to implement for custom tags, such as Geo, EXIF, OME, etc
23-
/// your type should also implement `Clone`
24-
// Send + Sync are required for Python, where `dyn ExtraTags` needs `Send` and `Sync`
25-
pub trait ExtraTags: ExtraTagsCloneArc + std::any::Any + Debug + Send + Sync {
26-
/// a list of tags this entry processes
27-
/// e.g. for Geo this would be [34735, 34736, 34737]
28-
fn tags(&self) -> &'static [Tag];
29-
/// process a single tag
30-
fn process_tag(&mut self, tag: u16, value: Value) -> AsyncTiffResult<()>;
31-
}
32-
33-
// we need to do a little dance to do an object-safe deep clone
34-
// https://stackoverflow.com/a/30353928/14681457
35-
pub trait ExtraTagsCloneArc {
36-
fn clone_arc(&self) -> Arc<dyn ExtraTags>;
37-
}
38-
39-
impl<T> ExtraTagsCloneArc for T
40-
where
41-
T: 'static + ExtraTags + Clone
42-
{
43-
fn clone_arc(&self) -> Arc<dyn ExtraTags> {
44-
Arc::new(self.clone())
45-
}
46-
}
47-
48-
#[derive(Debug, Clone)]
49-
pub struct ExtraTagsRegistry(HashMap<Tag, Arc<dyn ExtraTags>>);
50-
51-
impl ExtraTagsRegistry {
52-
/// Create a new, empty `ExtraTagsRegistry`
53-
pub fn new() -> Self{
54-
Self(HashMap::new())
55-
}
56-
/// Register an ExtraTags so their tags are parsed and stored in the ifd's `extra_tags``
57-
pub fn register(&mut self, tags: Arc<dyn ExtraTags>) -> AsyncTiffResult<()> {
58-
// check for duplicates
59-
for tag in tags.tags() {
60-
if self.0.contains_key(tag) {
61-
return Err(AsyncTiffError::General(format!("Tag {tag:?} already registered in {self:?}!")));
62-
}
63-
}
64-
// add to self
65-
for tag in tags.tags() {
66-
self.0.insert(*tag, tags.clone());
67-
}
68-
Ok(())
69-
}
70-
71-
/// deep clone so we have different registries per IFD
72-
pub(crate) fn deep_clone(&self) -> Self {
73-
let mut new_registry = ExtraTagsRegistry::new();
74-
75-
76-
// we need to do some magic, because we can have multiple tags pointing to the same arc
77-
let mut seen = HashSet::new();
78-
for extra_tags in self.0.values() {
79-
// only add if this is the first encountered reference to this arc
80-
// (using thin pointer equality: https://stackoverflow.com/a/67114787/14681457 ; https://github.com/rust-lang/rust/issues/46139#issuecomment-346971153)
81-
if seen.insert(Arc::as_ptr(extra_tags) as *const ()) {
82-
if let Err(e) = new_registry.register(extra_tags.clone_arc()) {
83-
panic!("{e}");
84-
}
85-
}
86-
}
87-
88-
new_registry
89-
}
90-
}
91-
92-
impl Default for ExtraTagsRegistry {
93-
fn default() -> Self {
94-
Self::new() // add e.g. geo tags later
95-
}
96-
}
97-
9822
/// An ImageFileDirectory representing Image content
9923
// The ordering of these tags matches the sorted order in TIFF spec Appendix A
10024
#[allow(dead_code)]
@@ -229,7 +153,7 @@ impl ImageFileDirectory {
229153
pub fn from_tags(
230154
tag_data: HashMap<Tag, Value>,
231155
endianness: Endianness,
232-
extra_tags_registry: ExtraTagsRegistry
156+
extra_tags_registry: ExtraTagsRegistry,
233157
) -> AsyncTiffResult<Self> {
234158
let mut new_subfile_type = None;
235159
let mut image_width = None;

‎src/metadata/extra_tags.rs

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
use crate::error::{AsyncTiffError, AsyncTiffResult};
2+
use crate::tiff::tags::Tag;
3+
use crate::tiff::Value;
4+
use std::collections::{HashMap, HashSet};
5+
use std::fmt::Debug;
6+
use std::sync::Arc;
7+
8+
/// Trait to implement for custom tags, such as Geo, EXIF, OME, etc
9+
/// your type should also implement `Clone`
10+
// Send + Sync are required for Python, where `dyn ExtraTags` needs `Send` and `Sync`
11+
pub trait ExtraTags: ExtraTagsCloneArc + std::any::Any + Debug + Send + Sync {
12+
/// a list of tags this entry processes
13+
/// e.g. for Geo this would be [34735, 34736, 34737]
14+
fn tags(&self) -> &'static [Tag];
15+
/// process a single tag
16+
fn process_tag(&mut self, tag: u16, value: Value) -> AsyncTiffResult<()>;
17+
}
18+
19+
// we need to do a little dance to do an object-safe deep clone
20+
// https://stackoverflow.com/a/30353928/14681457
21+
pub trait ExtraTagsCloneArc {
22+
fn clone_arc(&self) -> Arc<dyn ExtraTags>;
23+
}
24+
25+
impl<T> ExtraTagsCloneArc for T
26+
where
27+
T: 'static + ExtraTags + Clone,
28+
{
29+
fn clone_arc(&self) -> Arc<dyn ExtraTags> {
30+
Arc::new(self.clone())
31+
}
32+
}
33+
34+
/// The registry in which extra tags (parsers) are registered
35+
/// This is passed to TODO
36+
#[derive(Debug, Clone)]
37+
pub struct ExtraTagsRegistry(HashMap<Tag, Arc<dyn ExtraTags>>);
38+
39+
impl ExtraTagsRegistry {
40+
/// Create a new, empty `ExtraTagsRegistry`
41+
pub fn new() -> Self {
42+
Self(HashMap::new())
43+
}
44+
/// Register an ExtraTags so their tags are parsed and stored in the ifd's `extra_tags``
45+
pub fn register(&mut self, tags: Arc<dyn ExtraTags>) -> AsyncTiffResult<()> {
46+
// check for duplicates
47+
for tag in tags.tags() {
48+
if self.0.contains_key(tag) {
49+
return Err(AsyncTiffError::General(format!(
50+
"Tag {tag:?} already registered in {self:?}!"
51+
)));
52+
}
53+
}
54+
// add to self
55+
for tag in tags.tags() {
56+
self.0.insert(*tag, tags.clone());
57+
}
58+
Ok(())
59+
}
60+
61+
/// deep clone so we have different registries per IFD
62+
pub(crate) fn deep_clone(&self) -> Self {
63+
let mut new_registry = ExtraTagsRegistry::new();
64+
65+
// we need to do some magic, because we can have multiple tags pointing to the same arc
66+
let mut seen = HashSet::new();
67+
for extra_tags in self.0.values() {
68+
// only add if this is the first encountered reference to this arc
69+
// (using thin pointer equality: https://stackoverflow.com/a/67114787/14681457 ; https://github.com/rust-lang/rust/issues/46139#issuecomment-346971153)
70+
if seen.insert(Arc::as_ptr(extra_tags) as *const ()) {
71+
if let Err(e) = new_registry.register(extra_tags.clone_arc()) {
72+
panic!("{e}");
73+
}
74+
}
75+
}
76+
77+
new_registry
78+
}
79+
}
80+
81+
impl Default for ExtraTagsRegistry {
82+
fn default() -> Self {
83+
Self::new() // add e.g. geo tags later
84+
}
85+
}
86+
87+
#[cfg(test)]
88+
mod tests {
89+
use super::*;
90+
use std::sync::LazyLock;
91+
92+
#[derive(Debug, Clone, PartialEq)]
93+
struct TestyTag;
94+
95+
static TESTY_TAGS: LazyLock<Vec<Tag>> = LazyLock::new(|| {
96+
vec![
97+
Tag::from_u16_exhaustive(u16::MAX),
98+
Tag::from_u16_exhaustive(u16::MAX - 1),
99+
]
100+
});
101+
102+
impl ExtraTags for TestyTag {
103+
fn tags(&self) -> &'static [Tag] {
104+
&TESTY_TAGS
105+
}
106+
107+
fn process_tag(
108+
&mut self,
109+
tag: u16,
110+
value: crate::tiff::Value,
111+
) -> crate::error::AsyncTiffResult<()> {
112+
println!("received {tag:?}: {value:?}");
113+
Ok(())
114+
}
115+
}
116+
117+
#[test]
118+
fn test_register() {
119+
let mut registry = ExtraTagsRegistry::new();
120+
assert!(registry.0.is_empty());
121+
let a1: Arc<dyn ExtraTags> = Arc::new(TestyTag);
122+
registry.register(a1.clone()).unwrap();
123+
assert_eq!(registry.0.len(), TestyTag.tags().len());
124+
for tag in a1.tags() {
125+
// very strict equality check
126+
assert!(Arc::ptr_eq(&registry.0[tag], &a1));
127+
}
128+
}
129+
130+
#[test]
131+
fn test_overlap_err() {
132+
let mut registry = ExtraTagsRegistry::new();
133+
assert!(registry.0.is_empty());
134+
registry.register(Arc::new(TestyTag)).unwrap();
135+
assert!(matches!(
136+
registry.register(Arc::new(TestyTag)).unwrap_err(),
137+
AsyncTiffError::General(_)
138+
));
139+
}
140+
141+
#[test]
142+
fn test_deep_clone() {
143+
let mut registry = ExtraTagsRegistry::new();
144+
let a1: Arc<dyn ExtraTags> = Arc::new(TestyTag);
145+
registry.register(a1.clone()).unwrap();
146+
let r2 = registry.deep_clone();
147+
for tags in a1.tags().windows(2) {
148+
// all should refer to the same Arc
149+
assert!(Arc::ptr_eq(&r2.0[&tags[0]], &r2.0[&tags[1]]));
150+
// which is different from the previous
151+
assert!(!Arc::ptr_eq(&a1, &r2.0[&tags[0]]));
152+
assert!(!Arc::ptr_eq(&a1, &r2.0[&tags[1]]));
153+
}
154+
}
155+
}

‎src/metadata/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,10 @@
5858
//! fetches the first `N` bytes out of a file.
5959
//!
6060
61+
mod extra_tags;
6162
mod fetch;
6263
mod reader;
6364

65+
pub use extra_tags::{ExtraTags, ExtraTagsRegistry};
6466
pub use fetch::{MetadataFetch, PrefetchBuffer};
6567
pub use reader::{ImageFileDirectoryReader, TiffMetadataReader};

‎src/metadata/reader.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ use std::io::Read;
44
use bytes::Bytes;
55

66
use crate::error::{AsyncTiffError, AsyncTiffResult};
7-
use crate::ifd::ExtraTagsRegistry;
87
use crate::metadata::fetch::MetadataCursor;
8+
use crate::metadata::ExtraTagsRegistry;
99
use crate::metadata::MetadataFetch;
1010
use crate::reader::Endianness;
1111
use crate::tiff::tags::{Tag, Type};
@@ -134,7 +134,10 @@ impl TiffMetadataReader {
134134
) -> AsyncTiffResult<Vec<ImageFileDirectory>> {
135135
let mut ifds = vec![];
136136
// deep clone the extra_tags_registry so we can have different values
137-
while let Some(ifd) = self.read_next_ifd(fetch, extra_tags_registry.deep_clone()).await? {
137+
while let Some(ifd) = self
138+
.read_next_ifd(fetch, extra_tags_registry.deep_clone())
139+
.await?
140+
{
138141
ifds.push(ifd);
139142
}
140143
Ok(ifds)
@@ -224,7 +227,11 @@ impl ImageFileDirectoryReader {
224227
///
225228
/// Keep in mind that you'll still need to call [`finish`][Self::finish] to get the byte offset
226229
/// of the next IFD.
227-
pub async fn read<F: MetadataFetch>(&self, fetch: &F, extra_tags_registry: ExtraTagsRegistry) -> AsyncTiffResult<ImageFileDirectory> {
230+
pub async fn read<F: MetadataFetch>(
231+
&self,
232+
fetch: &F,
233+
extra_tags_registry: ExtraTagsRegistry,
234+
) -> AsyncTiffResult<ImageFileDirectory> {
228235
let mut tags = HashMap::with_capacity(self.tag_count as usize);
229236
for tag_idx in 0..self.tag_count {
230237
let (tag, value) = self.read_tag(fetch, tag_idx).await?;

‎tests/image_tiff/util.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ pub(crate) async fn open_tiff(filename: &str) -> TIFF {
1414
let reader = Arc::new(ObjectReader::new(store.clone(), path.as_str().into()))
1515
as Arc<dyn AsyncFileReader>;
1616
let mut metadata_reader = TiffMetadataReader::try_open(&reader).await.unwrap();
17-
let ifds = metadata_reader.read_all_ifds(&reader, Default::default()).await.unwrap();
17+
let ifds = metadata_reader
18+
.read_all_ifds(&reader, Default::default())
19+
.await
20+
.unwrap();
1821
TIFF::new(ifds)
1922
}

0 commit comments

Comments
 (0)