From 4276ed389bbe4436aaddaa0b059f381ef0e42704 Mon Sep 17 00:00:00 2001 From: KAMADA Ken'ichi Date: Sat, 14 Dec 2019 21:57:56 +0900 Subject: [PATCH] Change Reader::fields() to return an iterator instead of a slice. --- src/lib.rs | 1 + src/reader.rs | 82 +++++++++---------------------------------------- src/tag.rs | 4 +-- src/tiff.rs | 22 +++++++++++-- tests/rwrcmp.rs | 10 +++--- 5 files changed, 41 insertions(+), 78 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a410e2f..a5dade8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -59,6 +59,7 @@ //! they are now spelled in all uppercase (e.g., `Context::TIFF`). //! * `Value` became a self-contained type. The structures of `Value::Ascii` //! and `Value::Undefined` have been changed to use Vec instead of &[u8]. +//! * `Reader::fields` now returns an iterator instead of a slice. pub use error::Error; pub use jpeg::get_exif_attr as get_exif_attr_from_jpeg; diff --git a/src/reader.rs b/src/reader.rs index e330aa1..93ec439 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -24,16 +24,15 @@ // SUCH DAMAGE. // -use std::collections::HashMap; +use std::collections::BTreeMap; use std::io; use std::io::Read; -use std::mem; use crate::error::Error; use crate::jpeg; use crate::tag::Tag; use crate::tiff; -use crate::tiff::{Field, In, ProvideUnit}; +use crate::tiff::{Field, IfdEntry, In, ProvideUnit}; /// The `Reader` struct reads a JPEG or TIFF image, /// parses the Exif attributes in it, and holds the results. @@ -63,12 +62,11 @@ use crate::tiff::{Field, In, ProvideUnit}; pub struct Reader { // TIFF data. buf: Vec, - // Exif fields. - fields: Vec, + // Exif fields. `BTreeMap` is used so that `Reader::field` returns + // the fields in a stable order. + entries: BTreeMap<(In, Tag), IfdEntry>, // True if the TIFF data is little endian. little_endian: bool, - // HashMap to find a field quickly. - field_map: HashMap<(Tag, In), &'static Field>, } impl Reader { @@ -89,20 +87,14 @@ impl Reader { return Err(Error::InvalidFormat("Unknown image format")); } - let (fields, le) = tiff::parse_exif_compat03(&buf)?; - - // Initialize the HashMap of all fields. - let mut field_map = HashMap::new(); - for f in &fields { - field_map.insert((f.tag, f.ifd_num), - unsafe { mem::transmute::<&Field, &Field>(f) }); - } + let (entries, le) = tiff::parse_exif(&buf)?; + let entries = entries.into_iter() + .map(|e| (e.ifd_num_tag(), e)).collect(); Ok(Reader { buf: buf, - fields: fields, + entries: entries, little_endian: le, - field_map: field_map }) } @@ -114,8 +106,9 @@ impl Reader { /// Returns a slice of Exif fields. #[inline] - pub fn fields<'a>(&'a self) -> &[Field] { - &self.fields + pub fn fields<'a>(&'a self) -> impl ExactSizeIterator { + self.entries.values() + .map(move |e| e.ref_field(&self.buf, self.little_endian)) } /// Returns true if the TIFF data is in the little-endian byte order. @@ -128,7 +121,8 @@ impl Reader { /// and the IFD number. #[inline] pub fn get_field(&self, tag: Tag, ifd_num: In) -> Option<&Field> { - self.field_map.get(&(tag, ifd_num)).map(|&f| f) + self.entries.get(&(ifd_num, tag)) + .map(|e| e.ref_field(&self.buf, self.little_endian)) } } @@ -145,54 +139,6 @@ mod tests { use crate::value::Value; use super::*; - static TIFF_ASCII: &'static [u8] = - b"MM\0\x2a\0\0\0\x08\0\x01\x01\x0e\0\x02\0\0\0\x04ABC\0\0\0\0\0"; - - // Test if moving a `Reader` does not invalidate the references in it. - // The referer is in the heap and does not move, so this test is not - // so meaningful. - #[test] - fn move_reader() { - let r1 = Reader::new(&mut BufReader::new(TIFF_ASCII)).unwrap(); - let ptr = &r1 as *const _; - move_in_and_drop(r1, ptr); - - let (r2, ptr) = move_out_and_drop(); - assert!(ptr != &r2 as *const _, "not moved"); - check_abc(r2.fields()); - - box_and_drop(); - } - - #[inline(never)] - fn move_in_and_drop(r1: Reader, ptr: *const Reader) { - assert!(ptr != &r1 as *const _, "not moved"); - check_abc(r1.fields()); - } - - #[inline(never)] - fn move_out_and_drop() -> (Reader, *const Reader) { - let r2 = Reader::new(&mut BufReader::new(TIFF_ASCII)).unwrap(); - let ptr = &r2 as *const _; - (r2, ptr) - } - - fn box_and_drop() { - let r = Reader::new(&mut BufReader::new(TIFF_ASCII)).unwrap(); - let ptr = &r as *const _; - let b = Box::new(r); - assert!(ptr != &*b as *const _, "not moved"); - check_abc(b.fields()); - } - - fn check_abc(fields: &[Field]) { - if let Value::Ascii(ref v) = fields[0].value { - assert_eq!(*v, vec![b"ABC"]); - } else { - panic!("TIFF ASCII field is expected"); - } - } - #[test] fn get_field() { let file = File::open("tests/yaminabe.tif").unwrap(); diff --git a/src/tag.rs b/src/tag.rs index 98f6da7..31f1a77 100644 --- a/src/tag.rs +++ b/src/tag.rs @@ -51,7 +51,7 @@ use crate::util::atou16; // PartialEq and Eq need to be _automatically derived_ for Tag to // emulate structural equivalency. // -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct Tag(pub Context, pub u16); impl Tag { @@ -110,7 +110,7 @@ impl fmt::Display for Tag { /// A type that indicates how a tag number is interpreted. // This type is not an enum to keep API compatibilities when // new contexts are introduced. -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct Context(std::num::NonZeroU16); macro_rules! generate_context { diff --git a/src/tiff.rs b/src/tiff.rs index 51d7607..26ded7e 100644 --- a/src/tiff.rs +++ b/src/tiff.rs @@ -53,6 +53,21 @@ pub struct IfdEntry { } impl IfdEntry { + pub fn ifd_num_tag(&self) -> (In, Tag) { + if self.field.is_fixed() { + let field = self.field.get_ref(); + (field.ifd_num, field.tag) + } else { + let field = self.field.get_mut(); + (field.ifd_num, field.tag) + } + } + + pub fn ref_field<'a>(&'a self, data: &[u8], le: bool) -> &'a Field { + self.parse(data, le); + self.field.get_ref() + } + fn into_field(self, data: &[u8], le: bool) -> Field { self.parse(data, le); self.field.into_inner() @@ -106,7 +121,7 @@ pub struct Field { /// assert_eq!(In::PRIMARY.index(), 0); /// assert_eq!(In::THUMBNAIL.index(), 1); /// ``` -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct In(pub u16); impl In { @@ -537,9 +552,10 @@ mod tests { fn unknown_field() { let data = b"MM\0\x2a\0\0\0\x08\ \0\x01\x01\0\xff\xff\0\0\0\x01\0\x14\0\0\0\0\0\0"; - let (v, _) = parse_exif_compat03(data).unwrap(); + let (v, le) = parse_exif(data).unwrap(); assert_eq!(v.len(), 1); - assert_pat!(v[0].value, Value::Unknown(0xffff, 1, 0x12)); + assert_pat!(v[0].ref_field(data, le).value, + Value::Unknown(0xffff, 1, 0x12)); } #[test] diff --git a/tests/rwrcmp.rs b/tests/rwrcmp.rs index 041a6b8..121cb72 100644 --- a/tests/rwrcmp.rs +++ b/tests/rwrcmp.rs @@ -111,13 +111,13 @@ fn rwr_compare

(path: P) where P: AsRef { let reader2 = Reader::new(&mut &out[..]).unwrap(); // Sort the fields (some files have wrong tag order). - let mut fields1 = reader1.fields().iter().map(|f| f).collect::>(); - fields1.sort_by_key(|f| f.tag.number()); - let mut fields2 = reader2.fields().iter().map(|f| f).collect::>(); - fields2.sort_by_key(|f| f.tag.number()); + let mut fields1 = reader1.fields().collect::>(); + fields1.sort_by_key(|f| (f.ifd_num, f.tag)); + let mut fields2 = reader2.fields().collect::>(); + fields2.sort_by_key(|f| (f.ifd_num, f.tag)); // Compare. - assert_eq!(reader1.fields().len(), reader2.fields().len()); + assert_eq!(fields1.len(), fields2.len()); for (f1, f2) in fields1.iter().zip(fields2.iter()) { assert_eq!(f1.tag, f2.tag); assert_eq!(f1.ifd_num, f2.ifd_num);