[libcamera-devel] [PATCH 18/19] v4l2: Remove internal thread
Niklas Söderlund
niklas.soderlund at ragnatech.se
Wed Jan 22 17:39:18 CET 2020
Hi Laurent,
Thanks for your work.
On 2020-01-20 02:24:36 +0200, Laurent Pinchart wrote:
> Now that libcamera creates threads internally and doesn't rely on an
> application-provided event loop, remove the thread from the V4L2
> compatibility layer. The split between the V4L2CameraProxy and
> V4L2Camera classes is still kept to separate the V4L2 adaptation from
> camera operation. This may be further refactored later.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> src/v4l2/v4l2_camera.h | 2 +-
> src/v4l2/v4l2_camera_proxy.cpp | 43 ++++++++++------------------
> src/v4l2/v4l2_compat_manager.cpp | 48 +++++++++-----------------------
> src/v4l2/v4l2_compat_manager.h | 13 ++-------
> 4 files changed, 31 insertions(+), 75 deletions(-)
>
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> index f1f04d9ef6ed..37bd358462db 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -21,7 +21,7 @@
>
> using namespace libcamera;
>
> -class V4L2Camera : public Object
> +class V4L2Camera
> {
> public:
> struct Buffer {
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index e58fd6a0d8b5..622520479be0 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -41,8 +41,7 @@ int V4L2CameraProxy::open(bool nonBlocking)
> {
> LOG(V4L2Compat, Debug) << "Servicing open";
>
> - int ret = vcam_->invokeMethod(&V4L2Camera::open,
> - ConnectionTypeBlocking);
> + int ret = vcam_->open();
> if (ret < 0) {
> errno = -ret;
> return -1;
> @@ -50,8 +49,7 @@ int V4L2CameraProxy::open(bool nonBlocking)
>
> nonBlocking_ = nonBlocking;
>
> - vcam_->invokeMethod(&V4L2Camera::getStreamConfig,
> - ConnectionTypeBlocking, &streamConfig_);
> + vcam_->getStreamConfig(&streamConfig_);
> setFmtFromConfig(streamConfig_);
> sizeimage_ = calculateSizeImage(streamConfig_);
>
> @@ -72,7 +70,7 @@ void V4L2CameraProxy::close()
> if (--refcount_ > 0)
> return;
>
> - vcam_->invokeMethod(&V4L2Camera::close, ConnectionTypeBlocking);
> + vcam_->close();
> }
>
> void *V4L2CameraProxy::mmap(void *addr, size_t length, int prot, int flags,
> @@ -284,11 +282,9 @@ int V4L2CameraProxy::vidioc_s_fmt(struct v4l2_format *arg)
> tryFormat(arg);
>
> Size size(arg->fmt.pix.width, arg->fmt.pix.height);
> - int ret = vcam_->invokeMethod(&V4L2Camera::configure,
> - ConnectionTypeBlocking,
> - &streamConfig_, size,
> - v4l2ToDrm(arg->fmt.pix.pixelformat),
> - bufferCount_);
> + int ret = vcam_->configure(&streamConfig_, size,
> + v4l2ToDrm(arg->fmt.pix.pixelformat),
> + bufferCount_);
> if (ret < 0)
> return -EINVAL;
>
> @@ -319,13 +315,12 @@ int V4L2CameraProxy::freeBuffers()
> {
> LOG(V4L2Compat, Debug) << "Freeing libcamera bufs";
>
> - int ret = vcam_->invokeMethod(&V4L2Camera::streamOff,
> - ConnectionTypeBlocking);
> + int ret = vcam_->streamOff();
> if (ret < 0) {
> LOG(V4L2Compat, Error) << "Failed to stop stream";
> return ret;
> }
> - vcam_->invokeMethod(&V4L2Camera::freeBuffers, ConnectionTypeBlocking);
> + vcam_->freeBuffers();
> bufferCount_ = 0;
>
> return 0;
> @@ -349,11 +344,9 @@ int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg)
> return freeBuffers();
>
> Size size(curV4L2Format_.fmt.pix.width, curV4L2Format_.fmt.pix.height);
> - ret = vcam_->invokeMethod(&V4L2Camera::configure,
> - ConnectionTypeBlocking,
> - &streamConfig_, size,
> - v4l2ToDrm(curV4L2Format_.fmt.pix.pixelformat),
> - arg->count);
> + ret = vcam_->configure(&streamConfig_, size,
> + v4l2ToDrm(curV4L2Format_.fmt.pix.pixelformat),
> + arg->count);
> if (ret < 0)
> return -EINVAL;
>
> @@ -366,8 +359,7 @@ int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg)
> arg->count = streamConfig_.bufferCount;
> bufferCount_ = arg->count;
>
> - ret = vcam_->invokeMethod(&V4L2Camera::allocBuffers,
> - ConnectionTypeBlocking, arg->count);
> + ret = vcam_->allocBuffers(arg->count);
> if (ret < 0) {
> arg->count = 0;
> return ret;
> @@ -415,8 +407,7 @@ int V4L2CameraProxy::vidioc_qbuf(struct v4l2_buffer *arg)
> arg->index >= bufferCount_)
> return -EINVAL;
>
> - int ret = vcam_->invokeMethod(&V4L2Camera::qbuf, ConnectionTypeBlocking,
> - arg->index);
> + int ret = vcam_->qbuf(arg->index);
> if (ret < 0)
> return ret;
>
> @@ -459,10 +450,7 @@ int V4L2CameraProxy::vidioc_streamon(int *arg)
> if (!validateBufferType(*arg))
> return -EINVAL;
>
> - int ret = vcam_->invokeMethod(&V4L2Camera::streamOn,
> - ConnectionTypeBlocking);
> -
> - return ret;
> + return vcam_->streamOn();
> }
>
> int V4L2CameraProxy::vidioc_streamoff(int *arg)
> @@ -472,8 +460,7 @@ int V4L2CameraProxy::vidioc_streamoff(int *arg)
> if (!validateBufferType(*arg))
> return -EINVAL;
>
> - int ret = vcam_->invokeMethod(&V4L2Camera::streamOff,
> - ConnectionTypeBlocking);
> + int ret = vcam_->streamOff();
>
> for (struct v4l2_buffer &buf : buffers_)
> buf.flags &= ~(V4L2_BUF_FLAG_QUEUED | V4L2_BUF_FLAG_DONE);
> diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp
> index f5a7b2ac4229..961d06b3e39a 100644
> --- a/src/v4l2/v4l2_compat_manager.cpp
> +++ b/src/v4l2/v4l2_compat_manager.cpp
> @@ -37,7 +37,7 @@ void get_symbol(T &func, const char *name)
> } /* namespace */
>
> V4L2CompatManager::V4L2CompatManager()
> - : cm_(nullptr), initialized_(false)
> + : cm_(nullptr)
> {
> get_symbol(fops_.openat, "openat");
> get_symbol(fops_.dup, "dup");
> @@ -52,24 +52,15 @@ V4L2CompatManager::~V4L2CompatManager()
> devices_.clear();
> mmaps_.clear();
>
> - if (isRunning()) {
> - exit(0);
> - /* \todo Wait with a timeout, just in case. */
> - wait();
> + if (cm_) {
> + proxies_.clear();
> + cm_->stop();
> + delete cm_;
> + cm_ = nullptr;
> }
> }
>
> -int V4L2CompatManager::init()
> -{
> - start();
> -
> - MutexLocker locker(mutex_);
> - cv_.wait(locker, [&] { return initialized_; });
> -
> - return 0;
> -}
> -
> -void V4L2CompatManager::run()
> +int V4L2CompatManager::start()
> {
> cm_ = new CameraManager();
>
> @@ -77,7 +68,9 @@ void V4L2CompatManager::run()
> if (ret) {
> LOG(V4L2Compat, Error) << "Failed to start camera manager: "
> << strerror(-ret);
> - return;
> + delete cm_;
> + cm_ = nullptr;
> + return ret;
> }
>
> LOG(V4L2Compat, Debug) << "Started camera manager";
> @@ -93,22 +86,7 @@ void V4L2CompatManager::run()
> ++index;
> }
>
> - /*
> - * libcamera has been initialized. Unlock the init() caller as we're
> - * now ready to handle calls from the framework.
> - */
> - mutex_.lock();
> - initialized_ = true;
> - mutex_.unlock();
> - cv_.notify_one();
> -
> - /* Now start processing events and messages. */
> - exec();
> -
> - proxies_.clear();
> - cm_->stop();
> - delete cm_;
> - cm_ = nullptr;
> + return 0;
> }
>
> V4L2CompatManager *V4L2CompatManager::instance()
> @@ -159,8 +137,8 @@ int V4L2CompatManager::openat(int dirfd, const char *path, int oflag, mode_t mod
> major(statbuf.st_rdev) != 81)
> return fd;
>
> - if (!isRunning())
> - init();
> + if (!cm_)
> + start();
>
> ret = getCameraIndex(fd);
> if (ret < 0) {
> diff --git a/src/v4l2/v4l2_compat_manager.h b/src/v4l2/v4l2_compat_manager.h
> index d51b5953d930..872c7c3b10e8 100644
> --- a/src/v4l2/v4l2_compat_manager.h
> +++ b/src/v4l2/v4l2_compat_manager.h
> @@ -8,22 +8,19 @@
> #ifndef __V4L2_COMPAT_MANAGER_H__
> #define __V4L2_COMPAT_MANAGER_H__
>
> -#include <condition_variable>
> #include <fcntl.h>
> #include <map>
> #include <memory>
> -#include <mutex>
> #include <sys/types.h>
> #include <vector>
>
> #include <libcamera/camera_manager.h>
>
> -#include "thread.h"
> #include "v4l2_camera_proxy.h"
>
> using namespace libcamera;
>
> -class V4L2CompatManager : public Thread
> +class V4L2CompatManager
> {
> public:
> struct FileOperations {
> @@ -46,8 +43,6 @@ public:
>
> static V4L2CompatManager *instance();
>
> - int init();
> -
> V4L2CameraProxy *getProxy(int fd);
> const FileOperations &fops() const { return fops_; }
>
> @@ -64,17 +59,13 @@ private:
> V4L2CompatManager();
> ~V4L2CompatManager();
>
> - void run() override;
> + int start();
> int getCameraIndex(int fd);
>
> FileOperations fops_;
>
> CameraManager *cm_;
>
> - std::mutex mutex_;
> - std::condition_variable cv_;
> - bool initialized_;
> -
> std::vector<std::unique_ptr<V4L2CameraProxy>> proxies_;
> std::map<int, V4L2CameraProxy *> devices_;
> std::map<void *, V4L2CameraProxy *> mmaps_;
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list