[{"id":27272,"web_url":"https://patchwork.libcamera.org/comment/27272/","msgid":"<168c-647f1e80-5-2235d640@253258028>","date":"2023-06-06T11:55:43","subject":"Re: [libcamera-devel] =?utf-8?q?=5BPATCH_v3_2/5=5D_libcamera=3A_?=\n\t=?utf-8?q?camera=5Fmanager=3A_Move_private_implementation_to_inter?=\n\t=?utf-8?q?nal?=","submitter":{"id":165,"url":"https://patchwork.libcamera.org/api/people/165/","name":"Ashok Sidipotu","email":"ashok.sidipotu@collabora.com"},"content":"Tested-by: Ashok Sidipotu \nOn Monday, May 15, 2023 18:15 IST, Kieran Bingham wrote:\n The CameraManager makes use of the Extensible pattern to provide an\ninternal private implementation that is not exposed in the public API.\n\nMove the Private declaration to an internal header to make it available\nfrom other internal components in preperation for reducing the surface\narea of the public interface of the Camera Manager.\n\nSigned-off-by: Kieran Bingham \nReviewed-by: Jacopo Mondi \n---\ninclude/libcamera/internal/camera_manager.h | 64 +++++++++++++++++++++\ninclude/libcamera/internal/meson.build | 1 +\nsrc/libcamera/camera_manager.cpp | 50 ++--------------\n3 files changed, 69 insertions(+), 46 deletions(-)\ncreate mode 100644 include/libcamera/internal/camera_manager.h\n\ndiff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\nnew file mode 100644\nindex 000000000000..05a1e4df8add\n--- /dev/null\n+++ b/include/libcamera/internal/camera_manager.h\n@@ -0,0 +1,64 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2023, Ideas on Board Oy.\n+ *\n+ * camera_manager.h - Camera manager private data\n+ */\n+\n+#pragma once\n+\n+#include \n+\n+#include \n+#include \n+\n+#include \n+\n+#include \"libcamera/internal/ipa_manager.h\"\n+#include \"libcamera/internal/process.h\"\n+\n+namespace libcamera {\n+\n+class DeviceEnumerator;\n+\n+class CameraManager::Private : public Extensible::Private, public Thread\n+{\n+ LIBCAMERA_DECLARE_PUBLIC(CameraManager)\n+\n+public:\n+ Private();\n+\n+ int start();\n+ void addCamera(std::shared_ptr camera,\n+ const std::vector &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);\n+ void removeCamera(Camera *camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n+\n+ /*\n+ * This mutex protects\n+ *\n+ * - initialized_ and status_ during initialization\n+ * - cameras_ and camerasByDevnum_ after initialization\n+ */\n+ mutable Mutex mutex_;\n+ std::vector> cameras_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n+ std::map> camerasByDevnum_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n+\n+protected:\n+ void run() override;\n+\n+private:\n+ int init();\n+ void createPipelineHandlers();\n+ void cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);\n+\n+ ConditionVariable cv_;\n+ bool initialized_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n+ int status_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n+\n+ std::unique_ptr enumerator_;\n+\n+ IPAManager ipaManager_;\n+ ProcessManager processManager_;\n+};\n+\n+} /* namespace libcamera */\ndiff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\nindex d75088059996..0028ed0dc27f 100644\n--- a/include/libcamera/internal/meson.build\n+++ b/include/libcamera/internal/meson.build\n@@ -13,6 +13,7 @@ libcamera_internal_headers = files([\n'bayer_format.h',\n'byte_stream_buffer.h',\n'camera.h',\n+ 'camera_manager.h',\n'camera_controls.h',\n'camera_lens.h',\n'camera_sensor.h',\ndiff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\nindex c1edefdad160..d3c297b888d8 100644\n--- a/src/libcamera/camera_manager.cpp\n+++ b/src/libcamera/camera_manager.cpp\n@@ -5,24 +5,22 @@\n* camera_manager.h - Camera management\n*/\n\n-#include \n+#include \"libcamera/internal/camera_manager.h\"\n\n#include \n\n-#include \n-\n#include \n#include \n#include \n#include \n\n+#include \n+\n#include \"libcamera/internal/device_enumerator.h\"\n-#include \"libcamera/internal/ipa_manager.h\"\n#include \"libcamera/internal/pipeline_handler.h\"\n-#include \"libcamera/internal/process.h\"\n\n/**\n- * \\file camera_manager.h\n+ * \\file libcamera/camera_manager.h\n* \\brief The camera manager\n*/\n\n@@ -33,46 +31,6 @@ namespace libcamera {\n\nLOG_DEFINE_CATEGORY(Camera)\n\n-class CameraManager::Private : public Extensible::Private, public Thread\n-{\n- LIBCAMERA_DECLARE_PUBLIC(CameraManager)\n-\n-public:\n- Private();\n-\n- int start();\n- void addCamera(std::shared_ptr camera,\n- const std::vector &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);\n- void removeCamera(Camera *camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n-\n- /*\n- * This mutex protects\n- *\n- * - initialized_ and status_ during initialization\n- * - cameras_ and camerasByDevnum_ after initialization\n- */\n- mutable Mutex mutex_;\n- std::vector> cameras_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n- std::map> camerasByDevnum_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n-\n-protected:\n- void run() override;\n-\n-private:\n- int init();\n- void createPipelineHandlers();\n- void cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);\n-\n- ConditionVariable cv_;\n- bool initialized_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n- int status_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n-\n- std::unique_ptr enumerator_;\n-\n- IPAManager ipaManager_;\n- ProcessManager processManager_;\n-};\n-\nCameraManager::Private::Private()\n: initialized_(false)\n{\n--\n2.34.1","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 6D960C31E9\n\tfor ;\n\tTue, 6 Jun 2023 12:03:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CC00662898;\n\tTue, 6 Jun 2023 14:03:28 +0200 (CEST)","from madras.collabora.co.uk (madras.collabora.co.uk\n\t[46.235.227.172])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C9E5762722\n\tfor ;\n\tTue, 6 Jun 2023 13:55:43 +0200 (CEST)","from hamburger.collabora.co.uk (hamburger.collabora.co.uk\n\t[IPv6:2a01:4f8:1c1c:f269::1])\n\tby madras.collabora.co.uk (Postfix) with ESMTP id 5A4686606EAD;\n\tTue, 6 Jun 2023 12:55:43 +0100 (BST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686053008;\n\tbh=ZB6HgoVmNTQPCrssngz/nSZdNeQ7HKxGEYFxk86fdLE=;\n\th=In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:\n\tList-Post:List-Help:List-Subscribe:From:Reply-To:Cc:From;\n\tb=q75PSQq/MvcgIu74EwR4DuF3ilssizndS45ebcjklIye+h1i8EvjOSm5gD5nGxTId\n\t+pmSQWzi97KA4DIcZUsJ3E+0T9Hb4B7ZkwdDBafWA9p9nln+D+AE5lAuiyW2w2Hgwy\n\thGSzErJZCqM6U0HZ3rG+bRPsa9nx6P5Kpe7JoR/OirkA2yx61OGOv91bsqnP2ISP9O\n\t6K5+URjA8iXpgXTn/hXbm5HUgheGcHAnRnTF/GjB4Uy5AuiuNsxwhFAJer731KOWLu\n\tiSwOTDzMuAZ8CqOc9hoBe5zgfSUIY+byE3EgapJE+VgE2ajg1AcBgvJNx5zKyiQis7\n\tziHMQ6HL6OdZw==","In-Reply-To":"<20230515124550.3601128-3-kieran.bingham@ideasonboard.com>","Content-Type":"multipart/alternative;\n\tboundary=\"----=_=-_OpenGroupware_org_NGMime-5772-1686052543.111434-1------\"","X-Forward":"127.0.0.1","Date":"Tue, 06 Jun 2023 12:55:43 +0100","To":"\"Kieran Bingham\" ","MIME-Version":"1.0","Message-ID":"<168c-647f1e80-5-2235d640@253258028>","User-Agent":"SOGoMail 5.8.0","X-Mailman-Approved-At":"Tue, 06 Jun 2023 14:03:27 +0200","Subject":"Re: [libcamera-devel] =?utf-8?q?=5BPATCH_v3_2/5=5D_libcamera=3A_?=\n\t=?utf-8?q?camera=5Fmanager=3A_Move_private_implementation_to_inter?=\n\t=?utf-8?q?nal?=","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":"Ashok Sidipotu via libcamera-devel\n\t","Reply-To":"Ashok Sidipotu ","Cc":"Jacopo Mondi ,\n\tlibcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27288,"web_url":"https://patchwork.libcamera.org/comment/27288/","msgid":"<20230606160535.GD25679@pendragon.ideasonboard.com>","date":"2023-06-06T16:05:35","subject":"Re: [libcamera-devel] [PATCH v3 2/5] libcamera: camera_manager:\n\tMove private implementation to internal","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Mon, May 15, 2023 at 01:45:47PM +0100, Kieran Bingham via libcamera-devel wrote:\n> The CameraManager makes use of the Extensible pattern to provide an\n> internal private implementation that is not exposed in the public API.\n> \n> Move the Private declaration to an internal header to make it available\n> from other internal components in preperation for reducing the surface\n> area of the public interface of the Camera Manager.\n\ns/Camera Manager/CameraManager/\n\n> Signed-off-by: Kieran Bingham \n> Reviewed-by: Jacopo Mondi \n> ---\n> include/libcamera/internal/camera_manager.h | 64 +++++++++++++++++++++\n> include/libcamera/internal/meson.build | 1 +\n> src/libcamera/camera_manager.cpp | 50 ++--------------\n> 3 files changed, 69 insertions(+), 46 deletions(-)\n> create mode 100644 include/libcamera/internal/camera_manager.h\n> \n> diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n> new file mode 100644\n> index 000000000000..05a1e4df8add\n> --- /dev/null\n> +++ b/include/libcamera/internal/camera_manager.h\n> @@ -0,0 +1,64 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Ideas on Board Oy.\n> + *\n> + * camera_manager.h - Camera manager private data\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n\n#include \n\nfor std::shared_ptr, std::unique_ptr and std::weak_ptr, and\n\n#include \n\nfor std::vector.\n\n> +\n\n#include \n\nfor Extensible::Private.\n\n> +#include \n> +#include \n> +\n> +#include \n\nShould this go first to improve the self-contained header test coverage\n?\n\n> +\n> +#include \"libcamera/internal/ipa_manager.h\"\n> +#include \"libcamera/internal/process.h\"\n> +\n> +namespace libcamera {\n> +\n\nclass Camera;\n\n> +class DeviceEnumerator;\n> +\n> +class CameraManager::Private : public Extensible::Private, public Thread\n> +{\n> +\tLIBCAMERA_DECLARE_PUBLIC(CameraManager)\n> +\n> +public:\n> +\tPrivate();\n> +\n> +\tint start();\n> +\tvoid addCamera(std::shared_ptr camera,\n> +\t\t const std::vector &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> +\tvoid removeCamera(Camera *camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> +\n> +\t/*\n> +\t * This mutex protects\n> +\t *\n> +\t * - initialized_ and status_ during initialization\n> +\t * - cameras_ and camerasByDevnum_ after initialization\n> +\t */\n> +\tmutable Mutex mutex_;\n> +\tstd::vector> cameras_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n> +\tstd::map> camerasByDevnum_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n> +\n> +protected:\n> +\tvoid run() override;\n> +\n> +private:\n> +\tint init();\n> +\tvoid createPipelineHandlers();\n> +\tvoid cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);\n> +\n> +\tConditionVariable cv_;\n> +\tbool initialized_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n> +\tint status_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n> +\n> +\tstd::unique_ptr enumerator_;\n> +\n> +\tIPAManager ipaManager_;\n> +\tProcessManager processManager_;\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index d75088059996..0028ed0dc27f 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -13,6 +13,7 @@ libcamera_internal_headers = files([\n> 'bayer_format.h',\n> 'byte_stream_buffer.h',\n> 'camera.h',\n> + 'camera_manager.h',\n\nAlphabetical order.\n\n> 'camera_controls.h',\n> 'camera_lens.h',\n> 'camera_sensor.h',\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index c1edefdad160..d3c297b888d8 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -5,24 +5,22 @@\n> * camera_manager.h - Camera management\n> */\n> \n> -#include \n> +#include \"libcamera/internal/camera_manager.h\"\n> \n> #include \n\nYou can drop this.\n\n> \n> -#include \n> -\n> #include \n> #include \n> #include \n\nAnd mutex.h and thread.h too.\n\n> #include \n> \n> +#include \n> +\n> #include \"libcamera/internal/device_enumerator.h\"\n> -#include \"libcamera/internal/ipa_manager.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> -#include \"libcamera/internal/process.h\"\n> \n> /**\n> - * \\file camera_manager.h\n> + * \\file libcamera/camera_manager.h\n\nI think you need both, see framebuffer.cpp.\n\n> * \\brief The camera manager\n> */\n> \n> @@ -33,46 +31,6 @@ namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(Camera)\n> \n> -class CameraManager::Private : public Extensible::Private, public Thread\n> -{\n> -\tLIBCAMERA_DECLARE_PUBLIC(CameraManager)\n> -\n> -public:\n> -\tPrivate();\n> -\n> -\tint start();\n> -\tvoid addCamera(std::shared_ptr camera,\n> -\t\t const std::vector &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> -\tvoid removeCamera(Camera *camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> -\n> -\t/*\n> -\t * This mutex protects\n> -\t *\n> -\t * - initialized_ and status_ during initialization\n> -\t * - cameras_ and camerasByDevnum_ after initialization\n> -\t */\n> -\tmutable Mutex mutex_;\n> -\tstd::vector> cameras_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n> -\tstd::map> camerasByDevnum_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n> -\n> -protected:\n> -\tvoid run() override;\n> -\n> -private:\n> -\tint init();\n> -\tvoid createPipelineHandlers();\n> -\tvoid cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);\n> -\n> -\tConditionVariable cv_;\n> -\tbool initialized_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n> -\tint status_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n> -\n> -\tstd::unique_ptr enumerator_;\n> -\n> -\tIPAManager ipaManager_;\n> -\tProcessManager processManager_;\n> -};\n> -\n> CameraManager::Private::Private()\n> \t: initialized_(false)\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 2347FC31E9\n\tfor ;\n\tTue, 6 Jun 2023 16:05:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 487DC62893;\n\tTue, 6 Jun 2023 18:05:39 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 140B862722\n\tfor ;\n\tTue, 6 Jun 2023 18:05:38 +0200 (CEST)","from pendragon.ideasonboard.com (om126253223039.31.openmobile.ne.jp\n\t[126.253.223.39])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8AD66283;\n\tTue, 6 Jun 2023 18:05:11 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686067539;\n\tbh=4vyw1Vrwu6aW6S0invscCydA3sMJWOoZF3hb7nxI/n0=;\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=r5YUbNJnOTsXWEOzzS0l1FrjgRg1+eAOxSfTBkPD3hWccljaPVJHQvr8Dq9mNHl5c\n\tdvhL5xrLefXN0jGkvIvkYEqLmn2yCOMSS0MRxMrrfKRLoDWhiEP2AeWlarDER4cuwd\n\t6QrcJR9ciS4muTK8GDSbEhKv+AFj1kIAp2uRGXhLiQjIVnWkXqzgjumHQvXXZaN3Co\n\tTB+pFqzHTtgikiyVw5tvwqX+LuzRCeA42eVY2U6bLGYhdcF0aL/PV37tDSg5qstdXS\n\tKC8H33OzO3k4iDHUCJbFpu+fKTysooE6fYiPeHOhZUayD1PW0ap5qO91HPLXi8HW/1\n\tqH9R6kBXhwlQQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686067512;\n\tbh=4vyw1Vrwu6aW6S0invscCydA3sMJWOoZF3hb7nxI/n0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=X3F63FCkyECZqIDzR5BJdwP1fkpeGakaesGxPskABUYoFYynGZM3NACCaeuG/FsKf\n\tp6+GtZFFR24bsfAmTIAE1x7yi1CaEzxJzT5TdK7lM96IwqbuYoysrcsLL6MBm4zcs/\n\tUHxfU1IXJ+2FN9LiYb7TeK0XC6jvbuhIWdgyjaSI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"X3F63FCk\"; dkim-atps=neutral","Date":"Tue, 6 Jun 2023 19:05:35 +0300","To":"Kieran Bingham ","Message-ID":"<20230606160535.GD25679@pendragon.ideasonboard.com>","References":"<20230515124550.3601128-1-kieran.bingham@ideasonboard.com>\n\t<20230515124550.3601128-3-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230515124550.3601128-3-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 2/5] libcamera: camera_manager:\n\tMove private implementation to internal","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"Jacopo Mondi ,\n\tlibcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27301,"web_url":"https://patchwork.libcamera.org/comment/27301/","msgid":"<168614554930.2889415.11370787098019306570@Monstersaurus>","date":"2023-06-07T13:45:49","subject":"Re: [libcamera-devel] [PATCH v3 2/5] libcamera: camera_manager:\n\tMove private implementation to internal","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2023-06-06 17:05:35)\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Mon, May 15, 2023 at 01:45:47PM +0100, Kieran Bingham via libcamera-devel wrote:\n> > The CameraManager makes use of the Extensible pattern to provide an\n> > internal private implementation that is not exposed in the public API.\n> > \n> > Move the Private declaration to an internal header to make it available\n> > from other internal components in preperation for reducing the surface\n> > area of the public interface of the Camera Manager.\n> \n> s/Camera Manager/CameraManager/\n> \n> > Signed-off-by: Kieran Bingham \n> > Reviewed-by: Jacopo Mondi \n> > ---\n> > include/libcamera/internal/camera_manager.h | 64 +++++++++++++++++++++\n> > include/libcamera/internal/meson.build | 1 +\n> > src/libcamera/camera_manager.cpp | 50 ++--------------\n> > 3 files changed, 69 insertions(+), 46 deletions(-)\n> > create mode 100644 include/libcamera/internal/camera_manager.h\n> > \n> > diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n> > new file mode 100644\n> > index 000000000000..05a1e4df8add\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/camera_manager.h\n> > @@ -0,0 +1,64 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2023, Ideas on Board Oy.\n> > + *\n> > + * camera_manager.h - Camera manager private data\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> \n> #include \n> \n> for std::shared_ptr, std::unique_ptr and std::weak_ptr, and\n> \n> #include \n> \n> for std::vector.\n\nAlso #include for dev_t;\n\n> \n> > +\n> \n> #include \n> \n> for Extensible::Private.\n> \n> > +#include \n> > +#include \n> > +\n> > +#include \n> \n> Should this go first to improve the self-contained header test coverage\n> ?\n\nclang-format really doesn't like that ;-) I don't think 'headers'\nincluding a component header have any concept in the include handling\nthere.\n\nBut as the core camera_manager.cpp now includes this file first, indeed\nI think it should come first to complete the chain.\n\n> \n> > +\n> > +#include \"libcamera/internal/ipa_manager.h\"\n> > +#include \"libcamera/internal/process.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> \n> class Camera;\n> \n> > +class DeviceEnumerator;\n> > +\n> > +class CameraManager::Private : public Extensible::Private, public Thread\n> > +{\n> > + LIBCAMERA_DECLARE_PUBLIC(CameraManager)\n> > +\n> > +public:\n> > + Private();\n> > +\n> > + int start();\n> > + void addCamera(std::shared_ptr camera,\n> > + const std::vector &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > + void removeCamera(Camera *camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > +\n> > + /*\n> > + * This mutex protects\n> > + *\n> > + * - initialized_ and status_ during initialization\n> > + * - cameras_ and camerasByDevnum_ after initialization\n> > + */\n> > + mutable Mutex mutex_;\n> > + std::vector> cameras_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n> > + std::map> camerasByDevnum_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n> > +\n> > +protected:\n> > + void run() override;\n> > +\n> > +private:\n> > + int init();\n> > + void createPipelineHandlers();\n> > + void cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > +\n> > + ConditionVariable cv_;\n> > + bool initialized_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n> > + int status_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n> > +\n> > + std::unique_ptr enumerator_;\n> > +\n> > + IPAManager ipaManager_;\n> > + ProcessManager processManager_;\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> > index d75088059996..0028ed0dc27f 100644\n> > --- a/include/libcamera/internal/meson.build\n> > +++ b/include/libcamera/internal/meson.build\n> > @@ -13,6 +13,7 @@ libcamera_internal_headers = files([\n> > 'bayer_format.h',\n> > 'byte_stream_buffer.h',\n> > 'camera.h',\n> > + 'camera_manager.h',\n> \n> Alphabetical order.\n> \n> > 'camera_controls.h',\n> > 'camera_lens.h',\n> > 'camera_sensor.h',\n> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > index c1edefdad160..d3c297b888d8 100644\n> > --- a/src/libcamera/camera_manager.cpp\n> > +++ b/src/libcamera/camera_manager.cpp\n> > @@ -5,24 +5,22 @@\n> > * camera_manager.h - Camera management\n> > */\n> > \n> > -#include \n> > +#include \"libcamera/internal/camera_manager.h\"\n> > \n> > #include \n> \n> You can drop this.\n> \n> > \n> > -#include \n> > -\n> > #include \n> > #include \n> > #include \n> \n> And mutex.h and thread.h too.\n> \n> > #include \n> > \n> > +#include \n> > +\n> > #include \"libcamera/internal/device_enumerator.h\"\n> > -#include \"libcamera/internal/ipa_manager.h\"\n> > #include \"libcamera/internal/pipeline_handler.h\"\n> > -#include \"libcamera/internal/process.h\"\n> > \n> > /**\n> > - * \\file camera_manager.h\n> > + * \\file libcamera/camera_manager.h\n> \n> I think you need both, see framebuffer.cpp.\n\nAh, yes that's what I was missing.\n\n> \n> > * \\brief The camera manager\n> > */\n> > \n> > @@ -33,46 +31,6 @@ namespace libcamera {\n> > \n> > LOG_DEFINE_CATEGORY(Camera)\n> > \n> > -class CameraManager::Private : public Extensible::Private, public Thread\n> > -{\n> > - LIBCAMERA_DECLARE_PUBLIC(CameraManager)\n> > -\n> > -public:\n> > - Private();\n> > -\n> > - int start();\n> > - void addCamera(std::shared_ptr camera,\n> > - const std::vector &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > - void removeCamera(Camera *camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > -\n> > - /*\n> > - * This mutex protects\n> > - *\n> > - * - initialized_ and status_ during initialization\n> > - * - cameras_ and camerasByDevnum_ after initialization\n> > - */\n> > - mutable Mutex mutex_;\n> > - std::vector> cameras_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n> > - std::map> camerasByDevnum_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n> > -\n> > -protected:\n> > - void run() override;\n> > -\n> > -private:\n> > - int init();\n> > - void createPipelineHandlers();\n> > - void cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > -\n> > - ConditionVariable cv_;\n> > - bool initialized_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n> > - int status_ LIBCAMERA_TSA_GUARDED_BY(mutex_);\n> > -\n> > - std::unique_ptr enumerator_;\n> > -\n> > - IPAManager ipaManager_;\n> > - ProcessManager processManager_;\n> > -};\n> > -\n> > CameraManager::Private::Private()\n> > : initialized_(false)\n> > {\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 1DE9BC3200\n\tfor ;\n\tWed, 7 Jun 2023 13:45:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 600B762894;\n\tWed, 7 Jun 2023 15:45:54 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5CACB60579\n\tfor ;\n\tWed, 7 Jun 2023 15:45:52 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EB72F2B6;\n\tWed, 7 Jun 2023 15:45:25 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686145554;\n\tbh=Ovhph+w9fKlbQH1CZ3GMAPfeuYnu7EhaI77oUbE1/bA=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=CzOGOO9OY2oYVdww332GhbH6fUdQ7QOxUIziNJUbuJyOPw8fAelUii/4SWa8uDhvP\n\tfthBXI9l8HRZlGhdk4IRwm63VnYKN+fEy0xPdl5U8nysv8rwDw9eirmjhOJyafLnhI\n\tM8ybA24ZdTgNOHdWYEX8ePU6GwxDDEPYz/CZo5aa38lVqlkXqXVneKGA9pzjWHv5ed\n\tRjxXjeEBDkRV2p5rl3DZQbARvmYO2nJ42DUPJj0MueLTqlW1gJwgaCwtPzzaPm1yaF\n\tf1n2lgU1xK4d5AWEspX3lfLsgZrGH2SWyUCtW4VzNk7GGCpzsWIpnGyUsFsRptukQK\n\tLQs7Tly3FPjsg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686145526;\n\tbh=Ovhph+w9fKlbQH1CZ3GMAPfeuYnu7EhaI77oUbE1/bA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=la71s5veEvIk20WpbPphQ/d26zm+uCiXGXFZbHROkUxFWNI0RDmplZZ9AtOt5PYxO\n\taYwKc9G9UQj6NVi+WaQlLhPn+rX/lVlnzUGfPE2cf0y2s+JO3pWUJsSTX1mc3t+ZG1\n\t4fMaZLe2s95AErSqho2aMAVm0WU7ktxlyacvnkfs="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"la71s5ve\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230606160535.GD25679@pendragon.ideasonboard.com>","References":"<20230515124550.3601128-1-kieran.bingham@ideasonboard.com>\n\t<20230515124550.3601128-3-kieran.bingham@ideasonboard.com>\n\t<20230606160535.GD25679@pendragon.ideasonboard.com>","To":"Laurent Pinchart ","Date":"Wed, 07 Jun 2023 14:45:49 +0100","Message-ID":"<168614554930.2889415.11370787098019306570@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 2/5] libcamera: camera_manager:\n\tMove private implementation to internal","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":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"Jacopo Mondi ,\n\tlibcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]