| Message ID | 20200828065727.9909-3-email@uajain.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. On Fri, Aug 28, 2020 at 06:57:40AM +0000, 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@ideasonboard.com> > Signed-off-by: Umang Jain <email@uajain.com> > Reviewed-by: Kieran Bingham <kieran.bingham@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 | 62 ++++++++++++++++++++++++++++ > src/android/jpeg/exif.h | 9 ++++ > 6 files changed, 100 insertions(+), 4 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(); > + > + 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 b49b538..3fbb117 100644 > --- a/src/android/jpeg/exif.cpp > +++ b/src/android/jpeg/exif.cpp > @@ -157,6 +157,68 @@ 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) const Size &size ? > +{ > + setShort(EXIF_IFD_0, EXIF_TAG_IMAGE_LENGTH, size.height); The EXIF specification says that EXIF_TAG_IMAGE_LENGTH and EXIF_TAG_IMAGE_WIDTH should not be set for JPEG compressed images. X_DIMENSION and Y_DIMENSION are fine. > + 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[20]; > + std::strftime(str, sizeof(str), "%Y:%m:%d %H:%M:%S", > + std::localtime(×tamp)); > + std::string ts(str); > + int ret = -1; > + > + 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; This makes me think that skipping error handling and relying on valid_ would be a good idea :-) Doesn't have to be part of this series, but adding the timezone would be great (EXIF_TAG_TIME_ZONE_OFFSET). > + > + return ret; > +} > + > +int Exif::setOrientation(int 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); > +} > + > int Exif::generate() > { > if (exifData_) { > diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h > index 6c10f94..f15afb9 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> I missed this on 1/2, but we usually put the C/C++ headers first. > > class Exif > @@ -19,6 +21,13 @@ public: > Exif(); > ~Exif(); > > + 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); No need for const when passing by value :-) > + > libcamera::Span<const uint8_t> data() const { return { exifData_, size_ }; } > int generate(); >
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(); + + 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 b49b538..3fbb117 100644 --- a/src/android/jpeg/exif.cpp +++ b/src/android/jpeg/exif.cpp @@ -157,6 +157,68 @@ 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[20]; + std::strftime(str, sizeof(str), "%Y:%m:%d %H:%M:%S", + std::localtime(×tamp)); + std::string ts(str); + int ret = -1; + + 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) +{ + 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); +} + int Exif::generate() { if (exifData_) { diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h index 6c10f94..f15afb9 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 @@ -19,6 +21,13 @@ public: Exif(); ~Exif(); + 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<const uint8_t> data() const { return { exifData_, size_ }; } int generate();