[{"id":14416,"web_url":"https://patchwork.libcamera.org/comment/14416/","msgid":"<X+xzAhxJu6YTyuur@pendragon.ideasonboard.com>","date":"2020-12-30T12:30:58","subject":"Re: [libcamera-devel] [PATCH v2 4/5] libcamera: camera_sensor: Add\n\tcontrols() method","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 Mon, Dec 28, 2020 at 05:55:59PM +0100, Jacopo Mondi wrote:\n> Add a controls() method to retrieve the map of libcamera controls\n> registered by the CameraSensor class.\n> \n> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  include/libcamera/internal/camera_sensor.h | 1 +\n>  src/libcamera/camera_sensor.cpp            | 6 ++++++\n>  2 files changed, 7 insertions(+)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index 841c7f4bef0f..10877b9d1d9d 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -62,6 +62,7 @@ public:\n>  \tint setControls(ControlList *ctrls);\n>  \n>  \tconst ControlList &properties() const { return properties_; }\n> +\tconst ControlInfoMap &controls() const { return controls_; }\n>  \tint sensorInfo(CameraSensorInfo *info) const;\n>  \n>  protected:\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 337d73c524bf..ea57943647b1 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -653,6 +653,12 @@ ControlList CameraSensor::getControls(const std::vector<uint32_t> &ids)\n>   * \\return The list of camera sensor properties\n>   */\n>  \n> +/**\n> + * \\fn CameraSensor::controls()\n> + * \\brief Retrieve the camera sensor controls info\n\nThis needs to describe what those controls are. You're using libcamera\ncontrols at the moment, which is OK as an interim measure, but we'll\nneed to define sensor-specific controls sooner than latter. This should\nbe mentioned, with a warning stating that those controls must not be\ncopied verbatim to the controls exposed by the Camera to applications.\nSimilarly, the subdevControls() documentation should state that the\nfunction is deprecated.\n\nThis brings a second question, which is whether using controls for this\nis actually the right API. There's little that the camera sensor needs\nto expose, and a specific API may make more sense.\n\n> + * \\return The map of camera sensor controls info\n> + */\n> +\n>  /**\n>   * \\brief Write controls to the sensor\n>   * \\param[in] ctrls The list of controls to write","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 14761C0F1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 30 Dec 2020 12:31:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D41F9615B2;\n\tWed, 30 Dec 2020 13:31:10 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7FDB660320\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 30 Dec 2020 13:31:09 +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 1EDB72A3;\n\tWed, 30 Dec 2020 13:31:09 +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=\"jtM4J0Eg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1609331469;\n\tbh=pnr4P0SEWZ47wn7rfPfKuHHNjBkz5C7+6BQbY3Ni1xU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jtM4J0EgdftdcG8tsuRzK2OcCS6edaPsc+ztmwm1KUrbCQgNPeaZVBFnk+GPUpy0l\n\tOsTvk+F7kEFCESBg508S1//tgFXIMqDybKxZKPA8SlGq9wXSgabgk7cBP85wlG+Rq3\n\tq6xwzvCZLEzcAsP6C4851wyoM4hrSGtIMO3EircA=","Date":"Wed, 30 Dec 2020 14:30:58 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<X+xzAhxJu6YTyuur@pendragon.ideasonboard.com>","References":"<20201228165600.53987-1-jacopo@jmondi.org>\n\t<20201228165600.53987-5-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201228165600.53987-5-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 4/5] libcamera: camera_sensor: Add\n\tcontrols() method","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>"}}]