Message ID | 20210123051704.188117-6-paul.elder@ideasonboard.com |
---|---|
State | Superseded |
Delegated to: | Paul Elder |
Headers | show |
Series |
|
Related | show |
Hi Paul, Thank you for the patch. On Sat, Jan 23, 2021 at 02:17:01PM +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@ideasonboard.com> > > --- > 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 | 22 +++++++- > src/android/camera_stream.cpp | 7 ++- > src/android/camera_stream.h | 4 +- > src/android/jpeg/post_processor_jpeg.cpp | 64 +++++++++++++++++++----- > src/android/jpeg/post_processor_jpeg.h | 3 +- > src/android/post_processor.h | 3 +- > 6 files changed, 84 insertions(+), 19 deletions(-) > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > index 592e2d43..d0922db7 100644 > --- a/src/android/camera_device.cpp > +++ b/src/android/camera_device.cpp > @@ -1688,6 +1688,8 @@ 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 */ > + CameraMetadata settings(camera3Request->settings); This variable seems unused. > Camera3RequestDescriptor *descriptor = > new Camera3RequestDescriptor(camera_.get(), camera3Request); > > @@ -1817,6 +1819,7 @@ void CameraDevice::requestComplete(Request *request) > } > > int ret = cameraStream->process(*buffer, &mapped, > + descriptor->settings_, > resultMetadata.get()); It may make sense to store the result metadata in the descriptor, and pass the descriptor pointer to the process() function, but that's a potential candidate for a later refactoring. > if (ret) { > status = CAMERA3_BUFFER_STATUS_ERROR; > @@ -1913,14 +1916,21 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor, > > /* > * \todo Keep this in sync with the actual number of entries. > - * Currently: 33 entries, 75 bytes > + * Currently: 40 entries, 156 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 Should this be 40, given the encoding prefix ? > + * ANDROID_JPEG_GPS_TIMESTAMP (int64) = 8 bytes > * ANDROID_JPEG_ORIENTATION (int32_t) = 3 entries, 9 bytes. > + * ANDROID_JPEG_THUMBNAIL_QUALITY (byte) = 1 byte > + * ANDROID_JPEG_THUMBNAIL_SIZE (int32 x 2) = 8 bytes These two have been moved to a separate patch. > + * ANDROID_LENS_APERTURE (float) = 4 bytes > + * ANDROID_LENS_FOCAL_LENGTH (float) = 4 bytes > */ > std::unique_ptr<CameraMetadata> resultMetadata = > - std::make_unique<CameraMetadata>(33, 75); > + std::make_unique<CameraMetadata>(40, 156); > if (!resultMetadata->isValid()) { > LOG(HAL, Error) << "Failed to allocate static metadata"; > return nullptr; > @@ -1985,6 +1995,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); > + > 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 436a50f8..b325a06a 100644 > --- a/src/android/jpeg/post_processor_jpeg.cpp > +++ b/src/android/jpeg/post_processor_jpeg.cpp > @@ -80,30 +80,71 @@ 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. > * Since the precision we need for EXIF timestamp is only one > * second, it is good enough. > */ > - exif.setTimestamp(std::time(nullptr)); > + exif.setTimestamp(std::time(nullptr), 0); This belongs to a previous patch. > + > + exif.setExposureTime(0); > + 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); > + } > > std::vector<unsigned char> thumbnail; > generateThumbnail(source, &thumbnail); > if (!thumbnail.empty()) > exif.setThumbnail(thumbnail, Exif::Compression::JPEG); > > + 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); > + } > + Could you move these two above thumbnail generation, to group them with the GPS timestamp ? > if (exif.generate() != 0) > LOG(JPEG, Error) << "Failed to generate valid EXIF data"; > > @@ -133,13 +174,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 jpeg_quality = ret ? *entry.data.u8 : 95; s/jpeg_quality/jpegQuality/ Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + resultMetadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 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__ */
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 592e2d43..d0922db7 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -1688,6 +1688,8 @@ 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 */ + CameraMetadata settings(camera3Request->settings); Camera3RequestDescriptor *descriptor = new Camera3RequestDescriptor(camera_.get(), camera3Request); @@ -1817,6 +1819,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 +1916,21 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor, /* * \todo Keep this in sync with the actual number of entries. - * Currently: 33 entries, 75 bytes + * Currently: 40 entries, 156 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. + * ANDROID_JPEG_THUMBNAIL_QUALITY (byte) = 1 byte + * ANDROID_JPEG_THUMBNAIL_SIZE (int32 x 2) = 8 bytes + * ANDROID_LENS_APERTURE (float) = 4 bytes + * ANDROID_LENS_FOCAL_LENGTH (float) = 4 bytes */ std::unique_ptr<CameraMetadata> resultMetadata = - std::make_unique<CameraMetadata>(33, 75); + std::make_unique<CameraMetadata>(40, 156); if (!resultMetadata->isValid()) { LOG(HAL, Error) << "Failed to allocate static metadata"; return nullptr; @@ -1985,6 +1995,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); + 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 436a50f8..b325a06a 100644 --- a/src/android/jpeg/post_processor_jpeg.cpp +++ b/src/android/jpeg/post_processor_jpeg.cpp @@ -80,30 +80,71 @@ 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. * Since the precision we need for EXIF timestamp is only one * second, it is good enough. */ - exif.setTimestamp(std::time(nullptr)); + exif.setTimestamp(std::time(nullptr), 0); + + exif.setExposureTime(0); + 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); + } std::vector<unsigned char> thumbnail; generateThumbnail(source, &thumbnail); if (!thumbnail.empty()) exif.setThumbnail(thumbnail, Exif::Compression::JPEG); + 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); + } + if (exif.generate() != 0) LOG(JPEG, Error) << "Failed to generate valid EXIF data"; @@ -133,13 +174,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 jpeg_quality = ret ? *entry.data.u8 : 95; + resultMetadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 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__ */
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@ideasonboard.com> --- 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 | 22 +++++++- src/android/camera_stream.cpp | 7 ++- src/android/camera_stream.h | 4 +- src/android/jpeg/post_processor_jpeg.cpp | 64 +++++++++++++++++++----- src/android/jpeg/post_processor_jpeg.h | 3 +- src/android/post_processor.h | 3 +- 6 files changed, 84 insertions(+), 19 deletions(-)