-
Notifications
You must be signed in to change notification settings - Fork 206
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
consolidate exif parsing libraries - rework of timestamps #829
Conversation
* exifr is used for most tags now * New timestamp handling, removed exif-parser, date supported for png * Removed offset from testhelper. It's optional * explanations * Feature/timestamps (#3) * preparing for further timestamp test * Added more test and fixed offset calculation bug * Revered old dimension test, added new timestamp tests, some bug fixes * Renamed png-test because faces overrule keywords
I realize that the timestamp handling change may impact users of pigallery who live far away from UTC. Without this change, dates are parsed as local time on the server that runs pigallery, that is how Javascript Date works. To amend it, I have made the offset time available in the metadata, but it has not yet been used. Until the time that the UI can utilize this offset, this change will cause a daytime photo taken in e.g. Sydney to show up with a night time (or evening or morning) timestamp in Europe and vice versa. However for the overall correctness, I believe that this approach is correct. |
Added recognition of the offset value in the UI. It will be displayed if available. Caveat: Search will not take offset into account. A new year's picture taken in Sydney the 1st of January 2019 00:15:00 GMT+11, is technically taken in 31st of December 2018 in UTC. Therefore this picture won't show of in seaches where the after: parameter is set to 1st of january 2019. This is both correct and wrong at the same time. UTC-wise it is correct, local time it is not correct. I guess most people would find local time most untuitive, so there is room for improvement of the search. :)
Most of these changes have now been mitigated with updates to the UI. Offset will be displayed and taken into account when available. One caveat: Search using before and after date will occasionally miss a picture, because search is done by UTC time. So a new-year photo from Australia early the 1st of January, will technically still be 31st of December UTC, so it won't show up when using after: and first of January. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
By the way. The photos added for demo and test are from https://unsplash.com/ and free to use for any purpose except reselling or competing with unsplash. |
Hey @bpatrik When you get to review this, which is a combined metadataloader refactor and use of timestamps with offsets. You may want to take a look at this small additional change, which I have ready for merge aswell, if you like it: In the lightbox and infopanel for photos, it will show all of the location metadata: city, state and country separated by comma. Any empty value will be left out of course. My reasoning for suggesting this change is programs like GeoSetter and GetTagNinja fill out these values, so you can display "Los Angeles, California, USA" or "Birmingham, England, Great Britain" for example. Many countries don't use the "State" attribute. There it will just be like "Athens, Greece" for example. I can make it a part of this pull request or make another pull request if you think it's a good idea. No rush ;) Kind regards. |
Just to double check. After these change, this means that If I take a photo at noon (12:00) in London and someone else takes a photo 5 hours later at noon (12:00) in New York. Both of them would be shown in the app as they would have been created at the same time at noon. Even if I watch the photos in Sydney time. That would be the expected behavior as when you look back to your holidays photos, you do not expect to see a lunch photo taken at 2am. One future feature I can imagen is to add a switch to the lightbox menu that switches between local time and UTC (or however we would call it) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think its fine if the search update is not part of this PR.
But can you at least add a \TODO with some explanation to the right places in the file.
One sample place:
`media.metadata.creationDate ${relation} :from${queryId}`, |
Also thank you very much. This is huge contribution to the app! |
Yes. Lightbox etc. where time is displayed for human viewing, will show 12:00Z in London and 12:00-05:00 for New York (the default for Datepipe formatting is that UTC+00:00 is displayed at Z). This is regardless of where Pigallery is running. Only when absolutely no information about offset is available, the local time of the pigallery2 server will be used. So if your pigallery is running in timezone +1 and you show a picture from New York without offset. It would probably show the time wrong by 6 hours (the difference between the -5 and +1 offset). That is how it works now without this pull request, if my experiments are correct. Most modern photos won't have this problem, but incomplete metadata from old cameras, scanned photos may not have saved any timezone info. But I would suggest to the owner of such photos to fix this using exiftool or one of the other tools available for this :) Sure. A feature "show timestamps as UTC" could be made, that would replace all offsets with the fixed offset value +00:00. That's the beautiful thing about how you kept the MS format for storing timestamps. It's global, so applying offset is just a matter of how to show the timestamp in the places you want to show it. |
* effective storage of offset * added comments to searchmanager.ts fixed linting error in utils
Added a big TODO explanation here, and a bunch of smaller ones. |
Great! Thank you very much for your contribution! I think I can go ahead to merge it. |
ahh this PR ended up breaking tests :( https://github.com/bpatrik/pigallery2/actions/runs/8125512654/job/22208202972 for |
Ok. I will take a look asap. I will prioritize this as much as possible. |
Hi @bpatrik It also explains why I didn't get the error in the pull request build, because it was before the 29th of february. I'm thinking that replacing the format string "%j" (dayofyear) with "%m%d" (month and date) will help. I will try this out and report back (hopefully with a pull request that has a fix) |
I'm sorry, but apparently there were not enough tests on metadataloader and the timestamps for some photos are borken now in the demo folder. These photos have a proper date: |
Here is a quick var console.log dump. I will let this with you as I suspect the issue is with the new exifr part: Input file: wild-1.jpg from the demo folder.
The 'solution' should be |
Maybe this should be
pls doublecheck. |
Metadata Parsing
-- Rather than getting rid of image-size, it appears to be a good idea to use an independent source for image dimensions and only use tags as fallback (judging from a thread on exiftool's forums most software do not tag the dimensions of an image correctly).
I believe this comes pretty close to resolving #277
Timestamp handling
metadata.CreatedDate is now milliseconds based on UTC, where enough information was available. The value is read from the most accurate tag that is available. Preceedence is
Offsets are pulled directly from the metadata if availalbe. Same preceedence as above. If no offset is available, but GPS is, the offset is calculated from GPS timestamp (which is always UTC by spec). If no offset is available by metadata or calculation, offset is not defined.
Examples
These changes mean that a picture taken in London and a picture taken in Sydney at the exact same millisecond will have the same CreatedDate value.
The offset (e.g. +00:00 or +01:00 for London and +10:00 or +11:00 for Sydney) has been added to the photometadata, where available. This can be used to give the user better information in the UI.