jokiazhang / metadata-extractor

Automatically exported from code.google.com/p/metadata-extractor
0 stars 0 forks source link

Directory.getDate() uses system timezone #11

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
When the metadata-extractor is used on a server, Directory.getDate() uses the 
server's timezone for parsing, which can be different from the timezone of the 
user's camera (e.g. when uploading photos to a photo community).

What steps will reproduce the problem?
1. Use metadata-extractor on a server set to GMT
2. Upload a photo taken at 12:34 GMT+2
3. Use Directory.getDate() to find out when the photo was taken

What is the expected output? What do you see instead?
Expected: 10:34 GMT (which is 12:34 GMT+2)
Actual: 12:34 GMT (which is 14:34 GMT+2)

Proposed change:

Add a method Directory.getDate(int tagType, TimeZone tz), which accepts a 
TimeZone that is used when the Exif timestamp is parsed. Directory.getDate(int 
tagType) continues to use the system's TimeZone, to maintain downward 
compatibility.

Original issue reported on code.google.com by shredz...@gmail.com on 26 Apr 2011 at 10:41

GoogleCodeExporter commented 9 years ago
Thank you for your issue report.

This is an interesting one... at first glance I agreed completely, but 
unfortunately there is a problem.  The date is not always stored as a string, 
so therefore it is not always parsed.  If parsing does not occur, then the 
timezone passed to the method will have no effect.  I think that's confusing 
for users of the API.  Can you think of a better solution here?

The workaround is, of course, to call Directory.getString and parse the string 
yourself, providing the time zone.

I'll give this one some more thought.

Original comment by drewnoakes on 27 Apr 2011 at 1:09

GoogleCodeExporter commented 9 years ago
Well, I don't really think it would be confusing to the user. It is quite 
common in Java APIs to pass in a second parameter for overriding a system 
default. And a null result would still mean "sorry, there was no date set at 
all", IMO.

Hmmm... Another approach would be to add a Directory.getCalendar() method, 
which returns a Calendar object. The invoker could set the proper timezone at 
the returned Calendar. However I think it is more irritating to the user that 
the time stored in a Calendar is not changed when a different timezone is set. 
What do you think?

A different solution: The timezone is connected to the image that is read, so 
maybe a TimeZone parameter should be added to the MetadataReader methods, where 
the actual image file is read. The disadvantage is that the TimeZone value 
would need to be stored in the Metadata and Directory classes. The impact to 
the API would be much higher, as more classes and methods are affected. I also 
dislike to redundantly keep a value in several objects.

Personally, I would prefer the former solution because it is very simple, but 
certainly it is a matter of taste. :-)

> The workaround is, of course, to call Directory.getString and parse the string
> yourself, providing the time zone.

This is what I am currently doing, but it is quite tedious because of the 
different possible date formats. Also, the expertise about how to read EXIF 
data should be kept inside the Metadata Extractor library.

Original comment by shredz...@gmail.com on 27 Apr 2011 at 6:54

GoogleCodeExporter commented 9 years ago
For documentation purposes, this is the workaround I am using:

    private Date getDate(Directory dir, int tagType, TimeZone tz) throws MetadataException {
        Object o = dir.getObject(tagType);
        if (o==null) {
            throw new MetadataException("Tag " + dir.getTagName(tagType) + " has not been set -- check using containsTag() first");
        } else if (o instanceof java.util.Date) {
            return (java.util.Date)o;
        } else if (o instanceof String) {
            // add new dateformat strings to make this method even smarter
            // so far, this seems to cover all known date strings
            // (for example, AM and PM strings are not supported...)
            String datePatterns[] = {
                "yyyy:MM:dd HH:mm:ss",
                "yyyy:MM:dd HH:mm",
                "yyyy-MM-dd HH:mm:ss",
                "yyyy-MM-dd HH:mm"};
            String dateString = (String)o;
            for (int i = 0; i<datePatterns.length; i++) {
                try {
                    DateFormat parser = new java.text.SimpleDateFormat(datePatterns[i]);
                    if (tz != null) {
                        parser.setTimeZone(tz);
                    }
                    return parser.parse(dateString);
                } catch (java.text.ParseException ex) {
                    // simply try the next pattern
                }
            }
        }
        throw new MetadataException("Tag '" + tagType + "' cannot be cast to a java.util.Date.  It is of type '" + o.getClass() + "'.");
    }

Original comment by shredz...@gmail.com on 27 Apr 2011 at 8:12

GoogleCodeExporter commented 9 years ago
Hi Richard,

Sorry it's taken me a few days to get to this.  I've been working through a 
backlog in chronological order.

The only issue I have with the above code is that if the tag is already stored 
as a java.util.Date, then the TimeZone is not applied.  I guess that's what I 
was trying to say above.  But, looking at it, I guess it could be applied to 
the Date object, right?  I'm not that familiar with the Java Date/Calendar API. 
 Could you recommend an amendment to the method you post that shows how this 
might work?  Any unit tests you could provide would be even better.

Many thanks.

Original comment by drewnoakes on 1 May 2011 at 5:24

GoogleCodeExporter commented 9 years ago
A Date object is always using UTC by definition, so there is no way to apply a 
TimeZone to it. A timezone is required when you convert a calendary date to a 
Date object, and vice versa. If the date was already stored as a Date object, 
it should be assumed that it has been properly converted.

Original comment by shredz...@gmail.com on 1 May 2011 at 10:33

GoogleCodeExporter commented 9 years ago
Hi Richard,

Thanks for the explanation.  This change has been committed in revision 68.  
I'll close this issue now, but please comment back if you do not believe it's 
been addressed adequately.

I'll get a released version out soon, but for now if you'd like this feature 
then please work from the SVN head.

Drew.

Original comment by drewnoakes on 3 May 2011 at 2:24

GoogleCodeExporter commented 9 years ago
The change looks good. Thank you! :-)

Original comment by shredz...@gmail.com on 3 May 2011 at 9:34