[{"id":25872,"web_url":"https://patchwork.libcamera.org/comment/25872/","msgid":"","date":"2022-11-23T11:55:34","subject":"Re: [libcamera-devel] [PATCH v2 1/1] libcamera: camera_sensor:\n\tReset HBLANK when setting the format","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nThanks for fixing this.\n\nOn Wed, 23 Nov 2022 at 11:36, David Plowman via libcamera-devel <\nlibcamera-devel@lists.libcamera.org> wrote:\n\n> We no longer reset the HBLANK in init because we may not own the\n> camera and the operation may fail. Instead, we reset it when we set\n> the camera format so that all pipeline handlers, even ones that do not\n> control HBLANK themselves, will have a well-defined value.\n>\n> Signed-off-by: David Plowman \n>\n\nSeems reasonable!\n\nReviewed-By: Naushir Patuck \n\n\n> ---\n> src/libcamera/camera_sensor.cpp | 53 ++++++++++++++++-----------------\n> 1 file changed, 26 insertions(+), 27 deletions(-)\n>\n> diff --git a/src/libcamera/camera_sensor.cpp\n> b/src/libcamera/camera_sensor.cpp\n> index 572a313a..ec7bb0e7 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -176,32 +176,6 @@ int CameraSensor::init()\n> if (ret)\n> return ret;\n>\n> - /*\n> - * Set HBLANK to the minimum to start with a well-defined line\n> length,\n> - * allowing IPA modules that do not modify HBLANK to use the sensor\n> - * minimum line length in their calculations.\n> - *\n> - * At present, there is no way of knowing if a control is\n> read-only.\n> - * As a workaround, assume that if the minimum and maximum values\n> of\n> - * the V4L2_CID_HBLANK control are the same, it implies the control\n> - * is read-only.\n> - *\n> - * \\todo The control API ought to have a flag to specify if a\n> control\n> - * is read-only which could be used below.\n> - */\n> - const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);\n> - const int32_t hblankMin = hblank.min().get();\n> - const int32_t hblankMax = hblank.max().get();\n> -\n> - if (hblankMin != hblankMax) {\n> - ControlList ctrl(subdev_->controls());\n> -\n> - ctrl.set(V4L2_CID_HBLANK, hblankMin);\n> - ret = subdev_->setControls(&ctrl);\n> - if (ret)\n> - return ret;\n> - }\n> -\n> return\n> applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);\n> }\n>\n> @@ -748,7 +722,32 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat\n> *format)\n> return ret;\n>\n> updateControlInfo();\n> - return 0;\n> +\n> + /*\n> + * Set HBLANK to the minimum to start with a well-defined line\n> length,\n> + * allowing IPA modules that do not modify HBLANK to use the sensor\n> + * minimum line length in their calculations.\n> + *\n> + * At present, there is no way of knowing if a control is\n> read-only.\n> + * As a workaround, assume that if the minimum and maximum values\n> of\n> + * the V4L2_CID_HBLANK control are the same, it implies the control\n> + * is read-only.\n> + *\n> + * \\todo The control API ought to have a flag to specify if a\n> control\n> + * is read-only which could be used below.\n> + */\n> + const ControlInfo hblank = controls().at(V4L2_CID_HBLANK);\n> + const int32_t hblankMin = hblank.min().get();\n> + const int32_t hblankMax = hblank.max().get();\n> +\n> + if (hblankMin != hblankMax) {\n> + ControlList ctrl(subdev_->controls());\n> +\n> + ctrl.set(V4L2_CID_HBLANK, hblankMin);\n> + ret = subdev_->setControls(&ctrl);\n> + }\n> +\n> + return ret;\n> }\n>\n> /**\n> --\n> 2.30.2\n>\n>","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 18861BDE6B\n\tfor ;\n\tWed, 23 Nov 2022 11:55:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7FCB16331A;\n\tWed, 23 Nov 2022 12:55:52 +0100 (CET)","from mail-il1-x12a.google.com (mail-il1-x12a.google.com\n\t[IPv6:2607:f8b0:4864:20::12a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1AF1D63311\n\tfor ;\n\tWed, 23 Nov 2022 12:55:51 +0100 (CET)","by mail-il1-x12a.google.com with SMTP id o17so2339485ilg.12\n\tfor ;\n\tWed, 23 Nov 2022 03:55:51 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669204552;\n\tbh=3M23jJznTx/R+zIjGI47rLAC67+mlIs8dNFAHGkUZdA=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=ij3Ndi4Vl3VEuytNsSPrNBmzPkwtrF7JpgXYtApPu7kDp3rVWULLwM8W1YlUmd1lT\n\t3xPxh44mGc7YPh9MGIOHLpkjIji8IR5snScpQiMFREtBYFeeByvhxvptRNrL93ct24\n\tTV4LVuWR2tjeoxkkinWvCIE8PwNgVU08SaEfFYJHYbzCt7RrFPPe2A5mJP+isVvh5u\n\ti5L7M92saoyTgf3cP7eVxKfZPf7YJ8rmgkjrr/GTX5n8BYV/p77KvCg8UzlTeu7hmY\n\tfcx83OnbPrsNiOwNEH93C8vIXLZTFwuUQWjVQ8M/S4TUMkTYt7I2aLtZ7nK8oIyEJJ\n\tJIow1/s6jSpbQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=zILBpT3OGoxHajgJpI42I1+GmDa/KnKIss0U9e5LInk=;\n\tb=asBuMjFQPXANxmOslVsNFSr6QmwqxFDzU844cRQJX9MTp3li/Pttftk51kJV3OYvBU\n\trAB3buRKINAKtEjMJ0lJ6Ww9RiMnb3Emo9c9Kpb5N6Y6/AadTVlWKuxgzTYBiwQgV7BN\n\tKcrsH+QGkTtd06S9XXUsVTDkhyKWnoPQsL4cIwEfz1objcZKsvwGlsstFIMxSOigbEHE\n\tdUOpGqxzbnpVcY3rxDaFNdxc9pNdn9wnMOP20AczTbE1RQ4MLZn71B2Brj3s0nBoTxkb\n\teWBrHMgkFSqUrBx3sQ6lDWGaKxFSoyQFqo0uE82sX0cWAuNQL2xhOV01mtNVGrwIGNkJ\n\tuAnA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"asBuMjFQ\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=zILBpT3OGoxHajgJpI42I1+GmDa/KnKIss0U9e5LInk=;\n\tb=E6kuO6IXyhxEE/26c+m33aYa2q42tl4p8wh9nAvFkm6UuCv5Xi4zPiq/Dne24+K2Xa\n\tsWKGGmLif3iT3oN6+U7otl4pwa0GhQ/1swHWfM7u2/zYELv6SO7y2jUaPEZ+oeJ9wbI4\n\tjzKhm3VbLQWkGbhcSh+h+42+6UkGIHoFDJ58pKqybyIKOPzZ1V1mPMk7F/1iDVSZVhd8\n\ty6epmecVfBKA+uUSuDh3qNQCejBj2zM8vv7um2vuZ6JkjYZcOjDOdwnBxUdcYCdmH0jh\n\tSGkHdLpAcQpi8J7sO5CsMT+HDWEykYNNaj0dsnrhsVW0L8zbaFPv3d0bJ6i7D02tWUPs\n\t3pug==","X-Gm-Message-State":"ANoB5pkmV52nK5CNU/Uua/k6yvTXqObkMMZkOcc3YlhOonSUkVUSB3FZ\n\tGDeg8AXnCAe0U/LvJW5wwvfLx/ftiTVp/VIGFzK41YaolFY=","X-Google-Smtp-Source":"AA0mqf6yBA6uDI/hK/ikuarhXcdhPVEassXNgpzWH0Dr1fG94EB0KmP4jIqvBAPOfN2GA5+Vy7YPm3QIvT7CAuCsnBY=","X-Received":"by 2002:a92:8748:0:b0:2f9:b1d0:2f24 with SMTP id\n\td8-20020a928748000000b002f9b1d02f24mr12462740ilm.181.1669204549787;\n\tWed, 23 Nov 2022 03:55:49 -0800 (PST)","MIME-Version":"1.0","References":"<20221123113644.1778-1-david.plowman@raspberrypi.com>\n\t<20221123113644.1778-2-david.plowman@raspberrypi.com>","In-Reply-To":"<20221123113644.1778-2-david.plowman@raspberrypi.com>","Date":"Wed, 23 Nov 2022 11:55:34 +0000","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"0000000000003f7b5b05ee21f832\"","Subject":"Re: [libcamera-devel] [PATCH v2 1/1] libcamera: camera_sensor:\n\tReset HBLANK when setting the format","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","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25901,"web_url":"https://patchwork.libcamera.org/comment/25901/","msgid":"<20221124141544.4yllwzne56bh6jgq@uno.localdomain>","date":"2022-11-24T14:15:44","subject":"Re: [libcamera-devel] [PATCH v2 1/1] libcamera: camera_sensor:\n\tReset HBLANK when setting the format","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi David\n\nOn Wed, Nov 23, 2022 at 11:36:44AM +0000, David Plowman via libcamera-devel wrote:\n> We no longer reset the HBLANK in init because we may not own the\n> camera and the operation may fail. Instead, we reset it when we set\n> the camera format so that all pipeline handlers, even ones that do not\n> control HBLANK themselves, will have a well-defined value.\n>\n> Signed-off-by: David Plowman \n> ---\n> src/libcamera/camera_sensor.cpp | 53 ++++++++++++++++-----------------\n> 1 file changed, 26 insertions(+), 27 deletions(-)\n>\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 572a313a..ec7bb0e7 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -176,32 +176,6 @@ int CameraSensor::init()\n> \tif (ret)\n> \t\treturn ret;\n>\n> -\t/*\n> -\t * Set HBLANK to the minimum to start with a well-defined line length,\n> -\t * allowing IPA modules that do not modify HBLANK to use the sensor\n> -\t * minimum line length in their calculations.\n> -\t *\n> -\t * At present, there is no way of knowing if a control is read-only.\n> -\t * As a workaround, assume that if the minimum and maximum values of\n> -\t * the V4L2_CID_HBLANK control are the same, it implies the control\n> -\t * is read-only.\n> -\t *\n> -\t * \\todo The control API ought to have a flag to specify if a control\n> -\t * is read-only which could be used below.\n> -\t */\n> -\tconst ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);\n> -\tconst int32_t hblankMin = hblank.min().get();\n> -\tconst int32_t hblankMax = hblank.max().get();\n> -\n> -\tif (hblankMin != hblankMax) {\n> -\t\tControlList ctrl(subdev_->controls());\n> -\n> -\t\tctrl.set(V4L2_CID_HBLANK, hblankMin);\n> -\t\tret = subdev_->setControls(&ctrl);\n> -\t\tif (ret)\n> -\t\t\treturn ret;\n> -\t}\n> -\n> \treturn applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);\n> }\n>\n> @@ -748,7 +722,32 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)\n> \t\treturn ret;\n>\n> \tupdateControlInfo();\n\nI would do that -before- updating the controls info.\n\n> -\treturn 0;\n> +\n> +\t/*\n> +\t * Set HBLANK to the minimum to start with a well-defined line length,\n> +\t * allowing IPA modules that do not modify HBLANK to use the sensor\n> +\t * minimum line length in their calculations.\n> +\t *\n> +\t * At present, there is no way of knowing if a control is read-only.\n\nI know it was already like this, but can't we use\n\nconst struct v4l2_query_ext_ctrl *V4L2Device::controlInfo(uint32_t id) const\n\nand inspect the flags ?\n\n> +\t * As a workaround, assume that if the minimum and maximum values of\n> +\t * the V4L2_CID_HBLANK control are the same, it implies the control\n> +\t * is read-only.\n> +\t *\n> +\t * \\todo The control API ought to have a flag to specify if a control\n> +\t * is read-only which could be used below.\n> +\t */\n> +\tconst ControlInfo hblank = controls().at(V4L2_CID_HBLANK);\n> +\tconst int32_t hblankMin = hblank.min().get();\n> +\tconst int32_t hblankMax = hblank.max().get();\n> +\n> +\tif (hblankMin != hblankMax) {\n> +\t\tControlList ctrl(subdev_->controls());\n> +\n> +\t\tctrl.set(V4L2_CID_HBLANK, hblankMin);\n> +\t\tret = subdev_->setControls(&ctrl);\n> +\t}\n> +\n> +\treturn ret;\n> }\n\nI could take this one on top of the flip series maybe, if you agree\nwith the above changes ?\n\nThanks\n j\n\n>\n> /**\n> --\n> 2.30.2\n>","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 A7490BDE6B\n\tfor ;\n\tThu, 24 Nov 2022 14:15:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 63E2A6331F;\n\tThu, 24 Nov 2022 15:15:47 +0100 (CET)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::228])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E6E866330D\n\tfor ;\n\tThu, 24 Nov 2022 15:15:45 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 8EB5D1BF20B;\n\tThu, 24 Nov 2022 14:15:45 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669299347;\n\tbh=PIVWiMfrRz/MgLzJ/7EbsS7LAVLd+Ks4+L/m5uVBpzE=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=tPtmMjOLeD6V8uaiyg7viZSa+PNQNu0KbONwlrGSGB0sDmIy7MSAhT7a7dwhdG9lI\n\tU1cL/M0NbxHUdX/K4hRk3vwoB4ANNHSgs29IkeAG5yw7rOj6Hh4pVEG++bwEqc1SEQ\n\tswr9uIMe0dQTbnMEk8VntHy8oWpK6eB6uUKNSDpOzHxAQULkJcVrd88enzvxJghvJQ\n\tDhlqciDikXl5e/2qpLar3qyBWlNKF642liVKyg6cuahEv6EFzWrvGtR6hvynrWCmH7\n\tGDyJ25EX9lABdStTjqhYpr+Bq30GxdfXRkmhwqVSK4NNXmnVjURvUjfWR2UP/x4f3t\n\tqHafF3e6ly7QQ==","Date":"Thu, 24 Nov 2022 15:15:44 +0100","To":"David Plowman ","Message-ID":"<20221124141544.4yllwzne56bh6jgq@uno.localdomain>","References":"<20221123113644.1778-1-david.plowman@raspberrypi.com>\n\t<20221123113644.1778-2-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221123113644.1778-2-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 1/1] libcamera: camera_sensor:\n\tReset HBLANK when setting the format","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]