Skip to content

correct epoch_1_jan_1904 #498

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

Merged
merged 1 commit into from
Sep 8, 2020

Conversation

choim4389
Copy link

No description provided.

@Nadahar
Copy link
Contributor

Nadahar commented Sep 4, 2020

@drewnoakes This might be nothing, but isn't it a bit strange how you could have come up with "the wrong constant"? I'm thinking that maybe the "Java epoch" isn't always the same for all platforms, and that this might explain how this came to be.

The constant was introduced when you refactored #410 in be7c4c8 as far as I can understand. The way I originally made it, the difference was "calculated" by each individual system. I'm just wondering if that might ultimately be the correct solution. This was the original code:

        // Calculate the difference between the Java Epoch and the MP4 Epoch
        Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
        calendar.set(1904, 0, 1, 0, 0, 0); // January 1, 1904  -  Macintosh/MP4/QuickTime Time Epoch
        MP4_EPOCH_OFFSET = calendar.getTime().getTime();

The initialization was done in a static initializer, so I don't thing the "cost" of doing it this way matters much.

@choim4389
Copy link
Author

@Nadahar
calendar.set(1904, 0, 1, 0, 0, 0) doesn't set milliseconds. So, previous milliseconds value would be remained.
Therefore every set() could returns the different ms value.

To avoid that call calendar.clear() before set(). it clears remained ms value and after that calendar.getTime().getTime() always returns -2082844800000.

@Nadahar
Copy link
Contributor

Nadahar commented Sep 5, 2020

@choim4389 Ah, that explains everything then. I must say that it's a strange behavior on the part of Calendar that I wasn't aware of, but it indicates that using a (correct) constant is the best solution.

@drewnoakes I'd sayt just merge this 😉

@drewnoakes drewnoakes merged commit bc6b90e into drewnoakes:master Sep 8, 2020
@drewnoakes
Copy link
Owner

Thanks all for the investigation here. At least we understand why we had a weird constant there.

@drewnoakes
Copy link
Owner

Released in 2.15.0.

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.

None yet

4 participants