[{"id":14452,"web_url":"https://patchwork.libcamera.org/comment/14452/","msgid":"","date":"2021-01-06T10:39:02","subject":"Re: [libcamera-devel] [PATCH v5 03/10] libcamera: camera_sensor:\n\tProvide fall-back for sensor properties","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your work.\n\nOn 2021-01-05 13:31:21 +0100, Jacopo Mondi wrote:\n> Support for the V4L2 selection API is currently optional in the\n> CameraSensor class. Properties registered by using values read through\n> that API are defaulted in several different places (the Android camera\n> HAL or the CameraSensor class).\n> \n> In future support for the selection API will be made mandatory, but to\n> give time to sensor drivers in all test platforms to be updated, use\n> sensor resolution as fallback values for sensor pixel array properties\n> and cache them as class member variables.\n> \n> Signed-off-by: Jacopo Mondi \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> include/libcamera/internal/camera_sensor.h | 3 ++\n> src/libcamera/camera_sensor.cpp | 57 ++++++++--------------\n> 2 files changed, 24 insertions(+), 36 deletions(-)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index aee10aa6e3c7..86902b85ada8 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -84,6 +84,9 @@ private:\n> \tstd::vector mbusCodes_;\n> \tstd::vector sizes_;\n> \n> +\tSize pixelArraySize_;\n> +\tRectangle activeArea_;\n> +\n> \tControlList properties_;\n> };\n> \n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 048fcd6a1fae..92c90862215d 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -269,15 +269,21 @@ int CameraSensor::validateSensorDriver()\n> \tRectangle rect;\n> \tint ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &rect);\n> \tif (ret) {\n> +\t\trect = Rectangle(resolution());\n> \t\tLOG(CameraSensor, Warning)\n> -\t\t\t<< \"Failed to retrieve the readable pixel array size\";\n> +\t\t\t<< \"The PixelArraySize property has been defaulted to \"\n> +\t\t\t<< rect.toString();\n> \t\terr = -EINVAL;\n> \t}\n> +\tpixelArraySize_.width = rect.width;\n> +\tpixelArraySize_.height = rect.height;\n> \n> -\tret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &rect);\n> +\tret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &activeArea_);\n> \tif (ret) {\n> +\t\tactiveArea_ = Rectangle(pixelArraySize_);\n> \t\tLOG(CameraSensor, Warning)\n> -\t\t\t<< \"Failed to retrieve the active pixel array size\";\n> +\t\t\t<< \"The PixelArrayActiveAreas property has been defaulted to: \"\n> +\t\t\t<< activeArea_.toString();\n> \t\terr = -EINVAL;\n> \t}\n> \n> @@ -373,24 +379,8 @@ int CameraSensor::initProperties()\n> \t\tpropertyValue = 0;\n> \tproperties_.set(properties::Rotation, propertyValue);\n> \n> -\tRectangle bounds;\n> -\tret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &bounds);\n> -\tif (!ret)\n> -\t\tproperties_.set(properties::PixelArraySize, bounds.size());\n> -\n> -\tRectangle crop;\n> -\tret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &crop);\n> -\tif (!ret) {\n> -\t\t/*\n> -\t\t * V4L2_SEL_TGT_CROP_DEFAULT and V4L2_SEL_TGT_CROP_BOUNDS are\n> -\t\t * defined relatively to the sensor full pixel array size,\n> -\t\t * while properties::PixelArrayActiveAreas is defined relatively\n> -\t\t * to properties::PixelArraySize. Adjust it.\n> -\t\t */\n> -\t\tcrop.x -= bounds.x;\n> -\t\tcrop.y -= bounds.y;\n> -\t\tproperties_.set(properties::PixelArrayActiveAreas, { crop });\n> -\t}\n> +\tproperties_.set(properties::PixelArraySize, pixelArraySize_);\n> +\tproperties_.set(properties::PixelArrayActiveAreas, { activeArea_ });\n> \n> \t/* Color filter array pattern, register only for RAW sensors. */\n> \tfor (const auto &format : formats_) {\n> @@ -646,20 +636,15 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> {\n> \tinfo->model = model();\n> \n> -\t/* Get the active area size. */\n> -\tRectangle rect;\n> -\tint ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &rect);\n> -\tif (ret) {\n> -\t\tLOG(CameraSensor, Error)\n> -\t\t\t<< \"Failed to construct camera sensor info: \"\n> -\t\t\t<< \"the camera sensor does not report the active area\";\n> -\n> -\t\treturn ret;\n> -\t}\n> -\tinfo->activeAreaSize = { rect.width, rect.height };\n> +\t/*\n> +\t * The active area size is a static property, while the crop\n> +\t * rectangle needs to be re-read as it depends on the sensor\n> +\t * configuration.\n> +\t */\n> +\tinfo->activeAreaSize = { activeArea_.width, activeArea_.height };\n> \n> \t/* It's mandatory for the subdevice to report its crop rectangle. */\n> -\tret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &info->analogCrop);\n> +\tint ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &info->analogCrop);\n> \tif (ret) {\n> \t\tLOG(CameraSensor, Error)\n> \t\t\t<< \"Failed to construct camera sensor info: \"\n> @@ -672,10 +657,10 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> \t * are defined relatively to the active pixel area, while V4L2's\n> \t * TGT_CROP target is defined in respect to the full pixel array.\n> \t *\n> -\t * Compensate it by subtracting the active areas offset.\n> +\t * Compensate it by subtracting the active area offset.\n> \t */\n> -\tinfo->analogCrop.x -= rect.x;\n> -\tinfo->analogCrop.y -= rect.y;\n> +\tinfo->analogCrop.x -= activeArea_.x;\n> +\tinfo->analogCrop.y -= activeArea_.y;\n> \n> \t/* The bit depth and image size depend on the currently applied format. */\n> \tV4L2SubdeviceFormat format{};\n> -- \n> 2.29.2\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 07E85C0F1C\n\tfor ;\n\tWed, 6 Jan 2021 10:39:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7D30B630BB;\n\tWed, 6 Jan 2021 11:39:06 +0100 (CET)","from mail-lf1-x133.google.com (mail-lf1-x133.google.com\n\t[IPv6:2a00:1450:4864:20::133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0F9A76010B\n\tfor ;\n\tWed, 6 Jan 2021 11:39:05 +0100 (CET)","by mail-lf1-x133.google.com with SMTP id h205so5506680lfd.5\n\tfor ;\n\tWed, 06 Jan 2021 02:39:05 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tm24sm292509ljj.62.2021.01.06.02.39.03\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 06 Jan 2021 02:39:03 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"p2lklKoj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=VyWMyVrPw3hgykCt1GSIEkpNkP397SUY1R/hKXn0uZo=;\n\tb=p2lklKoj/2b9ioJgeYT+7Tvet0qT8nXLQxuWWMbKS9z4laTgUOM3Nl7Pz94R1kgbMa\n\tpeZD/fp5EiSRLfWnAUuon7YM7N3UIcdEV5OqEd6juvLmbi/1Nk3CAeukMwnhU2hQAcB/\n\t6qCBYp7/AoQ71q5bxhteL7a2gFxh5jdwLeDAP9vLBRIV+Boc2L0ku6FJ9Mx3d+H3vI8G\n\thgixawIE0DIlV1SRW2mr5uDfk3Yt9MUelEayfnCvYVPPrruLuKWN65dUlEgvgffUpyUw\n\tqVCurTYOY/kJEpwwQRI7la9fM4Grasll0qMuMAiEATj5Fc9/d7oQ49sn/FTk9R07hMEW\n\tIWKg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=VyWMyVrPw3hgykCt1GSIEkpNkP397SUY1R/hKXn0uZo=;\n\tb=BdB0gtajeBym9ufpjeTt/gqn48NJVGQsEhToTZttq3e0rHXTlvsU1ysckZbopm6Ztm\n\tZxjIISUFTZy2gklqna8oEXjS+yHwBxS1iwJZu1ErWJtrRrmGNVaS13yRqR1tiitei1j2\n\tFPJOsaoLFqs7Ou+XCKkMVhPeZoCXA/l6YX7JwWunbybaw9a8QZR4C/RHwgPxIZVvN3y4\n\tutjlrWcVmv9IgLP80aaveghEXdsQ7CHnHHx8j7+UM3MMie2fDtSiyMn1euAn3IMegUEV\n\tLaBeOl2t18rfRxVZK4K8QtfCtsUbQ6Mzlk62ZAwzyWJfClHbSyqgcCoYA+0rlZ1MoHpA\n\txekQ==","X-Gm-Message-State":"AOAM530yCWw0w3rdYET0C7uZ0SgzNv8Zdlro2gv8t3Hw1QX6qhtJlk/r\n\twysQwraSJIIci80X8bvVED+aojDGvhvtcQ==","X-Google-Smtp-Source":"ABdhPJxfqRF98hqrwxL1chEDGi0I86he/MrgBMEtOg3RImyqA4oG4z0ET9CWAUhK8ymYDe361Tmowg==","X-Received":"by 2002:a19:c001:: with SMTP id q1mr1612589lff.55.1609929544249; \n\tWed, 06 Jan 2021 02:39:04 -0800 (PST)","Date":"Wed, 6 Jan 2021 11:39:02 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210105123128.617543-1-jacopo@jmondi.org>\n\t<20210105123128.617543-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210105123128.617543-4-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v5 03/10] libcamera: camera_sensor:\n\tProvide fall-back for sensor properties","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14457,"web_url":"https://patchwork.libcamera.org/comment/14457/","msgid":"","date":"2021-01-07T02:32:50","subject":"Re: [libcamera-devel] [PATCH v5 03/10] libcamera: camera_sensor:\n\tProvide fall-back for sensor properties","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 Tue, Jan 05, 2021 at 01:31:21PM +0100, Jacopo Mondi wrote:\n> Support for the V4L2 selection API is currently optional in the\n> CameraSensor class. Properties registered by using values read through\n> that API are defaulted in several different places (the Android camera\n> HAL or the CameraSensor class).\n> \n> In future support for the selection API will be made mandatory, but to\n\ns/In future/In the future/\n\n> give time to sensor drivers in all test platforms to be updated, use\n> sensor resolution as fallback values for sensor pixel array properties\n> and cache them as class member variables.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> include/libcamera/internal/camera_sensor.h | 3 ++\n> src/libcamera/camera_sensor.cpp | 57 ++++++++--------------\n> 2 files changed, 24 insertions(+), 36 deletions(-)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index aee10aa6e3c7..86902b85ada8 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -84,6 +84,9 @@ private:\n> \tstd::vector mbusCodes_;\n> \tstd::vector sizes_;\n> \n> +\tSize pixelArraySize_;\n> +\tRectangle activeArea_;\n> +\n> \tControlList properties_;\n> };\n> \n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 048fcd6a1fae..92c90862215d 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -269,15 +269,21 @@ int CameraSensor::validateSensorDriver()\n> \tRectangle rect;\n> \tint ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &rect);\n> \tif (ret) {\n> +\t\trect = Rectangle(resolution());\n> \t\tLOG(CameraSensor, Warning)\n> -\t\t\t<< \"Failed to retrieve the readable pixel array size\";\n> +\t\t\t<< \"The PixelArraySize property has been defaulted to \"\n> +\t\t\t<< rect.toString();\n> \t\terr = -EINVAL;\n> \t}\n> +\tpixelArraySize_.width = rect.width;\n> +\tpixelArraySize_.height = rect.height;\n> \n> -\tret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &rect);\n> +\tret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &activeArea_);\n> \tif (ret) {\n> +\t\tactiveArea_ = Rectangle(pixelArraySize_);\n> \t\tLOG(CameraSensor, Warning)\n> -\t\t\t<< \"Failed to retrieve the active pixel array size\";\n> +\t\t\t<< \"The PixelArrayActiveAreas property has been defaulted to: \"\n\ns/to:/to/\n\nReviewed-by: Laurent Pinchart \n\n> +\t\t\t<< activeArea_.toString();\n> \t\terr = -EINVAL;\n> \t}\n> \n> @@ -373,24 +379,8 @@ int CameraSensor::initProperties()\n> \t\tpropertyValue = 0;\n> \tproperties_.set(properties::Rotation, propertyValue);\n> \n> -\tRectangle bounds;\n> -\tret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &bounds);\n> -\tif (!ret)\n> -\t\tproperties_.set(properties::PixelArraySize, bounds.size());\n> -\n> -\tRectangle crop;\n> -\tret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &crop);\n> -\tif (!ret) {\n> -\t\t/*\n> -\t\t * V4L2_SEL_TGT_CROP_DEFAULT and V4L2_SEL_TGT_CROP_BOUNDS are\n> -\t\t * defined relatively to the sensor full pixel array size,\n> -\t\t * while properties::PixelArrayActiveAreas is defined relatively\n> -\t\t * to properties::PixelArraySize. Adjust it.\n> -\t\t */\n> -\t\tcrop.x -= bounds.x;\n> -\t\tcrop.y -= bounds.y;\n> -\t\tproperties_.set(properties::PixelArrayActiveAreas, { crop });\n> -\t}\n> +\tproperties_.set(properties::PixelArraySize, pixelArraySize_);\n> +\tproperties_.set(properties::PixelArrayActiveAreas, { activeArea_ });\n> \n> \t/* Color filter array pattern, register only for RAW sensors. */\n> \tfor (const auto &format : formats_) {\n> @@ -646,20 +636,15 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> {\n> \tinfo->model = model();\n> \n> -\t/* Get the active area size. */\n> -\tRectangle rect;\n> -\tint ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &rect);\n> -\tif (ret) {\n> -\t\tLOG(CameraSensor, Error)\n> -\t\t\t<< \"Failed to construct camera sensor info: \"\n> -\t\t\t<< \"the camera sensor does not report the active area\";\n> -\n> -\t\treturn ret;\n> -\t}\n> -\tinfo->activeAreaSize = { rect.width, rect.height };\n> +\t/*\n> +\t * The active area size is a static property, while the crop\n> +\t * rectangle needs to be re-read as it depends on the sensor\n> +\t * configuration.\n> +\t */\n> +\tinfo->activeAreaSize = { activeArea_.width, activeArea_.height };\n> \n> \t/* It's mandatory for the subdevice to report its crop rectangle. */\n> -\tret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &info->analogCrop);\n> +\tint ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &info->analogCrop);\n> \tif (ret) {\n> \t\tLOG(CameraSensor, Error)\n> \t\t\t<< \"Failed to construct camera sensor info: \"\n> @@ -672,10 +657,10 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> \t * are defined relatively to the active pixel area, while V4L2's\n> \t * TGT_CROP target is defined in respect to the full pixel array.\n> \t *\n> -\t * Compensate it by subtracting the active areas offset.\n> +\t * Compensate it by subtracting the active area offset.\n> \t */\n> -\tinfo->analogCrop.x -= rect.x;\n> -\tinfo->analogCrop.y -= rect.y;\n> +\tinfo->analogCrop.x -= activeArea_.x;\n> +\tinfo->analogCrop.y -= activeArea_.y;\n> \n> \t/* The bit depth and image size depend on the currently applied format. */\n> \tV4L2SubdeviceFormat format{};","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 BAAA9C0F1A\n\tfor ;\n\tThu, 7 Jan 2021 02:33:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 35F80631AB;\n\tThu, 7 Jan 2021 03:33:05 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9EE556010E\n\tfor ;\n\tThu, 7 Jan 2021 03:33:03 +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 15B8C2E0;\n\tThu, 7 Jan 2021 03:33:03 +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=\"C4r33F2x\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1609986783;\n\tbh=kH1dkb8emWTLqVuVD+htvhKjbhyTyDMJ9lGYNa7paeI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=C4r33F2xPrr48nGvDmDiDcrcWtp6RsTWbC4Mpoene5jPfwwgRZWuOaU0vQNFXHfmA\n\tYsT/6QF83LqHZFAqY8aBvUK2krn4DrRFFYvkVHN5d0k2O6y/EE6aecdSclf5pL+ibk\n\txXfqcj/Tdf2OE3a3ENrGxM+UqCB0lX/V+qEnec2g=","Date":"Thu, 7 Jan 2021 04:32:50 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210105123128.617543-1-jacopo@jmondi.org>\n\t<20210105123128.617543-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210105123128.617543-4-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v5 03/10] libcamera: camera_sensor:\n\tProvide fall-back for sensor properties","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\" "}}]