[{"id":2210,"web_url":"https://patchwork.libcamera.org/comment/2210/","msgid":"<20190711050019.GI1557@wyvern>","date":"2019-07-11T05:00:19","subject":"Re: [libcamera-devel] [PATCH 1/6] libcamera: Add thread support","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 patch.\n\nOn 2019-07-10 22:17:03 +0300, Laurent Pinchart wrote:\n> The new Thread class wraps std::thread in order to integrate it with the\n> Object, Signal and EventDispatcher classes. By default new threads run\n> an internal event loop, and their run() method can be overloaded to\n> provide a custom thread loop.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nI like it,\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  include/libcamera/camera_manager.h |   2 -\n>  src/libcamera/camera_manager.cpp   |  15 +-\n>  src/libcamera/include/thread.h     |  61 ++++++\n>  src/libcamera/meson.build          |   3 +\n>  src/libcamera/thread.cpp           | 335 +++++++++++++++++++++++++++++\n>  5 files changed, 403 insertions(+), 13 deletions(-)\n>  create mode 100644 src/libcamera/include/thread.h\n>  create mode 100644 src/libcamera/thread.cpp\n> \n> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\n> index 633d27d17ebf..0e8881baba40 100644\n> --- a/include/libcamera/camera_manager.h\n> +++ b/include/libcamera/camera_manager.h\n> @@ -46,8 +46,6 @@ private:\n>  \tstd::vector<std::shared_ptr<PipelineHandler>> pipes_;\n>  \tstd::vector<std::shared_ptr<Camera>> cameras_;\n>  \n> -\tstd::unique_ptr<EventDispatcher> dispatcher_;\n> -\n>  \tstatic const std::string version_;\n>  };\n>  \n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index 337496c21cfc..2cf014233b05 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -14,6 +14,7 @@\n>  #include \"event_dispatcher_poll.h\"\n>  #include \"log.h\"\n>  #include \"pipeline_handler.h\"\n> +#include \"thread.h\"\n>  #include \"utils.h\"\n>  \n>  /**\n> @@ -56,7 +57,7 @@ LOG_DEFINE_CATEGORY(Camera)\n>   */\n>  \n>  CameraManager::CameraManager()\n> -\t: enumerator_(nullptr), dispatcher_(nullptr)\n> +\t: enumerator_(nullptr)\n>  {\n>  }\n>  \n> @@ -247,12 +248,7 @@ CameraManager *CameraManager::instance()\n>   */\n>  void CameraManager::setEventDispatcher(std::unique_ptr<EventDispatcher> dispatcher)\n>  {\n> -\tif (dispatcher_) {\n> -\t\tLOG(Camera, Warning) << \"Event dispatcher is already set\";\n> -\t\treturn;\n> -\t}\n> -\n> -\tdispatcher_ = std::move(dispatcher);\n> +\tThread::current()->setEventDispatcher(std::move(dispatcher));\n>  }\n>  \n>  /**\n> @@ -268,10 +264,7 @@ void CameraManager::setEventDispatcher(std::unique_ptr<EventDispatcher> dispatch\n>   */\n>  EventDispatcher *CameraManager::eventDispatcher()\n>  {\n> -\tif (!dispatcher_)\n> -\t\tdispatcher_ = utils::make_unique<EventDispatcherPoll>();\n> -\n> -\treturn dispatcher_.get();\n> +\treturn Thread::current()->eventDispatcher();\n>  }\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/include/thread.h b/src/libcamera/include/thread.h\n> new file mode 100644\n> index 000000000000..e881d90e9367\n> --- /dev/null\n> +++ b/src/libcamera/include/thread.h\n> @@ -0,0 +1,61 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * thread.h - Thread support\n> + */\n> +#ifndef __LIBCAMERA_THREAD_H__\n> +#define __LIBCAMERA_THREAD_H__\n> +\n> +#include <memory>\n> +#include <mutex>\n> +#include <thread>\n> +\n> +#include <libcamera/signal.h>\n> +\n> +namespace libcamera {\n> +\n> +class EventDispatcher;\n> +class ThreadData;\n> +class ThreadMain;\n> +\n> +using Mutex = std::mutex;\n> +using MutexLocker = std::unique_lock<std::mutex>;\n> +\n> +class Thread\n> +{\n> +public:\n> +\tThread();\n> +\tvirtual ~Thread();\n> +\n> +\tvoid start();\n> +\tvoid exit(int code = 0);\n> +\tvoid wait();\n> +\n> +\tbool isRunning();\n> +\n> +\tSignal<Thread *> finished;\n> +\n> +\tstatic Thread *current();\n> +\n> +\tEventDispatcher *eventDispatcher();\n> +\tvoid setEventDispatcher(std::unique_ptr<EventDispatcher> dispatcher);\n> +\n> +protected:\n> +\tint exec();\n> +\tvirtual void run();\n> +\n> +private:\n> +\tvoid startThread();\n> +\tvoid finishThread();\n> +\n> +\tfriend class ThreadData;\n> +\tfriend class ThreadMain;\n> +\n> +\tstd::thread thread_;\n> +\tThreadData *data_;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_THREAD_H__ */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 97ff86e2167f..bf71524f768c 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -23,6 +23,7 @@ libcamera_sources = files([\n>      'request.cpp',\n>      'signal.cpp',\n>      'stream.cpp',\n> +    'thread.cpp',\n>      'timer.cpp',\n>      'utils.cpp',\n>      'v4l2_controls.cpp',\n> @@ -45,6 +46,7 @@ libcamera_headers = files([\n>      'include/media_device.h',\n>      'include/media_object.h',\n>      'include/pipeline_handler.h',\n> +    'include/thread.h',\n>      'include/utils.h',\n>      'include/v4l2_device.h',\n>      'include/v4l2_subdevice.h',\n> @@ -91,6 +93,7 @@ libcamera_sources += version_cpp\n>  libcamera_deps = [\n>      cc.find_library('dl'),\n>      libudev,\n> +    dependency('threads'),\n>  ]\n>  \n>  libcamera = shared_library('camera',\n> diff --git a/src/libcamera/thread.cpp b/src/libcamera/thread.cpp\n> new file mode 100644\n> index 000000000000..95636ecaab53\n> --- /dev/null\n> +++ b/src/libcamera/thread.cpp\n> @@ -0,0 +1,335 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * thread.cpp - Thread support\n> + */\n> +\n> +#include \"thread.h\"\n> +\n> +#include <atomic>\n> +\n> +#include <libcamera/event_dispatcher.h>\n> +\n> +#include \"event_dispatcher_poll.h\"\n> +#include \"log.h\"\n> +\n> +/**\n> + * \\file thread.h\n> + * \\brief Thread support\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(Thread)\n> +\n> +class ThreadMain;\n> +\n> +/**\n> + * \\brief Thread-local internal data\n> + */\n> +class ThreadData\n> +{\n> +public:\n> +\tThreadData()\n> +\t\t: thread_(nullptr), running_(false), dispatcher_(nullptr)\n> +\t{\n> +\t}\n> +\n> +\tstatic ThreadData *current();\n> +\n> +private:\n> +\tfriend class Thread;\n> +\tfriend class ThreadMain;\n> +\n> +\tThread *thread_;\n> +\tbool running_;\n> +\n> +\tMutex mutex_;\n> +\n> +\tstd::atomic<EventDispatcher *> dispatcher_;\n> +\n> +\tstd::atomic<bool> exit_;\n> +\tint exitCode_;\n> +};\n> +\n> +/**\n> + * \\brief Thread wrapper for the main thread\n> + */\n> +class ThreadMain : public Thread\n> +{\n> +public:\n> +\tThreadMain()\n> +\t{\n> +\t\tdata_->running_ = true;\n> +\t}\n> +\n> +protected:\n> +\tvoid run() override\n> +\t{\n> +\t\tLOG(Thread, Fatal) << \"The main thread can't be restarted\";\n> +\t}\n> +};\n> +\n> +static thread_local ThreadData *currentThreadData = nullptr;\n> +static ThreadMain mainThread;\n> +\n> +/**\n> + * \\brief Retrieve thread-local internal data for the current thread\n> + * \\return The thread-local internal data for the current thread\n> + */\n> +ThreadData *ThreadData::current()\n> +{\n> +\tif (currentThreadData)\n> +\t\treturn currentThreadData;\n> +\n> +\t/*\n> +\t * The main thread doesn't receive thread-local data when it is\n> +\t * started, set it here.\n> +\t */\n> +\tThreadData *data = mainThread.data_;\n> +\tcurrentThreadData = data;\n> +\treturn data;\n> +}\n> +\n> +/**\n> + * \\typedef Mutex\n> + * \\brief An alias for std::mutex\n> + */\n> +\n> +/**\n> + * \\typedef MutexLocker\n> + * \\brief An alias for std::unique_lock<std::mutex>\n> + */\n> +\n> +/**\n> + * \\class Thread\n> + * \\brief A thread of execution\n> + *\n> + * The Thread class is a wrapper around std::thread that handles integration\n> + * with the Object, Signal and EventDispatcher classes.\n> + *\n> + * Thread instances by default run an event loop until the exit() method is\n> + * called. A custom event dispatcher may be installed with\n> + * setEventDispatcher(), otherwise a poll-based event dispatcher is used. This\n> + * behaviour can be overriden by overloading the run() method.\n> + */\n> +\n> +/**\n> + * \\brief Create a thread\n> + */\n> +Thread::Thread()\n> +{\n> +\tdata_ = new ThreadData;\n> +\tdata_->thread_ = this;\n> +}\n> +\n> +Thread::~Thread()\n> +{\n> +\tdelete data_->dispatcher_.load(std::memory_order_relaxed);\n> +\tdelete data_;\n> +}\n> +\n> +/**\n> + * \\brief Start the thread\n> + */\n> +void Thread::start()\n> +{\n> +\tMutexLocker locker(data_->mutex_);\n> +\n> +\tif (data_->running_)\n> +\t\treturn;\n> +\n> +\tdata_->running_ = true;\n> +\tdata_->exitCode_ = -1;\n> +\tdata_->exit_.store(false, std::memory_order_relaxed);\n> +\n> +\tthread_ = std::thread(&Thread::startThread, this);\n> +}\n> +\n> +void Thread::startThread()\n> +{\n> +\tstruct ThreadCleaner {\n> +\t\tThreadCleaner(Thread *thread, void (Thread::*cleaner)())\n> +\t\t\t: thread_(thread), cleaner_(cleaner)\n> +\t\t{\n> +\t\t}\n> +\t\t~ThreadCleaner()\n> +\t\t{\n> +\t\t\t(thread_->*cleaner_)();\n> +\t\t}\n> +\n> +\t\tThread *thread_;\n> +\t\tvoid (Thread::*cleaner_)();\n> +\t};\n> +\n> +\t/*\n> +\t * Make sure the thread is cleaned up even if the run method exits\n> +\t * abnormally (for instance via a direct call to pthread_cancel()).\n> +\t */\n> +\tthread_local ThreadCleaner cleaner(this, &Thread::finishThread);\n> +\n> +\tcurrentThreadData = data_;\n> +\n> +\trun();\n> +}\n> +\n> +/**\n> + * \\brief Enter the event loop\n> + *\n> + * This method enter an event loop based on the event dispatcher instance for\n> + * the thread, and blocks until the exit() method is called. It is meant to be\n> + * called within the thread from the run() method and shall not be called\n> + * outside of the thread.\n> + *\n> + * \\return The exit code passed to the exit() method\n> + */\n> +int Thread::exec()\n> +{\n> +\tMutexLocker locker(data_->mutex_);\n> +\n> +\tEventDispatcher *dispatcher = eventDispatcher();\n> +\n> +\tlocker.unlock();\n> +\n> +\twhile (!data_->exit_.load(std::memory_order_acquire))\n> +\t\tdispatcher->processEvents();\n> +\n> +\tlocker.lock();\n> +\n> +\treturn data_->exitCode_;\n> +}\n> +\n> +/**\n> + * \\brief Main method of the thread\n> + *\n> + * When the thread is started with start(), it calls this method in the context\n> + * of the new thread. The run() method can be overloaded to perform custom\n> + * work. When this method returns the thread execution is stopped, and the \\ref\n> + * finished signal is emitted.\n> + *\n> + * The base implementation just calls exec().\n> + */\n> +void Thread::run()\n> +{\n> +\texec();\n> +}\n> +\n> +void Thread::finishThread()\n> +{\n> +\tdata_->mutex_.lock();\n> +\tdata_->running_ = false;\n> +\tdata_->mutex_.unlock();\n> +\n> +\tfinished.emit(this);\n> +}\n> +\n> +/**\n> + * \\brief Stop the thread's event loop\n> + * \\param[in] code The exit code\n> + *\n> + * This method interrupts the event loop started by the exec() method, causing\n> + * exec() to return \\a code.\n> + *\n> + * Calling exit() on a thread that reimplements the run() method and doesn't\n> + * call exec() will likely have no effect.\n> + */\n> +void Thread::exit(int code)\n> +{\n> +\tdata_->exitCode_ = code;\n> +\tdata_->exit_.store(true, std::memory_order_release);\n> +\n> +\tEventDispatcher *dispatcher = data_->dispatcher_.load(std::memory_order_relaxed);\n> +\tif (!dispatcher)\n> +\t\treturn;\n> +\n> +\tdispatcher->interrupt();\n> +}\n> +\n> +/**\n> + * \\brief Wait for the thread to finish\n> + *\n> + * This method waits until the thread finishes, or returns immediately if the\n> + * thread is not running.\n> + */\n> +void Thread::wait()\n> +{\n> +\tif (thread_.joinable())\n> +\t\tthread_.join();\n> +}\n> +\n> +/**\n> + * \\brief Check if the thread is running\n> + *\n> + * A Thread instance is considered as running once the underlying thread has\n> + * started. This method guarantees that it returns true after the start()\n> + * method returns, and false after the wait() method returns.\n> + *\n> + * \\return True if the thread is running, false otherwise\n> + */\n> +bool Thread::isRunning()\n> +{\n> +\tMutexLocker locker(data_->mutex_);\n> +\treturn data_->running_;\n> +}\n> +\n> +/**\n> + * \\var Thread::finished\n> + * \\brief Signal the end of thread execution\n> + */\n> +\n> +/**\n> + * \\brief Retrieve the Thread instance for the current thread\n> + * \\return The Thread instance for the current thread\n> + */\n> +Thread *Thread::current()\n> +{\n> +\tThreadData *data = ThreadData::current();\n> +\treturn data->thread_;\n> +}\n> +\n> +/**\n> + * \\brief Set the event dispatcher\n> + * \\param[in] dispatcher Pointer to the event dispatcher\n> + *\n> + * Threads that run an event loop require an event dispatcher to integrate\n> + * event notification and timers with the loop. Users that want to provide\n> + * their own event dispatcher shall call this method once and only once before\n> + * the thread is started with start(). If no event dispatcher is provided, a\n> + * default poll-based implementation will be used.\n> + *\n> + * The Thread takes ownership of the event dispatcher and will delete it when\n> + * the thread is destroyed.\n> + */\n> +void Thread::setEventDispatcher(std::unique_ptr<EventDispatcher> dispatcher)\n> +{\n> +\tif (data_->dispatcher_.load(std::memory_order_relaxed)) {\n> +\t\tLOG(Thread, Warning) << \"Event dispatcher is already set\";\n> +\t\treturn;\n> +\t}\n> +\n> +\tdata_->dispatcher_.store(dispatcher.release(),\n> +\t\t\t\t std::memory_order_relaxed);\n> +}\n> +\n> +/**\n> + * \\brief Retrieve the event dispatcher\n> + *\n> + * This method retrieves the event dispatcher set with setEventDispatcher().\n> + * If no dispatcher has been set, a default poll-based implementation is created\n> + * and returned, and no custom event dispatcher may be installed anymore.\n> + *\n> + * The returned event dispatcher is valid until the thread is destroyed.\n> + *\n> + * \\return Pointer to the event dispatcher\n> + */\n> +EventDispatcher *Thread::eventDispatcher()\n> +{\n> +\tif (!data_->dispatcher_.load(std::memory_order_relaxed))\n> +\t\tdata_->dispatcher_.store(new EventDispatcherPoll(),\n> +\t\t\t\t\t std::memory_order_release);\n> +\n> +\treturn data_->dispatcher_.load(std::memory_order_relaxed);\n> +}\n> +\n> +}; /* namespace libcamera */\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-pg1-x543.google.com (mail-pg1-x543.google.com\n\t[IPv6:2607:f8b0:4864:20::543])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7CF6460C23\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 11 Jul 2019 07:00:24 +0200 (CEST)","by mail-pg1-x543.google.com with SMTP id o13so2287497pgp.12\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 10 Jul 2019 22:00:24 -0700 (PDT)","from localhost (softbank126163157105.bbtec.net. [126.163.157.105])\n\tby smtp.gmail.com with ESMTPSA id\n\tx14sm5057421pfq.158.2019.07.10.22.00.21\n\t(version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256);\n\tWed, 10 Jul 2019 22:00:21 -0700 (PDT)"],"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\t:user-agent; bh=27WfnKjsHFOXlU3/Ar9TLTH3bZ4WHw+924rv4cekvAg=;\n\tb=W2nr6oheNm93qbAaofrZLKT8ejbWwT8pT/leFs91IZU/W8Z0TBaROSwp4qQ12cuBxS\n\tOfs2DiQEJyRcLMSrf1d+Cdk2jZiP9SniGMV7gY2TuTBqmWsYRMkcTsTzHUFJWcUcEhbO\n\tv7K5lvrdP/8ifqU6p8WFHzcIgL03TTDO3OJuTuFAaAX/dzvA64X9zpf9mBVYIGNDTL5E\n\t0jNxJFp8yopDkNRkuKfnthFhHu1vVEP4vioix78vjyVF7La+U5m6Kg664La44tMMkb53\n\tlz5hk4x1qHZmhILAJBoaC7cGawyeAxuNsEITYQieTzdZot3m4/VFeomfXscZ7ChmZMOT\n\tLhdQ==","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:user-agent;\n\tbh=27WfnKjsHFOXlU3/Ar9TLTH3bZ4WHw+924rv4cekvAg=;\n\tb=Dqv8gihuaGZtEm8L2FZAQ3seZEDQPenwoCUQL5Ef4QzWj3l/4mKl4CYdfW054zGl15\n\t05+6XhkHMBUZpvp0zKEoveSV54CE8HGM3Sk9gKu61OE32AmUaVAUxGphf7gNxF3PvonH\n\tvBkQOlhL82dp0L/9LJPuNipbWN8KDOefqW89RzN3GZPhiDqIjym1I7FyUmQTuDuFGy/5\n\trWig+n3vPrxubmLUsDtrslfgzP2j169/zKT5UpVnN/Ff5isucIS8Jy3oxPgGOBopRHuH\n\tlV0JG7fB/aWPeWkfluZahnJH9qlfrTGL4ZpcKQon9CMIs8xgqqvGNxcqrzMqVbNjA4d5\n\tbSyw==","X-Gm-Message-State":"APjAAAViF+9Gd0QvRhzrEH6R8GBotKaq11yZZ1fT/k0Ta/zZGzugFNQ3\n\t1e1PjmOC4qDGPzZiVyZSVUgESU93id4=","X-Google-Smtp-Source":"APXvYqyBdxUO1vlUXCEtTWL1iEx1AC+Rmp2mg7X1j7USYl52yrg1RaoaXPneZAlYIA4eiyJnM8xhyQ==","X-Received":"by 2002:a17:90a:5288:: with SMTP id\n\tw8mr2511488pjh.61.1562821222736; \n\tWed, 10 Jul 2019 22:00:22 -0700 (PDT)","Date":"Thu, 11 Jul 2019 14:00:19 +0900","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":"<20190711050019.GI1557@wyvern>","References":"<20190710191708.13049-1-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":"<20190710191708.13049-1-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH 1/6] libcamera: Add thread support","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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":"Thu, 11 Jul 2019 05:00:25 -0000"}}]