[libcamera-devel] [PATCH v3 1/2] libcamera: android: Add EXIF infrastructure

[Kieran Bingham]

Hi Umang,

On 27/08/2020 10:49, Umang Jain wrote:
> From: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> Provide helper classes to utilise the libexif interfaces and link
> against libexif to support tag additions when creating JPEG images.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Signed-off-by: Umang Jain <email at uajain.com>
> ---
>  src/android/jpeg/exif.cpp | 172 ++++++++++++++++++++++++++++++++++++++
>  src/android/jpeg/exif.h   |  44 ++++++++++
>  src/android/meson.build   |   2 +
>  3 files changed, 218 insertions(+)
>  create mode 100644 src/android/jpeg/exif.cpp
>  create mode 100644 src/android/jpeg/exif.h
> 
> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
> new file mode 100644
> index 0000000..ce9c69b
> --- /dev/null
> +++ b/src/android/jpeg/exif.cpp
> @@ -0,0 +1,172 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * exif.cpp - EXIF tag creation using libexif
> + */
> +
> +#include "exif.h"
> +
> +#include "libcamera/internal/log.h"
> +
> +using namespace libcamera;
> +
> +LOG_DEFINE_CATEGORY(EXIF)
> +
> +Exif::Exif()
> +	: valid_(false), exif_data_(0), size_(0)
> +{
> +	/* Create an ExifMem allocator to construct entries. */
> +	mem_ = exif_mem_new_default();
> +	if (!mem_) {
> +		LOG(EXIF, Fatal) << "Failed to allocate ExifMem Allocator";
> +		return;
> +	}
> +
> +	data_ = exif_data_new_mem(mem_);
> +	if (!data_) {
> +		LOG(EXIF, Fatal) << "Failed to allocate an ExifData structure";
> +		return;
> +	}
> +
> +	valid_ = true;
> +
> +	exif_data_set_option(data_, EXIF_DATA_OPTION_FOLLOW_SPECIFICATION);
> +	exif_data_set_data_type(data_, EXIF_DATA_TYPE_COMPRESSED);
> +
> +	/*
> +	 * Big-Endian: EXIF_BYTE_ORDER_MOTOROLA
> +	 * Little Endian: EXIF_BYTE_ORDER_INTEL
> +	 */
> +	exif_data_set_byte_order(data_, EXIF_BYTE_ORDER_INTEL);
> +
> +	/* Create the mandatory EXIF fields with default data. */
> +	exif_data_fix(data_);
> +}
> +
> +Exif::~Exif()
> +{
> +	if (exif_data_)
> +		free(exif_data_);
> +
> +	if (data_)
> +		exif_data_unref(data_);
> +
> +	if (mem_)
> +		exif_mem_unref(mem_);
> +}
> +
> +ExifEntry *Exif::createEntry(ExifIfd ifd, ExifTag tag)
> +{
> +	ExifContent *content = data_->ifd[ifd];
> +	ExifEntry *entry = exif_content_get_entry(content, tag);
> +
> +	if (entry) {
> +		exif_entry_ref(entry);
> +		return entry;
> +	}
> +
> +	entry = exif_entry_new_mem(mem_);
> +	if (!entry) {
> +		LOG(EXIF, Fatal) << "Failed to allocated new entry";
> +		return nullptr;
> +	}
> +
> +	entry->tag = tag;
> +
> +	exif_content_add_entry(content, entry);
> +	exif_entry_initialize(entry, tag);
> +
> +	return entry;
> +}
> +
> +ExifEntry *Exif::createEntry(ExifIfd ifd, ExifTag tag, ExifFormat format,
> +			     uint64_t components, unsigned int size)
> +{
> +	ExifContent *content = data_->ifd[ifd];
> +
> +	/* Replace any existing entry with the same tag. */
> +	ExifEntry *existing = exif_content_get_entry(content, tag);
> +	exif_content_remove_entry(content, existing);
> +
> +	ExifEntry *entry = exif_entry_new_mem(mem_);
> +	if (!entry) {
> +		LOG(EXIF, Fatal) << "Failed to allocated new entry";
> +		return nullptr;
> +	}
> +
> +	void *buffer = exif_mem_alloc(mem_, size);
> +	if (!buffer) {
> +		LOG(EXIF, Fatal) << "Failed to allocate buffer for variable entry";
> +		exif_mem_unref(mem_);
> +		return nullptr;
> +	}
> +
> +	entry->data = static_cast<unsigned char *>(buffer);
> +	entry->components = components;
> +	entry->format = format;
> +	entry->size = size;
> +	entry->tag = tag;
> +
> +	exif_content_add_entry(content, entry);
> +
> +	return entry;
> +}
> +
> +int Exif::setShort(ExifIfd ifd, ExifTag tag, uint16_t item)
> +{
> +	ExifEntry *entry = createEntry(ifd, tag);
> +
> +	exif_set_short(entry->data, EXIF_BYTE_ORDER_INTEL, item);
> +	exif_entry_unref(entry);
> +
> +	return 0;
> +}
> +
> +int Exif::setLong(ExifIfd ifd, ExifTag tag, uint32_t item)
> +{
> +	ExifEntry *entry = createEntry(ifd, tag);
> +
> +	exif_set_long(entry->data, EXIF_BYTE_ORDER_INTEL, item);
> +	exif_entry_unref(entry);
> +
> +	return 0;
> +}
> +
> +int Exif::setRational(ExifIfd ifd, ExifTag tag, uint32_t numerator, uint32_t denominator)
> +{
> +	ExifEntry *entry = createEntry(ifd, tag);
> +	ExifRational item{ numerator, denominator };
> +
> +	exif_set_rational(entry->data, EXIF_BYTE_ORDER_INTEL, item);
> +	exif_entry_unref(entry);
> +
> +	return 0;
> +}
> +
> +int Exif::setString(ExifIfd ifd, ExifTag tag, ExifFormat format, const std::string &item)
> +{
> +	/* Pad 1 extra byte for null-terminated string. */
> +	size_t length = item.length() + 1;
> +
> +	ExifEntry *entry = createEntry(ifd, tag, format, length, length);
> +
> +	memcpy(entry->data, item.c_str(), length);
> +	exif_entry_unref(entry);
> +
> +	return 0;
> +}
> +
> +int Exif::generate()
> +{
> +	if (exif_data_) {
> +		free(exif_data_);
> +		exif_data_ = nullptr;
> +	}
> +
> +	exif_data_save_data(data_, &exif_data_, &size_);
> +
> +	LOG(EXIF, Debug) << "Created EXIF instance (" << size_ << " bytes)";
> +
> +	return 0;
> +}
> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h
> new file mode 100644
> index 0000000..3a132ba
> --- /dev/null
> +++ b/src/android/jpeg/exif.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * exif.h - EXIF tag creator using libexif
> + */
> +#ifndef __ANDROID_JPEG_EXIF_H__
> +#define __ANDROID_JPEG_EXIF_H__
> +
> +#include <libexif/exif-data.h>
> +
> +#include <libcamera/span.h>
> +
> +#include <string>
> +
> +class Exif
> +{
> +public:
> +	Exif();
> +	~Exif();
> +
> +	libcamera::Span<const uint8_t> data() const { return { exif_data_, size_ }; }
> +	int generate();
> +
> +private:
> +	ExifEntry *createEntry(ExifIfd ifd, ExifTag tag);
> +	ExifEntry *createEntry(ExifIfd ifd, ExifTag tag, ExifFormat format,
> +			       uint64_t components, unsigned int size);
> +
> +	int setShort(ExifIfd ifd, ExifTag tag, uint16_t item);
> +	int setLong(ExifIfd ifd, ExifTag tag, uint32_t item);
> +	int setString(ExifIfd ifd, ExifTag tag, ExifFormat format, const std::string &item);
> +	int setRational(ExifIfd ifd, ExifTag tag, uint32_t numerator, uint32_t denominator);
> +
> +	bool valid_;
> +
> +	ExifData *data_;
> +	ExifMem *mem_;
> +
> +	unsigned char *exif_data_;

This should be exifData_ to follow our coding style.


> +	unsigned int size_;
> +};
> +
> +#endif /* __LIBCAMERA_EXIF_H__ */
> diff --git a/src/android/meson.build b/src/android/meson.build
> index f7b81a4..0f49b25 100644
> --- a/src/android/meson.build
> +++ b/src/android/meson.build
> @@ -7,6 +7,7 @@ android_hal_sources = files([
>      'camera_metadata.cpp',
>      'camera_ops.cpp',
>      'jpeg/encoder_libjpeg.cpp',
> +    'jpeg/exif.cpp',
>  ])
>  
>  android_camera_metadata_sources = files([
> @@ -15,6 +16,7 @@ android_camera_metadata_sources = files([
>  
>  android_deps = [
>      dependency('libjpeg'),
> +    dependency('libexif'),

Laurent asked for those to be alphabetically sorted.

Other than those two, I think this is ready for integration., but I'm
biased as I started it... so I'll await a tag/comment from someone else.

If you post a new version with those two comments fixed, add:

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Or I could handle them when applying otherwise if someone else gives a
tag already.


>  ]
>  
>  android_camera_metadata = static_library('camera_metadata',
> 

-- 
Regards
--
Kieran