Skip to content
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

Improve printing of GPS info #1551

Merged
merged 3 commits into from
Apr 14, 2021
Merged

Improve printing of GPS info #1551

merged 3 commits into from
Apr 14, 2021

Conversation

kmilos
Copy link
Collaborator

@kmilos kmilos commented Apr 13, 2021

Addresses #1541

clanmills
clanmills previously approved these changes Apr 13, 2021
Copy link
Collaborator

@clanmills clanmills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Milos. That looks fine. Thank You for dealing with this.

knot is a measure of speed...
Copy link
Collaborator

@clanmills clanmills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. A few comments:

1 Converting rationalToFloat()

I'm a little surprised that so much code is necessary in printDegrees(). Is there a lesson to be learned here. There is a API:

EXIV2API Rational floatToRationalCast(float f);

I thought there are one to convert rationalToFloat(), or Long, or Double. I can't find it. Is there something special and unusual about degrees() such as doing arithmetic on several values.

  1. Is the result the same as ExifTool?

  2. Well spotted that I put "knots" in the man page. It should of course be nautical miles.

@kmilos
Copy link
Collaborator Author

kmilos commented Apr 14, 2021

Is there something special and unusual about degrees() such as doing arithmetic on several values.

There are two ways of lat/lon encoding w/ 3 RATIONALs according to Exif spec: integer degrees and minutes and then anything goes for seconds (integer or fractional) like [ DD/1, MM/1, SS/1 or SSSS/frac], or fractional minutes with seconds "unused" [ DD/1, MMMM/frac, 0/1 ]. We need to convert the second case (I chose to make it fit the first one), and then print consistently in the usual degree/minute/second format...

Is the result the same as ExifTool?

One exception: ExifTool prints .00" even when seconds are integers...

@clanmills
Copy link
Collaborator

Right. You've got your head round this one. That cosmetic difference with ExifTool seems OK to me. I'm happy for you to merge.

@clanmills
Copy link
Collaborator

Let's agree on a team convention. The person who opens a PR also does the merge. Why? Because if you suddenly think of a modification to the PR, you can update it before merging.

@kmilos
Copy link
Collaborator Author

kmilos commented Apr 14, 2021

Thanks. Fine by me.

@kmilos kmilos merged commit db86f8c into main Apr 14, 2021
@kmilos kmilos deleted the fix_1541_gps_print branch April 14, 2021 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants