[{"id":14723,"web_url":"https://patchwork.libcamera.org/comment/14723/","msgid":"","date":"2021-01-23T09:21:15","subject":"Re: [libcamera-devel] [PATCH v3 5/8] android: Set result metadata\n\tand EXIF fields based on request metadata","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Sat, Jan 23, 2021 at 02:17:01PM +0900, Paul Elder wrote:\n> Set the following android result metadata:\n> - ANDROID_LENS_FOCAL_LENGTH\n> - ANDROID_LENS_APERTURE\n> - ANDROID_JPEG_GPS_TIMESTAMP\n> - ANDROID_JPEG_GPS_COORDINATES\n> - ANDROID_JPEG_GPS_PROCESSING_METHOD\n> - ANDROID_JPEG_SIZE\n> - ANDROID_JPEG_QUALITY\n> - ANDROID_JPEG_ORIENTATION\n> \n> And the following EXIF fields:\n> - GPSDatestamp\n> - GPSTimestamp\n> - GPSLocation\n> - GPSLatitudeRef\n> - GPSLatitude\n> - GPSLongitudeRef\n> - GPSLongitude\n> - GPSAltitudeRef\n> - GPSAltitude\n> - GPSProcessingMethod\n> - FocalLength\n> - ExposureTime\n> - FNumber\n> - ISO\n> - Flash\n> - WhiteBalance\n> - SubsecTime\n> - SubsecTimeOriginal\n> - SubsecTimeDigitized\n> \n> Based on android request metadata.\n> \n> This allows the following CTS tests to pass:\n> - android.hardware.camera2.cts.StillCaptureTest#testFocalLengths\n> - android.hardware.camera2.cts.StillCaptureTest#testJpegExif\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> Changes in v3:\n> - fix metadata entries and byte count\n> - fix gps processing method string truncation\n> - move thumbnail handling to a different patch\n> \n> Changes in v2:\n> - break out processControls and thumbnailer configuration into\n> different\n> patches\n> - fix android metadata entry number and size\n> - other small changes\n> ---\n> src/android/camera_device.cpp | 22 +++++++-\n> src/android/camera_stream.cpp | 7 ++-\n> src/android/camera_stream.h | 4 +-\n> src/android/jpeg/post_processor_jpeg.cpp | 64 +++++++++++++++++++-----\n> src/android/jpeg/post_processor_jpeg.h | 3 +-\n> src/android/post_processor.h | 3 +-\n> 6 files changed, 84 insertions(+), 19 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 592e2d43..d0922db7 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1688,6 +1688,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> \t * The descriptor and the associated memory reserved here are freed\n> \t * at request complete time.\n> \t */\n> +\t/* \\todo Handle null request settings */\n> +\tCameraMetadata settings(camera3Request->settings);\n\nThis variable seems unused.\n\n> \tCamera3RequestDescriptor *descriptor =\n> \t\tnew Camera3RequestDescriptor(camera_.get(), camera3Request);\n> \n> @@ -1817,6 +1819,7 @@ void CameraDevice::requestComplete(Request *request)\n> \t\t}\n> \n> \t\tint ret = cameraStream->process(*buffer, &mapped,\n> +\t\t\t\t\t\tdescriptor->settings_,\n> \t\t\t\t\t\tresultMetadata.get());\n\nIt may make sense to store the result metadata in the descriptor, and\npass the descriptor pointer to the process() function, but that's a\npotential candidate for a later refactoring.\n\n> \t\tif (ret) {\n> \t\t\tstatus = CAMERA3_BUFFER_STATUS_ERROR;\n> @@ -1913,14 +1916,21 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,\n> \n> \t/*\n> \t * \\todo Keep this in sync with the actual number of entries.\n> -\t * Currently: 33 entries, 75 bytes\n> +\t * Currently: 40 entries, 156 bytes\n> \t *\n> \t * Reserve more space for the JPEG metadata set by the post-processor.\n> \t * Currently: ANDROID_JPEG_SIZE (int32_t), ANDROID_JPEG_QUALITY (byte),\n> +\t * ANDROID_JPEG_GPS_COORDINATES (double x 3) = 24 bytes\n> +\t * ANDROID_JPEG_GPS_PROCESSING_METHOD (byte x 32) = 32 bytes\n\nShould this be 40, given the encoding prefix ?\n\n> +\t * ANDROID_JPEG_GPS_TIMESTAMP (int64) = 8 bytes\n> \t * ANDROID_JPEG_ORIENTATION (int32_t) = 3 entries, 9 bytes.\n> +\t * ANDROID_JPEG_THUMBNAIL_QUALITY (byte) = 1 byte\n> +\t * ANDROID_JPEG_THUMBNAIL_SIZE (int32 x 2) = 8 bytes\n\nThese two have been moved to a separate patch.\n\n> +\t * ANDROID_LENS_APERTURE (float) = 4 bytes\n> +\t * ANDROID_LENS_FOCAL_LENGTH (float) = 4 bytes\n> \t */\n> \tstd::unique_ptr resultMetadata =\n> -\t\tstd::make_unique(33, 75);\n> +\t\tstd::make_unique(40, 156);\n> \tif (!resultMetadata->isValid()) {\n> \t\tLOG(HAL, Error) << \"Failed to allocate static metadata\";\n> \t\treturn nullptr;\n> @@ -1985,6 +1995,14 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,\n> \tvalue = ANDROID_FLASH_STATE_UNAVAILABLE;\n> \tresultMetadata->addEntry(ANDROID_FLASH_STATE, &value, 1);\n> \n> +\tcamera_metadata_ro_entry_t entry;\n> +\tint ret = descriptor->settings_.getEntry(ANDROID_LENS_APERTURE, &entry);\n> +\tif (ret)\n> +\t\tresultMetadata->addEntry(ANDROID_LENS_APERTURE, entry.data.f, 1);\n> +\n> +\tfloat focal_length = 1.0;\n> +\tresultMetadata->addEntry(ANDROID_LENS_FOCAL_LENGTH, &focal_length, 1);\n> +\n> \tvalue = ANDROID_LENS_STATE_STATIONARY;\n> \tresultMetadata->addEntry(ANDROID_LENS_STATE, &value, 1);\n> \n> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> index dba351a4..4c8020e5 100644\n> --- a/src/android/camera_stream.cpp\n> +++ b/src/android/camera_stream.cpp\n> @@ -96,12 +96,15 @@ int CameraStream::configure()\n> }\n> \n> int CameraStream::process(const libcamera::FrameBuffer &source,\n> -\t\t\t MappedCamera3Buffer *dest, CameraMetadata *metadata)\n> +\t\t\t MappedCamera3Buffer *dest,\n> +\t\t\t const CameraMetadata &requestMetadata,\n> +\t\t\t CameraMetadata *resultMetadata)\n> {\n> \tif (!postProcessor_)\n> \t\treturn 0;\n> \n> -\treturn postProcessor_->process(source, dest->maps()[0], metadata);\n> +\treturn postProcessor_->process(source, dest->maps()[0],\n> +\t\t\t\t requestMetadata, resultMetadata);\n> }\n> \n> FrameBuffer *CameraStream::getBuffer()\n> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> index cc9d5470..298ffbf6 100644\n> --- a/src/android/camera_stream.h\n> +++ b/src/android/camera_stream.h\n> @@ -119,7 +119,9 @@ public:\n> \n> \tint configure();\n> \tint process(const libcamera::FrameBuffer &source,\n> -\t\t MappedCamera3Buffer *dest, CameraMetadata *metadata);\n> +\t\t MappedCamera3Buffer *dest,\n> +\t\t const CameraMetadata &requestMetadata,\n> +\t\t CameraMetadata *resultMetadata);\n> \tlibcamera::FrameBuffer *getBuffer();\n> \tvoid putBuffer(libcamera::FrameBuffer *buffer);\n> \n> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp\n> index 436a50f8..b325a06a 100644\n> --- a/src/android/jpeg/post_processor_jpeg.cpp\n> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> @@ -80,30 +80,71 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,\n> \n> int PostProcessorJpeg::process(const FrameBuffer &source,\n> \t\t\t Span destination,\n> -\t\t\t CameraMetadata *metadata)\n> +\t\t\t const CameraMetadata &requestMetadata,\n> +\t\t\t CameraMetadata *resultMetadata)\n> {\n> \tif (!encoder_)\n> \t\treturn 0;\n> \n> +\tcamera_metadata_ro_entry_t entry;\n> +\tint ret;\n> +\n> \t/* Set EXIF metadata for various tags. */\n> \tExif exif;\n> -\t/* \\todo Set Make and Model from external vendor tags. */\n> -\texif.setMake(\"libcamera\");\n> -\texif.setModel(\"cameraModel\");\n> -\texif.setOrientation(cameraDevice_->orientation());\n> +\texif.setMake(cameraDevice_->cameraMake());\n> +\texif.setModel(cameraDevice_->cameraModel());\n> +\n> +\tret = requestMetadata.getEntry(ANDROID_JPEG_ORIENTATION, &entry);\n> +\n> +\tconst uint32_t jpegOrientation = ret ? *entry.data.i32 : 0;\n> +\tresultMetadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpegOrientation, 1);\n> +\texif.setOrientation(jpegOrientation);\n> +\n> \texif.setSize(streamSize_);\n> \t/*\n> \t * We set the frame's EXIF timestamp as the time of encode.\n> \t * Since the precision we need for EXIF timestamp is only one\n> \t * second, it is good enough.\n> \t */\n> -\texif.setTimestamp(std::time(nullptr));\n> +\texif.setTimestamp(std::time(nullptr), 0);\n\nThis belongs to a previous patch.\n\n> +\n> +\texif.setExposureTime(0);\n> +\tret = requestMetadata.getEntry(ANDROID_LENS_APERTURE, &entry);\n> +\tif (ret)\n> +\t\texif.setAperture(*entry.data.f);\n> +\texif.setISO(100);\n> +\texif.setFlash(Exif::Flash::FlashNotPresent);\n> +\texif.setWhiteBalance(Exif::WhiteBalance::Auto);\n> +\n> +\texif.setFocalLength(1.0);\n> +\n> +\tret = requestMetadata.getEntry(ANDROID_JPEG_GPS_TIMESTAMP, &entry);\n> +\tif (ret) {\n> +\t\texif.setGPSDateTimestamp(*entry.data.i64);\n> +\t\tresultMetadata->addEntry(ANDROID_JPEG_GPS_TIMESTAMP,\n> +\t\t\t\t\t entry.data.i64, 1);\n> +\t}\n> \n> \tstd::vector thumbnail;\n> \tgenerateThumbnail(source, &thumbnail);\n> \tif (!thumbnail.empty())\n> \t\texif.setThumbnail(thumbnail, Exif::Compression::JPEG);\n> \n> +\tret = requestMetadata.getEntry(ANDROID_JPEG_GPS_COORDINATES, &entry);\n> +\tif (ret) {\n> +\t\texif.setGPSLocation(entry.data.d);\n> +\t\tresultMetadata->addEntry(ANDROID_JPEG_GPS_COORDINATES,\n> +\t\t\t\t\t entry.data.d, 3);\n> +\t}\n> +\n> +\tret = requestMetadata.getEntry(ANDROID_JPEG_GPS_PROCESSING_METHOD, &entry);\n> +\tif (ret) {\n> +\t\tstd::string method(entry.data.u8, entry.data.u8 + entry.count);\n> +\t\texif.setGPSMethod(method);\n> +\t\tresultMetadata->addEntry(ANDROID_JPEG_GPS_PROCESSING_METHOD,\n> +\t\t\t\t\t entry.data.u8, entry.count);\n> +\t}\n> +\n\nCould you move these two above thumbnail generation, to group them with\nthe GPS timestamp ?\n\n> \tif (exif.generate() != 0)\n> \t\tLOG(JPEG, Error) << \"Failed to generate valid EXIF data\";\n> \n> @@ -133,13 +174,12 @@ int PostProcessorJpeg::process(const FrameBuffer &source,\n> \tblob->jpeg_size = jpeg_size;\n> \n> \t/* Update the JPEG result Metadata. */\n> -\tmetadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);\n> -\n> -\tconst uint32_t jpeg_quality = 95;\n> -\tmetadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);\n> +\tresultMetadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);\n> \n> -\tconst uint32_t jpeg_orientation = 0;\n> -\tmetadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);\n> +\t/* \\todo Configure JPEG encoder with this */\n> +\tret = requestMetadata.getEntry(ANDROID_JPEG_QUALITY, &entry);\n> +\tconst uint32_t jpeg_quality = ret ? *entry.data.u8 : 95;\n\ns/jpeg_quality/jpegQuality/\n\nReviewed-by: Laurent Pinchart \n\n> +\tresultMetadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);\n> \n> \treturn 0;\n> }\n> diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h\n> index 5afa831c..d721d1b9 100644\n> --- a/src/android/jpeg/post_processor_jpeg.h\n> +++ b/src/android/jpeg/post_processor_jpeg.h\n> @@ -26,7 +26,8 @@ public:\n> \t\t const libcamera::StreamConfiguration &outcfg) override;\n> \tint process(const libcamera::FrameBuffer &source,\n> \t\t libcamera::Span destination,\n> -\t\t CameraMetadata *metadata) override;\n> +\t\t const CameraMetadata &requestMetadata,\n> +\t\t CameraMetadata *resultMetadata) override;\n> \n> private:\n> \tvoid generateThumbnail(const libcamera::FrameBuffer &source,\n> diff --git a/src/android/post_processor.h b/src/android/post_processor.h\n> index e0f91880..bda93bb4 100644\n> --- a/src/android/post_processor.h\n> +++ b/src/android/post_processor.h\n> @@ -22,7 +22,8 @@ public:\n> \t\t\t const libcamera::StreamConfiguration &outCfg) = 0;\n> \tvirtual int process(const libcamera::FrameBuffer &source,\n> \t\t\t libcamera::Span destination,\n> -\t\t\t CameraMetadata *metadata) = 0;\n> +\t\t\t const CameraMetadata &requestMetadata,\n> +\t\t\t CameraMetadata *resultMetadata) = 0;\n> };\n> \n> #endif /* __ANDROID_POST_PROCESSOR_H__ */","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A96EABD808\n\tfor ;\n\tSat, 23 Jan 2021 09:36:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2B61F68294;\n\tSat, 23 Jan 2021 10:21:37 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 214056827C\n\tfor ;\n\tSat, 23 Jan 2021 10:21:36 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 71EE71491;\n\tSat, 23 Jan 2021 10:21:35 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"vmSVM/z+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611393695;\n\tbh=wprPVDQELOVJs9TjxNIx9ANc4cpSbeqeWUd17LRB3D8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vmSVM/z+kz7yLDHk5bTfYzvFg3mEHTCMAWGNZio9FHftTqH717atfYXa/zmFRf66j\n\tLIsXWbYcjqN6zwA6Pi94UIOvDrtA2HcnoDHXyozzRyQO8wpPCdJaJpPwATl9hDG3mJ\n\tjE4cW15mlboUZW8GSTCI7THchYDqamcZyZWDnWnU=","Date":"Sat, 23 Jan 2021 11:21:15 +0200","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"","References":"<20210123051704.188117-1-paul.elder@ideasonboard.com>\n\t<20210123051704.188117-6-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210123051704.188117-6-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 5/8] android: Set result metadata\n\tand EXIF fields based on request metadata","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]