[libcamera-devel] [RFC v1 PATCH] android: jpeg: exif: Set timezone information

[Umang Jain]

Hi,

On 9/22/20 9:56 PM, Umang Jain wrote:
> Get timezone information from the timestamp, although the resolution
> for EXIF_TAG_TIME_ZONE_OFFSET is fairly limited (per-hour only).
>
> Experimentation with 'exiftool', commandline utility to read/write
> exif metadata on images, resulted in rounding off the hours if the
> minutes came out to >= 30. Hence, the behaviour is inspired from
> exiftool itself. For e.g.,
>
> Timezone      Tag's value
>   +1015     =>     10
>   +0945     =>     10
>   -1145     =>    -12
>
> The EXIF specification defines three other tags (OffsetTime,
> OffsetTimeOriginal, OffsetTimeDigitized), in the EXIF IFD.
> These are not supported by libexif yet.
>
> Signed-off-by: Umang Jain <email at uajain.com>
> ---
>   src/android/jpeg/exif.cpp | 31 +++++++++++++++++++++++++++++++
>   src/android/jpeg/exif.h   |  1 +
>   2 files changed, 32 insertions(+)
>
> ---
>
> Hi Laurent/Kieran,
>
> Can you soft review(or test) this patch to see if anything
> is wrong with it?
> The reason I am asking, because I cannot make it work :(
> exiftool with a captured frame shows empty Timezone Offset tag as:
> ```
> Time Zone Offset                :
> ```
>
> exif tool shows nothing.
>
> I made sure a valid value being generated for timezone
> offset in setTimeStamp().
> I also checked the case where it might be preset using
> exif_data_fix and not been able to re-set again.
> But no, that doesn't seem the problem in my testing.
>
>
> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
> index c0dbfcc..9c23cfb 100644
> --- a/src/android/jpeg/exif.cpp
> +++ b/src/android/jpeg/exif.cpp
> @@ -7,6 +7,8 @@
>   
>   #include "exif.h"
>   
> +#include <stdlib.h>
> +
>   #include "libcamera/internal/log.h"
>   
>   using namespace libcamera;
> @@ -135,6 +137,16 @@ void Exif::setShort(ExifIfd ifd, ExifTag tag, uint16_t item)
>   	exif_entry_unref(entry);
>   }
>   
> +void Exif::setSShort(ExifIfd ifd, ExifTag tag, int16_t item)
> +{
> +	ExifEntry *entry = createEntry(ifd, tag);
> +	if (!entry)
> +		return;
> +
> +	exif_set_sshort(entry->data, EXIF_BYTE_ORDER_INTEL, item);
Getting to bottom of this in the morning, exif_set_sshort seems to 
convert its passed int16_t value to character. I don't know what's the 
logic behind this [1] (apart from EXIF's quirky-ness), but this simply 
doesn't look right then. So..
> +	exif_entry_unref(entry);
> +}
> +
>   void Exif::setLong(ExifIfd ifd, ExifTag tag, uint32_t item)
>   {
>   	ExifEntry *entry = createEntry(ifd, tag);
> @@ -196,6 +208,25 @@ void Exif::setTimestamp(time_t timestamp)
>   	setString(EXIF_IFD_0, EXIF_TAG_DATE_TIME, EXIF_FORMAT_ASCII, ts);
>   	setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_ORIGINAL, EXIF_FORMAT_ASCII, ts);
>   	setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_DIGITIZED, EXIF_FORMAT_ASCII, ts);
> +
> +	/*
> +	 * If possible, query and set timezone information via
> +	 * EXIF_TAG_TIME_ZONE_OFFSET. There is only per-hour resolution for tag
> +	 * hence, round up hours if minutes >= 30.
> +	 */
> +	int r = strftime(str, sizeof(str), "%z", &tm);
> +	if (r > 0) {
> +		int16_t timezone = atoi(str);
> +		int16_t hour = timezone / 100;
> +		int16_t min = timezone % 100;;
Complier should have errored out on ;; , but it didn't.
Rectified locally on noticing.
> +
> +		if (min <= -30)
> +			hour--;
> +		else if (min >= 30)
> +			hour++;
> +
> +		setSShort(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, hour);
Instead of setSShort call, if we use setString, things seems to get 
moving. Something like:
setString(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, EXIF_FORMAT_ASCII, "6");

got parsed correctly by exiftool and exif commandline utilities.

[1]: 
https://github.com/libexif/libexif/blob/master/libexif/exif-utils.c#L107-L121 

> +	}
>   }
>   
>   void Exif::setOrientation(int orientation)
> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h
> index f04cefc..9c9cc3b 100644
> --- a/src/android/jpeg/exif.h
> +++ b/src/android/jpeg/exif.h
> @@ -37,6 +37,7 @@ private:
>   			       unsigned long components, unsigned int size);
>   
>   	void setShort(ExifIfd ifd, ExifTag tag, uint16_t item);
> +	void setSShort(ExifIfd ifd, ExifTag tag, int16_t item);
>   	void setLong(ExifIfd ifd, ExifTag tag, uint32_t item);
>   	void setString(ExifIfd ifd, ExifTag tag, ExifFormat format,
>   		       const std::string &item);