[{"id":12612,"web_url":"https://patchwork.libcamera.org/comment/12612/","msgid":"<20200921092439.GK2692107@oden.dyn.berto.se>","date":"2020-09-21T09:24:39","subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: camera_manager:\n\tInherit from Extensible","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2020-09-21 06:10:56 +0300, Laurent Pinchart wrote:\n> Use the d-pointer infrastructure offered by the Extensible class to\n> replace the custom implementation.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> include/libcamera/camera_manager.h | 7 ++--\n> src/libcamera/camera_manager.cpp | 55 +++++++++++++++++++-----------\n> 2 files changed, 38 insertions(+), 24 deletions(-)\n> \n> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\n> index 9eb2b6f5a5f5..6d5341c76412 100644\n> --- a/include/libcamera/camera_manager.h\n> +++ b/include/libcamera/camera_manager.h\n> @@ -12,6 +12,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> #include \n> \n> @@ -20,8 +21,9 @@ namespace libcamera {\n> class Camera;\n> class EventDispatcher;\n> \n> -class CameraManager : public Object\n> +class CameraManager : public Object, public Extensible\n> {\n> +\tLIBCAMERA_DECLARE_PRIVATE(CameraManager)\n> public:\n> \tCameraManager();\n> \tCameraManager(const CameraManager &) = delete;\n> @@ -50,9 +52,6 @@ public:\n> private:\n> \tstatic const std::string version_;\n> \tstatic CameraManager *self_;\n> -\n> -\tclass Private;\n> -\tstd::unique_ptr p_;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index 780a66a7ac10..77b49709cd96 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -30,8 +30,10 @@ namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(Camera)\n> \n> -class CameraManager::Private : public Thread\n> +class CameraManager::Private : public Extensible::Private, public Thread\n> {\n> +\tLIBCAMERA_DECLARE_PUBLIC(CameraManager)\n> +\n> public:\n> \tPrivate(CameraManager *cm);\n> \n> @@ -58,8 +60,6 @@ private:\n> \tvoid createPipelineHandlers();\n> \tvoid cleanup();\n> \n> -\tCameraManager *cm_;\n> -\n> \tstd::condition_variable cv_;\n> \tbool initialized_;\n> \tint status_;\n> @@ -70,7 +70,7 @@ private:\n> };\n> \n> CameraManager::Private::Private(CameraManager *cm)\n> -\t: cm_(cm), initialized_(false)\n> +\t: Extensible::Private(cm), initialized_(false)\n> {\n> }\n> \n> @@ -131,6 +131,8 @@ int CameraManager::Private::init()\n> \n> void CameraManager::Private::createPipelineHandlers()\n> {\n> +\tLIBCAMERA_O_PTR(CameraManager);\n> +\n> \t/*\n> \t * \\todo Try to read handlers and order from configuration\n> \t * file and only fallback on all handlers if there is no\n> @@ -145,7 +147,7 @@ void CameraManager::Private::createPipelineHandlers()\n> \t\t * all pipelines it can provide.\n> \t\t */\n> \t\twhile (1) {\n> -\t\t\tstd::shared_ptr pipe = factory->create(cm_);\n> +\t\t\tstd::shared_ptr pipe = factory->create(o);\n> \t\t\tif (!pipe->match(enumerator_.get()))\n> \t\t\t\tbreak;\n> \n> @@ -256,7 +258,7 @@ void CameraManager::Private::removeCamera(Camera *camera)\n> CameraManager *CameraManager::self_ = nullptr;\n> \n> CameraManager::CameraManager()\n> -\t: p_(new CameraManager::Private(this))\n> +\t: Extensible(new CameraManager::Private(this))\n> {\n> \tif (self_)\n> \t\tLOG(Camera, Fatal)\n> @@ -284,9 +286,11 @@ CameraManager::~CameraManager()\n> */\n> int CameraManager::start()\n> {\n> +\tLIBCAMERA_D_PTR(CameraManager);\n> +\n> \tLOG(Camera, Info) << \"libcamera \" << version_;\n> \n> -\tint ret = p_->start();\n> +\tint ret = d->start();\n> \tif (ret)\n> \t\tLOG(Camera, Error) << \"Failed to start camera manager: \"\n> \t\t\t\t << strerror(-ret);\n> @@ -306,8 +310,9 @@ int CameraManager::start()\n> */\n> void CameraManager::stop()\n> {\n> -\tp_->exit();\n> -\tp_->wait();\n> +\tLIBCAMERA_D_PTR(CameraManager);\n> +\td->exit();\n> +\td->wait();\n> }\n> \n> /**\n> @@ -323,9 +328,11 @@ void CameraManager::stop()\n> */\n> std::vector> CameraManager::cameras() const\n> {\n> -\tMutexLocker locker(p_->mutex_);\n> +\tLIBCAMERA_D_PTR(CameraManager);\n> \n> -\treturn p_->cameras_;\n> +\tMutexLocker locker(d->mutex_);\n> +\n> +\treturn d->cameras_;\n> }\n> \n> /**\n> @@ -341,9 +348,11 @@ std::vector> CameraManager::cameras() const\n> */\n> std::shared_ptr CameraManager::get(const std::string &id)\n> {\n> -\tMutexLocker locker(p_->mutex_);\n> +\tLIBCAMERA_D_PTR(CameraManager);\n> \n> -\tfor (std::shared_ptr camera : p_->cameras_) {\n> +\tMutexLocker locker(d->mutex_);\n> +\n> +\tfor (std::shared_ptr camera : d->cameras_) {\n> \t\tif (camera->id() == id)\n> \t\t\treturn camera;\n> \t}\n> @@ -369,10 +378,12 @@ std::shared_ptr CameraManager::get(const std::string &id)\n> */\n> std::shared_ptr CameraManager::get(dev_t devnum)\n> {\n> -\tMutexLocker locker(p_->mutex_);\n> +\tLIBCAMERA_D_PTR(CameraManager);\n> \n> -\tauto iter = p_->camerasByDevnum_.find(devnum);\n> -\tif (iter == p_->camerasByDevnum_.end())\n> +\tMutexLocker locker(d->mutex_);\n> +\n> +\tauto iter = d->camerasByDevnum_.find(devnum);\n> +\tif (iter == d->camerasByDevnum_.end())\n> \t\treturn nullptr;\n> \n> \treturn iter->second.lock();\n> @@ -423,9 +434,11 @@ std::shared_ptr CameraManager::get(dev_t devnum)\n> void CameraManager::addCamera(std::shared_ptr camera,\n> \t\t\t const std::vector &devnums)\n> {\n> -\tASSERT(Thread::current() == p_.get());\n> +\tLIBCAMERA_D_PTR(CameraManager);\n> \n> -\tp_->addCamera(camera, devnums);\n> +\tASSERT(Thread::current() == d);\n> +\n> +\td->addCamera(camera, devnums);\n> \tcameraAdded.emit(camera);\n> }\n> \n> @@ -441,9 +454,11 @@ void CameraManager::addCamera(std::shared_ptr camera,\n> */\n> void CameraManager::removeCamera(std::shared_ptr camera)\n> {\n> -\tASSERT(Thread::current() == p_.get());\n> +\tLIBCAMERA_D_PTR(CameraManager);\n> \n> -\tp_->removeCamera(camera.get());\n> +\tASSERT(Thread::current() == d);\n> +\n> +\td->removeCamera(camera.get());\n> \tcameraRemoved.emit(camera);\n> }\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 164F0C3B5B\n\tfor ;\n\tMon, 21 Sep 2020 09:24:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A49BC62FAD;\n\tMon, 21 Sep 2020 11:24:41 +0200 (CEST)","from mail-lj1-x236.google.com (mail-lj1-x236.google.com\n\t[IPv6:2a00:1450:4864:20::236])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F3EC60367\n\tfor ;\n\tMon, 21 Sep 2020 11:24:40 +0200 (CEST)","by mail-lj1-x236.google.com with SMTP id b19so10454653lji.11\n\tfor ;\n\tMon, 21 Sep 2020 02:24:40 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tw4sm2403902lfq.75.2020.09.21.02.24.39\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 21 Sep 2020 02:24:39 -0700 (PDT)"],"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=\"xs3yoXeU\"; 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=8hxXjTGwT3YOVh2m7shaZbBZkU8FxCVQZx//AVmfKU8=;\n\tb=xs3yoXeURldJYajjqAJOfxEDG9TfKRVZ8cvyRJR3tYuMcS0hfPshgZbrRZLCR7XMmY\n\tRIk15DCZi1VxMh6Gkg9ELs2NZsdga5j4beGiugQq5miahkkWUe/YjhVo7sLx3+0VYAEn\n\t+sP3penzKs/lNQ0AIDCJDcZMkFGsrEFK3uisGU68tZujT+HiDzMqM+l0XpzX+odmpsr9\n\tsWjOlsSr8/T1Cu6a9nK3GVYnLz+z9t60rNbIVFckWjuzI4u8w2AmPZ34WFO/6Rr5fgCb\n\tyBn/1f3hrrC7raKP1pL1H7LwWLxstXQaUMAfeG8C9+DeK9nSGvVKns00X87hHVFH0D0W\n\t1hkA==","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=8hxXjTGwT3YOVh2m7shaZbBZkU8FxCVQZx//AVmfKU8=;\n\tb=dii/7sMJcCZ0thvMGP5zVu2mfUeeyfB9+uwIqylMd7gHBshqeyFZQVEBBG8amZLnRr\n\tZPqvAvhlFRZ/9d3vsB7gVnUR0lp+OinsXoktyqBWJzbNe7tJO8kht8M94p8xXsKNf2pZ\n\tN1ALBjuRwsQXCxWD/47vVuYvnSoPTYe3blra6z2hEmAnD34LHUjKE73lTnPBZZqaErGv\n\t4gHOp+11UNh2dnYQ4TVVGmaJe+gbMU4fYrZToIVrGGoO4e8dZfL0PiRS+QGG6l8LZwQI\n\tDequoDNBExH0HegFg6JaqMGP7omreA1SEfqUaMnv7R7KI2dyZLHFOvv5GAcfhfd3tYCM\n\trl2A==","X-Gm-Message-State":"AOAM5314YrjrXZ+A95QHcXu4J1s/Htg0LHOlHbT0Rs3ITeD3iJETCLUD\n\tuXsvZtuX36uSUOq83JBi3LP9XTM18l00aw==","X-Google-Smtp-Source":"ABdhPJx7kNpN8AhnxcOpivYGgE9zwZaUnbWce5e7qFNvtOrA0kxWioYFihv+NtN9mbouXaKikIDfqA==","X-Received":"by 2002:a2e:9655:: with SMTP id\n\tz21mr15958995ljh.410.1600680279951; \n\tMon, 21 Sep 2020 02:24:39 -0700 (PDT)","Date":"Mon, 21 Sep 2020 11:24:39 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"<20200921092439.GK2692107@oden.dyn.berto.se>","References":"<20200921031057.3564-1-laurent.pinchart@ideasonboard.com>\n\t<20200921031057.3564-4-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200921031057.3564-4-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: camera_manager:\n\tInherit from Extensible","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","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]