[{"id":16621,"web_url":"https://patchwork.libcamera.org/comment/16621/","msgid":"","date":"2021-04-27T05:21:44","subject":"Re: [libcamera-devel] [RFC PATCH v2 12/12] android camera_device:\n\tSet result metadata from libcamera 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 Thu, Apr 22, 2021 at 06:41:02PM +0900, Paul Elder wrote:\n> Set the result metadata required for FULL hardware level based on the\n> metadata returned from libcamera.\n\nI'm afraid I have an annoying request: could you reorder patches to\navoid hardcoding values and only making them dynamic here ? It's hard to\nreview 07/12 otherwise. It should hopefully not cause conflicts. I think\n07/12 and 12/12 should be squashed.\n\nAnother option is to keep the current order, and remove from 07/12 the\nmetadata that is touched in 12/12, keeping only the HAL hardcoded values\nin 07/12. Not sure if there's any though, I haven't checked :-)\n\n> Signed-off-by: Paul Elder \n> ---\n> src/android/camera_device.cpp | 98 ++++++++++++++++-------------------\n> src/android/camera_device.h | 2 -\n> 2 files changed, 46 insertions(+), 54 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 30692a67..303767e5 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -402,7 +402,7 @@ CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor(\n> \n> CameraDevice::CameraDevice(unsigned int id, std::shared_ptr camera)\n> \t: id_(id), running_(false), camera_(std::move(camera)),\n> -\t facing_(CAMERA_FACING_FRONT), orientation_(0), lastTimestamp_(0)\n> +\t facing_(CAMERA_FACING_FRONT), orientation_(0)\n> {\n> \tcamera_->requestCompleted.connect(this, &CameraDevice::requestComplete);\n> \n> @@ -2326,10 +2326,6 @@ void CameraDevice::requestComplete(Request *request)\n> \n> \tresultMetadata = getResultMetadata(descriptor);\n> \n> -\tconst ControlList &metadata = descriptor->request_->metadata();\n> -\tif (metadata.contains(controls::SensorTimestamp))\n> -\t\tlastTimestamp_ = metadata.get(controls::SensorTimestamp);\n> -\n> \t/* Handle any JPEG compression. */\n> \tfor (camera3_stream_buffer_t &buffer : descriptor.buffers_) {\n> \t\tCameraStream *cameraStream =\n> @@ -2450,7 +2446,6 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \tconst ControlList &metadata = descriptor.request_->metadata();\n> \tconst CameraMetadata &settings = descriptor.settings_;\n> \tcamera_metadata_ro_entry_t entry;\n> -\tbool found;\n> \n> \t/*\n> \t * \\todo Keep this in sync with the actual number of entries.\n> @@ -2482,9 +2477,10 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \t */\n> \n> \t/* FULL */\n> -\tfound = settings.getEntry(ANDROID_BLACK_LEVEL_LOCK, &entry);\n> -\tbool valueBool = found ? *entry.data.u8 : false;\n> -\tresultMetadata->addEntry(ANDROID_BLACK_LEVEL_LOCK, &valueBool, 1);\n> +\tif (metadata.contains(controls::draft::BlackLevelLocked)) {\n> +\t\tbool valueBool = metadata.get(controls::draft::BlackLevelLocked);\n> +\t\tresultMetadata->addEntry(ANDROID_BLACK_LEVEL_LOCK, &valueBool, 1);\n> +\t}\n> \n> \tuint8_t value = ANDROID_COLOR_CORRECTION_ABERRATION_MODE_OFF;\n> \tresultMetadata->addEntry(ANDROID_COLOR_CORRECTION_ABERRATION_MODE,\n> @@ -2497,11 +2493,10 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \tresultMetadata->addEntry(ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION,\n> \t\t\t\t &value32, 1);\n> \n> -\t/* \\todo apply this */\n> -\tvalue = ANDROID_CONTROL_AE_LOCK_OFF;\n> -\tfound = settings.getEntry(ANDROID_CONTROL_AE_LOCK, &entry);\n> -\tresultMetadata->addEntry(ANDROID_CONTROL_AE_LOCK,\n> -\t\t\t\t found ? entry.data.u8 : &value, 1);\n> +\tif (metadata.contains(controls::AeLocked)) {\n> +\t\tvalue = metadata.get(controls::AeLocked);\n> +\t\tresultMetadata->addEntry(ANDROID_CONTROL_AE_LOCK, &value, 1);\n> +\t}\n> \n> \tvalue = ANDROID_CONTROL_AE_MODE_ON;\n> \tresultMetadata->addEntry(ANDROID_CONTROL_AE_MODE, &value, 1);\n> @@ -2515,10 +2510,11 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \t\tresultMetadata->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE,\n> \t\t\t\t\t entry.data.i32, 2);\n> \n> -\tvalue = ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER_IDLE;\n> -\tfound = settings.getEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, &entry);\n> -\tresultMetadata->addEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,\n> -\t\t\t\t found ? entry.data.u8 : &value, 1);\n> +\tif (metadata.contains(controls::draft::AePrecaptureTrigger)) {\n> +\t\tvalue = metadata.get(controls::draft::AePrecaptureTrigger);\n> +\t\tresultMetadata->addEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,\n> +\t\t\t\t\t &value, 1);\n> +\t}\n> \n> \tvalue = ANDROID_CONTROL_AE_STATE_CONVERGED;\n> \tresultMetadata->addEntry(ANDROID_CONTROL_AE_STATE, &value, 1);\n> @@ -2532,14 +2528,15 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \tvalue = ANDROID_CONTROL_AF_TRIGGER_IDLE;\n> \tresultMetadata->addEntry(ANDROID_CONTROL_AF_TRIGGER, &value, 1);\n> \n> -\tvalue = ANDROID_CONTROL_AWB_MODE_AUTO;\n> -\tfound = settings.getEntry(ANDROID_CONTROL_AWB_MODE, &entry);\n> -\tresultMetadata->addEntry(ANDROID_CONTROL_AWB_MODE,\n> -\t\t\t\t found ? entry.data.u8 : &value, 1);\n> +\tif (metadata.contains(controls::AwbMode)) {\n> +\t\tvalue = metadata.get(controls::AwbMode);\n> +\t\tresultMetadata->addEntry(ANDROID_CONTROL_AWB_MODE, &value, 1);\n> +\t}\n> \n> -\tfound = settings.getEntry(ANDROID_CONTROL_AWB_LOCK, &entry);\n> -\tvalue = found ? *entry.data.u8 : ANDROID_CONTROL_AWB_LOCK_OFF;\n> -\tresultMetadata->addEntry(ANDROID_CONTROL_AWB_LOCK, &value, 1);\n> +\tif (metadata.contains(controls::AwbLocked)) {\n> +\t\tvalue = metadata.get(controls::AwbLocked);\n> +\t\tresultMetadata->addEntry(ANDROID_CONTROL_AWB_LOCK, &value, 1);\n> +\t}\n> \n> \tvalue = value ? ANDROID_CONTROL_AWB_STATE_LOCKED :\n> \t\t\tANDROID_CONTROL_AWB_STATE_CONVERGED;\n> @@ -2560,9 +2557,10 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \tvalue = ANDROID_CONTROL_VIDEO_STABILIZATION_MODE_OFF;\n> \tresultMetadata->addEntry(ANDROID_CONTROL_VIDEO_STABILIZATION_MODE, &value, 1);\n> \n> -\tfound = settings.getEntry(ANDROID_EDGE_MODE, &entry);\n> -\tvalue = found ? *entry.data.u8 : ANDROID_EDGE_MODE_OFF;\n> -\tresultMetadata->addEntry(ANDROID_EDGE_MODE, &value, 1);\n> +\tif (metadata.contains(controls::draft::EdgeMode)) {\n> +\t\tvalue = metadata.get(controls::draft::EdgeMode);\n> +\t\tresultMetadata->addEntry(ANDROID_EDGE_MODE, &value, 1);\n> +\t}\n> \n> \tvalue = ANDROID_FLASH_MODE_OFF;\n> \tresultMetadata->addEntry(ANDROID_FLASH_MODE, &value, 1);\n> @@ -2623,15 +2621,16 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \tresultMetadata->addEntry(ANDROID_STATISTICS_SCENE_FLICKER,\n> \t\t\t\t &value, 1);\n> \n> -\t/* \\todo handle this */\n> -\tfound = settings.getEntry(ANDROID_TONEMAP_MODE, &entry);\n> -\tvalue = found ? *entry.data.u8 : ANDROID_TONEMAP_MODE_FAST;\n> -\tresultMetadata->addEntry(ANDROID_TONEMAP_MODE, &value, 1);\n> +\tif (metadata.contains(controls::draft::TonemapMode)) {\n> +\t\tvalue = metadata.get(controls::draft::TonemapMode);\n> +\t\tresultMetadata->addEntry(ANDROID_TONEMAP_MODE, &value, 1);\n> +\t}\n> \n> -\tvalue = ANDROID_NOISE_REDUCTION_MODE_OFF;\n> -\tfound = settings.getEntry(ANDROID_NOISE_REDUCTION_MODE, &entry);\n> -\tresultMetadata->addEntry(ANDROID_NOISE_REDUCTION_MODE,\n> -\t\t\t\t found ? entry.data.u8 : &value, 1);\n> +\tif (metadata.contains(controls::draft::NoiseReductionMode)) {\n> +\t\tvalue = metadata.get(controls::draft::NoiseReductionMode);\n> +\t\tresultMetadata->addEntry(ANDROID_NOISE_REDUCTION_MODE,\n> +\t\t\t\t\t &value, 1);\n> +\t}\n> \n> \t/* 33.3 msec */\n> \tconst int64_t rolling_shutter_skew = 33300000;\n> @@ -2659,11 +2658,11 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \t * and copy to result\n> \t */\n> \n> -\t/* \\todo get this from camera */\n> -\tvalue32 = 32;\n> -\tfound = settings.getEntry(ANDROID_SENSOR_SENSITIVITY, &entry);\n> -\tresultMetadata->addEntry(ANDROID_SENSOR_SENSITIVITY,\n> -\t\t\t\t found ? entry.data.i32 : &value32, 1);\n> +\tif (metadata.contains(controls::draft::SensorSensitivity)) {\n> +\t\tvalue32 = metadata.get(controls::draft::SensorSensitivity);\n> +\t\tresultMetadata->addEntry(ANDROID_SENSOR_SENSITIVITY,\n> +\t\t\t\t\t &value32, 1);\n> +\t}\n> \n> \t/* Add metadata tags reported by libcamera. */\n> \tif (metadata.contains(controls::draft::PipelineDepth)) {\n> @@ -2673,26 +2672,21 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \t\t\t\t\t &pipeline_depth, 1);\n> \t}\n> \n> -\tfound = settings.getEntry(ANDROID_SENSOR_EXPOSURE_TIME, &entry);\n> -\tif (found || metadata.contains(controls::ExposureTime)) {\n> +\tif (metadata.contains(controls::ExposureTime)) {\n> \t\tint64_t exposure = metadata.get(controls::ExposureTime) * 1000ULL;\n> \t\tresultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME,\n> -\t\t\t\t\t found ? entry.data.i64 : &exposure, 1);\n> +\t\t\t\t\t &exposure, 1);\n> \t}\n> \n> \tif (metadata.contains(controls::SensorTimestamp)) {\n> \t\tint64_t timestamp = metadata.get(controls::SensorTimestamp);\n> \t\tresultMetadata->addEntry(ANDROID_SENSOR_TIMESTAMP, ×tamp, 1);\n> +\t}\n> \n> -\t\tint64_t frameDuration = timestamp - lastTimestamp_;\n> -\t\t/*\n> -\t\t * frame duration should be at last as long as the requested\n> -\t\t * exposure time, hardcode it for now\n> -\t\t */\n> -\t\tif (found && frameDuration < *entry.data.i64)\n> -\t\t\tframeDuration = *entry.data.i64;\n> +\tif (metadata.contains(controls::draft::FrameDuration)) {\n> +\t\tint64_t duration = metadata.get(controls::draft::FrameDuration);\n> \t\tresultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,\n> -\t\t\t\t\t &frameDuration, 1);\n> +\t\t\t\t\t &duration, 1);\n> \t}\n> \n> \tif (metadata.contains(controls::ScalerCrop)) {\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index fcd57fcd..8edbcdfd 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -139,8 +139,6 @@ private:\n> \n> \tunsigned int maxJpegBufferSize_;\n> \n> -\tint64_t lastTimestamp_;\n> -\n> \tCameraMetadata lastSettings_;\n> };\n>","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 94FC7BDCC3\n\tfor ;\n\tTue, 27 Apr 2021 05:21:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EBB54688AE;\n\tTue, 27 Apr 2021 07:21:51 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8BD1660512\n\tfor ;\n\tTue, 27 Apr 2021 07:21:50 +0200 (CEST)","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 1729EE9;\n\tTue, 27 Apr 2021 07:21:50 +0200 (CEST)"],"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=\"HY9u06T/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619500910;\n\tbh=8BiUAZeIFaivgdi5XKgTyDDDT61/97N+f0TAkEnFdkU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HY9u06T/cBTE6YAKm7lgcHzXkNnYIkQd8bs6+65Q3VexBEx7fl5nfXicNSKl51OJ1\n\tgWBEnL/LDq0M1+We2xkH1nb2rLri9ZDHbBCAM9EP7SkTaMetjtIU0qj/6fyGnPru9l\n\t0o66gthtGGL9MxSOk6zeyaBRogG7YXmyhVuv+Uus=","Date":"Tue, 27 Apr 2021 08:21:44 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"","References":"<20210422094102.371772-1-paul.elder@ideasonboard.com>\n\t<20210422094102.371772-13-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210422094102.371772-13-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH v2 12/12] android camera_device:\n\tSet result metadata from libcamera 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\" "}}]