Skip to content

added reading resolution info #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 8, 2021
Merged

Conversation

iikka-v
Copy link

@iikka-v iikka-v commented Oct 7, 2021

This is a fairly simple patch to add read access to resolution information of a TIFF file. The relevant IFD entries were already in place, so this just

  • adds the fields to the TinyTIFFReaderFrame internal struct,
  • provides the case branches to populate these fields during read operation and
  • provides accessor functions to read the values from the struct.

Although the resolution info is unimportant or not used in many applications, it does have high significance in certain special areas, such as industrial X-ray images, where TIFF data format is extensively used for the raw data. Thus, I'd kindly request you to implement reading the resolution information.

Signed-off-by: Iikka Virkkunen <iikka@virkkuset.com>
@jkriege2
Copy link
Owner

jkriege2 commented Oct 7, 2021

Hi!

nice addition, thanks. Could I ask you a favour: please rename TinyTIFFReader_GetResolutionUnit() to TinyTIFFReader_getResolutionUnit() (lower case g)

Best,
Jan

@jkriege2
Copy link
Owner

jkriege2 commented Oct 7, 2021

also please reference the constants in the getter function documentation

@jkriege2
Copy link
Owner

jkriege2 commented Oct 7, 2021

finally, please set default values for the new struct members in TinyTIFFReader_getEmptyFrame()

@jkriege2 jkriege2 self-assigned this Oct 7, 2021
…n and added resolution fields to empty frame initialization
@iikka-v
Copy link
Author

iikka-v commented Oct 7, 2021

Thank you for the quick and positive response. I've now made the requested changes. Thank you for the constructive comments (and sorry I wasn't more careful to begin with).

Iikka

@jkriege2
Copy link
Owner

jkriege2 commented Oct 8, 2021

thanks!

@jkriege2 jkriege2 merged commit f13ce29 into jkriege2:master Oct 8, 2021
@jkriege2 jkriege2 mentioned this pull request Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants