From patchwork Thu Jul 14 09:44:57 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 16625 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 1AD28BD1F1 for ; Thu, 14 Jul 2022 09:45:34 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id CC8AB6330E; Thu, 14 Jul 2022 11:45:33 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1657791933; bh=XgGbiTYW+9fw0c27glMEDMpa/zWn1AQo7K84I4fQ1pk=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=B9J+JS2He5UMGvfOf1ec430yteissMsMec2Bee9fP6r3frHg9zer+uSJ7xLuLAKIL SX9J1drPEa9AOg9ATG9G3Ds2GLa0RzkD9ECyF4RAuRAF3KjwXyq5mpW77G4nT8mVWW UhRMAiRssF8CZc09/sia3UMa81vmqKauR/pnCcCGg2E6S7ypA0YbRwnkGPZfe+sMNy xlseNUqJyYuBaYNVe4cylVVJ0Rkukw+71jfy5pW9m1M8LZdA0/JTMKiSrSYgfTklzh vE59vIL8p0kEpen3+eaZsayQ3grdPZ6r29t6GEhaeiio2Rxnigd4ADTjRaT2sR3D38 cvCuDqqpLBvsw== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7BA8263309 for ; Thu, 14 Jul 2022 11:45:32 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="F+c1Pam8"; dkim-atps=neutral Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BCE8C383; Thu, 14 Jul 2022 11:45:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1657791932; bh=XgGbiTYW+9fw0c27glMEDMpa/zWn1AQo7K84I4fQ1pk=; h=From:To:Cc:Subject:Date:From; b=F+c1Pam8fhT+WM0WOEXRkEZrP3va55o6drxf3q1B+Yz00yh+8UeyIl3Irz7gV3JFK +H/02gtt+/YKmFjxBmXRE6MUlA5+W//q1kf40QldIm1H89mf7c6t/k9+04FKJ2Q+Nk 3UEh/J9RfVoKq4BwIoGOjQs+Vjfss6+fdHxFO0Os= To: libcamera-devel@lists.libcamera.org Date: Thu, 14 Jul 2022 12:44:57 +0300 Message-Id: <20220714094457.6806-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.35.1 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2] libcamera: controls: Avoid double lookups X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Now that the ControlList::get() function returns an instance of std::optional<>, we can replace the ControlList::contains() calls with a nullopt check on the return value of get(). This avoids double lookups of controls through the code base. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Umang Jain --- Changes since v1: - Remove one more contains() call in src/cam/main.cpp --- This patch applies on top of "[PATCH v10 1/2] libcamera: controls: Use std::optional to handle invalid control values". --- src/android/camera_capabilities.cpp | 11 ++--- src/android/camera_device.cpp | 46 +++++++++---------- src/android/camera_hal_manager.cpp | 6 +-- src/cam/main.cpp | 11 +++-- src/libcamera/pipeline/ipu3/ipu3.cpp | 25 +++++----- .../pipeline/raspberrypi/raspberrypi.cpp | 14 +++--- src/qcam/dng_writer.cpp | 35 +++++++------- 7 files changed, 72 insertions(+), 76 deletions(-) diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp index 5304b2da1f93..94ebfc860040 100644 --- a/src/android/camera_capabilities.cpp +++ b/src/android/camera_capabilities.cpp @@ -1049,8 +1049,8 @@ int CameraCapabilities::initializeStaticMetadata() pixelArraySize); } - if (properties.contains(properties::UnitCellSize)) { - const auto &cellSize = properties.get(properties::UnitCellSize); + const auto &cellSize = properties.get(properties::UnitCellSize); + if (cellSize) { std::array physicalSize{ cellSize->width * pixelArraySize[0] / 1e6f, cellSize->height * pixelArraySize[1] / 1e6f @@ -1079,11 +1079,10 @@ int CameraCapabilities::initializeStaticMetadata() sensitivityRange); /* Report the color filter arrangement if the camera reports it. */ - if (properties.contains(properties::draft::ColorFilterArrangement)) { - uint8_t filterArr = *properties.get(properties::draft::ColorFilterArrangement); + const auto &filterArr = properties.get(properties::draft::ColorFilterArrangement); + if (filterArr) staticMetadata_->addEntry(ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT, - filterArr); - } + *filterArr); const auto &exposureInfo = controlsInfo.find(&controls::ExposureTime); if (exposureInfo != controlsInfo.end()) { diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 4662123210c2..b20e389b9d30 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -305,9 +305,9 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData) */ const ControlList &properties = camera_->properties(); - if (properties.contains(properties::Location)) { - int32_t location = *properties.get(properties::Location); - switch (location) { + const auto &location = properties.get(properties::Location); + if (location) { + switch (*location) { case properties::CameraLocationFront: facing_ = CAMERA_FACING_FRONT; break; @@ -355,9 +355,9 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData) * value for clockwise direction as required by the Android orientation * metadata. */ - if (properties.contains(properties::Rotation)) { - int rotation = *properties.get(properties::Rotation); - orientation_ = (360 - rotation) % 360; + const auto &rotation = properties.get(properties::Rotation); + if (rotation) { + orientation_ = (360 - *rotation) % 360; if (cameraConfigData && cameraConfigData->rotation != -1 && orientation_ != cameraConfigData->rotation) { LOG(HAL, Warning) @@ -1564,25 +1564,24 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons const int64_t timestamp = metadata.get(controls::SensorTimestamp).value_or(0); resultMetadata->addEntry(ANDROID_SENSOR_TIMESTAMP, timestamp); - if (metadata.contains(controls::draft::PipelineDepth)) { - uint8_t pipeline_depth = *metadata.get(controls::draft::PipelineDepth); + const auto &pipelineDepth = metadata.get(controls::draft::PipelineDepth); + if (pipelineDepth) resultMetadata->addEntry(ANDROID_REQUEST_PIPELINE_DEPTH, - pipeline_depth); - } + *pipelineDepth); - if (metadata.contains(controls::ExposureTime)) { - int64_t exposure = *metadata.get(controls::ExposureTime) * 1000ULL; - resultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure); - } + const auto &exposureTime = metadata.get(controls::ExposureTime); + if (exposureTime) + resultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, + *exposureTime * 1000ULL); - if (metadata.contains(controls::FrameDuration)) { - int64_t duration = *metadata.get(controls::FrameDuration) * 1000; + const auto &frameDuration = metadata.get(controls::FrameDuration); + if (frameDuration) resultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION, - duration); - } + *frameDuration * 1000); - if (metadata.contains(controls::ScalerCrop)) { - Rectangle crop = *metadata.get(controls::ScalerCrop); + const auto &scalerCrop = metadata.get(controls::ScalerCrop); + if (scalerCrop) { + const Rectangle &crop = *scalerCrop; int32_t cropRect[] = { crop.x, crop.y, static_cast(crop.width), static_cast(crop.height), @@ -1590,11 +1589,10 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons resultMetadata->addEntry(ANDROID_SCALER_CROP_REGION, cropRect); } - if (metadata.contains(controls::draft::TestPatternMode)) { - const int32_t testPatternMode = *metadata.get(controls::draft::TestPatternMode); + const auto &testPatternMode = metadata.get(controls::draft::TestPatternMode); + if (testPatternMode) resultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE, - testPatternMode); - } + *testPatternMode); /* * Return the result metadata pack even is not valid: get() will return diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp index 0bffe96f6dbc..7512cc4e11a4 100644 --- a/src/android/camera_hal_manager.cpp +++ b/src/android/camera_hal_manager.cpp @@ -228,11 +228,7 @@ void CameraHalManager::cameraRemoved(std::shared_ptr cam) int32_t CameraHalManager::cameraLocation(const Camera *cam) { - const ControlList &properties = cam->properties(); - if (!properties.contains(properties::Location)) - return -1; - - return *properties.get(properties::Location); + return cam->properties().get(properties::Location).value_or(-1); } CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id) diff --git a/src/cam/main.cpp b/src/cam/main.cpp index d8115cd86825..9533442f3bcb 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -300,8 +300,9 @@ std::string CamApp::cameraName(const Camera *camera) * Construct the name from the camera location, model and ID. The model * is only used if the location isn't present or is set to External. */ - if (props.contains(properties::Location)) { - switch (*props.get(properties::Location)) { + const auto &location = props.get(properties::Location); + if (location) { + switch (*location) { case properties::CameraLocationFront: addModel = false; name = "Internal front camera "; @@ -316,12 +317,14 @@ std::string CamApp::cameraName(const Camera *camera) } } - if (addModel && props.contains(properties::Model)) { + if (addModel) { /* * If the camera location is not availble use the camera model * to build the camera name. */ - name = "'" + *props.get(properties::Model) + "' "; + const auto model = props.get(properties::Model); + if (model) + name = "'" + *model + "' "; } name += "(" + camera->id() + ")"; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 43db7b68dac8..d60f20b08e27 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -1143,18 +1143,17 @@ int PipelineHandlerIPU3::registerCameras() &IPU3CameraData::frameStart); /* Convert the sensor rotation to a transformation */ - int32_t rotation = 0; - if (data->properties_.contains(properties::Rotation)) - rotation = *(data->properties_.get(properties::Rotation)); - else + const auto &rotation = data->properties_.get(properties::Rotation); + if (!rotation) LOG(IPU3, Warning) << "Rotation control not exposed by " << cio2->sensor()->id() << ". Assume rotation 0"; + int32_t rotationValue = rotation.value_or(0); bool success; - data->rotationTransform_ = transformFromRotation(rotation, &success); + data->rotationTransform_ = transformFromRotation(rotationValue, &success); if (!success) - LOG(IPU3, Warning) << "Invalid rotation of " << rotation + LOG(IPU3, Warning) << "Invalid rotation of " << rotationValue << " degrees: ignoring"; ControlList ctrls = cio2->sensor()->getControls({ V4L2_CID_HFLIP }); @@ -1330,8 +1329,9 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) request->metadata().set(controls::draft::PipelineDepth, 3); /* \todo Actually apply the scaler crop region to the ImgU. */ - if (request->controls().contains(controls::ScalerCrop)) - cropRegion_ = *(request->controls().get(controls::ScalerCrop)); + const auto &scalerCrop = request->controls().get(controls::ScalerCrop); + if (scalerCrop) + cropRegion_ = *scalerCrop; request->metadata().set(controls::ScalerCrop, cropRegion_); if (frameInfos_.tryComplete(info)) @@ -1455,13 +1455,12 @@ void IPU3CameraData::frameStart(uint32_t sequence) Request *request = processingRequests_.front(); processingRequests_.pop(); - if (!request->controls().contains(controls::draft::TestPatternMode)) + const auto &testPatternMode = request->controls().get(controls::draft::TestPatternMode); + if (!testPatternMode) return; - const int32_t testPatternMode = *(request->controls().get(controls::draft::TestPatternMode)); - int ret = cio2_.sensor()->setTestPatternMode( - static_cast(testPatternMode)); + static_cast(*testPatternMode)); if (ret) { LOG(IPU3, Error) << "Failed to set test pattern mode: " << ret; @@ -1469,7 +1468,7 @@ void IPU3CameraData::frameStart(uint32_t sequence) } request->metadata().set(controls::draft::TestPatternMode, - testPatternMode); + *testPatternMode); } REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3) diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 28f054cb84c8..2e19531eecc4 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1716,16 +1716,15 @@ void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList & * Inform the sensor of the latest colour gains if it has the * V4L2_CID_NOTIFY_GAINS control (which means notifyGainsUnity_ is set). */ - if (notifyGainsUnity_ && controls.contains(libcamera::controls::ColourGains)) { - libcamera::Span colourGains = - *controls.get(libcamera::controls::ColourGains); + const auto &colourGains = controls.get(libcamera::controls::ColourGains); + if (notifyGainsUnity_ && colourGains) { /* The control wants linear gains in the order B, Gb, Gr, R. */ ControlList ctrls(sensor_->controls()); std::array gains{ - static_cast(colourGains[1] * *notifyGainsUnity_), + static_cast((*colourGains)[1] * *notifyGainsUnity_), *notifyGainsUnity_, *notifyGainsUnity_, - static_cast(colourGains[0] * *notifyGainsUnity_) + static_cast((*colourGains)[0] * *notifyGainsUnity_) }; ctrls.set(V4L2_CID_NOTIFY_GAINS, Span{ gains }); @@ -2052,8 +2051,9 @@ Rectangle RPiCameraData::scaleIspCrop(const Rectangle &ispCrop) const void RPiCameraData::applyScalerCrop(const ControlList &controls) { - if (controls.contains(controls::ScalerCrop)) { - Rectangle nativeCrop = *controls.get(controls::ScalerCrop); + const auto &scalerCrop = controls.get(controls::ScalerCrop); + if (scalerCrop) { + Rectangle nativeCrop = *scalerCrop; if (!nativeCrop.width || !nativeCrop.height) nativeCrop = { 0, 0, 1, 1 }; diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp index 4b5d8276564f..5896485cda5b 100644 --- a/src/qcam/dng_writer.cpp +++ b/src/qcam/dng_writer.cpp @@ -391,9 +391,9 @@ int DNGWriter::write(const char *filename, const Camera *camera, TIFFSetField(tif, TIFFTAG_FILLORDER, FILLORDER_MSB2LSB); TIFFSetField(tif, TIFFTAG_MAKE, "libcamera"); - if (cameraProperties.contains(properties::Model)) { - std::string model = *cameraProperties.get(properties::Model); - TIFFSetField(tif, TIFFTAG_MODEL, model.c_str()); + const auto &model = cameraProperties.get(properties::Model); + if (model) { + TIFFSetField(tif, TIFFTAG_MODEL, model->c_str()); /* \todo set TIFFTAG_UNIQUECAMERAMODEL. */ } @@ -437,16 +437,18 @@ int DNGWriter::write(const char *filename, const Camera *camera, */ const double eps = 1e-2; - if (metadata.contains(controls::ColourGains)) { - const auto &colourGains = metadata.get(controls::ColourGains); + const auto &colourGains = metadata.get(controls::ColourGains); + if (colourGains) { if ((*colourGains)[0] > eps && (*colourGains)[1] > eps) { wbGain = Matrix3d::diag((*colourGains)[0], 1, (*colourGains)[1]); neutral[0] = 1.0 / (*colourGains)[0]; /* red */ neutral[2] = 1.0 / (*colourGains)[1]; /* blue */ } } - if (metadata.contains(controls::ColourCorrectionMatrix)) { - Matrix3d ccmSupplied(*metadata.get(controls::ColourCorrectionMatrix)); + + const auto &ccmControl = metadata.get(controls::ColourCorrectionMatrix); + if (ccmControl) { + Matrix3d ccmSupplied(*ccmControl); if (ccmSupplied.determinant() > eps) ccm = ccmSupplied; } @@ -513,9 +515,9 @@ int DNGWriter::write(const char *filename, const Camera *camera, float blackLevel[] = { 0.0f, 0.0f, 0.0f, 0.0f }; uint32_t whiteLevel = (1 << info->bitsPerSample) - 1; - if (metadata.contains(controls::SensorBlackLevels)) { - Span levels = - *metadata.get(controls::SensorBlackLevels); + const auto &blackLevels = metadata.get(controls::SensorBlackLevels); + if (blackLevels) { + Span levels = *blackLevels; /* * The black levels control is specified in R, Gr, Gb, B order. @@ -592,16 +594,15 @@ int DNGWriter::write(const char *filename, const Camera *camera, TIFFSetField(tif, EXIFTAG_DATETIMEORIGINAL, strTime); TIFFSetField(tif, EXIFTAG_DATETIMEDIGITIZED, strTime); - if (metadata.contains(controls::AnalogueGain)) { - float gain = *metadata.get(controls::AnalogueGain); - uint16_t iso = std::min(std::max(gain * 100, 0.0f), 65535.0f); + const auto &analogGain = metadata.get(controls::AnalogueGain); + if (analogGain) { + uint16_t iso = std::min(std::max(*analogGain * 100, 0.0f), 65535.0f); TIFFSetField(tif, EXIFTAG_ISOSPEEDRATINGS, 1, &iso); } - if (metadata.contains(controls::ExposureTime)) { - float exposureTime = *metadata.get(controls::ExposureTime) / 1e6; - TIFFSetField(tif, EXIFTAG_EXPOSURETIME, exposureTime); - } + const auto &exposureTime = metadata.get(controls::ExposureTime); + if (exposureTime) + TIFFSetField(tif, EXIFTAG_EXPOSURETIME, *exposureTime / 1e6); TIFFWriteCustomDirectory(tif, &exifIFDOffset);