[{"id":14779,"web_url":"https://patchwork.libcamera.org/comment/14779/","msgid":"","date":"2021-01-26T13:07:02","subject":"Re: [libcamera-devel] [PATCH v5 3/8] android: jpeg: exif: Add\n\tfunctions for setting various values","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Tue, Jan 26, 2021 at 07:28:20PM +0900, Paul Elder wrote:\n> Add functions for setting the following EXIF fields:\n> \n> - GPSDatestamp\n> - GPSTimestamp\n> - GPSLocation\n> - GPSLatitudeRef\n> - GPSLatitude\n> - GPSLongitudeRef\n> - GPSLongitude\n> - GPSAltitudeRef\n> - GPSAltitude\n> - GPSProcessingMethod\n> - FocalLength\n> - ExposureTime\n> - FNumber\n> - ISO\n> - Flash\n> - WhiteBalance\n> - SubsecTime\n> - SubsecTimeOriginal\n> - SubsecTimeDigitized\n> \n> These are in preparation for fixing the following CTS tests:\n> \n> - android.hardware.camera2.cts.StillCaptureTest#testFocalLengths\n> - android.hardware.camera2.cts.StillCaptureTest#testJpegExif\n> \n> Signed-off-by: Paul Elder \n> Reviewed-by: Laurent Pinchart \n> Reviewed-by: Jacopo Mondi \n> \n> ---\n> Changes in v5:\n> - fix number alignment in string in setting subsec values\n> - use ms operator from chrono for setting timestamp subsec\n> - other cosmetic changes\n> \n> Changes in v4:\n> - change type of subsec to std::chrono::milliseconds\n> \n> Changes in v3:\n> - merge setting subsec into setting the main timestamp\n> \n> Changes in v2:\n> - some cosmetic and precision changes\n> - use the new setString\n> ---\n> src/android/jpeg/exif.cpp | 181 +++++++++++++++++++++--\n> src/android/jpeg/exif.h | 39 ++++-\n> src/android/jpeg/post_processor_jpeg.cpp | 5 +-\n> 3 files changed, 209 insertions(+), 16 deletions(-)\n> \n> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp\n> index 204a237a..b4b3c0b7 100644\n> --- a/src/android/jpeg/exif.cpp\n> +++ b/src/android/jpeg/exif.cpp\n> @@ -7,7 +7,11 @@\n> \n> #include \"exif.h\"\n> \n> +#include \n> +#include \n> #include \n> +#include \n> +#include \n> #include \n> \n> #include \"libcamera/internal/log.h\"\n> @@ -151,6 +155,16 @@ ExifEntry *Exif::createEntry(ExifIfd ifd, ExifTag tag, ExifFormat format,\n> \treturn entry;\n> }\n> \n> +void Exif::setByte(ExifIfd ifd, ExifTag tag, uint8_t item)\n> +{\n> +\tExifEntry *entry = createEntry(ifd, tag, EXIF_FORMAT_BYTE, 1, 1);\n> +\tif (!entry)\n> +\t\treturn;\n> +\n> +\tentry->data[0] = item;\n> +\texif_entry_unref(entry);\n> +}\n> +\n> void Exif::setShort(ExifIfd ifd, ExifTag tag, uint16_t item)\n> {\n> \tExifEntry *entry = createEntry(ifd, tag);\n> @@ -267,7 +281,7 @@ void Exif::setSize(const Size &size)\n> \tsetLong(EXIF_IFD_EXIF, EXIF_TAG_PIXEL_X_DIMENSION, size.width);\n> }\n> \n> -void Exif::setTimestamp(time_t timestamp)\n> +void Exif::setTimestamp(time_t timestamp, std::chrono::milliseconds msec)\n> {\n> \tstruct tm tm;\n> \tlocaltime_r(×tamp, &tm);\n> @@ -282,19 +296,125 @@ void Exif::setTimestamp(time_t timestamp)\n> \n> \t/* Query and set timezone information if available. */\n> \tint r = strftime(str, sizeof(str), \"%z\", &tm);\n> -\tif (r > 0) {\n> -\t\tstd::string tz(str);\n> -\t\ttz.insert(3, 1, ':');\n> -\t\tsetString(EXIF_IFD_EXIF,\n> -\t\t\t static_cast(_ExifTag::OFFSET_TIME),\n> -\t\t\t EXIF_FORMAT_ASCII, tz);\n> -\t\tsetString(EXIF_IFD_EXIF,\n> -\t\t\t static_cast(_ExifTag::OFFSET_TIME_ORIGINAL),\n> -\t\t\t EXIF_FORMAT_ASCII, tz);\n> -\t\tsetString(EXIF_IFD_EXIF,\n> -\t\t\t static_cast(_ExifTag::OFFSET_TIME_DIGITIZED),\n> -\t\t\t EXIF_FORMAT_ASCII, tz);\n> -\t}\n> +\tif (r <= 0)\n> +\t\treturn;\n> +\n> +\tstd::string tz(str);\n> +\ttz.insert(3, 1, ':');\n> +\tsetString(EXIF_IFD_EXIF,\n> +\t\t static_cast(_ExifTag::OFFSET_TIME),\n> +\t\t EXIF_FORMAT_ASCII, tz);\n> +\tsetString(EXIF_IFD_EXIF,\n> +\t\t static_cast(_ExifTag::OFFSET_TIME_ORIGINAL),\n> +\t\t EXIF_FORMAT_ASCII, tz);\n> +\tsetString(EXIF_IFD_EXIF,\n> +\t\t static_cast(_ExifTag::OFFSET_TIME_DIGITIZED),\n> +\t\t EXIF_FORMAT_ASCII, tz);\n> +\n> +\tstd::stringstream subsec;\n> +\tsubsec << std::setfill('0') << std::setw(3) << std::to_string(msec.count());\n\nShouldn't you drop std::to_string() ?\n\n\tsubsec << std::setfill('0') << std::setw(3) << msec.count();\n\nYou could also catch the string instead of calling str() thrice.\n\n\tstd::stringstream sstr;\n\tsstr << std::setfill('0') << std::setw(3) << msec.count();\n\tstd::string subsec = subsec.str();\n\n> +\n> +\tsetString(EXIF_IFD_EXIF, EXIF_TAG_SUB_SEC_TIME,\n> +\t\t EXIF_FORMAT_ASCII, subsec.str());\n> +\tsetString(EXIF_IFD_EXIF, EXIF_TAG_SUB_SEC_TIME_ORIGINAL,\n> +\t\t EXIF_FORMAT_ASCII, subsec.str());\n> +\tsetString(EXIF_IFD_EXIF, EXIF_TAG_SUB_SEC_TIME_DIGITIZED,\n> +\t\t EXIF_FORMAT_ASCII, subsec.str());\n> +}\n> +\n> +void Exif::setGPSDateTimestamp(time_t timestamp)\n> +{\n> +\tstruct tm tm;\n> +\tgmtime_r(×tamp, &tm);\n> +\n> +\tchar str[11];\n> +\tstrftime(str, sizeof(str), \"%Y:%m:%d\", &tm);\n> +\tstd::string tsStr(str);\n> +\n> +\tsetString(EXIF_IFD_GPS, static_cast(EXIF_TAG_GPS_DATE_STAMP),\n> +\t\t EXIF_FORMAT_ASCII, tsStr);\n> +\n> +\t/* Set GPS_TIME_STAMP */\n> +\tExifEntry *entry =\n> +\t\tcreateEntry(EXIF_IFD_GPS,\n> +\t\t\t static_cast(EXIF_TAG_GPS_TIME_STAMP),\n> +\t\t\t EXIF_FORMAT_RATIONAL, 3, 3 * sizeof(ExifRational));\n> +\tif (!entry)\n> +\t\treturn;\n> +\n> +\tExifRational ts[] = {\n> +\t\t{ static_cast(tm.tm_hour), 1 },\n> +\t\t{ static_cast(tm.tm_min), 1 },\n> +\t\t{ static_cast(tm.tm_sec), 1 },\n> +\t};\n> +\n> +\tfor (int i = 0; i < 3; i++)\n> +\t\texif_set_rational(entry->data + i * sizeof(ExifRational),\n> +\t\t\t\t order_, ts[i]);\n> +\n> +\texif_entry_unref(entry);\n> +}\n> +\n> +std::tuple Exif::degreesToDMS(double decimalDegrees)\n> +{\n> +\tint degrees = std::trunc(decimalDegrees);\n> +\tdouble minutes = std::abs((decimalDegrees - degrees) * 60);\n> +\tdouble seconds = (minutes - std::trunc(minutes)) * 60;\n> +\n> +\treturn { degrees, std::trunc(minutes), std::round(seconds) };\n> +}\n> +\n> +void Exif::setGPSDMS(ExifIfd ifd, ExifTag tag, int deg, int min, int sec)\n> +{\n> +\tExifEntry *entry = createEntry(ifd, tag, EXIF_FORMAT_RATIONAL, 3,\n> +\t\t\t\t 3 * sizeof(ExifRational));\n> +\tif (!entry)\n> +\t\treturn;\n> +\n> +\tExifRational coords[] = {\n> +\t\t{ static_cast(deg), 1 },\n> +\t\t{ static_cast(min), 1 },\n> +\t\t{ static_cast(sec), 1 },\n> +\t};\n> +\n> +\tfor (int i = 0; i < 3; i++)\n> +\t\texif_set_rational(entry->data + i * sizeof(ExifRational),\n> +\t\t\t\t order_, coords[i]);\n> +\n> +\texif_entry_unref(entry);\n> +}\n> +\n> +/*\n> + * \\brief Set GPS location (lat, long, alt)\n> + * \\param[in] coords Pointer to coordinates latitude, longitude, and altitude,\n> + * first two in degrees, the third in meters\n> + */\n> +void Exif::setGPSLocation(const double *coords)\n> +{\n> +\tint deg, min, sec;\n> +\n> +\tstd::tie(deg, min, sec) = degreesToDMS(coords[0]);\n> +\tsetString(EXIF_IFD_GPS, static_cast(EXIF_TAG_GPS_LATITUDE_REF),\n> +\t\t EXIF_FORMAT_ASCII, deg >= 0 ? \"N\" : \"S\");\n> +\tsetGPSDMS(EXIF_IFD_GPS, static_cast(EXIF_TAG_GPS_LATITUDE),\n> +\t\t std::abs(deg), min, sec);\n> +\n> +\tstd::tie(deg, min, sec) = degreesToDMS(coords[1]);\n> +\tsetString(EXIF_IFD_GPS, static_cast(EXIF_TAG_GPS_LONGITUDE_REF),\n> +\t\t EXIF_FORMAT_ASCII, deg >= 0 ? \"E\" : \"W\");\n> +\tsetGPSDMS(EXIF_IFD_GPS, static_cast(EXIF_TAG_GPS_LATITUDE),\n> +\t\t std::abs(deg), min, sec);\n> +\n> +\tsetByte(EXIF_IFD_GPS, static_cast(EXIF_TAG_GPS_ALTITUDE_REF),\n> +\t\tcoords[2] >= 0 ? 0 : 1);\n> +\tsetRational(EXIF_IFD_GPS, static_cast(EXIF_TAG_GPS_ALTITUDE),\n> +\t\t ExifRational{ static_cast(std::abs(coords[2])), 1 });\n> +}\n> +\n> +void Exif::setGPSMethod(const std::string &method)\n> +{\n> +\tsetString(EXIF_IFD_GPS, static_cast(EXIF_TAG_GPS_PROCESSING_METHOD),\n> +\t\t EXIF_FORMAT_UNDEFINED, method, Unicode);\n> }\n> \n> void Exif::setOrientation(int orientation)\n> @@ -333,6 +453,39 @@ void Exif::setThumbnail(Span thumbnail,\n> \tsetShort(EXIF_IFD_0, EXIF_TAG_COMPRESSION, compression);\n> }\n> \n> +void Exif::setFocalLength(float length)\n> +{\n> +\tExifRational rational = { static_cast(length * 1000), 1000 };\n> +\tsetRational(EXIF_IFD_EXIF, EXIF_TAG_FOCAL_LENGTH, rational);\n> +}\n> +\n> +void Exif::setExposureTime(uint64_t nsec)\n> +{\n> +\tExifRational rational = { static_cast(nsec), 1000000000 };\n> +\tsetRational(EXIF_IFD_EXIF, EXIF_TAG_EXPOSURE_TIME, rational);\n> +}\n> +\n> +void Exif::setAperture(float size)\n> +{\n> +\tExifRational rational = { static_cast(size * 10000), 10000 };\n> +\tsetRational(EXIF_IFD_EXIF, EXIF_TAG_FNUMBER, rational);\n> +}\n> +\n> +void Exif::setISO(uint16_t iso)\n> +{\n> +\tsetShort(EXIF_IFD_EXIF, EXIF_TAG_ISO_SPEED_RATINGS, iso);\n> +}\n> +\n> +void Exif::setFlash(Flash flash)\n> +{\n> +\tsetShort(EXIF_IFD_EXIF, EXIF_TAG_FLASH, static_cast(flash));\n> +}\n> +\n> +void Exif::setWhiteBalance(WhiteBalance wb)\n> +{\n> +\tsetShort(EXIF_IFD_EXIF, EXIF_TAG_WHITE_BALANCE, static_cast(wb));\n> +}\n> +\n> /**\n> * \\brief Convert UTF-8 string to UTF-16 string\n> * \\param[in] str String to convert\n> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h\n> index 8b84165b..b0d61402 100644\n> --- a/src/android/jpeg/exif.h\n> +++ b/src/android/jpeg/exif.h\n> @@ -7,6 +7,7 @@\n> #ifndef __ANDROID_JPEG_EXIF_H__\n> #define __ANDROID_JPEG_EXIF_H__\n> \n> +#include \n> #include \n> #include \n> \n> @@ -26,6 +27,27 @@ public:\n> \t\tJPEG = 6,\n> \t};\n> \n> +\tenum Flash {\n> +\t\t/* bit 0 */\n> +\t\tFired = 0x01,\n> +\t\t/* bits 1 and 2 */\n> +\t\tStrobeDetected = 0x04,\n> +\t\tStrobeNotDetected = 0x06,\n> +\t\t/* bits 3 and 4 */\n> +\t\tModeCompulsoryFiring = 0x08,\n> +\t\tModeCompulsorySuppression = 0x10,\n> +\t\tModeAuto = 0x18,\n> +\t\t/* bit 5 */\n> +\t\tFlashNotPresent = 0x20,\n> +\t\t/* bit 6 */\n> +\t\tRedEye = 0x40,\n> +\t};\n> +\n> +\tenum WhiteBalance {\n> +\t\tAuto = 0,\n> +\t\tManual = 1,\n> +\t};\n> +\n> \tenum StringEncoding {\n> \t\tNoEncoding = 0,\n> \t\tASCII = 1,\n> @@ -39,7 +61,18 @@ public:\n> \tvoid setSize(const libcamera::Size &size);\n> \tvoid setThumbnail(libcamera::Span thumbnail,\n> \t\t\t Compression compression);\n> -\tvoid setTimestamp(time_t timestamp);\n> +\tvoid setTimestamp(time_t timestamp, std::chrono::milliseconds msec);\n> +\n> +\tvoid setGPSDateTimestamp(time_t timestamp);\n> +\tvoid setGPSLocation(const double *coords);\n> +\tvoid setGPSMethod(const std::string &method);\n> +\n> +\tvoid setFocalLength(float length);\n> +\tvoid setExposureTime(uint64_t nsec);\n> +\tvoid setAperture(float size);\n> +\tvoid setISO(uint16_t iso);\n> +\tvoid setFlash(Flash flash);\n> +\tvoid setWhiteBalance(WhiteBalance wb);\n> \n> \tlibcamera::Span data() const { return { exifData_, size_ }; }\n> \t[[nodiscard]] int generate();\n> @@ -49,6 +82,7 @@ private:\n> \tExifEntry *createEntry(ExifIfd ifd, ExifTag tag, ExifFormat format,\n> \t\t\t unsigned long components, unsigned int size);\n> \n> +\tvoid setByte(ExifIfd ifd, ExifTag tag, uint8_t item);\n> \tvoid setShort(ExifIfd ifd, ExifTag tag, uint16_t item);\n> \tvoid setLong(ExifIfd ifd, ExifTag tag, uint32_t item);\n> \tvoid setString(ExifIfd ifd, ExifTag tag, ExifFormat format,\n> @@ -56,6 +90,9 @@ private:\n> \t\t StringEncoding encoding = NoEncoding);\n> \tvoid setRational(ExifIfd ifd, ExifTag tag, ExifRational item);\n> \n> +\tstd::tuple degreesToDMS(double decimalDegrees);\n> +\tvoid setGPSDMS(ExifIfd ifd, ExifTag tag, int deg, int min, int sec);\n> +\n> \tstd::u16string utf8ToUtf16(const std::string &str);\n> \n> \tbool valid_;\n> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp\n> index 436a50f8..aa0f783d 100644\n> --- a/src/android/jpeg/post_processor_jpeg.cpp\n> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> @@ -7,6 +7,8 @@\n> \n> #include \"post_processor_jpeg.h\"\n> \n> +#include \n> +\n> #include \"../camera_device.h\"\n> #include \"../camera_metadata.h\"\n> #include \"encoder_libjpeg.h\"\n> @@ -17,6 +19,7 @@\n> #include \"libcamera/internal/log.h\"\n> \n> using namespace libcamera;\n> +using namespace std::chrono_literals;\n> \n> LOG_DEFINE_CATEGORY(JPEG)\n> \n> @@ -97,7 +100,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source,\n> \t * Since the precision we need for EXIF timestamp is only one\n> \t * second, it is good enough.\n> \t */\n> -\texif.setTimestamp(std::time(nullptr));\n> +\texif.setTimestamp(std::time(nullptr), 0ms);\n> \n> \tstd::vector thumbnail;\n> \tgenerateThumbnail(source, &thumbnail);","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0D9ECC0F2B\n\tfor ;\n\tTue, 26 Jan 2021 13:07:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 86F4868301;\n\tTue, 26 Jan 2021 14:07:23 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ED91B682EC\n\tfor ;\n\tTue, 26 Jan 2021 14:07:21 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 601802C1;\n\tTue, 26 Jan 2021 14:07:21 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"B9yHceUG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611666441;\n\tbh=Uj4v26wvzvuXiBemRjMzW/JbXzAaRWqB0LtVXCDzlKM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=B9yHceUGAHjkYrRaGL1TSXUb6cr4bfM3kT4ibuyjUTUAXHOiGv+oXpYVvnaZWf0Fq\n\tGeENPeAbSWy6td7W5wUJ0I9gW2WJSw2W8YHiIWnF63NYHvtfFf0aKZ113BYRr8yIse\n\tZW6eQycT2JUpW7w4xMlafOsNZvsZsgcQgyTBrbiw=","Date":"Tue, 26 Jan 2021 15:07:02 +0200","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"","References":"<20210126102825.147026-1-paul.elder@ideasonboard.com>\n\t<20210126102825.147026-4-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210126102825.147026-4-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 3/8] android: jpeg: exif: Add\n\tfunctions for setting various values","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]