[libcamera-devel,v2,2/2] android: jpeg: Support a initial set of EXIF metadata tags

Message ID 20200825201048.13608-1-email@uajain.com
State Superseded
Headers show
Series
  • Initial EXIF metadata support
Related show

Commit Message

Umang Jain Aug. 25, 2020, 8:10 p.m. UTC
From: Kieran Bingham <kieran.bingham@ideasonboard.com>

Add a Exif data placeholder inside CameraStream, to populate it
accordingly to the stream properties (size, orientation etc).
Static EXIF properties (such as make, model, size, orientation)
can be configured once when the stream is being configured.
Per-frame related metadata (such as timestamp) needs to set
just before encoding preferably in CameraDevice::requestComplete().

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Umang Jain <email@uajain.com>
---
 src/android/camera_device.cpp        | 22 +++++++++-
 src/android/camera_device.h          |  2 +
 src/android/jpeg/encoder.h           |  5 ++-
 src/android/jpeg/encoder_libjpeg.cpp |  9 +++-
 src/android/jpeg/encoder_libjpeg.h   |  3 +-
 src/android/jpeg/exif.cpp            | 66 ++++++++++++++++++++++++++++
 src/android/jpeg/exif.h              |  9 ++++
 7 files changed, 112 insertions(+), 4 deletions(-)

Comments

Laurent Pinchart Aug. 26, 2020, 12:40 a.m. UTC | #1
Hi Umang,

Thank you for the patch.

On Tue, Aug 25, 2020 at 08:10:53PM +0000, Umang Jain wrote:
> From: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> Add a Exif data placeholder inside CameraStream, to populate it
> accordingly to the stream properties (size, orientation etc).
> Static EXIF properties (such as make, model, size, orientation)
> can be configured once when the stream is being configured.
> Per-frame related metadata (such as timestamp) needs to set
> just before encoding preferably in CameraDevice::requestComplete().

As explained in the review of 1/2, I don't think we can reuse the EXIF
generator. Sorry :-(

> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Umang Jain <email@uajain.com>
> ---
>  src/android/camera_device.cpp        | 22 +++++++++-
>  src/android/camera_device.h          |  2 +
>  src/android/jpeg/encoder.h           |  5 ++-
>  src/android/jpeg/encoder_libjpeg.cpp |  9 +++-
>  src/android/jpeg/encoder_libjpeg.h   |  3 +-
>  src/android/jpeg/exif.cpp            | 66 ++++++++++++++++++++++++++++
>  src/android/jpeg/exif.h              |  9 ++++
>  7 files changed, 112 insertions(+), 4 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index de6f86f..8789a9e 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1245,6 +1245,17 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>  					<< "Failed to configure encoder";
>  				return ret;
>  			}
> +
> +			/*
> +			 * Set EXIF metadata for various tags.
> +			 * \todo Discuss setMake String, maybe vendor ID?
> +			 * KB suggested to leave it to "libcamera" for now.
> +			 * setModel should use the 'model' property of the camera.

Actually this should use the vendor and model of the end-product. We
will need to load this from a platform config/data file. It can stay
as-is for now.

> +			 */
> +			cameraStream->exif.setMake("libcamera");
> +			cameraStream->exif.setModel("cameraModel");
> +			cameraStream->exif.setOrientation(orientation_);
> +			cameraStream->exif.setSize(cfg.size);
>  		}
>  	}
>  
> @@ -1434,7 +1445,16 @@ void CameraDevice::requestComplete(Request *request)
>  			continue;
>  		}
>  
> -		int jpeg_size = encoder->encode(buffer, mapped.maps()[0]);
> +		/*
> +		 * We set the frame's EXIF timestamp as the time of encode. Since the

"the" ? the precision ?

> +		 * we need for EXIF is only one second, it is good enough.
> +		 */
> +		struct timespec tp;
> +		clock_gettime(CLOCK_REALTIME, &tp);
> +		cameraStream->exif.setTimestamp(tp.tv_sec);

This can be simplified to

		cameraStream->exif.setTimestamp(std::time(nullptr));

> +		Span<uint8_t> exif_data = cameraStream->exif.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/camera_device.h b/src/android/camera_device.h
> index 3934f19..3e8a119 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -42,6 +42,8 @@ struct CameraStream {
>  	libcamera::Size size;
>  
>  	Encoder *jpeg;
> +
> +	Exif exif;
>  };
>  
>  class CameraDevice : protected libcamera::Loggable
> diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h
> index f9eb88e..969f291 100644
> --- a/src/android/jpeg/encoder.h
> +++ b/src/android/jpeg/encoder.h
> @@ -7,6 +7,8 @@
>  #ifndef __ANDROID_JPEG_ENCODER_H__
>  #define __ANDROID_JPEG_ENCODER_H__
>  
> +#include "exif.h"
> +

I don't think you need to include exif.h here.

>  #include <libcamera/buffer.h>
>  #include <libcamera/span.h>
>  #include <libcamera/stream.h>
> @@ -18,7 +20,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<uint8_t> &exif_data) = 0;

The variable should be named exitData, or just exif.

>  };
>  
>  #endif /* __ANDROID_JPEG_ENCODER_H__ */
> diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp
> index 977538c..87464a4 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<uint8_t> &exif_data)
>  {
>  	MappedFrameBuffer frame(source, PROT_READ);
>  	if (!frame.isValid()) {
> @@ -214,5 +215,11 @@ int EncoderLibJpeg::encode(const FrameBuffer *source,
>  
>  	jpeg_finish_compress(&compress_);
>  
> +	if (exif_data.size())
> +		/* Store Exif data in the JPEG_APP1 data block. */
> +		jpeg_write_marker(&compress_, JPEG_APP0 + 1,
> +				  static_cast<const JOCTET *>(exif_data.data()),
> +				  exif_data.size());
> +
>  	return size;
>  }
> diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h
> index aed081a..88bc6e0 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<uint8_t> &exif_data) override;
>  
>  private:
>  	void compressRGB(const libcamera::MappedBuffer *frame);
> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
> index f6a9f5c..d2ef14e 100644
> --- a/src/android/jpeg/exif.cpp
> +++ b/src/android/jpeg/exif.cpp
> @@ -160,6 +160,72 @@ int Exif::setString(ExifIfd ifd, ExifTag tag, ExifFormat format, const std::stri
>  	return 0;
>  }
>  
> +int Exif::setMake(const std::string &make)
> +{
> +	return setString(EXIF_IFD_0, EXIF_TAG_MAKE, EXIF_FORMAT_ASCII, make);
> +}
> +
> +int Exif::setModel(const std::string &model)
> +{
> +	return setString(EXIF_IFD_0, EXIF_TAG_MODEL, EXIF_FORMAT_ASCII, model);
> +}
> +
> +int Exif::setSize(Size size)
> +{
> +	setShort(EXIF_IFD_0, EXIF_TAG_IMAGE_LENGTH, size.height);
> +	setLong(EXIF_IFD_EXIF, EXIF_TAG_PIXEL_Y_DIMENSION, size.height);
> +
> +	setShort(EXIF_IFD_0, EXIF_TAG_IMAGE_WIDTH, size.width);
> +	setLong(EXIF_IFD_EXIF, EXIF_TAG_PIXEL_X_DIMENSION, size.width);
> +
> +	return 0;
> +}
> +
> +int Exif::setTimestamp(const time_t timestamp)
> +{
> +	char str[40];
> +	size_t len = std::strftime(str, sizeof(str), "%c %Z",
> +				   std::localtime(&timestamp));

The EXIF specification requires dates to be in the "YYYY:MM:DD HH:MM:SS"
format. str can be shortened to 20.

> +	std::string ts(str);
> +	int ret = -1;
> +
> +	if (len > 0) {

This can't happen if str is large enough, so I'd skip this check.

> +		ret = setString(EXIF_IFD_0, EXIF_TAG_DATE_TIME, EXIF_FORMAT_ASCII, ts);
> +		if (ret < 0)
> +			return ret;
> +
> +		ret = setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_ORIGINAL, EXIF_FORMAT_ASCII, ts);
> +		if (ret < 0)
> +			return ret;
> +
> +		ret = setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_DIGITIZED, EXIF_FORMAT_ASCII, ts);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +

Extra blank line.

> +int Exif::setOrientation(int orientation)
> +{
> +	/* Orientation's tag value computed similar to Chrome HAL. */

I don't think the Chrome OS HAL is relevant :-) What's relevant is
turning the libcamera orientation into an EXIF orientation.

> +	int value = 1;
> +	switch (orientation) {
> +	case 90:
> +		value = 6;
> +		break;
> +	case 180:
> +		value = 3;
> +		break;
> +	case 270:
> +		value = 8;
> +		break;
> +	}
> +
> +	return setShort(EXIF_IFD_0, EXIF_TAG_ORIENTATION, value);
> +}
> +
>  Span<uint8_t> Exif::generate()
>  {
>  	if (exif_data_) {
> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h
> index 7df83c7..6cca578 100644
> --- a/src/android/jpeg/exif.h
> +++ b/src/android/jpeg/exif.h
> @@ -9,8 +9,10 @@
>  
>  #include <libexif/exif-data.h>
>  
> +#include <libcamera/geometry.h>
>  #include <libcamera/span.h>
>  
> +#include <ctime>
>  #include <string>
>  
>  class Exif
> @@ -24,6 +26,13 @@ public:
>  	int setString(ExifIfd ifd, ExifTag tag, ExifFormat format, const std::string &item);
>  	int setRational(ExifIfd ifd, ExifTag tag, uint32_t numerator, uint32_t denominator);
>  
> +	int setMake(const std::string &make);
> +	int setModel(const std::string &model);
> +
> +	int setOrientation(int orientation);
> +	int setSize(libcamera::Size size);
> +	int setTimestamp(const time_t timestamp);
> +
>  	libcamera::Span<uint8_t> generate();
>  	unsigned char *data() const { return exif_data_; }
>  	unsigned int size() const { return size_; }
Kieran Bingham Aug. 26, 2020, 8:11 a.m. UTC | #2
Hi Umang, Laurent,

On 26/08/2020 01:40, Laurent Pinchart wrote:
> Hi Umang,
> 
> Thank you for the patch.
> 
> On Tue, Aug 25, 2020 at 08:10:53PM +0000, Umang Jain wrote:
>> From: Kieran Bingham <kieran.bingham@ideasonboard.com>
>>
>> Add a Exif data placeholder inside CameraStream, to populate it
>> accordingly to the stream properties (size, orientation etc).
>> Static EXIF properties (such as make, model, size, orientation)
>> can be configured once when the stream is being configured.
>> Per-frame related metadata (such as timestamp) needs to set
>> just before encoding preferably in CameraDevice::requestComplete().
> 
> As explained in the review of 1/2, I don't think we can reuse the EXIF
> generator. Sorry :-(

I'm not yet convinced, but if you really want it to be a new object each
time that can be done too, and I guess it keeps all the code setting the
exif in camera_device in one place anyway.

> 
>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>> Signed-off-by: Umang Jain <email@uajain.com>
>> ---
>>  src/android/camera_device.cpp        | 22 +++++++++-
>>  src/android/camera_device.h          |  2 +
>>  src/android/jpeg/encoder.h           |  5 ++-
>>  src/android/jpeg/encoder_libjpeg.cpp |  9 +++-
>>  src/android/jpeg/encoder_libjpeg.h   |  3 +-
>>  src/android/jpeg/exif.cpp            | 66 ++++++++++++++++++++++++++++
>>  src/android/jpeg/exif.h              |  9 ++++
>>  7 files changed, 112 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>> index de6f86f..8789a9e 100644
>> --- a/src/android/camera_device.cpp
>> +++ b/src/android/camera_device.cpp
>> @@ -1245,6 +1245,17 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>>  					<< "Failed to configure encoder";
>>  				return ret;
>>  			}
>> +
>> +			/*
>> +			 * Set EXIF metadata for various tags.
>> +			 * \todo Discuss setMake String, maybe vendor ID?
>> +			 * KB suggested to leave it to "libcamera" for now.
>> +			 * setModel should use the 'model' property of the camera.
> 
> Actually this should use the vendor and model of the end-product. We
> will need to load this from a platform config/data file. It can stay
> as-is for now.

Hrm, indeed I was overthinking the 'model' property from the pipeline
handler.

I think we can adjust this to
 \todo Set Make and Model from external vendor tags.


> 
>> +			 */
>> +			cameraStream->exif.setMake("libcamera");
>> +			cameraStream->exif.setModel("cameraModel");
>> +			cameraStream->exif.setOrientation(orientation_);
>> +			cameraStream->exif.setSize(cfg.size);
>>  		}
>>  	}
>>  
>> @@ -1434,7 +1445,16 @@ void CameraDevice::requestComplete(Request *request)
>>  			continue;
>>  		}
>>  
>> -		int jpeg_size = encoder->encode(buffer, mapped.maps()[0]);
>> +		/*
>> +		 * We set the frame's EXIF timestamp as the time of encode. Since the
> 
> "the" ? the precision ?
> 
>> +		 * we need for EXIF is only one second, it is good enough.
>> +		 */
>> +		struct timespec tp;
>> +		clock_gettime(CLOCK_REALTIME, &tp);
>> +		cameraStream->exif.setTimestamp(tp.tv_sec);
> 
> This can be simplified to
> 
> 		cameraStream->exif.setTimestamp(std::time(nullptr));
> 
>> +		Span<uint8_t> exif_data = cameraStream->exif.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/camera_device.h b/src/android/camera_device.h
>> index 3934f19..3e8a119 100644
>> --- a/src/android/camera_device.h
>> +++ b/src/android/camera_device.h
>> @@ -42,6 +42,8 @@ struct CameraStream {
>>  	libcamera::Size size;
>>  
>>  	Encoder *jpeg;
>> +
>> +	Exif exif;
>>  };
>>  
>>  class CameraDevice : protected libcamera::Loggable
>> diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h
>> index f9eb88e..969f291 100644
>> --- a/src/android/jpeg/encoder.h
>> +++ b/src/android/jpeg/encoder.h
>> @@ -7,6 +7,8 @@
>>  #ifndef __ANDROID_JPEG_ENCODER_H__
>>  #define __ANDROID_JPEG_ENCODER_H__
>>  
>> +#include "exif.h"
>> +
> 
> I don't think you need to include exif.h here.

Indeed, the benefit of using the Span :-)

> 
>>  #include <libcamera/buffer.h>
>>  #include <libcamera/span.h>
>>  #include <libcamera/stream.h>
>> @@ -18,7 +20,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<uint8_t> &exif_data) = 0;
> 
> The variable should be named exitData, or just exif.
> 
>>  };
>>  
>>  #endif /* __ANDROID_JPEG_ENCODER_H__ */
>> diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp
>> index 977538c..87464a4 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<uint8_t> &exif_data)
>>  {
>>  	MappedFrameBuffer frame(source, PROT_READ);
>>  	if (!frame.isValid()) {
>> @@ -214,5 +215,11 @@ int EncoderLibJpeg::encode(const FrameBuffer *source,
>>  
>>  	jpeg_finish_compress(&compress_);
>>  
>> +	if (exif_data.size())
>> +		/* Store Exif data in the JPEG_APP1 data block. */
>> +		jpeg_write_marker(&compress_, JPEG_APP0 + 1,
>> +				  static_cast<const JOCTET *>(exif_data.data()),
>> +				  exif_data.size());
>> +
>>  	return size;
>>  }
>> diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h
>> index aed081a..88bc6e0 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<uint8_t> &exif_data) override;
>>  
>>  private:
>>  	void compressRGB(const libcamera::MappedBuffer *frame);
>> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
>> index f6a9f5c..d2ef14e 100644
>> --- a/src/android/jpeg/exif.cpp
>> +++ b/src/android/jpeg/exif.cpp
>> @@ -160,6 +160,72 @@ int Exif::setString(ExifIfd ifd, ExifTag tag, ExifFormat format, const std::stri
>>  	return 0;
>>  }
>>  
>> +int Exif::setMake(const std::string &make)
>> +{
>> +	return setString(EXIF_IFD_0, EXIF_TAG_MAKE, EXIF_FORMAT_ASCII, make);
>> +}
>> +
>> +int Exif::setModel(const std::string &model)
>> +{
>> +	return setString(EXIF_IFD_0, EXIF_TAG_MODEL, EXIF_FORMAT_ASCII, model);
>> +}
>> +
>> +int Exif::setSize(Size size)
>> +{
>> +	setShort(EXIF_IFD_0, EXIF_TAG_IMAGE_LENGTH, size.height);
>> +	setLong(EXIF_IFD_EXIF, EXIF_TAG_PIXEL_Y_DIMENSION, size.height);
>> +
>> +	setShort(EXIF_IFD_0, EXIF_TAG_IMAGE_WIDTH, size.width);
>> +	setLong(EXIF_IFD_EXIF, EXIF_TAG_PIXEL_X_DIMENSION, size.width);
>> +
>> +	return 0;
>> +}
>> +
>> +int Exif::setTimestamp(const time_t timestamp)
>> +{
>> +	char str[40];
>> +	size_t len = std::strftime(str, sizeof(str), "%c %Z",
>> +				   std::localtime(&timestamp));
> 
> The EXIF specification requires dates to be in the "YYYY:MM:DD HH:MM:SS"
> format. str can be shortened to 20.
> 
>> +	std::string ts(str);
>> +	int ret = -1;
>> +
>> +	if (len > 0) {
> 
> This can't happen if str is large enough, so I'd skip this check.
> 
>> +		ret = setString(EXIF_IFD_0, EXIF_TAG_DATE_TIME, EXIF_FORMAT_ASCII, ts);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		ret = setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_ORIGINAL, EXIF_FORMAT_ASCII, ts);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		ret = setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_DIGITIZED, EXIF_FORMAT_ASCII, ts);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +
> 
> Extra blank line.
> 
>> +int Exif::setOrientation(int orientation)
>> +{
>> +	/* Orientation's tag value computed similar to Chrome HAL. */
> 
> I don't think the Chrome OS HAL is relevant :-) What's relevant is
> turning the libcamera orientation into an EXIF orientation.
> 
>> +	int value = 1;
>> +	switch (orientation) {
>> +	case 90:
>> +		value = 6;
>> +		break;
>> +	case 180:
>> +		value = 3;
>> +		break;
>> +	case 270:
>> +		value = 8;
>> +		break;
>> +	}
>> +
>> +	return setShort(EXIF_IFD_0, EXIF_TAG_ORIENTATION, value);
>> +}
>> +
>>  Span<uint8_t> Exif::generate()
>>  {
>>  	if (exif_data_) {
>> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h
>> index 7df83c7..6cca578 100644
>> --- a/src/android/jpeg/exif.h
>> +++ b/src/android/jpeg/exif.h
>> @@ -9,8 +9,10 @@
>>  
>>  #include <libexif/exif-data.h>
>>  
>> +#include <libcamera/geometry.h>
>>  #include <libcamera/span.h>
>>  
>> +#include <ctime>
>>  #include <string>
>>  
>>  class Exif
>> @@ -24,6 +26,13 @@ public:
>>  	int setString(ExifIfd ifd, ExifTag tag, ExifFormat format, const std::string &item);
>>  	int setRational(ExifIfd ifd, ExifTag tag, uint32_t numerator, uint32_t denominator);
>>  
>> +	int setMake(const std::string &make);
>> +	int setModel(const std::string &model);
>> +
>> +	int setOrientation(int orientation);
>> +	int setSize(libcamera::Size size);
>> +	int setTimestamp(const time_t timestamp);
>> +
>>  	libcamera::Span<uint8_t> generate();
>>  	unsigned char *data() const { return exif_data_; }
>>  	unsigned int size() const { return size_; }
>

Patch

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index de6f86f..8789a9e 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -1245,6 +1245,17 @@  int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
 					<< "Failed to configure encoder";
 				return ret;
 			}
+
+			/*
+			 * Set EXIF metadata for various tags.
+			 * \todo Discuss setMake String, maybe vendor ID?
+			 * KB suggested to leave it to "libcamera" for now.
+			 * setModel should use the 'model' property of the camera.
+			 */
+			cameraStream->exif.setMake("libcamera");
+			cameraStream->exif.setModel("cameraModel");
+			cameraStream->exif.setOrientation(orientation_);
+			cameraStream->exif.setSize(cfg.size);
 		}
 	}
 
@@ -1434,7 +1445,16 @@  void CameraDevice::requestComplete(Request *request)
 			continue;
 		}
 
-		int jpeg_size = encoder->encode(buffer, mapped.maps()[0]);
+		/*
+		 * We set the frame's EXIF timestamp as the time of encode. Since the
+		 * we need for EXIF is only one second, it is good enough.
+		 */
+		struct timespec tp;
+		clock_gettime(CLOCK_REALTIME, &tp);
+		cameraStream->exif.setTimestamp(tp.tv_sec);
+		Span<uint8_t> exif_data = cameraStream->exif.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/camera_device.h b/src/android/camera_device.h
index 3934f19..3e8a119 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -42,6 +42,8 @@  struct CameraStream {
 	libcamera::Size size;
 
 	Encoder *jpeg;
+
+	Exif exif;
 };
 
 class CameraDevice : protected libcamera::Loggable
diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h
index f9eb88e..969f291 100644
--- a/src/android/jpeg/encoder.h
+++ b/src/android/jpeg/encoder.h
@@ -7,6 +7,8 @@ 
 #ifndef __ANDROID_JPEG_ENCODER_H__
 #define __ANDROID_JPEG_ENCODER_H__
 
+#include "exif.h"
+
 #include <libcamera/buffer.h>
 #include <libcamera/span.h>
 #include <libcamera/stream.h>
@@ -18,7 +20,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<uint8_t> &exif_data) = 0;
 };
 
 #endif /* __ANDROID_JPEG_ENCODER_H__ */
diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp
index 977538c..87464a4 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<uint8_t> &exif_data)
 {
 	MappedFrameBuffer frame(source, PROT_READ);
 	if (!frame.isValid()) {
@@ -214,5 +215,11 @@  int EncoderLibJpeg::encode(const FrameBuffer *source,
 
 	jpeg_finish_compress(&compress_);
 
+	if (exif_data.size())
+		/* Store Exif data in the JPEG_APP1 data block. */
+		jpeg_write_marker(&compress_, JPEG_APP0 + 1,
+				  static_cast<const JOCTET *>(exif_data.data()),
+				  exif_data.size());
+
 	return size;
 }
diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h
index aed081a..88bc6e0 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<uint8_t> &exif_data) override;
 
 private:
 	void compressRGB(const libcamera::MappedBuffer *frame);
diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
index f6a9f5c..d2ef14e 100644
--- a/src/android/jpeg/exif.cpp
+++ b/src/android/jpeg/exif.cpp
@@ -160,6 +160,72 @@  int Exif::setString(ExifIfd ifd, ExifTag tag, ExifFormat format, const std::stri
 	return 0;
 }
 
+int Exif::setMake(const std::string &make)
+{
+	return setString(EXIF_IFD_0, EXIF_TAG_MAKE, EXIF_FORMAT_ASCII, make);
+}
+
+int Exif::setModel(const std::string &model)
+{
+	return setString(EXIF_IFD_0, EXIF_TAG_MODEL, EXIF_FORMAT_ASCII, model);
+}
+
+int Exif::setSize(Size size)
+{
+	setShort(EXIF_IFD_0, EXIF_TAG_IMAGE_LENGTH, size.height);
+	setLong(EXIF_IFD_EXIF, EXIF_TAG_PIXEL_Y_DIMENSION, size.height);
+
+	setShort(EXIF_IFD_0, EXIF_TAG_IMAGE_WIDTH, size.width);
+	setLong(EXIF_IFD_EXIF, EXIF_TAG_PIXEL_X_DIMENSION, size.width);
+
+	return 0;
+}
+
+int Exif::setTimestamp(const time_t timestamp)
+{
+	char str[40];
+	size_t len = std::strftime(str, sizeof(str), "%c %Z",
+				   std::localtime(&timestamp));
+	std::string ts(str);
+	int ret = -1;
+
+	if (len > 0) {
+		ret = setString(EXIF_IFD_0, EXIF_TAG_DATE_TIME, EXIF_FORMAT_ASCII, ts);
+		if (ret < 0)
+			return ret;
+
+		ret = setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_ORIGINAL, EXIF_FORMAT_ASCII, ts);
+		if (ret < 0)
+			return ret;
+
+		ret = setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_DIGITIZED, EXIF_FORMAT_ASCII, ts);
+		if (ret < 0)
+			return ret;
+	}
+
+	return ret;
+}
+
+
+int Exif::setOrientation(int orientation)
+{
+	/* Orientation's tag value computed similar to Chrome HAL. */
+	int value = 1;
+	switch (orientation) {
+	case 90:
+		value = 6;
+		break;
+	case 180:
+		value = 3;
+		break;
+	case 270:
+		value = 8;
+		break;
+	}
+
+	return setShort(EXIF_IFD_0, EXIF_TAG_ORIENTATION, value);
+}
+
 Span<uint8_t> Exif::generate()
 {
 	if (exif_data_) {
diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h
index 7df83c7..6cca578 100644
--- a/src/android/jpeg/exif.h
+++ b/src/android/jpeg/exif.h
@@ -9,8 +9,10 @@ 
 
 #include <libexif/exif-data.h>
 
+#include <libcamera/geometry.h>
 #include <libcamera/span.h>
 
+#include <ctime>
 #include <string>
 
 class Exif
@@ -24,6 +26,13 @@  public:
 	int setString(ExifIfd ifd, ExifTag tag, ExifFormat format, const std::string &item);
 	int setRational(ExifIfd ifd, ExifTag tag, uint32_t numerator, uint32_t denominator);
 
+	int setMake(const std::string &make);
+	int setModel(const std::string &model);
+
+	int setOrientation(int orientation);
+	int setSize(libcamera::Size size);
+	int setTimestamp(const time_t timestamp);
+
 	libcamera::Span<uint8_t> generate();
 	unsigned char *data() const { return exif_data_; }
 	unsigned int size() const { return size_; }