Skip to content

Conversation

@grasdk
Copy link
Contributor

@grasdk grasdk commented Mar 5, 2024

Seaching during leap years fixed
Small optimization in metadataloader, so the buffer size is the minimum of the total file size and the configured size
Fixed a small sorting service bug, utilizing timezoneoffset, instead of UTC when available

@grasdk
Copy link
Contributor Author

grasdk commented Mar 6, 2024

If you want to review the actual changes to MetadataLoader.ts, looking at this commit is probably the easiest:
f11a133

The whole file is changed after switching to LF instead of CRLF for newline.

@grasdk grasdk marked this pull request as ready for review March 6, 2024 20:41
@bpatrik
Copy link
Owner

bpatrik commented Mar 6, 2024

Great! Thank you for the quick fix!

Thanks for the extra pointer for the diff. I was tired last time when I hit merge and did not see that the whole file changed (I also trusted the unit tests. Metadataloader is one of the most most tested part of the app)

Btw, my hope with the metadataloader fix was to get rid of the whole custom magic buffer loader. I tried it once but (as you also found) exifr is not powerful enough and the development stopped. (my guess the guy finished schooled and found a job).

ultimate solution would be ExifTool but that was super slow when I tried once and never looked deep enough if there is tweaking opportunity (my assumption that it would not be fast enough).

@bpatrik bpatrik merged commit 5d5be06 into bpatrik:master Mar 6, 2024
@grasdk
Copy link
Contributor Author

grasdk commented Mar 6, 2024

Glad to be of help. Yeah, too bad with exifr, but luckily it does a lot. Including sidecar. I don't use them myself, but the other PR about sidecar got me interested in fixing that too. Maybe some fun for a bit later, depending on what the author of the other PR will end up with. :)

Btw. the leap year stuff was crazy. Took me a long while to realize what the problem was, but it was good timing. If the metadataloader change had been in april, we might not have discovered the issue ;)

@grasdk grasdk deleted the feature/timestamp_rework_test_fix branch March 6, 2024 22:06
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