[libcamera-devel] [PATCH v3 3/8] android: jpeg: exif: Add functions for setting various values

Laurent Pinchart laurent.pinchart at ideasonboard.com
Sat Jan 23 09:58:26 CET 2021


Hi Paul,

Thank you for the patch.

On Sat, Jan 23, 2021 at 02:16:59PM +0900, Paul Elder wrote:
> Add functions for setting the following EXIF fields:
> 
> - GPSDatestamp
> - GPSTimestamp
> - GPSLocation
>   - GPSLatitudeRef
>   - GPSLatitude
>   - GPSLongitudeRef
>   - GPSLongitude
>   - GPSAltitudeRef
>   - GPSAltitude
> - GPSProcessingMethod
> - FocalLength
> - ExposureTime
> - FNumber
> - ISO
> - Flash
> - WhiteBalance
> - SubsecTime
> - SubsecTimeOriginal
> - SubsecTimeDigitized
> 
> These are in preparation for fixing the following CTS tests:
> 
> - android.hardware.camera2.cts.StillCaptureTest#testFocalLengths
> - android.hardware.camera2.cts.StillCaptureTest#testJpegExif
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> ---
> Changes in v3:
> - merge setting subsec into setting the main timestamp
> 
> Changes in v2:
> - some cosmetic and precision changes
> - use the new setString
> ---
>  src/android/jpeg/exif.cpp | 175 +++++++++++++++++++++++++++++++++++---
>  src/android/jpeg/exif.h   |  38 ++++++++-
>  2 files changed, 198 insertions(+), 15 deletions(-)
> 
> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
> index d4f86006..06cb4808 100644
> --- a/src/android/jpeg/exif.cpp
> +++ b/src/android/jpeg/exif.cpp
> @@ -7,7 +7,9 @@
>  
>  #include "exif.h"
>  
> +#include <cmath>
>  #include <map>
> +#include <tuple>
>  #include <uchar.h>
>  
>  #include "libcamera/internal/log.h"
> @@ -151,6 +153,16 @@ ExifEntry *Exif::createEntry(ExifIfd ifd, ExifTag tag, ExifFormat format,
>  	return entry;
>  }
>  
> +void Exif::setByte(ExifIfd ifd, ExifTag tag, uint8_t item)
> +{
> +	ExifEntry *entry = createEntry(ifd, tag, EXIF_FORMAT_BYTE, 1, 1);
> +	if (!entry)
> +		return;
> +
> +	entry->data[0] = item;
> +	exif_entry_unref(entry);
> +}
> +
>  void Exif::setShort(ExifIfd ifd, ExifTag tag, uint16_t item)
>  {
>  	ExifEntry *entry = createEntry(ifd, tag);
> @@ -268,7 +280,7 @@ void Exif::setSize(const Size &size)
>  	setLong(EXIF_IFD_EXIF, EXIF_TAG_PIXEL_X_DIMENSION, size.width);
>  }
>  
> -void Exif::setTimestamp(time_t timestamp)
> +void Exif::setTimestamp(time_t timestamp, uint64_t subsec)

Could we rename subsec to state the unit explicitly ? nsecs if it's
expressed in nanoseconds for instance ? And as I assume you won't need a
higher precision than nanoseconds, uint32_t should be enough.

Another option would be to use

void Exif::setTimestamp(time_t timestamp, std::chrono::nanoseconds nsecs)

for additional type safety. You can then call the function with a ns
literal (see https://en.cppreference.com/w/cpp/chrono/operator%22%22ns):

	exif.setTimestamp(..., 0ns);

Calling it with

	exif.setTimestamp(..., 0);

would work, as the corresponding std::chrono::duration is explicit.

Does CTS require any specific precision ? If not, nanoseconds may be a
bit too much possibly ? Up to you.

>  {
>  	struct tm tm;
>  	localtime_r(&timestamp, &tm);
> @@ -283,19 +295,121 @@ void Exif::setTimestamp(time_t timestamp)
>  
>  	/* Query and set timezone information if available. */
>  	int r = strftime(str, sizeof(str), "%z", &tm);
> -	if (r > 0) {
> -		std::string tz(str);
> -		tz.insert(3, 1, ':');
> -		setString(EXIF_IFD_EXIF,
> -			  static_cast<ExifTag>(_ExifTag::OFFSET_TIME),
> -			  EXIF_FORMAT_ASCII, tz);
> -		setString(EXIF_IFD_EXIF,
> -			  static_cast<ExifTag>(_ExifTag::OFFSET_TIME_ORIGINAL),
> -			  EXIF_FORMAT_ASCII, tz);
> -		setString(EXIF_IFD_EXIF,
> -			  static_cast<ExifTag>(_ExifTag::OFFSET_TIME_DIGITIZED),
> -			  EXIF_FORMAT_ASCII, tz);
> -	}
> +	if (r <= 0)
> +		return;
> +
> +	std::string tz(str);
> +	tz.insert(3, 1, ':');
> +	setString(EXIF_IFD_EXIF,
> +		  static_cast<ExifTag>(_ExifTag::OFFSET_TIME),
> +		  EXIF_FORMAT_ASCII, tz);
> +	setString(EXIF_IFD_EXIF,
> +		  static_cast<ExifTag>(_ExifTag::OFFSET_TIME_ORIGINAL),
> +		  EXIF_FORMAT_ASCII, tz);
> +	setString(EXIF_IFD_EXIF,
> +		  static_cast<ExifTag>(_ExifTag::OFFSET_TIME_DIGITIZED),
> +		  EXIF_FORMAT_ASCII, tz);
> +

	std::string subsec = std::to_string(nsecs);

to avoid computing for every call ?

I got confused previously, but I think you still need to pad wih 0's on
the left. The example given in the Exif documentation encodes September
9, 1998, 9:15:30.130 as "1998:09:01 09:15:30\0" for DateTime (quite
obviously incorrect btw, the date should be 1998:09:09), and "130\0" for
SubsecTime. The unit isn't specified, but assuming milliseconds to match
the example, 09:15:30.012 should then be encoded as "012\0" in
SubsecTime, while the call to std:to_string() will turn 12 into "12\0".

> +	setString(EXIF_IFD_EXIF, EXIF_TAG_SUB_SEC_TIME,
> +		  EXIF_FORMAT_ASCII, std::to_string(subsec));
> +	setString(EXIF_IFD_EXIF, EXIF_TAG_SUB_SEC_TIME_ORIGINAL,
> +		  EXIF_FORMAT_ASCII, std::to_string(subsec));
> +	setString(EXIF_IFD_EXIF, EXIF_TAG_SUB_SEC_TIME_DIGITIZED,
> +		  EXIF_FORMAT_ASCII, std::to_string(subsec));
> +}
> +
> +void Exif::setGPSDateTimestamp(time_t timestamp)
> +{
> +	struct tm tm;
> +	gmtime_r(&timestamp, &tm);
> +
> +	char str[11];
> +	strftime(str, sizeof(str), "%Y:%m:%d", &tm);
> +	std::string tsStr(str);
> +
> +	setString(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_DATE_STAMP),
> +		  EXIF_FORMAT_ASCII, tsStr);
> +
> +	/* Set GPS_TIME_STAMP */
> +	ExifEntry *entry =
> +		createEntry(EXIF_IFD_GPS,
> +			    static_cast<ExifTag>(EXIF_TAG_GPS_TIME_STAMP),
> +			    EXIF_FORMAT_RATIONAL, 3, 3 * sizeof(ExifRational));
> +	if (!entry)
> +		return;
> +
> +	ExifRational ts[] = {
> +		{ static_cast<ExifLong>(tm.tm_hour), 1 },
> +		{ static_cast<ExifLong>(tm.tm_min),  1 },
> +		{ static_cast<ExifLong>(tm.tm_sec),  1 },
> +	};
> +
> +	for (int i = 0; i < 3; i++)
> +		exif_set_rational(entry->data + i * sizeof(ExifRational),
> +				  order_, ts[i]);
> +
> +	exif_entry_unref(entry);
> +}
> +
> +std::tuple<int, int, int> Exif::degreesToDMS(double decimalDegrees)
> +{
> +	int degrees = std::trunc(decimalDegrees);
> +	double minutes = std::abs((decimalDegrees - degrees) * 60);
> +	double seconds = (minutes - std::trunc(minutes)) * 60;
> +
> +	return { degrees, std::trunc(minutes), std::round(seconds) };
> +}
> +
> +void Exif::setGPSDMS(ExifIfd ifd, ExifTag tag, int deg, int min, int sec)
> +{
> +	ExifEntry *entry = createEntry(ifd, tag, EXIF_FORMAT_RATIONAL, 3,
> +				       3 * sizeof(ExifRational));
> +	if (!entry)
> +		return;
> +
> +	ExifRational coords[] = {
> +		{ static_cast<ExifLong>(deg), 1 },
> +		{ static_cast<ExifLong>(min), 1 },
> +		{ static_cast<ExifLong>(sec), 1 },
> +	};
> +
> +	for (int i = 0; i < 3; i++)
> +		exif_set_rational(entry->data + i * sizeof(ExifRational),
> +				  order_, coords[i]);
> +	exif_entry_unref(entry);
> +}
> +
> +/*
> + * \brief Set GPS location (lat, long, alt)
> + * \param[in] coords Pointer to coordinates latitude, longitude, and altitude,
> + *            first two in degrees, the third in meters
> + */
> +void Exif::setGPSLocation(const double *coords)
> +{
> +	int deg, min, sec;
> +
> +	std::tie<int, int, int>(deg, min, sec) = degreesToDMS(coords[0]);
> +	setString(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_LATITUDE_REF),
> +		  EXIF_FORMAT_ASCII, deg >= 0 ? "N" : "S");
> +	setGPSDMS(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_LATITUDE),
> +		  std::abs(deg), min, sec);
> +
> +	std::tie<int, int, int>(deg, min, sec) = degreesToDMS(coords[1]);
> +	setString(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_LONGITUDE_REF),
> +		  EXIF_FORMAT_ASCII, deg >= 0 ? "E" : "W");
> +	setGPSDMS(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_LATITUDE),
> +		  std::abs(deg), min, sec);
> +
> +	setByte(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_ALTITUDE_REF),
> +		coords[2] >= 0 ? 0 : 1);
> +	setRational(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_ALTITUDE),
> +		    ExifRational{ static_cast<ExifLong>(std::abs(coords[2])), 1 });
> +}
> +
> +void Exif::setGPSMethod(const std::string &method)
> +{
> +	setString(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_PROCESSING_METHOD),
> +		  EXIF_FORMAT_UNDEFINED, method, Unicode);
>  }
>  
>  void Exif::setOrientation(int orientation)
> @@ -334,6 +448,39 @@ void Exif::setThumbnail(Span<const unsigned char> thumbnail,
>  	setShort(EXIF_IFD_0, EXIF_TAG_COMPRESSION, compression);
>  }
>  
> +void Exif::setFocalLength(float length)
> +{
> +	ExifRational rational = { static_cast<ExifLong>(length * 1000), 1000 };
> +	setRational(EXIF_IFD_EXIF, EXIF_TAG_FOCAL_LENGTH, rational);
> +}
> +
> +void Exif::setExposureTime(uint64_t nsec)
> +{
> +	ExifRational rational = { static_cast<ExifLong>(nsec), 1000000000 };
> +	setRational(EXIF_IFD_EXIF, EXIF_TAG_EXPOSURE_TIME, rational);
> +}
> +
> +void Exif::setAperture(float size)
> +{
> +	ExifRational rational = { static_cast<ExifLong>(size * 10000), 10000 };
> +	setRational(EXIF_IFD_EXIF, EXIF_TAG_FNUMBER, rational);
> +}
> +
> +void Exif::setISO(uint16_t iso)
> +{
> +	setShort(EXIF_IFD_EXIF, EXIF_TAG_ISO_SPEED_RATINGS, iso);
> +}
> +
> +void Exif::setFlash(Flash flash)
> +{
> +	setShort(EXIF_IFD_EXIF, EXIF_TAG_FLASH, static_cast<ExifShort>(flash));
> +}
> +
> +void Exif::setWhiteBalance(WhiteBalance wb)
> +{
> +	setShort(EXIF_IFD_EXIF, EXIF_TAG_WHITE_BALANCE, static_cast<ExifShort>(wb));
> +}
> +
>  /**
>   * \brief Convert UTF-8 string to UTF-16 string
>   * \param[in] str String to convert
> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h
> index 73f231b2..7d8727ad 100644
> --- a/src/android/jpeg/exif.h
> +++ b/src/android/jpeg/exif.h
> @@ -26,6 +26,27 @@ public:
>  		JPEG = 6,
>  	};
>  
> +	enum Flash {
> +		/* bit 0 */
> +		Fired = 0x01,
> +		/* bits 1 and 2 */
> +		StrobeDetected = 0x04,
> +		StrobeNotDetected = 0x06,
> +		/* bits 3 and 4 */
> +		ModeCompulsoryFiring = 0x08,
> +		ModeCompulsorySuppression = 0x10,
> +		ModeAuto = 0x18,
> +		/* bit 5 */
> +		FlashNotPresent = 0x20,
> +		/* bit 6 */
> +		RedEye = 0x40,
> +	};
> +
> +	enum WhiteBalance {
> +		Auto = 0,
> +		Manual = 1,
> +	};
> +
>  	enum StringEncoding {
>  		NoEncoding = 0,
>  		ASCII = 1,
> @@ -39,7 +60,18 @@ public:
>  	void setSize(const libcamera::Size &size);
>  	void setThumbnail(libcamera::Span<const unsigned char> thumbnail,
>  			  Compression compression);
> -	void setTimestamp(time_t timestamp);
> +	void setTimestamp(time_t timestamp, uint64_t subsec);

Shouldn't you also update the caller in this patch ?

> +
> +	void setGPSDateTimestamp(time_t timestamp);
> +	void setGPSLocation(const double *coords);
> +	void setGPSMethod(const std::string &method);
> +
> +	void setFocalLength(float length);
> +	void setExposureTime(uint64_t nsec);
> +	void setAperture(float size);
> +	void setISO(uint16_t iso);
> +	void setFlash(Flash flash);
> +	void setWhiteBalance(WhiteBalance wb);
>  
>  	libcamera::Span<const uint8_t> data() const { return { exifData_, size_ }; }
>  	[[nodiscard]] int generate();
> @@ -49,12 +81,16 @@ private:
>  	ExifEntry *createEntry(ExifIfd ifd, ExifTag tag, ExifFormat format,
>  			       unsigned long components, unsigned int size);
>  
> +	void setByte(ExifIfd ifd, ExifTag tag, uint8_t item);
>  	void setShort(ExifIfd ifd, ExifTag tag, uint16_t item);
>  	void setLong(ExifIfd ifd, ExifTag tag, uint32_t item);
>  	void setString(ExifIfd ifd, ExifTag tag, ExifFormat format,
>  		       const std::string &item, StringEncoding encoding = NoEncoding);
>  	void setRational(ExifIfd ifd, ExifTag tag, ExifRational item);
>  
> +	std::tuple<int, int, int> degreesToDMS(double decimalDegrees);
> +	void setGPSDMS(ExifIfd ifd, ExifTag tag, int deg, int min, int sec);
> +
>  	std::u16string utf8ToUtf16(const std::string &str);
>  
>  	bool valid_;

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list