Closed Bug 819835 Opened 7 years ago Closed 7 years ago
[Camera] Date & time data in EXIF is not correct
All my photos taken by my dogfooding phone(unagi) have Date and Time (Origi|2002:12:08 12:00:00 Date and Time (Digit|2002:12:08 12:00:00 in the EXIF data.
blocking-basecamp: ? → -
Priority: -- → P4
dale, I think this is yours--updatePosition() needs to stuff in the position.timestamp field (in seconds from January 1, 1970) and optionally the altitude (in metres). It looks like a simple change--if you're too busy, I can make it.
Assignee: nobody → dale
Yeh I am good with the one, cheers
Component: General → Gaia::Camera
QA Contact: jhammink
Just to update, the camera now specifies the timestamp (of the geolocation fix) in the options, the exif data needs to be seperately specified probably inside the CameraControl code, leaving it assigned to me though
dale, did you still want to tackle this one? If not, I can look at it.
TEF? tracking-b2g18? many users will rely on EXIF information to keep track of photo taken dates and organize photos. All photos show 2012/12/08 12:00 isn't useful
I agree with Joe's nomination. Photo geeks and organizations depend on proper date taken in the EXIF. Normal people not so much initially when they get their first digital camera or cameraphone; after they have taken thousands of photos they do care because they have to organize their photos!
Triage: TEF-, tracking-b2g18+, but won't block ship due to this. Really would like to take a patch though
Ref. KEY_GPS_TIMESTAMP vs. KEY_EXIF_DATETIME. According to CameraParameters.h, the former is "UTC in seconds since January 1, 1970"; according to Camera.java, so is the latter. Easy enough.
daleharvey, what format would you like a 'dateTime' property in? It turns out the documentation in Camera.java is wrong, and KEY_EXIF_DATETIME just writes a 19-character string (ostensibly of the format "YYYY:MM:DD HH:MM:SS") into the corresponding EXIF header field. Date and Time (Origi|9221120237041090560 Date and Time (Digit|2002:12:08 12:00:00 I'm still not sure how to get the "Digitized" field filled in, but it looks like the same format.
FYI, it looks like the EXIF header stores the GPS_TIMESTAMP (passed as a double) in a hybrid format, where the date portion is stored as "YYYY:MM:DD", but the time portion is stored as an EXIF RATIONAL value (i.e. as separate numerators and denominators).
Currently this takes a timestamp in seconds from UTC January 1, 1970, and stores it as a local time string in the EXIF header. If this timestamp is not specified, the GPS timestamp is used in its place. This may change, depending on feedback from Gaia team.
Cheers for picking this up mike, Looks good, will give it a test asap (it doesnt merge against b2g18, should central be ok?) I dont think we should fallback to GPS timestamp, there is no reason for the app to not provide a timestamp and seems like it would hide possible bugs if the timestamp for autopopulated with seperate data.
sotaro, can you look this over, with particular attention to the proper use of localtime_r() and strftime()?
Removing errno/strerror calls from strftime() handler; according to , they aren't set. 1. http://pubs.opengroup.org/onlinepubs/009695399/functions/strftime.html
(In reply to Mike Habicher [:mikeh] from comment #9) > daleharvey, what format would you like a 'dateTime' property in? It turns > out the documentation in Camera.java is wrong, and KEY_EXIF_DATETIME just > writes a 19-character string (ostensibly of the format "YYYY:MM:DD > HH:MM:SS") into the corresponding EXIF header field. > It's strange, normal android source code do not have such function. It might be qcom dependent function and parameter.
(In reply to Sotaro Ikeda [:sotaro] from comment #15) > (In reply to Mike Habicher [:mikeh] from comment #9) > It's strange, normal android source code do not have such function. > It might be qcom dependent function and parameter. I understand that KEY_EXIF_DATETIME is added by qcom. qcom's API is inconsistent to KEY_GPS_TIMESTAMP. Then changed the parameter consistent to gps timestamp. Other non-qcom hw might not support KEY_EXIF_DATETIME.
Comment on attachment 703934 [details] [diff] [review] add support for specifying the EXIF DateTime field (v3) No problem about how to use of localtime_r() and strftime().
Attachment #703934 - Flags: review?(sotaro.ikeda.g) → review+
Comment on attachment 703934 [details] [diff] [review] add support for specifying the EXIF DateTime field (v3) [Approval Request Comment] Bug caused by (feature/regressing bug #): with setting dateTime, the EXIF header in the image always shows "2002:12:08 12:00:00", even if the GPS timestamp is set. User impact if declined: users may not be able to sort images based on EXIF data Testing completed: set .dateTime option when taking photo to X seconds from epoch UTC, verified that time string that appears in the EXIF header corresponds to equivalent local time (based on the current timezone settings of the DUT). Risk to taking this patch (and alternatives if risky): very low String or UUID changes made by this patch: none
Also, for Testing completed: I also verified that the code handles 0 and >32-bit values. The former case generates a time string around epoch (1969, actually, due to time zone differences); the latter overflows the bionic representation of time_t, which triggers an error in the logcat with no setting applied to the EXIF data.
This patch breaks B2G Arm opt, B2G Arm debug, and B2G Panda opt, because the 'KEY_EXIF_DATETIME' symbol doesn't exist on those platforms. Nevermind. Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/19b3551b0058 A fix does not exist for this problem that can be cleanly applied to unagi and !unagi.
Changed KEY_EXIF_DATETIME to raw "exif-datetime" string to not break !unagi builds. [Approval Request Comment] See comment 18.
Created bug 832494 to track eventual removal of the raw string.
try-server push: https://tbpl.mozilla.org/?tree=Try&rev=d3333019035a
Comment on attachment 704089 [details] [diff] [review] add support for specifying the EXIF DateTime field (v4), r=sotaro Mike told me that he thinks taking this patch is worth it and not risky. He's "tested the change with a variety of inputs, and it's solid ... this change doesn't fix any crashes, so whether or not it makes it to tef+ is less critical."
Comment on attachment 704089 [details] [diff] [review] add support for specifying the EXIF DateTime field (v4), r=sotaro low risk, high win on user experience so we'll take this on branches, please land to tip of mozilla-b2g18 before 1/25 as per https://wiki.mozilla.org/Release_Management/B2G_Landing
Attachment #704089 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
Re-try, just in case: https://tbpl.mozilla.org/?tree=Try&rev=5eaa06c03be2
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Over to Gaia for the field-stuffing.
You need to log in before you can comment on or make changes to this bug.