<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
Hi Laurent,<br>
<br>
<div class="moz-cite-prefix">On 9/4/20 9:04 PM, Laurent Pinchart
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:20200904153429.GE7518@pendragon.ideasonboard.com">
<pre class="moz-quote-pre" wrap="">Hi Umang,
Thank you for the patch.
On Fri, Sep 04, 2020 at 12:10:19PM +0530, Umang Jain wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Create a Exif object with various metadata tags set, just before
the encoder starts to encode the frame. The object is passed
directly as libcamera::Span<> to make sure EXIF tags can be set
in a single place i.e. in CameraDevice and the encoder only has
the job to write the data in the final output.
Signed-off-by: Kieran Bingham <a class="moz-txt-link-rfc2396E" href="mailto:kieran.bingham@ideasonboard.com"><kieran.bingham@ideasonboard.com></a>
Signed-off-by: Umang Jain <a class="moz-txt-link-rfc2396E" href="mailto:email@uajain.com"><email@uajain.com></a>
Reviewed-by: Kieran Bingham <a class="moz-txt-link-rfc2396E" href="mailto:kieran.bingham@ideasonboard.com"><kieran.bingham@ideasonboard.com></a>
Reviewed-by: Laurent Pinchart <a class="moz-txt-link-rfc2396E" href="mailto:laurent.pinchart@ideasonboard.com"><laurent.pinchart@ideasonboard.com></a>
---
src/android/camera_device.cpp | 19 ++++++++++-
src/android/jpeg/encoder.h | 3 +-
src/android/jpeg/encoder_libjpeg.cpp | 9 ++++-
src/android/jpeg/encoder_libjpeg.h | 3 +-
src/android/jpeg/exif.cpp | 50 ++++++++++++++++++++++++++++
src/android/jpeg/exif.h | 8 +++++
6 files changed, 88 insertions(+), 4 deletions(-)
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index de6f86f..0328ac6 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -24,6 +24,7 @@
#include "system/graphics.h"
#include "jpeg/encoder_libjpeg.h"
+#include "jpeg/exif.h"
using namespace libcamera;
@@ -1434,7 +1435,23 @@ void CameraDevice::requestComplete(Request *request)
continue;
}
- int jpeg_size = encoder->encode(buffer, mapped.maps()[0]);
+ /* Set EXIF metadata for various tags. */
+ Exif exif;
+ /* \todo Set Make and Model from external vendor tags. */
+ exif.setMake("libcamera");
+ exif.setModel("cameraModel");
+ exif.setOrientation(orientation_);
+ exif.setSize(cameraStream->size);
+ /*
+ * We set the frame's EXIF timestamp as the time of encode.
+ * Since the precision we need for EXIF timestamp is only one
+ * second, it is good enough.
+ */
+ exif.setTimestamp(std::time(nullptr));
+ if (exif.generate() != 0)
+ LOG(HAL, Error) << "Failed to get valid EXIF data";
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
s/get/generate/ ?
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+
+ int jpeg_size = encoder->encode(buffer, mapped.maps()[0], exif.data());
if (jpeg_size < 0) {
LOG(HAL, Error) << "Failed to encode stream image";
status = CAMERA3_BUFFER_STATUS_ERROR;
diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h
index f9eb88e..cf26d67 100644
--- a/src/android/jpeg/encoder.h
+++ b/src/android/jpeg/encoder.h
@@ -18,7 +18,8 @@ public:
virtual int configure(const libcamera::StreamConfiguration &cfg) = 0;
virtual int encode(const libcamera::FrameBuffer *source,
- const libcamera::Span<uint8_t> &destination) = 0;
+ const libcamera::Span<uint8_t> &destination,
+ const libcamera::Span<const uint8_t> &exifData) = 0;
};
#endif /* __ANDROID_JPEG_ENCODER_H__ */
diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp
index 977538c..510613c 100644
--- a/src/android/jpeg/encoder_libjpeg.cpp
+++ b/src/android/jpeg/encoder_libjpeg.cpp
@@ -180,7 +180,8 @@ void EncoderLibJpeg::compressNV(const libcamera::MappedBuffer *frame)
}
int EncoderLibJpeg::encode(const FrameBuffer *source,
- const libcamera::Span<uint8_t> &dest)
+ const libcamera::Span<uint8_t> &dest,
+ const libcamera::Span<const uint8_t> &exifData)
{
MappedFrameBuffer frame(source, PROT_READ);
if (!frame.isValid()) {
@@ -204,6 +205,12 @@ int EncoderLibJpeg::encode(const FrameBuffer *source,
jpeg_start_compress(&compress_, TRUE);
+ if (exifData.size())
+ /* Store Exif data in the JPEG_APP1 data block. */
+ jpeg_write_marker(&compress_, JPEG_APP0 + 1,
+ static_cast<const JOCTET *>(exifData.data()),
+ exifData.size());
+
LOG(JPEG, Debug) << "JPEG Encode Starting:" << compress_.image_width
<< "x" << compress_.image_height;
diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h
index aed081a..1e8df05 100644
--- a/src/android/jpeg/encoder_libjpeg.h
+++ b/src/android/jpeg/encoder_libjpeg.h
@@ -22,7 +22,8 @@ public:
int configure(const libcamera::StreamConfiguration &cfg) override;
int encode(const libcamera::FrameBuffer *source,
- const libcamera::Span<uint8_t> &destination) override;
+ const libcamera::Span<uint8_t> &destination,
+ const libcamera::Span<const uint8_t> &exifData) override;
private:
void compressRGB(const libcamera::MappedBuffer *frame);
diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
index 41ddf48..36922ef 100644
--- a/src/android/jpeg/exif.cpp
+++ b/src/android/jpeg/exif.cpp
@@ -168,6 +168,56 @@ void Exif::setString(ExifIfd ifd, ExifTag tag, ExifFormat format, const std::str
exif_entry_unref(entry);
}
+void Exif::setMake(const std::string &make)
+{
+ setString(EXIF_IFD_0, EXIF_TAG_MAKE, EXIF_FORMAT_ASCII, make);
+}
+
+void Exif::setModel(const std::string &model)
+{
+ setString(EXIF_IFD_0, EXIF_TAG_MODEL, EXIF_FORMAT_ASCII, model);
+}
+
+void Exif::setSize(const Size &size)
+{
+ setLong(EXIF_IFD_EXIF, EXIF_TAG_PIXEL_Y_DIMENSION, size.height);
+ setLong(EXIF_IFD_EXIF, EXIF_TAG_PIXEL_X_DIMENSION, size.width);
+}
+
+void Exif::setTimestamp(time_t timestamp)
+{
+ char str[20];
+ std::strftime(str, sizeof(str), "%Y:%m:%d %H:%M:%S",
+ std::localtime(×tamp));
+ std::string ts(str);
+
+ 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);
+}
+
+void Exif::setOrientation(int orientation)
+{
+ int value;
+ switch (orientation) {
+ case 0:
+ default:
+ value = 1;
+ break;
+ case 90:
+ value = 6;
+ break;
+ case 180:
+ value = 3;
+ break;
+ case 270:
+ value = 8;
+ break;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Shouldn't the the 90 and 270 values be swapped ? 6 means "The 0th row is
the visual right-hand side of the image, and the 0th column is the
visual top", and 8 means "The 0th row is the visual left-hand side of
the image, and the 0th column is the visual bottom".
Looking at the rotation property definition, the 90° rotation examples
shows
+-------------------------------------+
| _ _ |
| \ / |
| | | |
| | | |
| | > |
| < | |
| | | |
| . |
| V |
+-------------------------------------+
where the 0th row is the left side and the 0th column the bottom side.
I can fix those two small issues when applying, but I'd appreciate if
someone could double-check the rotation.</pre>
</blockquote>
Does the rotation property has to do anything with the fact that,
camera sensor(webcam use-case) is intentionally mounted upside down
to avoid rotation correction? A widely used example on the internet
is a big 'F' image denoting the value according to it's orientation
[1], which is the same as per the values used in the patch. On the
other hand, I can also find the code which satisfies Laurent's
review and the EXIF 'orientation' specification [2]<br>
<br>
[1]: <a
href="https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/camera/common/exif_utils.cc#376">https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/camera/common/exif_utils.cc#376</a><br>
[2]: <a
href="https://piexif.readthedocs.io/en/latest/sample.html#rotate-image-by-exif-orientation">https://piexif.readthedocs.io/en/latest/sample.html#rotate-image-by-exif-orientation</a><br>
<br>
<span style="color: rgb(0, 0, 0); font-size: 16px; font-style:
normal; font-variant-ligatures: normal; font-variant-caps: normal;
font-weight: 400; letter-spacing: normal; text-align: start;
text-indent: 0px; text-transform: none; white-space: normal;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
text-decoration-style: initial; text-decoration-color: initial;
display: inline !important; float: none;">¯\_(ツ)_/¯</span>
<blockquote type="cite"
cite="mid:20200904153429.GE7518@pendragon.ideasonboard.com">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ }
+
+ setShort(EXIF_IFD_0, EXIF_TAG_ORIENTATION, value);
+}
+
[[nodiscard]] int Exif::generate()
{
if (exifData_) {
diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h
index 8dfc324..622de4c 100644
--- a/src/android/jpeg/exif.h
+++ b/src/android/jpeg/exif.h
@@ -12,6 +12,7 @@
#include <libexif/exif-data.h>
+#include <libcamera/geometry.h>
#include <libcamera/span.h>
class Exif
@@ -20,6 +21,13 @@ public:
Exif();
~Exif();
+ void setMake(const std::string &make);
+ void setModel(const std::string &model);
+
+ void setOrientation(int orientation);
+ void setSize(const libcamera::Size &size);
+ void setTimestamp(time_t timestamp);
+
libcamera::Span<const uint8_t> data() const { return { exifData_, size_ }; }
[[nodiscard]] int generate();
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
</pre>
</blockquote>
<br>
</body>
</html>