[libcamera-devel] [PATCH v4 5/8] android: Set result metadata and EXIF fields based on request metadata
Jacopo Mondi
jacopo at jmondi.org
Mon Jan 25 12:51:34 CET 2021
Hi Paul,
On Mon, Jan 25, 2021 at 04:14:41PM +0900, Paul Elder wrote:
> Set the following android result metadata:
> - ANDROID_LENS_FOCAL_LENGTH
> - ANDROID_LENS_APERTURE
> - ANDROID_JPEG_GPS_TIMESTAMP
> - ANDROID_JPEG_GPS_COORDINATES
> - ANDROID_JPEG_GPS_PROCESSING_METHOD
> - ANDROID_JPEG_SIZE
> - ANDROID_JPEG_QUALITY
> - ANDROID_JPEG_ORIENTATION
>
> And the following EXIF fields:
> - GPSDatestamp
> - GPSTimestamp
> - GPSLocation
> - GPSLatitudeRef
> - GPSLatitude
> - GPSLongitudeRef
> - GPSLongitude
> - GPSAltitudeRef
> - GPSAltitude
> - GPSProcessingMethod
> - FocalLength
> - ExposureTime
> - FNumber
> - ISO
> - Flash
> - WhiteBalance
> - SubsecTime
> - SubsecTimeOriginal
> - SubsecTimeDigitized
>
> Based on android request metadata.
>
> This allows the following CTS tests to pass:
> - android.hardware.camera2.cts.StillCaptureTest#testFocalLengths
> - android.hardware.camera2.cts.StillCaptureTest#testJpegExif
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> ---
> Changes in v4:
> - remove unused variable
> - move android JPEG thumbnail tags allocation to the thumbnail patch
> - move setting the timestamp with subsec to the patch that adds set
> subsec to timestamp ("android: jpeg: exif: Add functions for
> setting various values")
> - group setting GPS-related tags
>
> Changes in v3:
> - fix metadata entries and byte count
> - fix gps processing method string truncation
> - move thumbnail handling to a different patch
>
> Changes in v2:
> - break out processControls and thumbnailer configuration into
> different
> patches
> - fix android metadata entry number and size
> - other small changes
> ---
> src/android/camera_device.cpp | 19 +++++++-
> src/android/camera_stream.cpp | 7 ++-
> src/android/camera_stream.h | 4 +-
> src/android/jpeg/post_processor_jpeg.cpp | 62 +++++++++++++++++++-----
> src/android/jpeg/post_processor_jpeg.h | 3 +-
> src/android/post_processor.h | 3 +-
> 6 files changed, 80 insertions(+), 18 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 592e2d43..2cd3c8a1 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1688,6 +1688,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> * The descriptor and the associated memory reserved here are freed
> * at request complete time.
> */
> + /* \todo Handle null request settings */
Should you know drop this ?
> Camera3RequestDescriptor *descriptor =
> new Camera3RequestDescriptor(camera_.get(), camera3Request);
>
> @@ -1817,6 +1818,7 @@ void CameraDevice::requestComplete(Request *request)
> }
>
> int ret = cameraStream->process(*buffer, &mapped,
> + descriptor->settings_,
> resultMetadata.get());
> if (ret) {
> status = CAMERA3_BUFFER_STATUS_ERROR;
> @@ -1913,14 +1915,19 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
>
> /*
> * \todo Keep this in sync with the actual number of entries.
> - * Currently: 33 entries, 75 bytes
> + * Currently: 38 entries, 147 bytes
> *
> * Reserve more space for the JPEG metadata set by the post-processor.
> * Currently: ANDROID_JPEG_SIZE (int32_t), ANDROID_JPEG_QUALITY (byte),
> + * ANDROID_JPEG_GPS_COORDINATES (double x 3) = 24 bytes
> + * ANDROID_JPEG_GPS_PROCESSING_METHOD (byte x 32) = 32 bytes
> + * ANDROID_JPEG_GPS_TIMESTAMP (int64) = 8 bytes
> * ANDROID_JPEG_ORIENTATION (int32_t) = 3 entries, 9 bytes.
This comment ("= 3 entries, 9 bytes") referred to the three already
populated metadata ANDROID_JPEG_SIZE, ANDROID_JPEG_QUALITY,
ANDROID_JPEG_ORIENTATION. As you're instead listing the size of each
metadata (and I think you should summarize the total size of the
JPEG-related metadata at the end of the comment, like it used to be)
this should just say '= 4 bytes'.
Also, on my previous question about the orientation and its
relationship with the camera rotation/location: as I read from the
description of the property this should not report the requested
orientation, but rather the correction that needs to be applied to
view the image upright. So it seems it really depends on the camera
rotation/orientation.
The property description also contain a code snippet that shows how
to combine the camera rotation and location to report the correct jpeg
orientation. Could you check if it matches your understanding ?
> + * ANDROID_LENS_APERTURE (float) = 4 bytes
> + * ANDROID_LENS_FOCAL_LENGTH (float) = 4 bytes
These are not metadata set by the jpeg post-processor, but they're set
here below. You should drop them from the comment, which serves for
the purpose of reporting what metadata (and how much space they
consume) are not set here but elsewhere.
> */
> std::unique_ptr<CameraMetadata> resultMetadata =
> - std::make_unique<CameraMetadata>(33, 75);
> + std::make_unique<CameraMetadata>(38, 147);
Metadata size and number of entries looks correct now!
> if (!resultMetadata->isValid()) {
> LOG(HAL, Error) << "Failed to allocate static metadata";
> return nullptr;
> @@ -1985,6 +1992,14 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
> value = ANDROID_FLASH_STATE_UNAVAILABLE;
> resultMetadata->addEntry(ANDROID_FLASH_STATE, &value, 1);
>
> + camera_metadata_ro_entry_t entry;
> + int ret = descriptor->settings_.getEntry(ANDROID_LENS_APERTURE, &entry);
> + if (ret)
> + resultMetadata->addEntry(ANDROID_LENS_APERTURE, entry.data.f, 1);
> +
> + float focal_length = 1.0;
> + resultMetadata->addEntry(ANDROID_LENS_FOCAL_LENGTH, &focal_length, 1);
> +
You're here adding 5 more metadata entries, they should be listed in
the availableResultKeys vector. Don't forget to add 20 more bytes to
the static metadata pack size.
> value = ANDROID_LENS_STATE_STATIONARY;
> resultMetadata->addEntry(ANDROID_LENS_STATE, &value, 1);
>
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index dba351a4..4c8020e5 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -96,12 +96,15 @@ int CameraStream::configure()
> }
>
> int CameraStream::process(const libcamera::FrameBuffer &source,
> - MappedCamera3Buffer *dest, CameraMetadata *metadata)
> + MappedCamera3Buffer *dest,
> + const CameraMetadata &requestMetadata,
> + CameraMetadata *resultMetadata)
> {
> if (!postProcessor_)
> return 0;
>
> - return postProcessor_->process(source, dest->maps()[0], metadata);
> + return postProcessor_->process(source, dest->maps()[0],
> + requestMetadata, resultMetadata);
> }
>
> FrameBuffer *CameraStream::getBuffer()
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index cc9d5470..298ffbf6 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -119,7 +119,9 @@ public:
>
> int configure();
> int process(const libcamera::FrameBuffer &source,
> - MappedCamera3Buffer *dest, CameraMetadata *metadata);
> + MappedCamera3Buffer *dest,
> + const CameraMetadata &requestMetadata,
> + CameraMetadata *resultMetadata);
> libcamera::FrameBuffer *getBuffer();
> void putBuffer(libcamera::FrameBuffer *buffer);
>
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index c29cb352..10c71a47 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -82,17 +82,26 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,
>
> int PostProcessorJpeg::process(const FrameBuffer &source,
> Span<uint8_t> destination,
> - CameraMetadata *metadata)
> + const CameraMetadata &requestMetadata,
> + CameraMetadata *resultMetadata)
> {
> if (!encoder_)
> return 0;
>
> + camera_metadata_ro_entry_t entry;
> + int ret;
> +
> /* 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(cameraDevice_->orientation());
> + exif.setMake(cameraDevice_->cameraMake());
> + exif.setModel(cameraDevice_->cameraModel());
> +
> + ret = requestMetadata.getEntry(ANDROID_JPEG_ORIENTATION, &entry);
> +
> + const uint32_t jpegOrientation = ret ? *entry.data.i32 : 0;
> + resultMetadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpegOrientation, 1);
> + exif.setOrientation(jpegOrientation);
> +
> exif.setSize(streamSize_);
> /*
> * We set the frame's EXIF timestamp as the time of encode.
> @@ -101,6 +110,38 @@ int PostProcessorJpeg::process(const FrameBuffer &source,
> */
> exif.setTimestamp(std::time(nullptr), std::chrono::milliseconds(0));
>
> + exif.setExposureTime(0);
You should record with a \todo that onces this information is
available from the libcamera::Request::metadata, it should be come
from there. Same from some of the fields below here.
> + ret = requestMetadata.getEntry(ANDROID_LENS_APERTURE, &entry);
> + if (ret)
> + exif.setAperture(*entry.data.f);
> + exif.setISO(100);
> + exif.setFlash(Exif::Flash::FlashNotPresent);
> + exif.setWhiteBalance(Exif::WhiteBalance::Auto);
> +
> + exif.setFocalLength(1.0);
> +
> + ret = requestMetadata.getEntry(ANDROID_JPEG_GPS_TIMESTAMP, &entry);
> + if (ret) {
> + exif.setGPSDateTimestamp(*entry.data.i64);
> + resultMetadata->addEntry(ANDROID_JPEG_GPS_TIMESTAMP,
> + entry.data.i64, 1);
> + }
> +
> + ret = requestMetadata.getEntry(ANDROID_JPEG_GPS_COORDINATES, &entry);
> + if (ret) {
> + exif.setGPSLocation(entry.data.d);
> + resultMetadata->addEntry(ANDROID_JPEG_GPS_COORDINATES,
> + entry.data.d, 3);
> + }
> +
> + ret = requestMetadata.getEntry(ANDROID_JPEG_GPS_PROCESSING_METHOD, &entry);
> + if (ret) {
> + std::string method(entry.data.u8, entry.data.u8 + entry.count);
> + exif.setGPSMethod(method);
> + resultMetadata->addEntry(ANDROID_JPEG_GPS_PROCESSING_METHOD,
> + entry.data.u8, entry.count);
> + }
> +
I'm a bit puzzled the only thing we have to do is record what the
camera framework tells us to. It might be possible that Android
collects this information from the GPS sub-system and pass it down to
us to record it in the Exif header, so it's not that strange after
all...
> std::vector<unsigned char> thumbnail;
> generateThumbnail(source, &thumbnail);
> if (!thumbnail.empty())
> @@ -135,13 +176,12 @@ int PostProcessorJpeg::process(const FrameBuffer &source,
> blob->jpeg_size = jpeg_size;
>
> /* Update the JPEG result Metadata. */
> - metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);
> -
> - const uint32_t jpeg_quality = 95;
> - metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);
> + resultMetadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);
>
> - const uint32_t jpeg_orientation = 0;
> - metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);
> + /* \todo Configure JPEG encoder with this */
> + ret = requestMetadata.getEntry(ANDROID_JPEG_QUALITY, &entry);
> + const uint32_t jpegQuality = ret ? *entry.data.u8 : 95;
> + resultMetadata->addEntry(ANDROID_JPEG_QUALITY, &jpegQuality, 1);
>
> return 0;
> }
> diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h
> index 5afa831c..d721d1b9 100644
> --- a/src/android/jpeg/post_processor_jpeg.h
> +++ b/src/android/jpeg/post_processor_jpeg.h
> @@ -26,7 +26,8 @@ public:
> const libcamera::StreamConfiguration &outcfg) override;
> int process(const libcamera::FrameBuffer &source,
> libcamera::Span<uint8_t> destination,
> - CameraMetadata *metadata) override;
> + const CameraMetadata &requestMetadata,
> + CameraMetadata *resultMetadata) override;
>
> private:
> void generateThumbnail(const libcamera::FrameBuffer &source,
> diff --git a/src/android/post_processor.h b/src/android/post_processor.h
> index e0f91880..bda93bb4 100644
> --- a/src/android/post_processor.h
> +++ b/src/android/post_processor.h
> @@ -22,7 +22,8 @@ public:
> const libcamera::StreamConfiguration &outCfg) = 0;
> virtual int process(const libcamera::FrameBuffer &source,
> libcamera::Span<uint8_t> destination,
> - CameraMetadata *metadata) = 0;
> + const CameraMetadata &requestMetadata,
> + CameraMetadata *resultMetadata) = 0;
> };
>
> #endif /* __ANDROID_POST_PROCESSOR_H__ */
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
More information about the libcamera-devel
mailing list