[{"id":13645,"web_url":"https://patchwork.libcamera.org/comment/13645/","msgid":"<20201109124344.GB88486@oden.dyn.berto.se>","date":"2020-11-09T12:43:44","subject":"Re: [libcamera-devel] [PATCH 2/4] libcamera: camera_sensor: Break\n\tout properties initialization","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-11-06 16:49:45 +0100, Jacopo Mondi wrote:\n> Break out initialization to its own function in preparation to\n> add more properties.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  include/libcamera/internal/camera_sensor.h |  1 +\n>  src/libcamera/camera_sensor.cpp            | 79 ++++++++++++----------\n>  2 files changed, 45 insertions(+), 35 deletions(-)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index b9eba89f00fb..f80d836161a5 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -69,6 +69,7 @@ protected:\n>  \n>  private:\n>  \tint generateId();\n> +\tint initProperties();\n>  \n>  \tconst MediaEntity *entity_;\n>  \tstd::unique_ptr<V4L2Subdevice> subdev_;\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 2af0b0a8db52..49b0a026125c 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -172,6 +172,49 @@ int CameraSensor::init()\n>  \t\treturn -EINVAL;\n>  \t}\n>  \n> +\t/* Create and open the subdev. */\n> +\tsubdev_ = std::make_unique<V4L2Subdevice>(entity_);\n> +\tint ret = subdev_->open();\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tret = initProperties();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\t/* Enumerate, sort and cache media bus codes and sizes. */\n> +\tformats_ = subdev_->formats(pad_);\n> +\tif (formats_.empty()) {\n> +\t\tLOG(CameraSensor, Error) << \"No image format found\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tmbusCodes_ = utils::map_keys(formats_);\n> +\tstd::sort(mbusCodes_.begin(), mbusCodes_.end());\n> +\n> +\tfor (const auto &format : formats_) {\n> +\t\tconst std::vector<SizeRange> &ranges = format.second;\n> +\t\tstd::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes_),\n> +\t\t\t       [](const SizeRange &range) { return range.max; });\n> +\t}\n> +\n> +\tstd::sort(sizes_.begin(), sizes_.end());\n> +\n> +\t/* Remove duplicates. */\n> +\tauto last = std::unique(sizes_.begin(), sizes_.end());\n> +\tsizes_.erase(last, sizes_.end());\n> +\n> +\t/*\n> +\t * The sizes_ vector is sorted in ascending order, the resolution is\n> +\t * thus the last element of the vector.\n> +\t */\n> +\tresolution_ = sizes_.back();\n> +\n> +\treturn 0;\n> +}\n> +\n> +int CameraSensor::initProperties()\n> +{\n>  \t/*\n>  \t * Extract the camera sensor model name from the media entity name.\n>  \t *\n> @@ -202,14 +245,8 @@ int CameraSensor::init()\n>  \n>  \tproperties_.set(properties::Model, utils::toAscii(model_));\n>  \n> -\t/* Create and open the subdev. */\n> -\tsubdev_ = std::make_unique<V4L2Subdevice>(entity_);\n> -\tint ret = subdev_->open();\n> -\tif (ret < 0)\n> -\t\treturn ret;\n> -\n>  \t/* Generate a unique ID for the sensor. */\n> -\tret = generateId();\n> +\tint ret = generateId();\n>  \tif (ret)\n>  \t\treturn ret;\n>  \n> @@ -251,34 +288,6 @@ int CameraSensor::init()\n>  \t\tpropertyValue = 0;\n>  \tproperties_.set(properties::Rotation, propertyValue);\n>  \n> -\t/* Enumerate, sort and cache media bus codes and sizes. */\n> -\tformats_ = subdev_->formats(pad_);\n> -\tif (formats_.empty()) {\n> -\t\tLOG(CameraSensor, Error) << \"No image format found\";\n> -\t\treturn -EINVAL;\n> -\t}\n> -\n> -\tmbusCodes_ = utils::map_keys(formats_);\n> -\tstd::sort(mbusCodes_.begin(), mbusCodes_.end());\n> -\n> -\tfor (const auto &format : formats_) {\n> -\t\tconst std::vector<SizeRange> &ranges = format.second;\n> -\t\tstd::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes_),\n> -\t\t\t       [](const SizeRange &range) { return range.max; });\n> -\t}\n> -\n> -\tstd::sort(sizes_.begin(), sizes_.end());\n> -\n> -\t/* Remove duplicates. */\n> -\tauto last = std::unique(sizes_.begin(), sizes_.end());\n> -\tsizes_.erase(last, sizes_.end());\n> -\n> -\t/*\n> -\t * The sizes_ vector is sorted in ascending order, the resolution is\n> -\t * thus the last element of the vector.\n> -\t */\n> -\tresolution_ = sizes_.back();\n> -\n>  \treturn 0;\n>  }\n>  \n> -- \n> 2.29.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 9F16ABDB89\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  9 Nov 2020 12:43:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6187663077;\n\tMon,  9 Nov 2020 13:43:47 +0100 (CET)","from mail-lf1-x143.google.com (mail-lf1-x143.google.com\n\t[IPv6:2a00:1450:4864:20::143])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4190B61E57\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  9 Nov 2020 13:43:46 +0100 (CET)","by mail-lf1-x143.google.com with SMTP id u18so12169398lfd.9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 09 Nov 2020 04:43:46 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tq4sm2178240ljh.38.2020.11.09.04.43.44\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 09 Nov 2020 04:43:44 -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=\"DbXtJIn3\"; 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=Jb5u7qtQT4EhdeKGqG2IwmUQCs6z4xMQRFZdUAYSo7g=;\n\tb=DbXtJIn3OjoGZga1fhy+PyjJ0hRDDKHuK70Ln1s3FlRpw8/p34q7PMxNkcwVKZUk8x\n\tu4k/p5R0dch81h81Ml3hr35w9ysvm5eROjsHjvaC5uUgOFD/BqvOw6UB2dPNAGtu4qHY\n\txCPTl4N+CB5WU04bFlSFu5S9PbtkCeYUN5duTa4U7DgMYa74DfjWR3RSKv2TcqZHbe+d\n\tAGeHyHCc2rivvC6XJD4i+3JvHO65lPX5MwsmyVRaInlq7pZET9hqOjHs2/G6m3whUMCv\n\tMe2GG0r7pqlQrNnjjWngA3y7d0EHXNXsCCQLF61F+zWQvAdvjdY7/X4XeOisuKpTVmoU\n\t9EJg==","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=Jb5u7qtQT4EhdeKGqG2IwmUQCs6z4xMQRFZdUAYSo7g=;\n\tb=XahhBNTnq4jT4SU3DJAuGuEDWLVwHYyps5C3ShzqYje5/T1bIIBwxfb1ZsN5Hu5zf8\n\tJKM4kvU1vZiOz1DYWIiiyn0JeouHINXnnucXpWgPExs0kjluOWQoGnvkx9SYSWarncEd\n\tWOytcws78qSw7CGOkwIkXeodlgRyNzh606mDJxiamoCGbMX10zoojAsjS3uS6jAlquIT\n\tknGJgOnjvK/fmQ+ssLaQ0KRSqUFC2makkQKv5CDQ4GfubtWj4gjPzgSnjAzq7yGyz4If\n\tuh8XIgTYPCree3RssqeQ6yAzc+V2TurKOHk3jg1Nyukt67p2qfF83gKxdLLCWy0PJBnb\n\t5/tA==","X-Gm-Message-State":"AOAM531fn9J9irEK/jafryNdoHrnD64QxJLz85GTYWq5aM2RAe7JdYRN\n\t6OdYBGLicTFOSGWkV0EEMm+UWA==","X-Google-Smtp-Source":"ABdhPJwyvPPEY6eNvK6hL10ddpzwHcgsHPkfnOB4YN1Gs6JLy39WXTbN0BnO4sgs0OoaXibJqQ9ARA==","X-Received":"by 2002:a19:6916:: with SMTP id e22mr5432251lfc.45.1604925825491;\n\tMon, 09 Nov 2020 04:43:45 -0800 (PST)","Date":"Mon, 9 Nov 2020 13:43:44 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20201109124344.GB88486@oden.dyn.berto.se>","References":"<20201106154947.261132-1-jacopo@jmondi.org>\n\t<20201106154947.261132-3-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201106154947.261132-3-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 2/4] libcamera: camera_sensor: Break\n\tout properties initialization","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14026,"web_url":"https://patchwork.libcamera.org/comment/14026/","msgid":"<20201201185721.GW4569@pendragon.ideasonboard.com>","date":"2020-12-01T18:57:21","subject":"Re: [libcamera-devel] [PATCH 2/4] libcamera: camera_sensor: Break\n\tout properties initialization","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 Fri, Nov 06, 2020 at 04:49:45PM +0100, Jacopo Mondi wrote:\n> Break out initialization to its own function in preparation to\n> add more properties.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> ---\n>  include/libcamera/internal/camera_sensor.h |  1 +\n>  src/libcamera/camera_sensor.cpp            | 79 ++++++++++++----------\n>  2 files changed, 45 insertions(+), 35 deletions(-)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index b9eba89f00fb..f80d836161a5 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -69,6 +69,7 @@ protected:\n>  \n>  private:\n>  \tint generateId();\n> +\tint initProperties();\n>  \n>  \tconst MediaEntity *entity_;\n>  \tstd::unique_ptr<V4L2Subdevice> subdev_;\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 2af0b0a8db52..49b0a026125c 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -172,6 +172,49 @@ int CameraSensor::init()\n>  \t\treturn -EINVAL;\n>  \t}\n>  \n> +\t/* Create and open the subdev. */\n> +\tsubdev_ = std::make_unique<V4L2Subdevice>(entity_);\n> +\tint ret = subdev_->open();\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tret = initProperties();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\t/* Enumerate, sort and cache media bus codes and sizes. */\n> +\tformats_ = subdev_->formats(pad_);\n> +\tif (formats_.empty()) {\n> +\t\tLOG(CameraSensor, Error) << \"No image format found\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tmbusCodes_ = utils::map_keys(formats_);\n> +\tstd::sort(mbusCodes_.begin(), mbusCodes_.end());\n> +\n> +\tfor (const auto &format : formats_) {\n> +\t\tconst std::vector<SizeRange> &ranges = format.second;\n> +\t\tstd::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes_),\n> +\t\t\t       [](const SizeRange &range) { return range.max; });\n> +\t}\n> +\n> +\tstd::sort(sizes_.begin(), sizes_.end());\n> +\n> +\t/* Remove duplicates. */\n> +\tauto last = std::unique(sizes_.begin(), sizes_.end());\n> +\tsizes_.erase(last, sizes_.end());\n> +\n> +\t/*\n> +\t * The sizes_ vector is sorted in ascending order, the resolution is\n> +\t * thus the last element of the vector.\n> +\t */\n> +\tresolution_ = sizes_.back();\n> +\n> +\treturn 0;\n> +}\n> +\n> +int CameraSensor::initProperties()\n> +{\n>  \t/*\n>  \t * Extract the camera sensor model name from the media entity name.\n>  \t *\n> @@ -202,14 +245,8 @@ int CameraSensor::init()\n>  \n>  \tproperties_.set(properties::Model, utils::toAscii(model_));\n>  \n> -\t/* Create and open the subdev. */\n> -\tsubdev_ = std::make_unique<V4L2Subdevice>(entity_);\n> -\tint ret = subdev_->open();\n> -\tif (ret < 0)\n> -\t\treturn ret;\n> -\n>  \t/* Generate a unique ID for the sensor. */\n> -\tret = generateId();\n> +\tint ret = generateId();\n>  \tif (ret)\n>  \t\treturn ret;\n>  \n> @@ -251,34 +288,6 @@ int CameraSensor::init()\n>  \t\tpropertyValue = 0;\n>  \tproperties_.set(properties::Rotation, propertyValue);\n>  \n> -\t/* Enumerate, sort and cache media bus codes and sizes. */\n> -\tformats_ = subdev_->formats(pad_);\n> -\tif (formats_.empty()) {\n> -\t\tLOG(CameraSensor, Error) << \"No image format found\";\n> -\t\treturn -EINVAL;\n> -\t}\n> -\n> -\tmbusCodes_ = utils::map_keys(formats_);\n> -\tstd::sort(mbusCodes_.begin(), mbusCodes_.end());\n> -\n> -\tfor (const auto &format : formats_) {\n> -\t\tconst std::vector<SizeRange> &ranges = format.second;\n> -\t\tstd::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes_),\n> -\t\t\t       [](const SizeRange &range) { return range.max; });\n> -\t}\n> -\n> -\tstd::sort(sizes_.begin(), sizes_.end());\n> -\n> -\t/* Remove duplicates. */\n> -\tauto last = std::unique(sizes_.begin(), sizes_.end());\n> -\tsizes_.erase(last, sizes_.end());\n> -\n> -\t/*\n> -\t * The sizes_ vector is sorted in ascending order, the resolution is\n> -\t * thus the last element of the vector.\n> -\t */\n> -\tresolution_ = sizes_.back();\n> -\n>  \treturn 0;\n>  }\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 6F7A4BE176\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  1 Dec 2020 18:57:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0A32663501;\n\tTue,  1 Dec 2020 19:57:32 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 934D363460\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  1 Dec 2020 19:57:30 +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 164C0DBD;\n\tTue,  1 Dec 2020 19:57:30 +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=\"cakinDe/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1606849050;\n\tbh=KoMdEmLk/Ks4OqwmRVjs0ErpdmJJODE1hkjFrQ6Guec=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cakinDe/FJfLiyrj+w9pWcjeTnFtFXfHUocJxH+jjavt/EExMux/Ccjpkb3wcA3fG\n\tpbVvnBfkowyoRFBlLcv0pgGOiYGvacyqTzMcPMW29EEqD/9+nemPbBIEiM7WZqLGfH\n\tW3hbAOf/yXMM1A2LNQ5Lzu4rOJhfpcjHu3IvZHmQ=","Date":"Tue, 1 Dec 2020 20:57:21 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20201201185721.GW4569@pendragon.ideasonboard.com>","References":"<20201106154947.261132-1-jacopo@jmondi.org>\n\t<20201106154947.261132-3-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201106154947.261132-3-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 2/4] libcamera: camera_sensor: Break\n\tout properties initialization","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","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\" <libcamera-devel-bounces@lists.libcamera.org>"}}]