[{"id":3575,"web_url":"https://patchwork.libcamera.org/comment/3575/","msgid":"<20200122161319.GU1124294@oden.dyn.berto.se>","date":"2020-01-22T16:13:19","subject":"Re: [libcamera-devel] [PATCH 13/19] libcamera: camera_manager: Run\n\tthe camera manager in a thread","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-01-20 02:24:31 +0200, Laurent Pinchart wrote:\n> Relying on the application event loop to process all our internal events\n> is a bad idea for multiple reasons. In many cases the user of libcamera\n> can't provide an event loop, for instance when running through one of\n> the adaptation layers. The Android camera HAL and V4L2 compatibility\n> layer create a thread for this reason, and the GStreamer element would\n> need to do so as well. Furthermore, relying on the application event\n> loop pushes libcamera's realtime constraints to the application, which\n> isn't manageable.\n> \n> For these reasons it's desirable to always run the camera manager, the\n> pipeline handlers and the cameras in a separate thread. Doing so isn't\n> too complicated, it only involves creating the thread internally when\n> starting the camera manager, and synchronizing a few methods of the\n> Camera class. Do so as a first step towards defining the threading model\n> of libcamera.\n> \n> Making CameraManager::cameras() thread-safe requires returning a copy of\n> the cameras vector instead of a reference. This is also required for\n> hot-plugging support and is thus desirable.\n> \n> The event dispatcher interface is still exposed to applications, to\n> enable cross-thread signal delivery if desired.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nThis patch is quiet hard to review as it do multiple things, would it be \npossible to split it a bit? I think it can be split into\n\n- Change the behavior of CameraManager::cameras()\n- Breakout of CameraManager::Private\n- New thread related code.\n\nSmall comments bellow tho.\n\n> ---\n>  Documentation/Doxyfile.in          |   1 +\n>  include/libcamera/camera_manager.h |  13 +-\n>  src/libcamera/camera_manager.cpp   | 295 +++++++++++++++++++++--------\n>  3 files changed, 224 insertions(+), 85 deletions(-)\n> \n> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> index 734672ed15dc..1c46b04b3f7e 100644\n> --- a/Documentation/Doxyfile.in\n> +++ b/Documentation/Doxyfile.in\n> @@ -879,6 +879,7 @@ EXCLUDE_SYMBOLS        = libcamera::BoundMethodArgs \\\n>                           libcamera::BoundMethodPackBase \\\n>                           libcamera::BoundMethodStatic \\\n>                           libcamera::SignalBase \\\n> +                         libcamera::*::Private \\\n>                           std::*\n>  \n>  # The EXAMPLE_PATH tag can be used to specify one or more files or directories\n> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\n> index 094197668698..079f848aec79 100644\n> --- a/include/libcamera/camera_manager.h\n> +++ b/include/libcamera/camera_manager.h\n> @@ -7,7 +7,6 @@\n>  #ifndef __LIBCAMERA_CAMERA_MANAGER_H__\n>  #define __LIBCAMERA_CAMERA_MANAGER_H__\n>  \n> -#include <map>\n>  #include <memory>\n>  #include <string>\n>  #include <sys/types.h>\n> @@ -18,9 +17,7 @@\n>  namespace libcamera {\n>  \n>  class Camera;\n> -class DeviceEnumerator;\n>  class EventDispatcher;\n> -class PipelineHandler;\n>  \n>  class CameraManager : public Object\n>  {\n> @@ -33,7 +30,7 @@ public:\n>  \tint start();\n>  \tvoid stop();\n>  \n> -\tconst std::vector<std::shared_ptr<Camera>> &cameras() const { return cameras_; }\n> +\tstd::vector<std::shared_ptr<Camera>> cameras() const;\n>  \tstd::shared_ptr<Camera> get(const std::string &name);\n>  \tstd::shared_ptr<Camera> get(dev_t devnum);\n>  \n> @@ -46,13 +43,11 @@ public:\n>  \tEventDispatcher *eventDispatcher();\n>  \n>  private:\n> -\tstd::unique_ptr<DeviceEnumerator> enumerator_;\n> -\tstd::vector<std::shared_ptr<PipelineHandler>> pipes_;\n> -\tstd::vector<std::shared_ptr<Camera>> cameras_;\n> -\tstd::map<dev_t, std::weak_ptr<Camera>> camerasByDevnum_;\n> -\n>  \tstatic const std::string version_;\n>  \tstatic CameraManager *self_;\n> +\n> +\tclass Private;\n> +\tstd::unique_ptr<Private> p_;\n>  };\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index a71caf8536bb..64aa89975701 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -7,6 +7,9 @@\n>  \n>  #include <libcamera/camera_manager.h>\n>  \n> +#include <condition_variable>\n> +#include <map>\n> +\n>  #include <libcamera/camera.h>\n>  #include <libcamera/event_dispatcher.h>\n>  \n> @@ -26,6 +29,184 @@ namespace libcamera {\n>  \n>  LOG_DEFINE_CATEGORY(Camera)\n>  \n> +class CameraManager::Private : public Thread\n> +{\n> +public:\n> +\tPrivate(CameraManager *cm);\n> +\n> +\tint start();\n> +\tvoid addCamera(std::shared_ptr<Camera> &camera, dev_t devnum);\n> +\tvoid removeCamera(Camera *camera);\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> +\tMutex mutex_;\n> +\tstd::vector<std::shared_ptr<Camera>> cameras_;\n> +\tstd::map<dev_t, std::weak_ptr<Camera>> camerasByDevnum_;\n> +\n> +protected:\n> +\tvoid run() override;\n> +\n> +private:\n> +\tint init();\n> +\tvoid cleanup();\n> +\n> +\tCameraManager *cm_;\n> +\n> +\tstd::condition_variable cv_;\n> +\tbool initialized_;\n> +\tint status_;\n> +\n> +\tstd::vector<std::shared_ptr<PipelineHandler>> pipes_;\n> +\tstd::unique_ptr<DeviceEnumerator> enumerator_;\n> +};\n> +\n> +CameraManager::Private::Private(CameraManager *cm)\n> +\t: cm_(cm), initialized_(false)\n> +{\n> +}\n> +\n> +int CameraManager::Private::start()\n> +{\n> +\t/* Start the thread and wait for initialization to complete. */\n> +\tThread::start();\n> +\n> +\t{\n> +\t\tMutexLocker locker(mutex_);\n> +\t\tcv_.wait(locker, [&] { return initialized_; });\n> +\t}\n\nFor my education, is this in its a own block so that locker will go out \nof scope and free the locker after the wait returns?\n\n> +\n> +\t/* If a failure happened during initialization, stop the thread. */\n> +\tif (status_ < 0) {\n> +\t\texit();\n> +\t\twait();\n> +\t\treturn status_;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +void CameraManager::Private::addCamera(std::shared_ptr<Camera> &camera,\n> +\t\t\t\t       dev_t devnum)\n> +{\n> +\tMutexLocker locker(mutex_);\n> +\n> +\tfor (std::shared_ptr<Camera> c : cameras_) {\n> +\t\tif (c->name() == camera->name()) {\n> +\t\t\tLOG(Camera, Warning)\n> +\t\t\t\t<< \"Registering camera with duplicate name '\"\n> +\t\t\t\t<< camera->name() << \"'\";\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +\n> +\tcameras_.push_back(std::move(camera));\n> +\n> +\tif (devnum) {\n> +\t\tunsigned int index = cameras_.size() - 1;\n> +\t\tcamerasByDevnum_[devnum] = cameras_[index];\n> +\t}\n> +}\n> +\n> +void CameraManager::Private::removeCamera(Camera *camera)\n> +{\n> +\tMutexLocker locker(mutex_);\n> +\n> +\tauto iter = std::find_if(cameras_.begin(), cameras_.end(),\n> +\t\t\t\t [camera](std::shared_ptr<Camera> &c) {\n> +\t\t\t\t\t return c.get() == camera;\n> +\t\t\t\t });\n> +\tif (iter == cameras_.end())\n> +\t\treturn;\n> +\n> +\tLOG(Camera, Debug)\n> +\t\t<< \"Unregistering camera '\" << camera->name() << \"'\";\n> +\n> +\tauto iter_d = std::find_if(camerasByDevnum_.begin(), camerasByDevnum_.end(),\n> +\t\t\t\t   [camera](const std::pair<dev_t, std::weak_ptr<Camera>> &p) {\n> +\t\t\t\t\t   return p.second.lock().get() == camera;\n> +\t\t\t\t   });\n> +\tif (iter_d != camerasByDevnum_.end())\n> +\t\tcamerasByDevnum_.erase(iter_d);\n> +\n> +\tcameras_.erase(iter);\n> +}\n> +\n> +void CameraManager::Private::run()\n> +{\n> +\tLOG(Camera, Debug) << \"Starting camera manager\";\n> +\n> +\tint ret = init();\n> +\n> +\tmutex_.lock();\n> +\tstatus_ = ret;\n> +\tinitialized_ = true;\n> +\tmutex_.unlock();\n> +\tcv_.notify_one();\n> +\n> +\tif (ret < 0)\n> +\t\treturn;\n> +\n> +\t/* Now start processing events and messages. */\n> +\texec();\n> +\n> +\tcleanup();\n> +}\n> +\n> +int CameraManager::Private::init()\n> +{\n> +\tenumerator_ = DeviceEnumerator::create();\n> +\tif (!enumerator_ || enumerator_->enumerate())\n> +\t\treturn -ENODEV;\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> +\t * configuration file.\n> +\t */\n> +\tstd::vector<PipelineHandlerFactory *> &factories = PipelineHandlerFactory::factories();\n> +\n> +\tfor (PipelineHandlerFactory *factory : factories) {\n> +\t\t/*\n> +\t\t * Try each pipeline handler until it exhaust\n> +\t\t * all pipelines it can provide.\n> +\t\t */\n> +\t\twhile (1) {\n> +\t\t\tstd::shared_ptr<PipelineHandler> pipe = factory->create(cm_);\n> +\t\t\tif (!pipe->match(enumerator_.get()))\n> +\t\t\t\tbreak;\n> +\n> +\t\t\tLOG(Camera, Debug)\n> +\t\t\t\t<< \"Pipeline handler \\\"\" << factory->name()\n> +\t\t\t\t<< \"\\\" matched\";\n> +\t\t\tpipes_.push_back(std::move(pipe));\n> +\t\t}\n> +\t}\n> +\n> +\t/* TODO: register hot-plug callback here */\n> +\n> +\treturn 0;\n> +}\n> +\n> +void CameraManager::Private::cleanup()\n> +{\n> +\t/* TODO: unregister hot-plug callback here */\n> +\n> +\t/*\n> +\t * Release all references to cameras and pipeline handlers to ensure\n> +\t * they all get destroyed before the device enumerator deletes the\n> +\t * media devices.\n> +\t */\n> +\tpipes_.clear();\n> +\tcameras_.clear();\n> +\n> +\tenumerator_.reset(nullptr);\n> +}\n> +\n>  /**\n>   * \\class CameraManager\n>   * \\brief Provide access and manage all cameras in the system\n> @@ -62,7 +243,7 @@ LOG_DEFINE_CATEGORY(Camera)\n>  CameraManager *CameraManager::self_ = nullptr;\n>  \n>  CameraManager::CameraManager()\n> -\t: enumerator_(nullptr)\n> +\t: p_(new CameraManager::Private(this))\n>  {\n>  \tif (self_)\n>  \t\tLOG(Camera, Fatal)\n> @@ -73,6 +254,8 @@ CameraManager::CameraManager()\n>  \n>  CameraManager::~CameraManager()\n>  {\n> +\tstop();\n> +\n>  \tself_ = nullptr;\n>  }\n>  \n> @@ -88,41 +271,16 @@ CameraManager::~CameraManager()\n>   */\n>  int CameraManager::start()\n>  {\n> -\tif (enumerator_)\n> -\t\treturn -EBUSY;\n> -\n>  \tLOG(Camera, Info) << \"libcamera \" << version_;\n>  \n> -\tenumerator_ = DeviceEnumerator::create();\n> -\tif (!enumerator_ || enumerator_->enumerate())\n> -\t\treturn -ENODEV;\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> -\t * configuration file.\n> -\t */\n> -\tstd::vector<PipelineHandlerFactory *> &factories = PipelineHandlerFactory::factories();\n> +\tint ret = p_->start();\n> +\tif (ret < 0) {\n> +\t\tLOG(Camera, Error) << \"Failed to start camera manager: \"\n> +\t\t\t\t   << strerror(-ret);\n>  \n> -\tfor (PipelineHandlerFactory *factory : factories) {\n> -\t\t/*\n> -\t\t * Try each pipeline handler until it exhaust\n> -\t\t * all pipelines it can provide.\n> -\t\t */\n> -\t\twhile (1) {\n> -\t\t\tstd::shared_ptr<PipelineHandler> pipe = factory->create(this);\n> -\t\t\tif (!pipe->match(enumerator_.get()))\n> -\t\t\t\tbreak;\n> -\n> -\t\t\tLOG(Camera, Debug)\n> -\t\t\t\t<< \"Pipeline handler \\\"\" << factory->name()\n> -\t\t\t\t<< \"\\\" matched\";\n> -\t\t\tpipes_.push_back(std::move(pipe));\n> -\t\t}\n> +\t\treturn ret;\n>  \t}\n>  \n> -\t/* TODO: register hot-plug callback here */\n> -\n>  \treturn 0;\n\nI think you can simplify this to\n\n        int ret = p_->start();\n        if (ret)\n\t\tLOG(Camera, Error) << \"Failed to start camera manager: \"\n\t\t\t\t   << strerror(-ret);\n\n        return ret;\n\n\nas start() shall return 0 on success.\n\n>  }\n>  \n> @@ -138,17 +296,8 @@ int CameraManager::start()\n>   */\n>  void CameraManager::stop()\n>  {\n> -\t/* TODO: unregister hot-plug callback here */\n> -\n> -\t/*\n> -\t * Release all references to cameras and pipeline handlers to ensure\n> -\t * they all get destroyed before the device enumerator deletes the\n> -\t * media devices.\n> -\t */\n> -\tpipes_.clear();\n> -\tcameras_.clear();\n> -\n> -\tenumerator_.reset(nullptr);\n> +\tp_->exit();\n> +\tp_->wait();\n>  }\n>  \n>  /**\n> @@ -158,8 +307,16 @@ void CameraManager::stop()\n>   * Before calling this function the caller is responsible for ensuring that\n>   * the camera manager is running.\n>   *\n> + * \\context This function is \\threadsafe.\n> + *\n>   * \\return List of all available cameras\n>   */\n> +std::vector<std::shared_ptr<Camera>> CameraManager::cameras() const\n> +{\n> +\tMutexLocker locker(p_->mutex_);\n> +\n> +\treturn p_->cameras_;\n> +}\n>  \n>  /**\n>   * \\brief Get a camera based on name\n> @@ -168,11 +325,15 @@ void CameraManager::stop()\n>   * Before calling this function the caller is responsible for ensuring that\n>   * the camera manager is running.\n>   *\n> + * \\context This function is \\threadsafe.\n> + *\n>   * \\return Shared pointer to Camera object or nullptr if camera not found\n>   */\n>  std::shared_ptr<Camera> CameraManager::get(const std::string &name)\n>  {\n> -\tfor (std::shared_ptr<Camera> camera : cameras_) {\n> +\tMutexLocker locker(p_->mutex_);\n> +\n> +\tfor (std::shared_ptr<Camera> camera : p_->cameras_) {\n>  \t\tif (camera->name() == name)\n>  \t\t\treturn camera;\n>  \t}\n> @@ -191,13 +352,17 @@ std::shared_ptr<Camera> CameraManager::get(const std::string &name)\n>   * Before calling this function the caller is responsible for ensuring that\n>   * the camera manager is running.\n>   *\n> + * \\context This function is \\threadsafe.\n> + *\n>   * \\return Shared pointer to Camera object, which is empty if the camera is\n>   * not found\n>   */\n>  std::shared_ptr<Camera> CameraManager::get(dev_t devnum)\n>  {\n> -\tauto iter = camerasByDevnum_.find(devnum);\n> -\tif (iter == camerasByDevnum_.end())\n> +\tMutexLocker locker(p_->mutex_);\n> +\n> +\tauto iter = p_->camerasByDevnum_.find(devnum);\n> +\tif (iter == p_->camerasByDevnum_.end())\n>  \t\treturn nullptr;\n>  \n>  \treturn iter->second.lock();\n> @@ -214,24 +379,14 @@ std::shared_ptr<Camera> CameraManager::get(dev_t devnum)\n>   *\n>   * \\a devnum is used by the V4L2 compatibility layer to map V4L2 device nodes\n>   * to Camera instances.\n> + *\n> + * \\context This function shall be called from the CameraManager thread.\n>   */\n>  void CameraManager::addCamera(std::shared_ptr<Camera> camera, dev_t devnum)\n>  {\n> -\tfor (std::shared_ptr<Camera> c : cameras_) {\n> -\t\tif (c->name() == camera->name()) {\n> -\t\t\tLOG(Camera, Warning)\n> -\t\t\t\t<< \"Registering camera with duplicate name '\"\n> -\t\t\t\t<< camera->name() << \"'\";\n> -\t\t\tbreak;\n> -\t\t}\n> -\t}\n> -\n> -\tcameras_.push_back(std::move(camera));\n> +\tASSERT(Thread::current() == p_.get());\n>  \n> -\tif (devnum) {\n> -\t\tunsigned int index = cameras_.size() - 1;\n> -\t\tcamerasByDevnum_[devnum] = cameras_[index];\n> -\t}\n> +\tp_->addCamera(camera, devnum);\n>  }\n>  \n>  /**\n> @@ -241,32 +396,20 @@ void CameraManager::addCamera(std::shared_ptr<Camera> camera, dev_t devnum)\n>   * This function is called by pipeline handlers to unregister cameras from the\n>   * camera manager. Unregistered cameras won't be reported anymore by the\n>   * cameras() and get() calls, but references may still exist in applications.\n> + *\n> + * \\context This function shall be called from the CameraManager thread.\n>   */\n>  void CameraManager::removeCamera(Camera *camera)\n>  {\n> -\tauto iter = std::find_if(cameras_.begin(), cameras_.end(),\n> -\t\t\t\t [camera](std::shared_ptr<Camera> &c) {\n> -\t\t\t\t\t return c.get() == camera;\n> -\t\t\t\t });\n> -\tif (iter == cameras_.end())\n> -\t\treturn;\n> -\n> -\tLOG(Camera, Debug)\n> -\t\t<< \"Unregistering camera '\" << camera->name() << \"'\";\n> -\n> -\tauto iter_d = std::find_if(camerasByDevnum_.begin(), camerasByDevnum_.end(),\n> -\t\t\t\t   [camera](const std::pair<dev_t, std::weak_ptr<Camera>> &p) {\n> -\t\t\t\t\t   return p.second.lock().get() == camera;\n> -\t\t\t\t   });\n> -\tif (iter_d != camerasByDevnum_.end())\n> -\t\tcamerasByDevnum_.erase(iter_d);\n> +\tASSERT(Thread::current() == p_.get());\n>  \n> -\tcameras_.erase(iter);\n> +\tp_->removeCamera(camera);\n>  }\n>  \n>  /**\n>   * \\fn const std::string &CameraManager::version()\n>   * \\brief Retrieve the libcamera version string\n> + * \\context This function is \\a threadsafe.\n>   * \\return The libcamera version string\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":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lf1-x134.google.com (mail-lf1-x134.google.com\n\t[IPv6:2a00:1450:4864:20::134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F29F260804\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 22 Jan 2020 17:13:21 +0100 (CET)","by mail-lf1-x134.google.com with SMTP id i23so35252lfo.7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 22 Jan 2020 08:13:21 -0800 (PST)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\tl21sm21037344lfh.74.2020.01.22.08.13.19\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 22 Jan 2020 08:13:20 -0800 (PST)"],"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=8Oex6siIc77/YaOvCHd3xlJnY2EqTvrTxVxUm933bCU=;\n\tb=Vk86xg+yn3qFm0MqTMFYNlR4KoBJU6OaRiZI1U7joBz9XsvrLW0rSDb7Fj64Gq5zUG\n\taupdu6kptqkq5eNZ9RMiC6rqz34aImCHVQnyfNRhGJjsEPrPc/zESQF9lNLc21y9PK5z\n\tlFHePLbnTSsXaSGoI9ecOSzgTJcTxE1gRy5tQ24fzQj4qyiuRz0CKeoOty+S/TvHkeZd\n\tAxafdsW7fPKe6Cd3oSE8LcWVhbnjp14azTkHYb5A5Rf0Yomlt2qANOSTKOZWhIa2Q71c\n\tzEKY4AVYLsSJ66JWz4lc7kK/gKeSWAWDkpPyTEVs5jtVKJV3PskckZqt5PpJgV5PPb+Y\n\tRizg==","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=8Oex6siIc77/YaOvCHd3xlJnY2EqTvrTxVxUm933bCU=;\n\tb=m8MLu83DVCXKSTH6RsWgNyzYvmLQrqOrPjxHsgqtndqKoPr4pQunYKf88ZEKGN4NMG\n\tkeWdYWXsrKWowsnSNaxB2FHGDYq9ki0g6S8TDdtOG4nLHtYUWyXfnYNryrcb3h/zASOz\n\tS0i0U+xNwhL5h4GdOmTRJbo/EvtJqiQNZ0QsrqP8CO31cQ7mH1dmNbpKnjbw0xjtqVHE\n\tplJ5+FsE0JbBdmVFUy2RoqgayCBbq6PtK5Fd6jp8DLxPOBKkckwZJUJ8GoV2W/rYOjKv\n\to03KNOFG0/6dsVcQhRzmwLaV2xqrPqFxPiqAOAsFVWMT1r388Zo+cHeto37+JefJ199k\n\tswjA==","X-Gm-Message-State":"APjAAAV9uGZ6CyfNlL8UjkLFPRWlQ+6s4lWbtpoVHjGkpVPunHdgf3eH\n\tr1zoOxlrZUcIFkceUhNls+4l/BZku8k=","X-Google-Smtp-Source":"APXvYqxEh3/9lr9Nh3C7WgHDifAKz4KJpO/s5/3Yiqz1uwUUEaJzQjYZbif28YMbJPQFd3z7Wvc0XA==","X-Received":"by 2002:a19:4ad0:: with SMTP id\n\tx199mr2163803lfa.13.1579709600955; \n\tWed, 22 Jan 2020 08:13:20 -0800 (PST)","Date":"Wed, 22 Jan 2020 17:13:19 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200122161319.GU1124294@oden.dyn.berto.se>","References":"<20200120002437.6633-1-laurent.pinchart@ideasonboard.com>\n\t<20200120002437.6633-14-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200120002437.6633-14-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 13/19] libcamera: camera_manager: Run\n\tthe camera manager in a thread","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>","X-List-Received-Date":"Wed, 22 Jan 2020 16:13:22 -0000"}},{"id":3583,"web_url":"https://patchwork.libcamera.org/comment/3583/","msgid":"<20200122193634.GF15239@pendragon.ideasonboard.com>","date":"2020-01-22T19:36:34","subject":"Re: [libcamera-devel] [PATCH 13/19] libcamera: camera_manager: Run\n\tthe camera manager in a thread","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Wed, Jan 22, 2020 at 05:13:19PM +0100, Niklas Söderlund wrote:\n> On 2020-01-20 02:24:31 +0200, Laurent Pinchart wrote:\n> > Relying on the application event loop to process all our internal events\n> > is a bad idea for multiple reasons. In many cases the user of libcamera\n> > can't provide an event loop, for instance when running through one of\n> > the adaptation layers. The Android camera HAL and V4L2 compatibility\n> > layer create a thread for this reason, and the GStreamer element would\n> > need to do so as well. Furthermore, relying on the application event\n> > loop pushes libcamera's realtime constraints to the application, which\n> > isn't manageable.\n> > \n> > For these reasons it's desirable to always run the camera manager, the\n> > pipeline handlers and the cameras in a separate thread. Doing so isn't\n> > too complicated, it only involves creating the thread internally when\n> > starting the camera manager, and synchronizing a few methods of the\n> > Camera class. Do so as a first step towards defining the threading model\n> > of libcamera.\n> > \n> > Making CameraManager::cameras() thread-safe requires returning a copy of\n> > the cameras vector instead of a reference. This is also required for\n> > hot-plugging support and is thus desirable.\n> > \n> > The event dispatcher interface is still exposed to applications, to\n> > enable cross-thread signal delivery if desired.\n> > \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> This patch is quiet hard to review as it do multiple things, would it be \n> possible to split it a bit? I think it can be split into\n> \n> - Change the behavior of CameraManager::cameras()\n> - Breakout of CameraManager::Private\n> - New thread related code.\n\nI'll do so.\n\n> Small comments bellow tho.\n> \n> > ---\n> >  Documentation/Doxyfile.in          |   1 +\n> >  include/libcamera/camera_manager.h |  13 +-\n> >  src/libcamera/camera_manager.cpp   | 295 +++++++++++++++++++++--------\n> >  3 files changed, 224 insertions(+), 85 deletions(-)\n> > \n> > diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> > index 734672ed15dc..1c46b04b3f7e 100644\n> > --- a/Documentation/Doxyfile.in\n> > +++ b/Documentation/Doxyfile.in\n> > @@ -879,6 +879,7 @@ EXCLUDE_SYMBOLS        = libcamera::BoundMethodArgs \\\n> >                           libcamera::BoundMethodPackBase \\\n> >                           libcamera::BoundMethodStatic \\\n> >                           libcamera::SignalBase \\\n> > +                         libcamera::*::Private \\\n> >                           std::*\n> >  \n> >  # The EXAMPLE_PATH tag can be used to specify one or more files or directories\n> > diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\n> > index 094197668698..079f848aec79 100644\n> > --- a/include/libcamera/camera_manager.h\n> > +++ b/include/libcamera/camera_manager.h\n> > @@ -7,7 +7,6 @@\n> >  #ifndef __LIBCAMERA_CAMERA_MANAGER_H__\n> >  #define __LIBCAMERA_CAMERA_MANAGER_H__\n> >  \n> > -#include <map>\n> >  #include <memory>\n> >  #include <string>\n> >  #include <sys/types.h>\n> > @@ -18,9 +17,7 @@\n> >  namespace libcamera {\n> >  \n> >  class Camera;\n> > -class DeviceEnumerator;\n> >  class EventDispatcher;\n> > -class PipelineHandler;\n> >  \n> >  class CameraManager : public Object\n> >  {\n> > @@ -33,7 +30,7 @@ public:\n> >  \tint start();\n> >  \tvoid stop();\n> >  \n> > -\tconst std::vector<std::shared_ptr<Camera>> &cameras() const { return cameras_; }\n> > +\tstd::vector<std::shared_ptr<Camera>> cameras() const;\n> >  \tstd::shared_ptr<Camera> get(const std::string &name);\n> >  \tstd::shared_ptr<Camera> get(dev_t devnum);\n> >  \n> > @@ -46,13 +43,11 @@ public:\n> >  \tEventDispatcher *eventDispatcher();\n> >  \n> >  private:\n> > -\tstd::unique_ptr<DeviceEnumerator> enumerator_;\n> > -\tstd::vector<std::shared_ptr<PipelineHandler>> pipes_;\n> > -\tstd::vector<std::shared_ptr<Camera>> cameras_;\n> > -\tstd::map<dev_t, std::weak_ptr<Camera>> camerasByDevnum_;\n> > -\n> >  \tstatic const std::string version_;\n> >  \tstatic CameraManager *self_;\n> > +\n> > +\tclass Private;\n> > +\tstd::unique_ptr<Private> p_;\n> >  };\n> >  \n> >  } /* namespace libcamera */\n> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > index a71caf8536bb..64aa89975701 100644\n> > --- a/src/libcamera/camera_manager.cpp\n> > +++ b/src/libcamera/camera_manager.cpp\n> > @@ -7,6 +7,9 @@\n> >  \n> >  #include <libcamera/camera_manager.h>\n> >  \n> > +#include <condition_variable>\n> > +#include <map>\n> > +\n> >  #include <libcamera/camera.h>\n> >  #include <libcamera/event_dispatcher.h>\n> >  \n> > @@ -26,6 +29,184 @@ namespace libcamera {\n> >  \n> >  LOG_DEFINE_CATEGORY(Camera)\n> >  \n> > +class CameraManager::Private : public Thread\n> > +{\n> > +public:\n> > +\tPrivate(CameraManager *cm);\n> > +\n> > +\tint start();\n> > +\tvoid addCamera(std::shared_ptr<Camera> &camera, dev_t devnum);\n> > +\tvoid removeCamera(Camera *camera);\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> > +\tMutex mutex_;\n> > +\tstd::vector<std::shared_ptr<Camera>> cameras_;\n> > +\tstd::map<dev_t, std::weak_ptr<Camera>> camerasByDevnum_;\n> > +\n> > +protected:\n> > +\tvoid run() override;\n> > +\n> > +private:\n> > +\tint init();\n> > +\tvoid cleanup();\n> > +\n> > +\tCameraManager *cm_;\n> > +\n> > +\tstd::condition_variable cv_;\n> > +\tbool initialized_;\n> > +\tint status_;\n> > +\n> > +\tstd::vector<std::shared_ptr<PipelineHandler>> pipes_;\n> > +\tstd::unique_ptr<DeviceEnumerator> enumerator_;\n> > +};\n> > +\n> > +CameraManager::Private::Private(CameraManager *cm)\n> > +\t: cm_(cm), initialized_(false)\n> > +{\n> > +}\n> > +\n> > +int CameraManager::Private::start()\n> > +{\n> > +\t/* Start the thread and wait for initialization to complete. */\n> > +\tThread::start();\n> > +\n> > +\t{\n> > +\t\tMutexLocker locker(mutex_);\n> > +\t\tcv_.wait(locker, [&] { return initialized_; });\n> > +\t}\n> \n> For my education, is this in its a own block so that locker will go out \n> of scope and free the locker after the wait returns?\n\nCorrect.\n\n> > +\n> > +\t/* If a failure happened during initialization, stop the thread. */\n> > +\tif (status_ < 0) {\n> > +\t\texit();\n> > +\t\twait();\n> > +\t\treturn status_;\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +void CameraManager::Private::addCamera(std::shared_ptr<Camera> &camera,\n> > +\t\t\t\t       dev_t devnum)\n> > +{\n> > +\tMutexLocker locker(mutex_);\n> > +\n> > +\tfor (std::shared_ptr<Camera> c : cameras_) {\n> > +\t\tif (c->name() == camera->name()) {\n> > +\t\t\tLOG(Camera, Warning)\n> > +\t\t\t\t<< \"Registering camera with duplicate name '\"\n> > +\t\t\t\t<< camera->name() << \"'\";\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\t}\n> > +\n> > +\tcameras_.push_back(std::move(camera));\n> > +\n> > +\tif (devnum) {\n> > +\t\tunsigned int index = cameras_.size() - 1;\n> > +\t\tcamerasByDevnum_[devnum] = cameras_[index];\n> > +\t}\n> > +}\n> > +\n> > +void CameraManager::Private::removeCamera(Camera *camera)\n> > +{\n> > +\tMutexLocker locker(mutex_);\n> > +\n> > +\tauto iter = std::find_if(cameras_.begin(), cameras_.end(),\n> > +\t\t\t\t [camera](std::shared_ptr<Camera> &c) {\n> > +\t\t\t\t\t return c.get() == camera;\n> > +\t\t\t\t });\n> > +\tif (iter == cameras_.end())\n> > +\t\treturn;\n> > +\n> > +\tLOG(Camera, Debug)\n> > +\t\t<< \"Unregistering camera '\" << camera->name() << \"'\";\n> > +\n> > +\tauto iter_d = std::find_if(camerasByDevnum_.begin(), camerasByDevnum_.end(),\n> > +\t\t\t\t   [camera](const std::pair<dev_t, std::weak_ptr<Camera>> &p) {\n> > +\t\t\t\t\t   return p.second.lock().get() == camera;\n> > +\t\t\t\t   });\n> > +\tif (iter_d != camerasByDevnum_.end())\n> > +\t\tcamerasByDevnum_.erase(iter_d);\n> > +\n> > +\tcameras_.erase(iter);\n> > +}\n> > +\n> > +void CameraManager::Private::run()\n> > +{\n> > +\tLOG(Camera, Debug) << \"Starting camera manager\";\n> > +\n> > +\tint ret = init();\n> > +\n> > +\tmutex_.lock();\n> > +\tstatus_ = ret;\n> > +\tinitialized_ = true;\n> > +\tmutex_.unlock();\n> > +\tcv_.notify_one();\n> > +\n> > +\tif (ret < 0)\n> > +\t\treturn;\n> > +\n> > +\t/* Now start processing events and messages. */\n> > +\texec();\n> > +\n> > +\tcleanup();\n> > +}\n> > +\n> > +int CameraManager::Private::init()\n> > +{\n> > +\tenumerator_ = DeviceEnumerator::create();\n> > +\tif (!enumerator_ || enumerator_->enumerate())\n> > +\t\treturn -ENODEV;\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> > +\t * configuration file.\n> > +\t */\n> > +\tstd::vector<PipelineHandlerFactory *> &factories = PipelineHandlerFactory::factories();\n> > +\n> > +\tfor (PipelineHandlerFactory *factory : factories) {\n> > +\t\t/*\n> > +\t\t * Try each pipeline handler until it exhaust\n> > +\t\t * all pipelines it can provide.\n> > +\t\t */\n> > +\t\twhile (1) {\n> > +\t\t\tstd::shared_ptr<PipelineHandler> pipe = factory->create(cm_);\n> > +\t\t\tif (!pipe->match(enumerator_.get()))\n> > +\t\t\t\tbreak;\n> > +\n> > +\t\t\tLOG(Camera, Debug)\n> > +\t\t\t\t<< \"Pipeline handler \\\"\" << factory->name()\n> > +\t\t\t\t<< \"\\\" matched\";\n> > +\t\t\tpipes_.push_back(std::move(pipe));\n> > +\t\t}\n> > +\t}\n> > +\n> > +\t/* TODO: register hot-plug callback here */\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +void CameraManager::Private::cleanup()\n> > +{\n> > +\t/* TODO: unregister hot-plug callback here */\n> > +\n> > +\t/*\n> > +\t * Release all references to cameras and pipeline handlers to ensure\n> > +\t * they all get destroyed before the device enumerator deletes the\n> > +\t * media devices.\n> > +\t */\n> > +\tpipes_.clear();\n> > +\tcameras_.clear();\n> > +\n> > +\tenumerator_.reset(nullptr);\n> > +}\n> > +\n> >  /**\n> >   * \\class CameraManager\n> >   * \\brief Provide access and manage all cameras in the system\n> > @@ -62,7 +243,7 @@ LOG_DEFINE_CATEGORY(Camera)\n> >  CameraManager *CameraManager::self_ = nullptr;\n> >  \n> >  CameraManager::CameraManager()\n> > -\t: enumerator_(nullptr)\n> > +\t: p_(new CameraManager::Private(this))\n> >  {\n> >  \tif (self_)\n> >  \t\tLOG(Camera, Fatal)\n> > @@ -73,6 +254,8 @@ CameraManager::CameraManager()\n> >  \n> >  CameraManager::~CameraManager()\n> >  {\n> > +\tstop();\n> > +\n> >  \tself_ = nullptr;\n> >  }\n> >  \n> > @@ -88,41 +271,16 @@ CameraManager::~CameraManager()\n> >   */\n> >  int CameraManager::start()\n> >  {\n> > -\tif (enumerator_)\n> > -\t\treturn -EBUSY;\n> > -\n> >  \tLOG(Camera, Info) << \"libcamera \" << version_;\n> >  \n> > -\tenumerator_ = DeviceEnumerator::create();\n> > -\tif (!enumerator_ || enumerator_->enumerate())\n> > -\t\treturn -ENODEV;\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> > -\t * configuration file.\n> > -\t */\n> > -\tstd::vector<PipelineHandlerFactory *> &factories = PipelineHandlerFactory::factories();\n> > +\tint ret = p_->start();\n> > +\tif (ret < 0) {\n> > +\t\tLOG(Camera, Error) << \"Failed to start camera manager: \"\n> > +\t\t\t\t   << strerror(-ret);\n> >  \n> > -\tfor (PipelineHandlerFactory *factory : factories) {\n> > -\t\t/*\n> > -\t\t * Try each pipeline handler until it exhaust\n> > -\t\t * all pipelines it can provide.\n> > -\t\t */\n> > -\t\twhile (1) {\n> > -\t\t\tstd::shared_ptr<PipelineHandler> pipe = factory->create(this);\n> > -\t\t\tif (!pipe->match(enumerator_.get()))\n> > -\t\t\t\tbreak;\n> > -\n> > -\t\t\tLOG(Camera, Debug)\n> > -\t\t\t\t<< \"Pipeline handler \\\"\" << factory->name()\n> > -\t\t\t\t<< \"\\\" matched\";\n> > -\t\t\tpipes_.push_back(std::move(pipe));\n> > -\t\t}\n> > +\t\treturn ret;\n> >  \t}\n> >  \n> > -\t/* TODO: register hot-plug callback here */\n> > -\n> >  \treturn 0;\n> \n> I think you can simplify this to\n> \n>         int ret = p_->start();\n>         if (ret)\n> \t\tLOG(Camera, Error) << \"Failed to start camera manager: \"\n> \t\t\t\t   << strerror(-ret);\n> \n>         return ret;\n> \n> \n> as start() shall return 0 on success.\n\nDone.\n\n> >  }\n> >  \n> > @@ -138,17 +296,8 @@ int CameraManager::start()\n> >   */\n> >  void CameraManager::stop()\n> >  {\n> > -\t/* TODO: unregister hot-plug callback here */\n> > -\n> > -\t/*\n> > -\t * Release all references to cameras and pipeline handlers to ensure\n> > -\t * they all get destroyed before the device enumerator deletes the\n> > -\t * media devices.\n> > -\t */\n> > -\tpipes_.clear();\n> > -\tcameras_.clear();\n> > -\n> > -\tenumerator_.reset(nullptr);\n> > +\tp_->exit();\n> > +\tp_->wait();\n> >  }\n> >  \n> >  /**\n> > @@ -158,8 +307,16 @@ void CameraManager::stop()\n> >   * Before calling this function the caller is responsible for ensuring that\n> >   * the camera manager is running.\n> >   *\n> > + * \\context This function is \\threadsafe.\n> > + *\n> >   * \\return List of all available cameras\n> >   */\n> > +std::vector<std::shared_ptr<Camera>> CameraManager::cameras() const\n> > +{\n> > +\tMutexLocker locker(p_->mutex_);\n> > +\n> > +\treturn p_->cameras_;\n> > +}\n> >  \n> >  /**\n> >   * \\brief Get a camera based on name\n> > @@ -168,11 +325,15 @@ void CameraManager::stop()\n> >   * Before calling this function the caller is responsible for ensuring that\n> >   * the camera manager is running.\n> >   *\n> > + * \\context This function is \\threadsafe.\n> > + *\n> >   * \\return Shared pointer to Camera object or nullptr if camera not found\n> >   */\n> >  std::shared_ptr<Camera> CameraManager::get(const std::string &name)\n> >  {\n> > -\tfor (std::shared_ptr<Camera> camera : cameras_) {\n> > +\tMutexLocker locker(p_->mutex_);\n> > +\n> > +\tfor (std::shared_ptr<Camera> camera : p_->cameras_) {\n> >  \t\tif (camera->name() == name)\n> >  \t\t\treturn camera;\n> >  \t}\n> > @@ -191,13 +352,17 @@ std::shared_ptr<Camera> CameraManager::get(const std::string &name)\n> >   * Before calling this function the caller is responsible for ensuring that\n> >   * the camera manager is running.\n> >   *\n> > + * \\context This function is \\threadsafe.\n> > + *\n> >   * \\return Shared pointer to Camera object, which is empty if the camera is\n> >   * not found\n> >   */\n> >  std::shared_ptr<Camera> CameraManager::get(dev_t devnum)\n> >  {\n> > -\tauto iter = camerasByDevnum_.find(devnum);\n> > -\tif (iter == camerasByDevnum_.end())\n> > +\tMutexLocker locker(p_->mutex_);\n> > +\n> > +\tauto iter = p_->camerasByDevnum_.find(devnum);\n> > +\tif (iter == p_->camerasByDevnum_.end())\n> >  \t\treturn nullptr;\n> >  \n> >  \treturn iter->second.lock();\n> > @@ -214,24 +379,14 @@ std::shared_ptr<Camera> CameraManager::get(dev_t devnum)\n> >   *\n> >   * \\a devnum is used by the V4L2 compatibility layer to map V4L2 device nodes\n> >   * to Camera instances.\n> > + *\n> > + * \\context This function shall be called from the CameraManager thread.\n> >   */\n> >  void CameraManager::addCamera(std::shared_ptr<Camera> camera, dev_t devnum)\n> >  {\n> > -\tfor (std::shared_ptr<Camera> c : cameras_) {\n> > -\t\tif (c->name() == camera->name()) {\n> > -\t\t\tLOG(Camera, Warning)\n> > -\t\t\t\t<< \"Registering camera with duplicate name '\"\n> > -\t\t\t\t<< camera->name() << \"'\";\n> > -\t\t\tbreak;\n> > -\t\t}\n> > -\t}\n> > -\n> > -\tcameras_.push_back(std::move(camera));\n> > +\tASSERT(Thread::current() == p_.get());\n> >  \n> > -\tif (devnum) {\n> > -\t\tunsigned int index = cameras_.size() - 1;\n> > -\t\tcamerasByDevnum_[devnum] = cameras_[index];\n> > -\t}\n> > +\tp_->addCamera(camera, devnum);\n> >  }\n> >  \n> >  /**\n> > @@ -241,32 +396,20 @@ void CameraManager::addCamera(std::shared_ptr<Camera> camera, dev_t devnum)\n> >   * This function is called by pipeline handlers to unregister cameras from the\n> >   * camera manager. Unregistered cameras won't be reported anymore by the\n> >   * cameras() and get() calls, but references may still exist in applications.\n> > + *\n> > + * \\context This function shall be called from the CameraManager thread.\n> >   */\n> >  void CameraManager::removeCamera(Camera *camera)\n> >  {\n> > -\tauto iter = std::find_if(cameras_.begin(), cameras_.end(),\n> > -\t\t\t\t [camera](std::shared_ptr<Camera> &c) {\n> > -\t\t\t\t\t return c.get() == camera;\n> > -\t\t\t\t });\n> > -\tif (iter == cameras_.end())\n> > -\t\treturn;\n> > -\n> > -\tLOG(Camera, Debug)\n> > -\t\t<< \"Unregistering camera '\" << camera->name() << \"'\";\n> > -\n> > -\tauto iter_d = std::find_if(camerasByDevnum_.begin(), camerasByDevnum_.end(),\n> > -\t\t\t\t   [camera](const std::pair<dev_t, std::weak_ptr<Camera>> &p) {\n> > -\t\t\t\t\t   return p.second.lock().get() == camera;\n> > -\t\t\t\t   });\n> > -\tif (iter_d != camerasByDevnum_.end())\n> > -\t\tcamerasByDevnum_.erase(iter_d);\n> > +\tASSERT(Thread::current() == p_.get());\n> >  \n> > -\tcameras_.erase(iter);\n> > +\tp_->removeCamera(camera);\n> >  }\n> >  \n> >  /**\n> >   * \\fn const std::string &CameraManager::version()\n> >   * \\brief Retrieve the libcamera version string\n> > + * \\context This function is \\a threadsafe.\n> >   * \\return The libcamera version string\n> >   */\n> >","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CA09260804\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 22 Jan 2020 20:36:50 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 396DD2E5;\n\tWed, 22 Jan 2020 20:36:50 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1579721810;\n\tbh=mQ9j6SeBDV8KqxWRyM/WgtRBkrVsMaBQ34xRgmMapxI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=U8hz8gY19MYzzzaBnkSDwNdpE/DuXgE8xG2wgST7OpwrHWvi+xz0j4npw5W2QVSrF\n\taIkQz7ijF83bmFLWiT+DU0lA9LBWpN7A4irDCWJ0Go+KTU9/99AXdOMrZmvKe1pwnP\n\tTMC6YI9GomxcZPnNFonfwxVv+0/xS6W4pmGMMV2U=","Date":"Wed, 22 Jan 2020 21:36:34 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200122193634.GF15239@pendragon.ideasonboard.com>","References":"<20200120002437.6633-1-laurent.pinchart@ideasonboard.com>\n\t<20200120002437.6633-14-laurent.pinchart@ideasonboard.com>\n\t<20200122161319.GU1124294@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200122161319.GU1124294@oden.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 13/19] libcamera: camera_manager: Run\n\tthe camera manager in a thread","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>","X-List-Received-Date":"Wed, 22 Jan 2020 19:36:51 -0000"}}]