[libcamera-devel] [PATCH v6 2/2] android: jpeg: Support an initial set of EXIF metadata tags
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Sep 4 17:34:29 CEST 2020
Hi Umang,
Thank you for the patch.
On Fri, Sep 04, 2020 at 12:10:19PM +0530, Umang Jain wrote:
> 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 <kieran.bingham at ideasonboard.com>
> Signed-off-by: Umang Jain <email at uajain.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> 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";
s/get/generate/ ?
> +
> + 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;
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.
> + }
> +
> + 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();
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list