[libcamera-devel] [PATCH v5 2/2] android: jpeg: Support an initial set of EXIF metadata tags
Umang Jain
email at uajain.com
Fri Sep 4 08:05:08 CEST 2020
Hi Laurent, Kieran,
On 9/4/20 4:10 AM, Laurent Pinchart wrote:
> On Thu, Sep 03, 2020 at 07:38:07PM +0100, Kieran Bingham wrote:
>> Hi Umang,
>>
>> On 03/09/2020 17:32, 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>
>>> ---
>>> src/android/camera_device.cpp | 18 ++++++++++-
>>> 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 | 48 +++++++++++++++++++++++++++-
>>> src/android/jpeg/exif.h | 8 +++++
>>> 6 files changed, 84 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>>> index de6f86f..54ca9c6 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,22 @@ 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));
>>> + exif.generate();
>> Hrm ... Did this pass compile tests?
> Likely not, given that generate() is defined as returning int in the
> header file and void in the implementation. Rebase issue, or sending the
> wrong version ?
Indeed, a faulty rebase on my end .. /o\
Although, it *did* pass the compile test and I can confirm
compiling the patches again (as is this series).
It's quite weird. GCC versions are :
C compiler for the host machine: cc (gcc 9.3.0 "cc (Ubuntu
9.3.0-10ubuntu2) 9.3.0")
C linker for the host machine: cc ld.bfd 2.34
C++ compiler for the host machine: c++ (gcc 9.3.0 "c++ (Ubuntu
9.3.0-10ubuntu2) 9.3.0")
C++ linker for the host machine: c++ ld.bfd 2.34
>
>> Didn't we add a [[nodiscard]] to this call?
>>
>> Although - if it does fail to generate, I think we'd still expect to
>> call into the encoder->encode() and I think the exif.data() would be
>> nullptr,0, (which is what we want), as the encoder checks the size to
>> determine if it should be added or not.
>>
>>> +
>>> + 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 9d8d9bc..5def7e3 100644
>>> --- a/src/android/jpeg/exif.cpp
>>> +++ b/src/android/jpeg/exif.cpp
>>> @@ -169,7 +169,53 @@ void Exif::setString(ExifIfd ifd, ExifTag tag, ExifFormat format, const std::str
>>> exif_entry_unref(entry);
>>> }
>>>
>>> -int Exif::generate()
>>> +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 = 1;
>>> + switch (orientation) {
>>> + case 90:
>>> + value = 6;
>>> + break;
>>> + case 180:
>>> + value = 3;
>>> + break;
>>> + case 270:
>>> + value = 8;
>>> + break;
>>> + }
>>> +
>>> + setShort(EXIF_IFD_0, EXIF_TAG_ORIENTATION, value);
>>> +}
>>> +
>>> +void Exif::generate()
>>> {
>>> if (exifData_) {
>>> free(exifData_);
>>> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h
>>> index 8fb8ffd..6113ca6 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_ }; }
>>> int [[nodiscard]] generate();
>>>
More information about the libcamera-devel
mailing list