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

Update CapturedAtTagNames.ts #159

Merged
merged 1 commit into from
Oct 2, 2023
Merged

Conversation

alextran1502
Copy link
Contributor

I found the issue that the method firstDateTime doesn't consider the file's creation date. So, it causes an issue for this file, which has the CreationDate is the actual file's created date and the CreateDate is the file's modified date

sample.zip

Copy link
Member

@mceachen mceachen left a comment

Choose a reason for hiding this comment

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

Thanks for the example video!

@mceachen mceachen merged commit 2ccb925 into photostructure:main Oct 2, 2023
@mceachen
Copy link
Member

mceachen commented Oct 2, 2023

Released: https://github.com/photostructure/exiftool-vendored.js/releases/tag/v23.2.0

@jrasm91
Copy link

jrasm91 commented Oct 3, 2023

Between when this PR was merged and 23.2.0 was released it looks like the order of these tags was changed.

From what I understand, more or less all videos have CreateDate. Some apple videos specifically have CreationDate, which should take precedence over CreateDate, but it is now later in the list, so it'll never be used.

Is there a reason you changed the order?

@mceachen
Copy link
Member

mceachen commented Oct 3, 2023

I pushed SubSec values to the front as they (sometimes, not always) have higher precision. The “demoting” of CreationDate wasn’t on purpose (I mistakenly assumed he added it in alphabetic order).

I actually don’t recommend simply returning the first defined value. I had to make PhotoStructure use additional heuristics (including precision-invariant sorting) to get the proper captured-at time. More details are here: https://photostructure.com/faq/captured-at/

As the list of tags is given to the function, and this const is simply the default, and there’s an order you prefer, perhaps use your own array? I’ll “demote” CreateDate later today.

@jrasm91
Copy link

jrasm91 commented Oct 3, 2023

OK. No worries, we can definitely use our own order. I was just curious if there was a specific reason for the change.

Applying additional heuristics and fallbacks is definitely a good idea. What you have on the FAQ is a good list/order - thanks for the link. Using firstDateTime seems to work pretty well for so far, and it is better than the one or two tags we had before.

Perhaps you can contribute to Immich a better "captured at" implementation in the future 😄

@mceachen
Copy link
Member

mceachen commented Oct 3, 2023

From what I understand, more or less all videos have CreateDate

Are you getting FileCreateDate (from stat()) mixed up with CreateDate?

CreateDate is indeed found in many .mp4, but I'm not seeing it in the 10+ other movie exemplar types I've collected.

@jrasm91
Copy link

jrasm91 commented Oct 3, 2023

No, I'm talking specifically about Quicktime:CreateDate. You are probably more familiar with the space than I am. The videos I have been working with are mostly phone/camera videos, most of which are mp4s with a CreateDate field.

Of the other movie types you've collected, do any of them have both CreateDate and CreationDate? Or, what is the common tag you use to captured at?

@mceachen
Copy link
Member

mceachen commented Oct 3, 2023

what is the common tag you use to captured at

LOL

Every camera manufacturer loves to be different, and no one follows the standards.

@jrasm91
Copy link

jrasm91 commented Oct 3, 2023

Lol, great. I'm sure we'll run into more edge cases soon enough :-).

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