Skip to content

Fix GPSTimeStamp for fractional seconds stored as low-denominator rationals - #56

Closed
tonimelisma wants to merge 1 commit into
bep:mainfrom
tonimelisma:fix/gps-timestamp-fractional-seconds
Closed

Fix GPSTimeStamp for fractional seconds stored as low-denominator rationals#56
tonimelisma wants to merge 1 commit into
bep:mainfrom
tonimelisma:fix/gps-timestamp-fractional-seconds

Conversation

@tonimelisma

Copy link
Copy Markdown
Contributor

The convertToTimestampString converter in helpers.go used ratNum() (numerator
only) for all three components, then formatted with %02d. It had a follow-up hack
that detected 10-character results and inserted a decimal point, e.g.:

4279/100 → numerator 4279 → "13:03:4279" (10 chars) → "13:03:42.79"  ✓

This only works when the seconds numerator happens to be exactly 4 digits. A rational
like 181/5 (= 36.2 seconds) has a 3-digit numerator and falls through:

181/5 → numerator 181 → "22:22:181" (9 chars) → no decimal inserted  ✗

Fix: compute the seconds value properly using toFloat64() (already used elsewhere
for GPS coordinate conversion) and format with strconv.FormatFloat(secs, 'f', -1, 64).
Integer seconds (e.g. 47/1) still produce "47" with no trailing decimal; fractional
seconds (e.g. 181/5) produce "36.2".

The existing goexif/has-lens-info.jpg golden test (seconds = 4279/100 = 42.79)
continues to pass as before.

This bug was discovered while adding HEIF support and testing against an iPhone 15 Pro
photo whose GPS log records seconds as 181/5.

The convertToTimestampString converter used ratNum() (numerator only) for
all three time components. It had a follow-up hack that detected 10-char
results and inserted a decimal point:

    4279/100 → numerator 4279 → "13:03:4279" (10 chars) → "13:03:42.79"

This only works when the seconds numerator is exactly 4 digits. A rational
like 181/5 (= 36.2 seconds) has a 3-digit numerator and falls through:

    181/5 → numerator 181 → "22:22:181" → no decimal inserted

Fix: use toFloat64() and strconv.FormatFloat for the seconds component.
Integer seconds still produce "47"; fractional seconds produce "36.2".

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@bep

bep commented Feb 21, 2026

Copy link
Copy Markdown
Owner

This needs to fix a failing test case.

@codecov-commenter

codecov-commenter commented Feb 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.40%. Comparing base (93d0b00) to head (13c518b).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #56      +/-   ##
==========================================
+ Coverage   79.38%   79.40%   +0.01%     
==========================================
  Files          14       14              
  Lines        1606     1607       +1     
==========================================
+ Hits         1275     1276       +1     
  Misses        228      228              
  Partials      103      103              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bep

bep commented Feb 21, 2026

Copy link
Copy Markdown
Owner

I'll handle this in #58

@bep bep closed this Feb 21, 2026
@tonimelisma
tonimelisma deleted the fix/gps-timestamp-fractional-seconds branch February 22, 2026 18:43
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.

3 participants