This repository was archived by the owner on Dec 28, 2021. It is now read-only.
Refactor a lot of library internals#10
Open
Ortham wants to merge 22 commits intoelvis-epx:masterfrom
Ortham:refactor
Open
Refactor a lot of library internals#10Ortham wants to merge 22 commits intoelvis-epx:masterfrom Ortham:refactor
Ortham wants to merge 22 commits intoelvis-epx:masterfrom
Ortham:refactor
Conversation
It's not useful to clients as where the data is stored doesn't matter once the image has been parsed, and just duplicates data.
It's not useful to store the offset vs. the actual data once the metadata has been parsed, and just duplicates data if there is no offset.
Don't allocate a different string for each and every entry, just return a static string slice. Also fix postprocessing setting a few units to value_more_readable, and unknown units being non-empty, both counter to the documented behaviour.
It's more idiomatic.
Make it a function on IfdFormat, not IfdEntry.
They're 16-bit values, use literals that match.
Use the names as given in the 2.31 spec.
This allows ExifEntry to directly preserve any tag values that don't map to ExifTag values, inching closer to obsoleting IfdEntry.
It's now obsolete, as all its data is duplicated in the other ExifEntry fields.
There's no reason to discard it, and the semantics could be useful.
Make it a consuming method on IfdEntry, and make the logic a bit more readable. It's also more efficient as it avoids cloning the value.
Instead of using -1 values to indicate there are no bounds.
It's a much cleaner API, and uses itertools to provide a robust join() implementation. There is one instance (rational_values()) where it's preferable to use itertools directly, as the trait implementation can't be specialised for Vec<URational> and mapping to Vec<f64> first is less efficient.
Use the identical strpass formatter instead.
There's probably a way to abstract out the remaining commonality, but I can't think of it.
It's cleaner and more idomatic.
Reimplement tag_value_new() as TagValue::new(), and put ToCsv in its own module.
Owner
|
Thanks for the message, I will check how to collaborate with the exif-rs project.
…--
Elvis Pfutzenreuter
+55 47 98801 6105
https://epxx.co
On 7 Jul 2017, at 13:38, Oliver Hamlet ***@***.***> wrote:
I was going to try adding limited write support for GPS tags to rexif for my own use, but found how it was implemented difficult to work with, so started to refactor the library internals to more easily accommodate write support, and add some tests.
However, since then I've noticed that kamadak/exif-rs <https://github.com/kamadak/exif-rs> does pretty much the same thing as rexif, but with an implementation that's neater than my own refactoring efforts, and it already has tests, so I've pretty much jumped ship. It would be great if the two of you could join forces, because it's a pretty small niche, and having one preferred pure-Rust EXIF parser would be great.
Either way I thought I'd make this PR in case you wanted to merge some or all of what I've done. I realise there's a lot of different bits to it, so if you like some but not others, I can split it up into smaller PRs.
You can view, comment on, or merge this pull request online at:
#10 <#10>
Commit Summary
Remove IfdEntry::ext_data field
Remove IfdEntry::ifd_data field
Replace ExifEntry::unit with a member function
Replace ifdformat_new() with IfdFormat::new()
Refactor IfdFormat size()
Use 16-bit literals for ExifTag values
Fix some Exif tag names
Tidy up self match references
Add IfdTag enum to wrap ExifTag
Remove ifd field from ExifEntry
Use underscores for unused match variables
Don't cast IfdFormat in TagValue::Invalid
Refactor IfdEntry -> ExifEntry conversion
Refactor IFD parsing slightly for clarity
Return count bounds as an optional tuple
Replace numarray_to_string with a ToCsv trait
Simplify other_tag() implementation
Remove nop formatter
Merge the rational_value and rational_values formatters
Refactor Exif postprocessing
Use "if let" for exifreadable functions
Split up the ifdformat module
File Changes
M Cargo.toml <https://github.com/elvis-epx/rexif/pull/10/files#diff-0> (3)
M src/exif.rs <https://github.com/elvis-epx/rexif/pull/10/files#diff-1> (396)
M src/exifpost.rs <https://github.com/elvis-epx/rexif/pull/10/files#diff-2> (349)
M src/exifreadable.rs <https://github.com/elvis-epx/rexif/pull/10/files#diff-3> (1092)
D src/ifdformat.rs <https://github.com/elvis-epx/rexif/pull/10/files#diff-4> (132)
M src/lib.rs <https://github.com/elvis-epx/rexif/pull/10/files#diff-5> (10)
M src/main.rs <https://github.com/elvis-epx/rexif/pull/10/files#diff-6> (18)
M src/tiff.rs <https://github.com/elvis-epx/rexif/pull/10/files#diff-7> (96)
A src/to_csv.rs <https://github.com/elvis-epx/rexif/pull/10/files#diff-8> (51)
M src/types.rs <https://github.com/elvis-epx/rexif/pull/10/files#diff-9> (259)
M src/types_impl.rs <https://github.com/elvis-epx/rexif/pull/10/files#diff-10> (343)
Patch Links:
https://github.com/elvis-epx/rexif/pull/10.patch <https://github.com/elvis-epx/rexif/pull/10.patch>
https://github.com/elvis-epx/rexif/pull/10.diff <https://github.com/elvis-epx/rexif/pull/10.diff>
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub <#10>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AExOgCS8pqVKQD4g4YVFeEikxfOe7qHIks5sLl8JgaJpZM4ORK6R>.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I was going to try adding limited write support for GPS tags to rexif for my own use, but found how it was implemented difficult to work with, so started to refactor the library internals to more easily accommodate write support, and add some tests.
However, since then I've noticed that kamadak/exif-rs does pretty much the same thing as rexif, but with an implementation that's neater than my own refactoring efforts, and it already has tests, so I've pretty much jumped ship. It would be great if the two of you could join forces, because it's a pretty small niche, and having one preferred pure-Rust EXIF parser would be great.
Either way I thought I'd make this PR in case you wanted to merge some or all of what I've done. I realise there's a lot of different bits to it, so if you like some but not others, I can split it up into smaller PRs.