drewnoakes / metadata-extractor

Extracts Exif, IPTC, XMP, ICC and other metadata from image, video and audio files
Apache License 2.0
2.53k stars 473 forks source link

wrong date/time while timezone is null #628

Closed Kumotaku closed 5 months ago

Kumotaku commented 9 months ago

date time shown with windows explorer: image

but read this: image

btw, i'm in China. timezone is gmt+8.00

i suggest not to set timezone here: image

StefanOltmann commented 5 months ago

Yes, many cameras don't even write timezone information.

It's best to just ignore time zones and display exactly the date as shown in the EXIF.

Edit: But I think that would be too much of a change by now considering the user base. So I would not recommend it.

drewnoakes commented 5 months ago

@Kumotaku thank you for reporting this.

I'm hoping you and/or @StefanOltmann can advise what the best approach would be here.

If we don't set unknown dates to GMT then unit tests fail. For example here in AEDT:

_directory.setString(1, "2002:01:30 23:59:59");

java.util.Date date = _directory.getDate(1, null);
java.lang.AssertionError: 
Expected :Thu Jan 31 10:59:59 AEDT 2002
Actual   :Wed Jan 30 23:59:59 AEDT 2002

I'm not very familiar with Java these days. What would be the best approach here, given we want both maximal information and a good experience for library users, and stable unit tests.

StefanOltmann commented 5 months ago

@drewnoakes

I am sure you wonโ€™t like my approach. ๐Ÿ˜…

https://github.com/Ashampoo/kim/blob/01fbb7825d3ccf53170c4625aad63b713614919e/src/commonMain/kotlin/com/ashampoo/kim/common/PhotoMetadataConverter.kt#L166

https://github.com/Ashampoo/kim/blob/01fbb7825d3ccf53170c4625aad63b713614919e/src/commonMain/kotlin/com/ashampoo/kim/Kim.kt#L51

This global variable is set from the unit tests. And yes, a user could set it at runtime and corrupt the results. As an alternative I can think of a environment variable that controls this.

StefanOltmann commented 5 months ago

Looking at the code I see that you already solved this for the PNG tests by just setting the default TimeZone to GMT there.

https://github.com/drewnoakes/metadata-extractor/blob/753cf1f514dff829af2782a60c81720ab161aa83/Tests/com/drew/imaging/png/PngMetadataReaderTest.java#L64

I will craft a PR and try the same for the other unit tests.

Nadahar commented 5 months ago

The TimeZone class has both getDefault() and setDefault(zone). setDefault() overrides the timezone retrieved from the OS for that JVM instance as I understand it. Thus, this should only apply "until the end of the application execution".

Wouldn't it be best to simply apply getDefault() when the timezone is missing, assuming that the computer and image belongs to the same geographical area - and then "prime" the test run with a call to setDefault(GMT) to in effect make sure that "unknown" timezones are always GMT during test runs?

@StefanOltmann You post popped up in the moment I pressed comment. But yeah, the idea is the same ๐Ÿ˜‰

StefanOltmann commented 5 months ago

@drewnoakes Your PR is ready. :)

@Nadahar Setting the default time zone to a new value just for the test is the best solution, indeed.

I wish I could do the same for my lib, but it's written in Kotlin. So I use kotlinx-datetime, which does not expose setDefault(zone) as not all platforms that I target allow this. Maybe that's the reason I was not thinking of that first.

drewnoakes commented 5 months ago

Thank you @StefanOltmann and @Nadahar, the approach in PR looks great. Merged.