[{"id":2585,"web_url":"https://patchwork.libcamera.org/comment/2585/","msgid":"<20190904124428.GB9548@pendragon.ideasonboard.com>","date":"2019-09-04T12:44:28","subject":"Re: [libcamera-devel] [PATCH v3 3/4] android: camera_device: Rework\n\ttemplate generation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Wed, Sep 04, 2019 at 02:08:46PM +0200, Jacopo Mondi wrote:\n> Rework the template generation procedure by adding two missing requested\n> keys, fixing the capture intent to update it based on the requested\n> template type, and removing static metadata keys which didn't belong\n> there.\n\nAs this is temporary code there's no need to spend too much time on\nthis, but having split this in one change per patch would have made it\neasier to review.\n\n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_device.cpp | 123 +++++++---------------------------\n> 1 file changed, 24 insertions(+), 99 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index dee60e3d2931..eaa0e1de12c2 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -544,52 +544,29 @@ const camera_metadata_t *CameraDevice::constructDefaultRequestSettings(int type)\n> \t\treturn nullptr;\n> \t}\n> \n> -\tif (requestTemplate_)\n> +\tif (requestTemplate_) {\n> +\t\tcamera_metadata_entry_t captureIntentEntry;\n> +\n> +\t\tfind_camera_metadata_entry(requestTemplate_,\n> +\t\t\tANDROID_CONTROL_CAPTURE_INTENT, &captureIntentEntry);\n> +\t\tret = update_camera_metadata_entry(requestTemplate_,\n> +\t\t\t\tcaptureIntentEntry.index,\n> +\t\t\t\t&captureIntent, 1, &captureIntentEntry);\n> +\t\tMETADATA_ASSERT(ret);\n> +\n> \t\treturn requestTemplate_;\n> +\t}\n\nAccording to the HAL documentation,\n\n * The HAL retains ownership of this structure, but the pointer to the\n * structure must be valid until the device is closed. The framework and the\n * HAL may not modify the buffer once it is returned by this call. The same\n * buffer may be returned for subsequent calls for the same template, or for\n * other templates.\n\nSo it looks like you will need to make turn the requestTemplate_ into a\nmap indexed by type :-S\n\n> \n> -\t/* \\todo Use correct sizes */\n> -\t#define REQUEST_TEMPLATE_ENTRIES\t 30\n> -\t#define REQUEST_TEMPLATE_DATA\t\t2048\n> -\trequestTemplate_ = allocate_camera_metadata(REQUEST_TEMPLATE_ENTRIES,\n> -\t\t\t\t\t\t REQUEST_TEMPLATE_DATA);\n> +\t/*\n> +\t * \\todo Keep this in sync with the actual number of entries.\n> +\t * Currently: 12 entries, 15 bytes\n> +\t */\n> +\trequestTemplate_ = allocate_camera_metadata(15, 20);\n> \tif (!requestTemplate_) {\n> \t\tLOG(HAL, Error) << \"Failed to allocate template metadata\";\n> \t\treturn nullptr;\n> \t}\n> \n> -\t/* Set to 0 the number of 'processed and stalling' streams (ie JPEG). */\n> -\tint32_t maxOutStream[] = { 0, 2, 0 };\n> -\tret = add_camera_metadata_entry(requestTemplate_,\n> -\t\t\tANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,\n> -\t\t\tmaxOutStream, 3);\n> -\tMETADATA_ASSERT(ret);\n> -\n> -\tuint8_t maxPipelineDepth = 5;\n> -\tret = add_camera_metadata_entry(requestTemplate_,\n> -\t\t\tANDROID_REQUEST_PIPELINE_MAX_DEPTH,\n> -\t\t\t&maxPipelineDepth, 1);\n> -\tMETADATA_ASSERT(ret);\n> -\n> -\tint32_t inputStreams = 0;\n> -\tret = add_camera_metadata_entry(requestTemplate_,\n> -\t\t\tANDROID_REQUEST_MAX_NUM_INPUT_STREAMS,\n> -\t\t\t&inputStreams, 1);\n> -\tMETADATA_ASSERT(ret);\n> -\n> -\tint32_t partialResultCount = 1;\n> -\tret = add_camera_metadata_entry(requestTemplate_,\n> -\t\t\tANDROID_REQUEST_PARTIAL_RESULT_COUNT,\n> -\t\t\t&partialResultCount, 1);\n> -\tMETADATA_ASSERT(ret);\n> -\n> -\tuint8_t availableCapabilities[] = {\n> -\t\tANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE,\n> -\t};\n> -\tret = add_camera_metadata_entry(requestTemplate_,\n> -\t\t\tANDROID_REQUEST_AVAILABLE_CAPABILITIES,\n> -\t\t\tavailableCapabilities, 1);\n> -\tMETADATA_ASSERT(ret);\n> -\n> \tuint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON;\n> \tret = add_camera_metadata_entry(requestTemplate_,\n> \t\t\tANDROID_CONTROL_AE_MODE,\n> @@ -632,12 +609,6 @@ const camera_metadata_t *CameraDevice::constructDefaultRequestSettings(int type)\n> \t\t\t&awbLock, 1);\n> \tMETADATA_ASSERT(ret);\n> \n> -\tuint8_t awbLockAvailable = ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE;\n> -\tret = add_camera_metadata_entry(requestTemplate_,\n> -\t\t\tANDROID_CONTROL_AWB_LOCK_AVAILABLE,\n> -\t\t\t&awbLockAvailable, 1);\n> -\tMETADATA_ASSERT(ret);\n> -\n> \tuint8_t flashMode = ANDROID_FLASH_MODE_OFF;\n> \tret = add_camera_metadata_entry(requestTemplate_,\n> \t\t\tANDROID_FLASH_MODE,\n> @@ -650,67 +621,21 @@ const camera_metadata_t *CameraDevice::constructDefaultRequestSettings(int type)\n> \t\t\t&faceDetectMode, 1);\n> \tMETADATA_ASSERT(ret);\n> \n> +\tuint8_t noiseReduction = ANDROID_NOISE_REDUCTION_MODE_OFF;\n> \tret = add_camera_metadata_entry(requestTemplate_,\n> -\t\t\tANDROID_CONTROL_CAPTURE_INTENT,\n> -\t\t\t&captureIntent, 1);\n> +\t\t\tANDROID_NOISE_REDUCTION_MODE, &noiseReduction, 1);\n> \tMETADATA_ASSERT(ret);\n> \n> -\t/*\n> -\t * This is quite hard to list at the moment wihtout knowing what\n> -\t * we could control.\n> -\t *\n> -\t * For now, just list in the available Request keys and in the available\n> -\t * result keys the control and reporting of the AE algorithm.\n> -\t */\n> -\tstd::vector availableRequestKeys = {\n> -\t\tANDROID_CONTROL_AE_MODE,\n> -\t\tANDROID_CONTROL_AE_EXPOSURE_COMPENSATION,\n> -\t\tANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,\n> -\t\tANDROID_CONTROL_AE_LOCK,\n> -\t\tANDROID_CONTROL_AF_TRIGGER,\n> -\t\tANDROID_CONTROL_AWB_MODE,\n> -\t\tANDROID_CONTROL_AWB_LOCK,\n> -\t\tANDROID_CONTROL_AWB_LOCK_AVAILABLE,\n> -\t\tANDROID_CONTROL_CAPTURE_INTENT,\n> -\t\tANDROID_FLASH_MODE,\n> -\t\tANDROID_STATISTICS_FACE_DETECT_MODE,\n> -\t};\n> -\n> +\tuint8_t aberrationMode = ANDROID_COLOR_CORRECTION_ABERRATION_MODE_OFF;\n> \tret = add_camera_metadata_entry(requestTemplate_,\n> -\t\t\tANDROID_REQUEST_AVAILABLE_REQUEST_KEYS,\n> -\t\t\tavailableRequestKeys.data(),\n> -\t\t\tavailableRequestKeys.size());\n> -\tMETADATA_ASSERT(ret);\n> -\n> -\tstd::vector availableResultKeys = {\n> -\t\tANDROID_CONTROL_AE_MODE,\n> -\t\tANDROID_CONTROL_AE_EXPOSURE_COMPENSATION,\n> -\t\tANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,\n> -\t\tANDROID_CONTROL_AE_LOCK,\n> -\t\tANDROID_CONTROL_AF_TRIGGER,\n> -\t\tANDROID_CONTROL_AWB_MODE,\n> -\t\tANDROID_CONTROL_AWB_LOCK,\n> -\t\tANDROID_CONTROL_AWB_LOCK_AVAILABLE,\n> -\t\tANDROID_CONTROL_CAPTURE_INTENT,\n> -\t\tANDROID_FLASH_MODE,\n> -\t\tANDROID_STATISTICS_FACE_DETECT_MODE,\n> -\t};\n> -\tret = add_camera_metadata_entry(requestTemplate_,\n> -\t\t\tANDROID_REQUEST_AVAILABLE_RESULT_KEYS,\n> -\t\t\tavailableResultKeys.data(),\n> -\t\t\tavailableResultKeys.size());\n> +\t\t\tANDROID_COLOR_CORRECTION_ABERRATION_MODE,\n> +\t\t\t&aberrationMode, 1);\n> \tMETADATA_ASSERT(ret);\n> \n> -\t/*\n> -\t * \\todo The available characteristics are be the tags reported\n> -\t * as part of the static metadata reported at hal_get_camera_info()\n> -\t * time. As of now, report an empty list.\n> -\t */\n> -\tstd::vector availableCharacteristicsKeys = {};\n> +\t/* Use the capture matching the requested template type. */\n> \tret = add_camera_metadata_entry(requestTemplate_,\n> -\t\t\tANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS,\n> -\t\t\tavailableCharacteristicsKeys.data(),\n> -\t\t\tavailableCharacteristicsKeys.size());\n> +\t\t\tANDROID_CONTROL_CAPTURE_INTENT,\n> +\t\t\t&captureIntent, 1);\n> \tMETADATA_ASSERT(ret);\n> \n> \treturn requestTemplate_;","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4CC8460BB2\n\tfor ;\n\tWed, 4 Sep 2019 14:44:38 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(mobile-access-bceeb5-228.dhcp.inet.fi [188.238.181.228])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D0D9B440;\n\tWed, 4 Sep 2019 14:44:36 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1567601077;\n\tbh=B7mjBExuha/r/aaYMt/ylIpHWPiPAg0FNdVD/lfnEe0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OtdqckPU0MA2Z5R3nO6qa/820P3dN4lEkr3o4mOIDAtLD5JZCmT6DXCH7qD5U5gFg\n\tzg4sX6kDEtCZDgkzYIFZpo0fMYexaEo7Cg9UJbgb11Nb/OujbhrflxDZSuuDeqRftB\n\tJAkhH9KRq4eUQweRXczcZDvYia2brBuSYZIXeiU8=","Date":"Wed, 4 Sep 2019 15:44:28 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org, jcliang@google.com, wtlee@google.com","Message-ID":"<20190904124428.GB9548@pendragon.ideasonboard.com>","References":"<20190904120847.11934-1-jacopo@jmondi.org>\n\t<20190904120847.11934-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190904120847.11934-4-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 3/4] android: camera_device: Rework\n\ttemplate generation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 04 Sep 2019 12:44:38 -0000"}}]