[{"id":28736,"web_url":"https://patchwork.libcamera.org/comment/28736/","msgid":"","date":"2024-02-26T14:21:07","subject":"Re: [PATCH] android: camera_device: The first valid settings should\n\tbe saved","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Fang Hui\n\nIn subject:\n\nandroid: camera_device: Save capture settings unconditionally\n\nOn Mon, Feb 26, 2024 at 11:08:12AM +0800, Fang Hui wrote:\n> If not, it will be deferred to the next frame.\n\nThis commit message is really too short and does not explain why this\nis an issue. With your ack I would change it to:\n\n------------------------------------------------------------------------------\nAs the Android framework sends to the camera device settings\nincrementally (only the ones that change are updated), the CameraDevice\nclass in the Android camera HAL keeps a copy of the last received\nsettings to be able to apply controls to the libcamera Camera and to\npopulate metadata correctly.\n\nWhen a valid 'camera3Request->settings' is provided, it gets saved to\n'lastSettings_' but 'descriptor->settings_' is not initialized until\nthe next frame (assuming it does not contain more settings).\n\nFix this by assigning to 'descriptor->settings_' the last saved\nsettings unconditionally.\n------------------------------------------------------------------------------\n\nAnd now that I wrote the last sentence, I realized lastSettings_ is\nnot re-initialized between streaming sessions.\n\nYou will likely need:\n\n--- a/src/android/camera_device.cpp\n+++ b/src/android/camera_device.cpp\n@@ -1108,6 +1108,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n }\n\n if (state_ == State::Stopped) {\n+ lastSettings_ = {};\n+\n ret = camera_->start();\n if (ret) {\n LOG(HAL, Error) << \"Failed to start camera\";\n\nWhich I can again add when applying with your ack.\n\nThanks\n j\n\n>\n> Signed-off-by: Fang Hui \n> ---\n> src/android/camera_device.cpp | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 25cedd44..d45ed1a5 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -952,8 +952,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> \t */\n> \tif (camera3Request->settings)\n> \t\tlastSettings_ = camera3Request->settings;\n> -\telse\n> -\t\tdescriptor->settings_ = lastSettings_;\n> +\n> +\tdescriptor->settings_ = lastSettings_;\n>\n> \tLOG(HAL, Debug) << \"Queueing request \" << descriptor->request_->cookie()\n> \t\t\t<< \" with \" << descriptor->buffers_.size() << \" streams\";\n> --\n> 2.25.1\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 CFD92BD160\n\tfor ;\n\tMon, 26 Feb 2024 14:21:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 286C062879;\n\tMon, 26 Feb 2024 15:21:12 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3E76E6285F\n\tfor ;\n\tMon, 26 Feb 2024 15:21:11 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B17BBCD1;\n\tMon, 26 Feb 2024 15:20:59 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"BwZb8qbf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1708957259;\n\tbh=bpYjq8PbMuKYuqeTtg6rdOAgXJ1asSnFv47F+C/+fr8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BwZb8qbfjrZSIZqM7Bi92UmEQWKKstcOtkbRDDF24FU6BlzrulGAd/hhb7mUpfP9K\n\tBkHW8mzkpSOv0MgtlKnC51ytCO8ZxaQn76Jx7upIqU6EpR8AAKO6G7hegYKxbx6Yy2\n\tJTBo6Ca/jWB3mkEiF+PEVlXoRU1YfXRhT9ylxWpM=","Date":"Mon, 26 Feb 2024 15:21:07 +0100","From":"Jacopo Mondi ","To":"Fang Hui ","Subject":"Re: [PATCH] android: camera_device: The first valid settings should\n\tbe saved","Message-ID":"","References":"<20240226030812.2225757-1-hui.fang@nxp.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240226030812.2225757-1-hui.fang@nxp.com>","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, biomifang118@gmail.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28737,"web_url":"https://patchwork.libcamera.org/comment/28737/","msgid":"","date":"2024-02-26T14:21:49","subject":"Re: [PATCH] android: camera_device: The first valid settings should\n\tbe saved","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Also,\n as there's a bug assigned to this issue\n\nOn Mon, Feb 26, 2024 at 11:08:12AM +0800, Fang Hui wrote:\n> If not, it will be deferred to the next frame.\n>\n\nBug: https://bugs.libcamera.org/show_bug.cgi?id=210\n\n> Signed-off-by: Fang Hui \n> ---\n> src/android/camera_device.cpp | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 25cedd44..d45ed1a5 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -952,8 +952,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> \t */\n> \tif (camera3Request->settings)\n> \t\tlastSettings_ = camera3Request->settings;\n> -\telse\n> -\t\tdescriptor->settings_ = lastSettings_;\n> +\n> +\tdescriptor->settings_ = lastSettings_;\n>\n> \tLOG(HAL, Debug) << \"Queueing request \" << descriptor->request_->cookie()\n> \t\t\t<< \" with \" << descriptor->buffers_.size() << \" streams\";\n> --\n> 2.25.1\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 2BD89BD160\n\tfor ;\n\tMon, 26 Feb 2024 14:21:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C1D6962871;\n\tMon, 26 Feb 2024 15:21:53 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 244E96286B\n\tfor ;\n\tMon, 26 Feb 2024 15:21:52 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BDEB2CD1;\n\tMon, 26 Feb 2024 15:21:40 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PQCbJOvI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1708957300;\n\tbh=OXj/p+19u/LVXgGJVEFYNqtlALoXoqCXt3w2gp25C8Q=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PQCbJOvIJ7b5jIt+AClAg9FjhKkRTs4iSBPZoR/ktOz8Q4N8a8mG5n3z2u8Gf/dfj\n\tuKplCVjaqgTK5X1CUYhas+XY1NYOAjtja1zDR0eu201OMuLA4w7UCeTrKz7LQ5iLuB\n\tN95Rq66sIb3fJdwjgqk8xtvvLz2cq8MCpHZ9Z/x8=","Date":"Mon, 26 Feb 2024 15:21:49 +0100","From":"Jacopo Mondi ","To":"Fang Hui ","Subject":"Re: [PATCH] android: camera_device: The first valid settings should\n\tbe saved","Message-ID":"","References":"<20240226030812.2225757-1-hui.fang@nxp.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240226030812.2225757-1-hui.fang@nxp.com>","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, biomifang118@gmail.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28772,"web_url":"https://patchwork.libcamera.org/comment/28772/","msgid":"","date":"2024-02-28T05:30:25","subject":"Re: [EXT] Re: [PATCH] android: camera_device: The first valid\n\tsettings should be saved","submitter":{"id":186,"url":"https://patchwork.libcamera.org/api/people/186/","name":"Fang Hui","email":"hui.fang@nxp.com"},"content":"commit 3a5da02954c7ab76a0c939eb1f806c3d529c7ec9 (HEAD -> master)\nAuthor: Fang Hui \nDate: Thu Jan 25 02:11:04 2024 +0800\n\n android: camera_device: Save capture settings unconditionally\n\n As the Android framework sends to the camera device settings\n incrementally (only the ones that change are updated), the CameraDevice\n class in the Android camera HAL keeps a copy of the last received\n settings to be able to apply controls to the libcamera Camera and to\n populate metadata correctly.\n\n When a valid 'camera3Request->settings' is provided, it gets saved to\n 'lastSettings_' but 'descriptor->settings_' is not initialized until\n the next frame (assuming it does not contain more settings).\n\n Fix this by assigning to 'descriptor->settings_' the last saved\n settings unconditionally.\n\n Bug: https://bugs.libcamera.org/show_bug.cgi?id=210\n\n Signed-off-by: Fang Hui \n\ndiff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\nindex 25cedd44..d2679a97 100644\n--- a/src/android/camera_device.cpp\n+++ b/src/android/camera_device.cpp\n@@ -952,8 +952,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n */\n if (camera3Request->settings)\n lastSettings_ = camera3Request->settings;\n- else\n- descriptor->settings_ = lastSettings_;\n+\n+ descriptor->settings_ = lastSettings_;\n\n LOG(HAL, Debug) << \"Queueing request \" << descriptor->request_->cookie()\n << \" with \" << descriptor->buffers_.size() << \" streams\";\n@@ -1108,6 +1108,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n }\n\n if (state_ == State::Stopped) {\n+ lastSettings_ = {};\n+\n ret = camera_->start();\n if (ret) {\n LOG(HAL, Error) << \"Failed to start camera\";","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 D5BA5BD160\n\tfor ;\n\tWed, 28 Feb 2024 05:30:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CBFD962867;\n\tWed, 28 Feb 2024 06:30:28 +0100 (CET)","from EUR03-AM7-obe.outbound.protection.outlook.com\n\t(mail-am7eur03on20600.outbound.protection.outlook.com\n\t[IPv6:2a01:111:f403:260e::600])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BE45A61C92\n\tfor ;\n\tWed, 28 Feb 2024 06:30:26 +0100 (CET)","from DB9PR04MB9284.eurprd04.prod.outlook.com (2603:10a6:10:36c::8)\n\tby PAXPR04MB8286.eurprd04.prod.outlook.com (2603:10a6:102:1cd::7)\n\twith Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7316.37;\n\tWed, 28 Feb 2024 05:30:25 +0000","from DB9PR04MB9284.eurprd04.prod.outlook.com\n\t([fe80::7d9b:76bb:b6c6:7f69]) by\n\tDB9PR04MB9284.eurprd04.prod.outlook.com\n\t([fe80::7d9b:76bb:b6c6:7f69%6]) with mapi id 15.20.7316.035;\n\tWed, 28 Feb 2024 05:30:25 +0000"],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"hULY9ZNz\";\n\tdkim-atps=neutral","dkim=none (message not signed)\n\theader.d=none;dmarc=none action=none header.from=nxp.com;"],"ARC-Seal":"i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;\n\tb=np6q2zQ/ytiyKO0xKmkestW3TL3uIKgkN0ZIImbM4sDA37Cc7dWu8eKAuADui9XL2StDpb0DxjPJjWFa/zFbVwUE9eSQ6iWxOdCr1Bs83CFRa7zJr+Kjq4Y50wq9PCo2YuR7APhvBMVY9ZZs+OGX1Vr/ncqCv82f2hZxEthb1uRW3YfzhFFMVETe/dDDFHg2RhLI0tfcdyJXLwDPLtwTNzJ7aOmiCizOUxll10BDkg27KUFxFXRse5Nw8YIWsw1GICUbtDr3hZjWFp8HmzRy9DBDW1LObuWGzcKcl71ttOG/KOq7ljcTV3RTqj7SMlnOZPg10DneKyqYneU9P+t0NA==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;\n\ts=arcselector9901;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;\n\tbh=da/GOr8dUNS9WphU3Ihd4sU6+dBKNQojnqU8YRWXguE=;\n\tb=MRmIRIoniUOloK21gpcTKxN/IwzqTX3yB9vrlrbl/EO9OdS6PNRGikSO7GeNhILHdp7J85QPmEsWdeAzl5H7GNs/NqMoyMD6AQmXNaTesMxcDthHeerIyDP+Jh0P+U7tiVskQurerh9dTfQrRWzUB76mskrDq6EQsySg+ETHaoXa64lTDdoZnMjOrJn3a+mcODy+5Yfayz0Nw1AwgLVf28O37rpYK6hVNhkLYTWTY6I8ot2l22HpJRn1cb4TYLKkejT9mAw0wFGWyEcWG1OyCZ47yoGNWLZ0kMDkWVOQW51K8d6iKuwgPtsYlhdemA8ccJVYB5Yate/ZDQF3kz3dxQ==","ARC-Authentication-Results":"i=1; mx.microsoft.com 1; spf=pass\n\tsmtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com;\n\tdkim=pass header.d=nxp.com; arc=none","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector2;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;\n\tbh=da/GOr8dUNS9WphU3Ihd4sU6+dBKNQojnqU8YRWXguE=;\n\tb=hULY9ZNzS4s7D0H4qt2nrHHdVyjijTpGw49dTnONrCwQen8LJXuFTN6n1ga4N10aJpqo4XpHroQdEK9e7lg1AvdLWdJ6GJREHxGaSQbwKOHx2c6avtWaYlL8ydL7+/AWawbdnLmjknELvVH23dDVwc9ZrhBTfjTZC9aUsDxCvPo=","From":"Hui Fang ","To":"Jacopo Mondi ","Subject":"Re: [EXT] Re: [PATCH] android: camera_device: The first valid\n\tsettings should be saved","Thread-Topic":"[EXT] Re: [PATCH] android: camera_device: The first valid\n\tsettings should be saved","Thread-Index":"AQHaaGWIegTkeApkY06F4hjjIpyTa7EcrXqAgAKPl8k=","Date":"Wed, 28 Feb 2024 05:30:25 +0000","Message-ID":"","References":"<20240226030812.2225757-1-hui.fang@nxp.com>\n\t","In-Reply-To":"","Accept-Language":"en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","msip_labels":"","authentication-results":["lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"hULY9ZNz\";\n\tdkim-atps=neutral","dkim=none (message not signed)\n\theader.d=none;dmarc=none action=none header.from=nxp.com;"],"x-ms-publictraffictype":"Email","x-ms-traffictypediagnostic":"DB9PR04MB9284:EE_|PAXPR04MB8286:EE_","x-ms-office365-filtering-correlation-id":"f5133a42-aebe-4eb6-0ea2-08dc381e5dd1","x-ms-exchange-senderadcheck":"1","x-ms-exchange-antispam-relay":"0","x-microsoft-antispam":"BCL:0;","x-microsoft-antispam-message-info":"qTEwzJilAJ9EUsmsiP2pdUTAzTHviH48L0D0b56ZkxWFs9baOUxe8Xej5I1WMQXkmAn0kwW8+zUsULjau0S6P2BzLGDtwXDUJF+jjDf69tDqhlWX+bIgdZKnHuCYp18CSKYbcH1uZgGAfLYC5XAecJblNF1WXFIXffXKEZoDveT4Etg1fdLRf8PbVEsePhdb2bmpbgROtlb83xWwUZ/FjAvN2b0LhQ5PV6jPjlf+2MaknZMcHxmIN97grLpIuSeHbXq80AGm2kP3/dZDme6UGxzVFmbYquMFmvbV95j/Os7RHp1Nre91z4HLBgt4I0HUgPlhUQDEdSnZsXy5lBCIJ6arCKZJs9Yg/MSKBDd9hqQxLbp75X5ksVVAilH5OgWgGfStbds/6bdwgBEiYaQDUoeTmvWMjPMz+WDhI31cKHUH+1vqTHDQPEESokBYgxtPCDijWgzfLXx7RGiAGYepMj3GlBjQ4YRghMLSZBb24JBTARI2xAihxjCm3q4bx+F4Pwk/I83cHsz2rW14uKnlqTKcGcwwxijGQY6t88kfYKsM34oGCYTqBEel8UhIGFtCB8+i0IARTC199IYizxsqQJXh8+Och2WxUn3zZq031S/OzMxbXLjVktJ9VZUX3La5BNTTKl2ylTcRRq1ZLDguN3yUljKrvRoNCetzsLC1srGZGRv1BvLRVqPiVuxgdSFEEQeXjEQg07JPrNY4ILwtnQ==","x-forefront-antispam-report":"CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;\n\tIPV:NLI; SFV:NSPM; H:DB9PR04MB9284.eurprd04.prod.outlook.com; PTR:;\n\tCAT:NONE; \n\tSFS:(13230031)(230273577357003)(38070700009); DIR:OUT; SFP:1101; ","x-ms-exchange-antispam-messagedata-chunkcount":"1","x-ms-exchange-antispam-messagedata-0":"rEiWogHnZSrbRrpqJGBEZpzwbeWWgICDeWjFSKylI/O6Qmux0C/Ct9j+MgSELe5wDo//8m2BOl2Hl0odDH+XQvVUkeriGxXzQBftOqOLPSt1uRunkYmLPhu1s+5zUWAX5zwBa4YQyxJJgKQXi0Zb6IDogURjVrUmdpfNaAzgXlRL0aUcGw3Dwcd4ui5+/UuIauYWYc9a8DCIPebBW2aAiOgdfotF9ziTdXN+17sPAcAKNGCcSrzS+4FyDqiy12jV/05ctNSt9dVWin//miSq5qWI8N+OwOzeYu40t8P489W4XvOfcQ0Isx6THMkmmEHVYAp87XkdFerrQ+C9HlAf1p3ECAEhyJaXdtGhWPP6CM6k/Fp7SseTMg8jT1ytozGtqqCUuSXxurYI7k2cNwO0/gnp5XzrT2CmGy+7K2GcafeUvGGjN8k1FHn4AtloZXU21qIWPVO1DpBilOPdGP2drkg9XG6qv9gAnYquMkKAq0eXNHMTTF1ajjDws8ts25GPSfbsvRliLZa84gbRBHCit7qrJHGCyLx9VWORD7iynQ6SaU8JQCVFWYhq5LKe4zOUxSUVTcf2lKjaajrYZ2jOmnJCuJvZNWYkZ1o2X0gp3L0yL5VgRBDKqpQwlKhJx9MgJxTX1oWixAba+sBdK3UabfBfOLRR50Go1D2ijbcIVB8jrsmikSSPwMno3Q6OYqb8W400/IIMr7F7cy6Cu72EfHp2B1BgKvVDMk5mJkUFaUjRZgIopOcWFUsAtibztM/xPPOIJppAsAUTxjthfncKZxggWPeKBZdBHZI5s5YpgOzu3dbdD0BwgLea9/vEmUAgnB6yGE07/344+67Kw4H51r+fMeq/jnCzjzqN7IoiKel83XwvwpEz05sBvJSLxE3SJQcP9UF4F5FSSChkKwCLaw6GjZFm/f7Glu4xjInHXrEPPeZmizc9wOU7/QYWfBH0IV7F+oNIgDutq2kVeQGW8Q4iu9qNYUBBI6zWcZJt5EA4N0+WgJMHZLk4d5lEKS4G4cP2aryQam+IGonImTERq6tYwvr6sS7ga6Al6cT+KflK+9kZX5rvMErKrg0AMc6OBLbAn1FoxghD6jEohcKQR9nAXR6z3QThfJbH5b+Oo1IYyYselsiBMV6GreMRJJ73oaLu4/n9dlFXUYhxrK9XQ2shBpLBGQZxiiFRWTUp20ipnUDH3rHeEYjXaiVDnVVOA08f+3y2K8jEsind4vcbSfnxzoO9MXLmOE08lQAMkkb316rirnNzaroGZAwCdkDwc0OpV52T7Fs1DylfqPucBMc1hRb7riDI5UeeRCKKBbD2UZNK5sr3An3JjIVt1gP6Iq0PDUarPOX82kzYWufW59t4tLmRauT6kXb0oKcwYNyaApQvXy+61RsS4qodCStxKJcXSlkpsrgWyzXJMy9jsodyzFjf1inBT/N1Wux6xPJMSRnpPXN36Y9S6ujaqSm+N5coczBYNitGEb8bs4aMbthV2eMnPYSNXhwALjtVlBkg+SAZgsj2gAWo//wGzbQA2P029vVfM177tNewRSXY4E/WfAyTSLHg2N1gp10JgKk=","Content-Type":"multipart/alternative;\n\tboundary=\"_000_DB9PR04MB9284386C13115F3917E9F6A087582DB9PR04MB9284eurp_\"","MIME-Version":"1.0","X-OriginatorOrg":"nxp.com","X-MS-Exchange-CrossTenant-AuthAs":"Internal","X-MS-Exchange-CrossTenant-AuthSource":"DB9PR04MB9284.eurprd04.prod.outlook.com","X-MS-Exchange-CrossTenant-Network-Message-Id":"f5133a42-aebe-4eb6-0ea2-08dc381e5dd1","X-MS-Exchange-CrossTenant-originalarrivaltime":"28 Feb 2024 05:30:25.3374\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635","X-MS-Exchange-CrossTenant-mailboxtype":"HOSTED","X-MS-Exchange-CrossTenant-userprincipalname":"BHyqK0f8eI16bXVuUE58Gzt72YF1+FusFPvgcCwHazs1yRbKyCioZ34dZWMZNEoFbZxRvIxAeckuEu7idJ6X1Q==","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"PAXPR04MB8286","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\"\n\t,\n\t\"biomifang118@gmail.com\" ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28774,"web_url":"https://patchwork.libcamera.org/comment/28774/","msgid":"<41c4b8f7-e087-4586-aaa4-9c06e103ff1f@ideasonboard.com>","date":"2024-02-28T08:05:20","subject":"Re: [PATCH] android: camera_device: The first valid settings should\n\tbe saved","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the discussion,\n\nOn 26/02/24 7:51 pm, Jacopo Mondi wrote:\n> Hi Fang Hui\n>\n> In subject:\n>\n> android: camera_device: Save capture settings unconditionally\n>\n> On Mon, Feb 26, 2024 at 11:08:12AM +0800, Fang Hui wrote:\n>> If not, it will be deferred to the next frame.\n> This commit message is really too short and does not explain why this\n> is an issue. With your ack I would change it to:\n>\n> ------------------------------------------------------------------------------\n> As the Android framework sends to the camera device settings\n> incrementally (only the ones that change are updated), the CameraDevice\n> class in the Android camera HAL keeps a copy of the last received\n> settings to be able to apply controls to the libcamera Camera and to\n> populate metadata correctly.\n>\n> When a valid 'camera3Request->settings' is provided, it gets saved to\n> 'lastSettings_' but 'descriptor->settings_' is not initialized until\n> the next frame (assuming it does not contain more settings).\n>\n> Fix this by assigning to 'descriptor->settings_' the last saved\n> settings unconditionally.\n> ------------------------------------------------------------------------------\n>\n> And now that I wrote the last sentence, I realized lastSettings_ is\n> not re-initialized between streaming sessions.\n>\n> You will likely need:\n>\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1108,6 +1108,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> }\n>\n> if (state_ == State::Stopped) {\n> + lastSettings_ = {};\n> +\n> ret = camera_->start();\n> if (ret) {\n> LOG(HAL, Error) << \"Failed to start camera\";\n>\n> Which I can again add when applying with your ack.\n\nWith the original patch hunk below and lastSettings_= {}; and commit \nmessage fixed,\n\nReviewed-by: Umang Jain \n\n>\n> Thanks\n> j\n>\n>> Signed-off-by: Fang Hui \n>> ---\n>> src/android/camera_device.cpp | 4 ++--\n>> 1 file changed, 2 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>> index 25cedd44..d45ed1a5 100644\n>> --- a/src/android/camera_device.cpp\n>> +++ b/src/android/camera_device.cpp\n>> @@ -952,8 +952,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n>> \t */\n>> \tif (camera3Request->settings)\n>> \t\tlastSettings_ = camera3Request->settings;\n>> -\telse\n>> -\t\tdescriptor->settings_ = lastSettings_;\n>> +\n>> +\tdescriptor->settings_ = lastSettings_;\n>>\n>> \tLOG(HAL, Debug) << \"Queueing request \" << descriptor->request_->cookie()\n>> \t\t\t<< \" with \" << descriptor->buffers_.size() << \" streams\";\n>> --\n>> 2.25.1\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 07FB2BD160\n\tfor ;\n\tWed, 28 Feb 2024 08:05:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DD96962865;\n\tWed, 28 Feb 2024 09:05:33 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7C00F627F9\n\tfor ;\n\tWed, 28 Feb 2024 09:05:31 +0100 (CET)","from [192.168.1.102] (unknown [103.251.226.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B6B01672;\n\tWed, 28 Feb 2024 09:05:16 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Lui3Xa4N\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709107518;\n\tbh=Nd+349uML9m9hSF7vYNGWsrgInFdnElpCaiKZgA4Gxc=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=Lui3Xa4NTvca/SImDCsQ0uoAeLearfy9G7Nx4NLHVcKA+zqPyq6qiSg2NftF1dozT\n\twouQq6l+vLk1P+VOmMio/bJL+VyJbB7//DUb3KKsMCULmVmSZ6Eox8AXF6AhnVNoyN\n\tYQt35Xato5R7IdEG6JAVYJQtNsevSrCcWxHvXmkM=","Message-ID":"<41c4b8f7-e087-4586-aaa4-9c06e103ff1f@ideasonboard.com>","Date":"Wed, 28 Feb 2024 13:35:20 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH] android: camera_device: The first valid settings should\n\tbe saved","To":"Jacopo Mondi , Fang Hui ","References":"<20240226030812.2225757-1-hui.fang@nxp.com>\n\t","Content-Language":"en-US","From":"Umang Jain ","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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, biomifang118@gmail.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29036,"web_url":"https://patchwork.libcamera.org/comment/29036/","msgid":"","date":"2024-03-22T10:11:53","subject":"Re: [EXT] Re: [PATCH] android: camera_device: The first valid\n\tsettings should be saved","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hello\n\nOn Wed, Feb 28, 2024 at 05:30:25AM +0000, Hui Fang wrote:\n> commit 3a5da02954c7ab76a0c939eb1f806c3d529c7ec9 (HEAD -> master)\n> Author: Fang Hui \n> Date: Thu Jan 25 02:11:04 2024 +0800\n>\n> android: camera_device: Save capture settings unconditionally\n>\n> As the Android framework sends to the camera device settings\n> incrementally (only the ones that change are updated), the CameraDevice\n> class in the Android camera HAL keeps a copy of the last received\n> settings to be able to apply controls to the libcamera Camera and to\n> populate metadata correctly.\n>\n> When a valid 'camera3Request->settings' is provided, it gets saved to\n> 'lastSettings_' but 'descriptor->settings_' is not initialized until\n> the next frame (assuming it does not contain more settings).\n>\n> Fix this by assigning to 'descriptor->settings_' the last saved\n> settings unconditionally.\n>\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=210\n>\n> Signed-off-by: Fang Hui \n\nNow tested with CTS, no regressions detected\n\nTested-by: Jacopo Mondi \n\nI'll merge it!\n\nThanks\n j\n\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 25cedd44..d2679a97 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -952,8 +952,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> */\n> if (camera3Request->settings)\n> lastSettings_ = camera3Request->settings;\n> - else\n> - descriptor->settings_ = lastSettings_;\n> +\n> + descriptor->settings_ = lastSettings_;\n>\n> LOG(HAL, Debug) << \"Queueing request \" << descriptor->request_->cookie()\n> << \" with \" << descriptor->buffers_.size() << \" streams\";\n> @@ -1108,6 +1108,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> }\n>\n> if (state_ == State::Stopped) {\n> + lastSettings_ = {};\n> +\n> ret = camera_->start();\n> if (ret) {\n> LOG(HAL, Error) << \"Failed to start camera\";\n> ________________________________\n> From: Jacopo Mondi \n> Sent: Monday, February 26, 2024 10:21 PM\n> To: Hui Fang \n> Cc: libcamera-devel@lists.libcamera.org ; biomifang118@gmail.com \n> Subject: [EXT] Re: [PATCH] android: camera_device: The first valid settings should be saved\n>\n> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button\n>\n>\n> Also,\n> as there's a bug assigned to this issue\n>\n> On Mon, Feb 26, 2024 at 11:08:12AM +0800, Fang Hui wrote:\n> > If not, it will be deferred to the next frame.\n> >\n>\n> Bug: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.libcamera.org%2Fshow_bug.cgi%3Fid%3D210&data=05%7C02%7Chui.fang%40nxp.com%7Cc9341f6fe9624af769f108dc36d64722%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638445541153242800%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=2JewcBFhgU4K%2BDofrUZLQUduptmwAwDcomPEXwQ3Ybw%3D&reserved=0\n>\n> > Signed-off-by: Fang Hui \n> > ---\n> > src/android/camera_device.cpp | 4 ++--\n> > 1 file changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 25cedd44..d45ed1a5 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -952,8 +952,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> > */\n> > if (camera3Request->settings)\n> > lastSettings_ = camera3Request->settings;\n> > - else\n> > - descriptor->settings_ = lastSettings_;\n> > +\n> > + descriptor->settings_ = lastSettings_;\n> >\n> > LOG(HAL, Debug) << \"Queueing request \" << descriptor->request_->cookie()\n> > << \" with \" << descriptor->buffers_.size() << \" streams\";\n> > --\n> > 2.25.1\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 F0789BD160\n\tfor ;\n\tFri, 22 Mar 2024 10:11:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0A7F462CA3;\n\tFri, 22 Mar 2024 11:11:59 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4007162CA3\n\tfor ;\n\tFri, 22 Mar 2024 11:11:57 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:cc1e:e404:491f:e6ea])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 25FEA82A;\n\tFri, 22 Mar 2024 11:11:28 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"dH4ZoR1A\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1711102288;\n\tbh=ea0LZYyCBV4QrzC2HF/eLl5kU4W6Ijs/qawPtQMP0Uk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dH4ZoR1AbRKoLHe5FBAfHhk20kZL8LbbNccpujzxT+/JBgg+Tp9fqUOJdUbZuziwc\n\tiESdEhK6FbbypgHq2fe1brogPGa7amOx6UyL8HAWqS8YB232mc7sd7n6kRFPLfvj5h\n\tlUcD2YletFx7Ji+mJm8gj+0bWz+UYo0QrWVkVTpc=","Date":"Fri, 22 Mar 2024 11:11:53 +0100","From":"Jacopo Mondi ","To":"Hui Fang ","Cc":"Jacopo Mondi , \n\t\"libcamera-devel@lists.libcamera.org\"\n\t, \n\t\"biomifang118@gmail.com\" ","Subject":"Re: [EXT] Re: [PATCH] android: camera_device: The first valid\n\tsettings should be saved","Message-ID":"","References":"<20240226030812.2225757-1-hui.fang@nxp.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]