[{"id":14741,"web_url":"https://patchwork.libcamera.org/comment/14741/","msgid":"<YA6VP4hX2YddzX+g@pendragon.ideasonboard.com>","date":"2021-01-25T09:54:07","subject":"Re: [libcamera-devel] [PATCH v4 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 Mon, Jan 25, 2021 at 04:14:41PM +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 <paul.elder@ideasonboard.com>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> ---\n> Changes in v4:\n> - remove unused variable\n> - move android JPEG thumbnail tags allocation to the thumbnail patch\n> - move setting the timestamp with subsec to the patch that adds set\n>   subsec to timestamp (\"android: jpeg: exif: Add functions for\n>   setting various values\")\n> - group setting GPS-related tags\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            | 19 +++++++-\n>  src/android/camera_stream.cpp            |  7 ++-\n>  src/android/camera_stream.h              |  4 +-\n>  src/android/jpeg/post_processor_jpeg.cpp | 62 +++++++++++++++++++-----\n>  src/android/jpeg/post_processor_jpeg.h   |  3 +-\n>  src/android/post_processor.h             |  3 +-\n>  6 files changed, 80 insertions(+), 18 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 592e2d43..2cd3c8a1 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1688,6 +1688,7 @@ 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>  \tCamera3RequestDescriptor *descriptor =\n>  \t\tnew Camera3RequestDescriptor(camera_.get(), camera3Request);\n>  \n> @@ -1817,6 +1818,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>  \t\tif (ret) {\n>  \t\t\tstatus = CAMERA3_BUFFER_STATUS_ERROR;\n> @@ -1913,14 +1915,19 @@ 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: 38 entries, 147 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_LENS_APERTURE (float) = 4 bytes\n> +\t * ANDROID_LENS_FOCAL_LENGTH (float) = 4 bytes\n>  \t */\n>  \tstd::unique_ptr<CameraMetadata> resultMetadata =\n> -\t\tstd::make_unique<CameraMetadata>(33, 75);\n> +\t\tstd::make_unique<CameraMetadata>(38, 147);\n>  \tif (!resultMetadata->isValid()) {\n>  \t\tLOG(HAL, Error) << \"Failed to allocate static metadata\";\n>  \t\treturn nullptr;\n> @@ -1985,6 +1992,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 c29cb352..10c71a47 100644\n> --- a/src/android/jpeg/post_processor_jpeg.cpp\n> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> @@ -82,17 +82,26 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,\n>  \n>  int PostProcessorJpeg::process(const FrameBuffer &source,\n>  \t\t\t       Span<uint8_t> 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> @@ -101,6 +110,38 @@ int PostProcessorJpeg::process(const FrameBuffer &source,\n>  \t */\n>  \texif.setTimestamp(std::time(nullptr), std::chrono::milliseconds(0));\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> +\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>  \tstd::vector<unsigned char> thumbnail;\n>  \tgenerateThumbnail(source, &thumbnail);\n>  \tif (!thumbnail.empty())\n> @@ -135,13 +176,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 jpegQuality = ret ? *entry.data.u8 : 95;\n\ns/uint32_t/uint8_t/ or the next call won't work on big endian platforms.\n\n> +\tresultMetadata->addEntry(ANDROID_JPEG_QUALITY, &jpegQuality, 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<uint8_t> 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<uint8_t> 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":"<libcamera-devel-bounces@lists.libcamera.org>","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 E64CFBD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 25 Jan 2021 09:54:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6AE77682B4;\n\tMon, 25 Jan 2021 10:54:29 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3F4296030B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Jan 2021 10:54:28 +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 9651E3E;\n\tMon, 25 Jan 2021 10:54:26 +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=\"ljtMsTQN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611568466;\n\tbh=ssJ+vN3ArKG5VTSUUQR2rFFi4RWKGhm93L0emXLm81s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ljtMsTQN7lpPnjgb/U7jOlPFaEGrivawgseuPJm2K0zpNoB/aJUxdRsnrg8oLHbhI\n\tPSFAoVc2xPkKreqamCouLMmAUww/8XtQZ6bi+GstscoWBCmXZHN/gO+soaxwYPU7Eg\n\tdROW/0PZ5yN9cRSVlqwFMVuvr7yOve57GwaOHy9w=","Date":"Mon, 25 Jan 2021 11:54:07 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>","Message-ID":"<YA6VP4hX2YddzX+g@pendragon.ideasonboard.com>","References":"<20210125071444.26252-1-paul.elder@ideasonboard.com>\n\t<20210125071444.26252-6-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210125071444.26252-6-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14743,"web_url":"https://patchwork.libcamera.org/comment/14743/","msgid":"<20210125101154.GA2215@pyrite.rasen.tech>","date":"2021-01-25T10:11:54","subject":"Re: [libcamera-devel] [PATCH v4 5/8] android: Set result metadata\n\tand EXIF fields based on request metadata","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Mon, Jan 25, 2021 at 11:54:07AM +0200, Laurent Pinchart wrote:\n> Hi Paul,\n> \n> Thank you for the patch.\n> \n> On Mon, Jan 25, 2021 at 04:14:41PM +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 <paul.elder@ideasonboard.com>\n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > \n> > ---\n> > Changes in v4:\n> > - remove unused variable\n> > - move android JPEG thumbnail tags allocation to the thumbnail patch\n> > - move setting the timestamp with subsec to the patch that adds set\n> >   subsec to timestamp (\"android: jpeg: exif: Add functions for\n> >   setting various values\")\n> > - group setting GPS-related tags\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            | 19 +++++++-\n> >  src/android/camera_stream.cpp            |  7 ++-\n> >  src/android/camera_stream.h              |  4 +-\n> >  src/android/jpeg/post_processor_jpeg.cpp | 62 +++++++++++++++++++-----\n> >  src/android/jpeg/post_processor_jpeg.h   |  3 +-\n> >  src/android/post_processor.h             |  3 +-\n> >  6 files changed, 80 insertions(+), 18 deletions(-)\n> > \n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 592e2d43..2cd3c8a1 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -1688,6 +1688,7 @@ 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> >  \tCamera3RequestDescriptor *descriptor =\n> >  \t\tnew Camera3RequestDescriptor(camera_.get(), camera3Request);\n> >  \n> > @@ -1817,6 +1818,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> >  \t\tif (ret) {\n> >  \t\t\tstatus = CAMERA3_BUFFER_STATUS_ERROR;\n> > @@ -1913,14 +1915,19 @@ 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: 38 entries, 147 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> \n> Should this be 40, given the encoding prefix \n\nNo, the encoding prefix is only for exif. Android specifically wants 32\nbytes of null-terminated UTF-8.\n\n> > +\t * ANDROID_JPEG_GPS_TIMESTAMP (int64) = 8 bytes\n> >  \t * ANDROID_JPEG_ORIENTATION (int32_t) = 3 entries, 9 bytes.\n> > +\t * ANDROID_LENS_APERTURE (float) = 4 bytes\n> > +\t * ANDROID_LENS_FOCAL_LENGTH (float) = 4 bytes\n> >  \t */\n> >  \tstd::unique_ptr<CameraMetadata> resultMetadata =\n> > -\t\tstd::make_unique<CameraMetadata>(33, 75);\n> > +\t\tstd::make_unique<CameraMetadata>(38, 147);\n> >  \tif (!resultMetadata->isValid()) {\n> >  \t\tLOG(HAL, Error) << \"Failed to allocate static metadata\";\n> >  \t\treturn nullptr;\n> > @@ -1985,6 +1992,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 c29cb352..10c71a47 100644\n> > --- a/src/android/jpeg/post_processor_jpeg.cpp\n> > +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> > @@ -82,17 +82,26 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,\n> >  \n> >  int PostProcessorJpeg::process(const FrameBuffer &source,\n> >  \t\t\t       Span<uint8_t> 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> > @@ -101,6 +110,38 @@ int PostProcessorJpeg::process(const FrameBuffer &source,\n> >  \t */\n> >  \texif.setTimestamp(std::time(nullptr), std::chrono::milliseconds(0));\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> > +\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> >  \tstd::vector<unsigned char> thumbnail;\n> >  \tgenerateThumbnail(source, &thumbnail);\n> >  \tif (!thumbnail.empty())\n> > @@ -135,13 +176,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 jpegQuality = ret ? *entry.data.u8 : 95;\n> \n> s/uint32_t/uint8_t/ or the next call won't work on big endian platforms.\n\nForgot about it here after I fixed it in a later patch.\n\n\nThanks,\n\nPaul\n\n> > +\tresultMetadata->addEntry(ANDROID_JPEG_QUALITY, &jpegQuality, 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<uint8_t> 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<uint8_t> 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":"<libcamera-devel-bounces@lists.libcamera.org>","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 16EBABD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 25 Jan 2021 10:12:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 95177682B0;\n\tMon, 25 Jan 2021 11:12:03 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 870DE6030B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Jan 2021 11:12:02 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D75753E;\n\tMon, 25 Jan 2021 11:12:00 +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=\"MepTyBUJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611569522;\n\tbh=x+jrnuOLeQUixgbiqswrgzAGilPISIPlS/N+Jtz5Cwc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=MepTyBUJfGdmgdnTknfBAKSV/+pOZGMZ/8M4dRafgslLthIgm8lYRPE2jSuJSPtsE\n\tfJJh+dzRKFzsmav+Tf6AKdpCAfWr99aEQoOp1niRBxJrGwtXHVy0M1hpfRSjp8M2cF\n\ta7TjMe8a75ESJFCiEjmYiPTY1AUEA4YVeEhr+Dyc=","Date":"Mon, 25 Jan 2021 19:11:54 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20210125101154.GA2215@pyrite.rasen.tech>","References":"<20210125071444.26252-1-paul.elder@ideasonboard.com>\n\t<20210125071444.26252-6-paul.elder@ideasonboard.com>\n\t<YA6VP4hX2YddzX+g@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<YA6VP4hX2YddzX+g@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14758,"web_url":"https://patchwork.libcamera.org/comment/14758/","msgid":"<20210125115134.k75gjmnohdyklirq@uno.localdomain>","date":"2021-01-25T11:51:34","subject":"Re: [libcamera-devel] [PATCH v4 5/8] android: Set result metadata\n\tand EXIF fields based on request metadata","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n\nOn Mon, Jan 25, 2021 at 04:14:41PM +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 <paul.elder@ideasonboard.com>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>\n> ---\n> Changes in v4:\n> - remove unused variable\n> - move android JPEG thumbnail tags allocation to the thumbnail patch\n> - move setting the timestamp with subsec to the patch that adds set\n>   subsec to timestamp (\"android: jpeg: exif: Add functions for\n>   setting various values\")\n> - group setting GPS-related tags\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            | 19 +++++++-\n>  src/android/camera_stream.cpp            |  7 ++-\n>  src/android/camera_stream.h              |  4 +-\n>  src/android/jpeg/post_processor_jpeg.cpp | 62 +++++++++++++++++++-----\n>  src/android/jpeg/post_processor_jpeg.h   |  3 +-\n>  src/android/post_processor.h             |  3 +-\n>  6 files changed, 80 insertions(+), 18 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 592e2d43..2cd3c8a1 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1688,6 +1688,7 @@ 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\nShould you know drop this ?\n\n>  \tCamera3RequestDescriptor *descriptor =\n>  \t\tnew Camera3RequestDescriptor(camera_.get(), camera3Request);\n>\n> @@ -1817,6 +1818,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>  \t\tif (ret) {\n>  \t\t\tstatus = CAMERA3_BUFFER_STATUS_ERROR;\n> @@ -1913,14 +1915,19 @@ 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: 38 entries, 147 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> +\t * ANDROID_JPEG_GPS_TIMESTAMP (int64) = 8 bytes\n>  \t * ANDROID_JPEG_ORIENTATION (int32_t) = 3 entries, 9 bytes.\n\nThis comment (\"= 3 entries, 9 bytes\") referred to the three already\npopulated metadata  ANDROID_JPEG_SIZE, ANDROID_JPEG_QUALITY,\nANDROID_JPEG_ORIENTATION. As you're instead listing the size of each\nmetadata (and I think you should summarize the total size of the\nJPEG-related metadata at the end of the comment, like it used to be)\nthis should just say '= 4 bytes'.\n\nAlso, on my previous question about the orientation and its\nrelationship with the camera rotation/location: as I read from the\ndescription of the property this should not report the requested\norientation, but rather the correction that needs to be applied to\nview the image upright. So it seems it really depends on the camera\nrotation/orientation.\n\nThe property description also contain a code snippet that shows how\nto combine the camera rotation and location to report the correct jpeg\norientation. Could you check if it matches your understanding ?\n\n> +\t * ANDROID_LENS_APERTURE (float) = 4 bytes\n> +\t * ANDROID_LENS_FOCAL_LENGTH (float) = 4 bytes\n\nThese are not metadata set by the jpeg post-processor, but they're set\nhere below. You should drop them from the comment, which serves for\nthe purpose of reporting what metadata (and how much space they\nconsume) are not set here but elsewhere.\n\n>  \t */\n>  \tstd::unique_ptr<CameraMetadata> resultMetadata =\n> -\t\tstd::make_unique<CameraMetadata>(33, 75);\n> +\t\tstd::make_unique<CameraMetadata>(38, 147);\n\nMetadata size and number of entries looks correct now!\n\n>  \tif (!resultMetadata->isValid()) {\n>  \t\tLOG(HAL, Error) << \"Failed to allocate static metadata\";\n>  \t\treturn nullptr;\n> @@ -1985,6 +1992,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\nYou're here adding 5 more metadata entries, they should be listed in\nthe availableResultKeys vector. Don't forget to add 20 more bytes to\nthe static metadata pack size.\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 c29cb352..10c71a47 100644\n> --- a/src/android/jpeg/post_processor_jpeg.cpp\n> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> @@ -82,17 +82,26 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,\n>\n>  int PostProcessorJpeg::process(const FrameBuffer &source,\n>  \t\t\t       Span<uint8_t> 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> @@ -101,6 +110,38 @@ int PostProcessorJpeg::process(const FrameBuffer &source,\n>  \t */\n>  \texif.setTimestamp(std::time(nullptr), std::chrono::milliseconds(0));\n>\n> +\texif.setExposureTime(0);\n\nYou should record with a \\todo that onces this information is\navailable from the libcamera::Request::metadata, it should be come\nfrom there. Same from some of the fields below here.\n\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> +\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\nI'm a bit puzzled the only thing we have to do is record what the\ncamera framework tells us to. It might be possible that Android\ncollects this information from the GPS sub-system and pass it down to\nus to record it in the Exif header, so it's not that strange after\nall...\n\n>  \tstd::vector<unsigned char> thumbnail;\n>  \tgenerateThumbnail(source, &thumbnail);\n>  \tif (!thumbnail.empty())\n> @@ -135,13 +176,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 jpegQuality = ret ? *entry.data.u8 : 95;\n> +\tresultMetadata->addEntry(ANDROID_JPEG_QUALITY, &jpegQuality, 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<uint8_t> 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<uint8_t> 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__ */\n> --\n> 2.27.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 920C9BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 25 Jan 2021 11:51:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E7A17682C6;\n\tMon, 25 Jan 2021 12:51:15 +0100 (CET)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4E0D9682BB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Jan 2021 12:51:14 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id D0A57C0003;\n\tMon, 25 Jan 2021 11:51:13 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 25 Jan 2021 12:51:34 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Paul Elder <paul.elder@ideasonboard.com>","Message-ID":"<20210125115134.k75gjmnohdyklirq@uno.localdomain>","References":"<20210125071444.26252-1-paul.elder@ideasonboard.com>\n\t<20210125071444.26252-6-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210125071444.26252-6-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14763,"web_url":"https://patchwork.libcamera.org/comment/14763/","msgid":"<YA6/1UKRGx/GN7qF@pendragon.ideasonboard.com>","date":"2021-01-25T12:55:49","subject":"Re: [libcamera-devel] [PATCH v4 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 Jacopo,\n\nOn Mon, Jan 25, 2021 at 12:51:34PM +0100, Jacopo Mondi wrote:\n> On Mon, Jan 25, 2021 at 04:14:41PM +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 <paul.elder@ideasonboard.com>\n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> >\n> > ---\n> > Changes in v4:\n> > - remove unused variable\n> > - move android JPEG thumbnail tags allocation to the thumbnail patch\n> > - move setting the timestamp with subsec to the patch that adds set\n> >   subsec to timestamp (\"android: jpeg: exif: Add functions for\n> >   setting various values\")\n> > - group setting GPS-related tags\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            | 19 +++++++-\n> >  src/android/camera_stream.cpp            |  7 ++-\n> >  src/android/camera_stream.h              |  4 +-\n> >  src/android/jpeg/post_processor_jpeg.cpp | 62 +++++++++++++++++++-----\n> >  src/android/jpeg/post_processor_jpeg.h   |  3 +-\n> >  src/android/post_processor.h             |  3 +-\n> >  6 files changed, 80 insertions(+), 18 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 592e2d43..2cd3c8a1 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -1688,6 +1688,7 @@ 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> \n> Should you know drop this ?\n> \n> >  \tCamera3RequestDescriptor *descriptor =\n> >  \t\tnew Camera3RequestDescriptor(camera_.get(), camera3Request);\n> >\n> > @@ -1817,6 +1818,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> >  \t\tif (ret) {\n> >  \t\t\tstatus = CAMERA3_BUFFER_STATUS_ERROR;\n> > @@ -1913,14 +1915,19 @@ 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: 38 entries, 147 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> > +\t * ANDROID_JPEG_GPS_TIMESTAMP (int64) = 8 bytes\n> >  \t * ANDROID_JPEG_ORIENTATION (int32_t) = 3 entries, 9 bytes.\n> \n> This comment (\"= 3 entries, 9 bytes\") referred to the three already\n> populated metadata  ANDROID_JPEG_SIZE, ANDROID_JPEG_QUALITY,\n> ANDROID_JPEG_ORIENTATION. As you're instead listing the size of each\n> metadata (and I think you should summarize the total size of the\n> JPEG-related metadata at the end of the comment, like it used to be)\n> this should just say '= 4 bytes'.\n> \n> Also, on my previous question about the orientation and its\n> relationship with the camera rotation/location: as I read from the\n> description of the property this should not report the requested\n> orientation, but rather the correction that needs to be applied to\n> view the image upright. So it seems it really depends on the camera\n> rotation/orientation.\n> \n> The property description also contain a code snippet that shows how\n> to combine the camera rotation and location to report the correct jpeg\n> orientation. Could you check if it matches your understanding ?\n\nAs far as I understand, that code should be used by the camera service\nto compute the JPEG orientation to pass to the HAL. From a HAL point of\nview, we get an orientation that we need to apply to the JPEG image,\neither by rotating the image, or by recording it in Exif. I'm not sure\nwhy we have to report it back in dynamic metadata as it should always\nmatch the value we get, but Android an I have no always agreed on design\nchoices :-)\n\n> > +\t * ANDROID_LENS_APERTURE (float) = 4 bytes\n> > +\t * ANDROID_LENS_FOCAL_LENGTH (float) = 4 bytes\n> \n> These are not metadata set by the jpeg post-processor, but they're set\n> here below. You should drop them from the comment, which serves for\n> the purpose of reporting what metadata (and how much space they\n> consume) are not set here but elsewhere.\n> \n> >  \t */\n> >  \tstd::unique_ptr<CameraMetadata> resultMetadata =\n> > -\t\tstd::make_unique<CameraMetadata>(33, 75);\n> > +\t\tstd::make_unique<CameraMetadata>(38, 147);\n> \n> Metadata size and number of entries looks correct now!\n> \n> >  \tif (!resultMetadata->isValid()) {\n> >  \t\tLOG(HAL, Error) << \"Failed to allocate static metadata\";\n> >  \t\treturn nullptr;\n> > @@ -1985,6 +1992,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> \n> You're here adding 5 more metadata entries, they should be listed in\n> the availableResultKeys vector. Don't forget to add 20 more bytes to\n> the static metadata pack size.\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 c29cb352..10c71a47 100644\n> > --- a/src/android/jpeg/post_processor_jpeg.cpp\n> > +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> > @@ -82,17 +82,26 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,\n> >\n> >  int PostProcessorJpeg::process(const FrameBuffer &source,\n> >  \t\t\t       Span<uint8_t> 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> > @@ -101,6 +110,38 @@ int PostProcessorJpeg::process(const FrameBuffer &source,\n> >  \t */\n> >  \texif.setTimestamp(std::time(nullptr), std::chrono::milliseconds(0));\n> >\n> > +\texif.setExposureTime(0);\n> \n> You should record with a \\todo that onces this information is\n> available from the libcamera::Request::metadata, it should be come\n> from there. Same from some of the fields below here.\n> \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> > +\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> \n> I'm a bit puzzled the only thing we have to do is record what the\n> camera framework tells us to. It might be possible that Android\n> collects this information from the GPS sub-system and pass it down to\n> us to record it in the Exif header, so it's not that strange after\n> all...\n\nAs far as I understand, yes, that's what happens. I wonder why the\nAndroid camera service doesn't update the Exif itself...\n\n> >  \tstd::vector<unsigned char> thumbnail;\n> >  \tgenerateThumbnail(source, &thumbnail);\n> >  \tif (!thumbnail.empty())\n> > @@ -135,13 +176,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 jpegQuality = ret ? *entry.data.u8 : 95;\n> > +\tresultMetadata->addEntry(ANDROID_JPEG_QUALITY, &jpegQuality, 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<uint8_t> 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<uint8_t> 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":"<libcamera-devel-bounces@lists.libcamera.org>","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 BD3E3C0F2B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 25 Jan 2021 12:56:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4927E682CC;\n\tMon, 25 Jan 2021 13:56:12 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 22507682C8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Jan 2021 13:56:10 +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 51FA5331;\n\tMon, 25 Jan 2021 13:56:08 +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=\"npsUM6JM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611579368;\n\tbh=5jGgh9GHcES3khlC/RS3CVW2nZfZpWAWBA/PPZHbFjI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=npsUM6JMkHk9EhsfK8tTtqOe5OxWszQvdl0SMNB7s88IcswFQ5LyDbrHsfSrD45n7\n\tz4bF+1m7T0bn2wB6n6DWqEt0QFnDhNViZSNJn1ECWVm7/vlsaPAlr71Hw+6iCyZv+X\n\twrM5IGbvAUt+zs47a4F2h2YL2BSa0LJ+NH7WN0fw=","Date":"Mon, 25 Jan 2021 14:55:49 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<YA6/1UKRGx/GN7qF@pendragon.ideasonboard.com>","References":"<20210125071444.26252-1-paul.elder@ideasonboard.com>\n\t<20210125071444.26252-6-paul.elder@ideasonboard.com>\n\t<20210125115134.k75gjmnohdyklirq@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210125115134.k75gjmnohdyklirq@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v4 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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","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\" <libcamera-devel-bounces@lists.libcamera.org>"}}]