[{"id":14438,"web_url":"https://patchwork.libcamera.org/comment/14438/","msgid":"","date":"2021-01-04T13:15:32","subject":"Re: [libcamera-devel] [PATCH v4 3/6] libcamera: camera_sensor:\n\tCache selection targets","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 2020-12-31 00:06:00 +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, provide\n> a default for the sensor pixel array properties and cache them as\n> class member variables.\n> \n> Reviewed-by: Laurent Pinchart \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 | 61 +++++++++-------------\n> 2 files changed, 28 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..bf0d6b06ae20 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -31,6 +31,9 @@ namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(CameraSensor)\n> \n> +static constexpr Size defaultPixelArraySize = { 2592, 1944 };\n> +static constexpr Rectangle defaultActiveArea = { 16, 12, 2560, 1920 };\n> +\n> /**\n> * \\struct CameraSensorInfo\n> * \\brief Report the image sensor characteristics\n> @@ -269,15 +272,22 @@ int CameraSensor::validateSensorDriver()\n> \tRectangle rect;\n> \tint ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &rect);\n> \tif (ret) {\n> +\t\trect.width = defaultPixelArraySize.width;\n> +\t\trect.height = defaultPixelArraySize.height;\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_ = defaultActiveArea;\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 +383,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 +640,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 +661,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 BDBA5C0F1C\n\tfor ;\n\tMon, 4 Jan 2021 13:15:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5549A6200B;\n\tMon, 4 Jan 2021 14:15:35 +0100 (CET)","from mail-lf1-x135.google.com (mail-lf1-x135.google.com\n\t[IPv6:2a00:1450:4864:20::135])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1531E60110\n\tfor ;\n\tMon, 4 Jan 2021 14:15:34 +0100 (CET)","by mail-lf1-x135.google.com with SMTP id m25so64116574lfc.11\n\tfor ;\n\tMon, 04 Jan 2021 05:15:34 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tj4sm7250901lfg.197.2021.01.04.05.15.32\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 04 Jan 2021 05:15:32 -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=\"HPr9hoth\"; 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=hQbsPgJg+gAI4v8YQvP89Vgq3sDX28corvIP/nO0CAM=;\n\tb=HPr9hothIHFaLMe0Ejx2TE9/bq0vXMvTP3hoatee4H1Y9pA6hU/Sibg+bXhfMxlBuq\n\tVOQWYXaCHJrh+/41Sr3Z2JWNMFuu01WrP7RYEUzoNJcrnvC7fjW2kqYbbRv4UKX5X7xV\n\tIQ4ZIXly/LNsvESzkXtZ2mBckiK5p5gH9wwdNK5p/ktwc6N537IA3N6gXz3DrtPGGLFG\n\t3qM4cV7AQFKybGsXW16qwL9crtlevEq+JYfIpQzfLkcwHev84kV4jtRWcKhLJfPSOZ6w\n\tPgo2x3YpuL6xvwseqIQGHctVL9weEDrd8eHkjoYSKkRk07uO95caFfIArkqH7xgwukK/\n\tTFQg==","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=hQbsPgJg+gAI4v8YQvP89Vgq3sDX28corvIP/nO0CAM=;\n\tb=YJs24K5F5z5lP8Obns4vM+xzVg5O0+aMoVvqPT0tj6IQKnMj3Yw+7iKhBySP06urzU\n\ti0pXVe5N5yvD6UUAnxUNN1XNLBsPbpaT6k794V6ct9gt10GelgklQGC79LiCfKoBuMZ2\n\t0H4vnfoPKJifvzHSogULObIH6RBSCSfkX0kPVGeVlEp6oG5B4txo6dVfxHVrtPOC1C89\n\tmuR/Y7lu6rbtsVXvt/FdW3o3RmKIzdYOhh8rD4MTPEQ4HwAEOvNKDqIfzXPVMSL6IEco\n\tsk4WJhJE//aBrpUxN9/5fgI8pR4E2Pax5iawVfgXE5AFHBbj6U7x3bxMgLwyl9nkrVe4\n\tn61w==","X-Gm-Message-State":"AOAM530vgJjhh2wLgKJnB8qeEBDBVxoYSVQjyT9DHt1g36PWV3HiN7iP\n\tnJeQQWXCliZ4Q67iMmNOK7wg1880QFTOpQ==","X-Google-Smtp-Source":"ABdhPJysM/SzLm24WacZsX3l0ezFHS8IrDdxPz1df+3MrUdoYjdZOomsmsPFxq1tjGNZAOweqC7O5A==","X-Received":"by 2002:a2e:98cc:: with SMTP id\n\ts12mr37570013ljj.325.1609766133453; \n\tMon, 04 Jan 2021 05:15:33 -0800 (PST)","Date":"Mon, 4 Jan 2021 14:15:32 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Message-ID":"","References":"<20201230230603.123486-1-jacopo@jmondi.org>\n\t<20201230230603.123486-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201230230603.123486-4-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v4 3/6] libcamera: camera_sensor:\n\tCache selection targets","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\" "}}]