[libcamera-devel] [PATCH v6 2/2] android: jpeg: Support an initial set of EXIF metadata tags
Umang Jain
email at uajain.com
Tue Sep 8 12:12:39 CEST 2020
Hi Laurent,
On 9/4/20 9:04 PM, Laurent Pinchart wrote:
> 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.
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]
[1]:
https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/camera/common/exif_utils.cc#376
[2]:
https://piexif.readthedocs.io/en/latest/sample.html#rotate-image-by-exif-orientation
¯\_(ツ)_/¯
>
>> + }
>> +
>> + 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();
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20200908/ad87901f/attachment.htm>
More information about the libcamera-devel
mailing list