@@ -97,7 +97,7 @@ for Python bindings: [optional]
pybind11-dev
for lc-compliance: [optional]
- libevent-dev libgtest-dev
+ libgtest-dev
for abi-compat.sh: [optional]
abi-compliance-checker
@@ -12,7 +12,7 @@
using namespace libcamera;
Capture::Capture(std::shared_ptr<Camera> camera)
- : loop_(nullptr), camera_(std::move(camera)),
+ : camera_(std::move(camera)),
allocator_(camera_)
{
}
@@ -52,6 +52,8 @@ void Capture::start()
camera_->requestCompleted.connect(this, &Capture::requestComplete);
+ result_.reset();
+
ASSERT_EQ(camera_->start(), 0) << "Failed to start camera";
}
@@ -62,6 +64,8 @@ void Capture::stop()
camera_->stop();
+ result_.reset();
+
camera_->requestCompleted.disconnect(this);
Stream *stream = config_->at(0).stream();
@@ -108,11 +112,10 @@ void CaptureBalanced::capture(unsigned int numRequests)
}
/* Run capture session. */
- loop_ = new EventLoop();
- loop_->exec();
+ int status = result_.wait();
stop();
- delete loop_;
+ ASSERT_EQ(status, 0);
ASSERT_EQ(captureCount_, captureLimit_);
}
@@ -132,13 +135,13 @@ void CaptureBalanced::requestComplete(Request *request)
captureCount_++;
if (captureCount_ >= captureLimit_) {
- loop_->exit(0);
+ result_.set(0);
return;
}
request->reuse(Request::ReuseBuffers);
if (queueRequest(request))
- loop_->exit(-EINVAL);
+ result_.set(-EINVAL);
}
/* CaptureUnbalanced */
@@ -171,10 +174,8 @@ void CaptureUnbalanced::capture(unsigned int numRequests)
}
/* Run capture session. */
- loop_ = new EventLoop();
- int status = loop_->exec();
+ int status = result_.wait();
stop();
- delete loop_;
ASSERT_EQ(status, 0);
}
@@ -183,7 +184,7 @@ void CaptureUnbalanced::requestComplete(Request *request)
{
captureCount_++;
if (captureCount_ >= captureLimit_) {
- loop_->exit(0);
+ result_.set(0);
return;
}
@@ -192,5 +193,5 @@ void CaptureUnbalanced::requestComplete(Request *request)
request->reuse(Request::ReuseBuffers);
if (camera_->queueRequest(request))
- loop_->exit(-EINVAL);
+ result_.set(-EINVAL);
}
@@ -7,12 +7,13 @@
#pragma once
+#include <condition_variable>
#include <memory>
+#include <mutex>
+#include <optional>
#include <libcamera/libcamera.h>
-#include "../common/event_loop.h"
-
class Capture
{
public:
@@ -27,12 +28,45 @@ protected:
virtual void requestComplete(libcamera::Request *request) = 0;
- EventLoop *loop_;
-
std::shared_ptr<libcamera::Camera> camera_;
libcamera::FrameBufferAllocator allocator_;
std::unique_ptr<libcamera::CameraConfiguration> config_;
std::vector<std::unique_ptr<libcamera::Request>> requests_;
+
+ struct
+ {
+ private:
+ std::mutex mutex_;
+ std::condition_variable cv_;
+ std::optional<int> value_;
+
+ public:
+ int wait()
+ {
+ std::unique_lock guard(mutex_);
+
+ cv_.wait(guard, [&] {
+ return value_.has_value();
+ });
+
+ return *value_;
+ }
+
+ void set(int value)
+ {
+ std::unique_lock guard(mutex_);
+
+ if (!value_)
+ value_ = value;
+
+ cv_.notify_all();
+ }
+
+ void reset()
+ {
+ value_.reset();
+ }
+ } result_;
};
class CaptureBalanced : public Capture
@@ -4,13 +4,11 @@ libgtest = dependency('gtest', version : '>=1.10.0',
required : get_option('lc-compliance'),
fallback : ['gtest', 'gtest_dep'])
-if opt_lc_compliance.disabled() or not libevent.found() or not libgtest.found()
- lc_compliance_enabled = false
+lc_compliance_enabled = opt_lc_compliance.allowed() and libgtest.found()
+if not lc_compliance_enabled
subdir_done()
endif
-lc_compliance_enabled = true
-
lc_compliance_sources = files([
'environment.cpp',
'helpers/capture.cpp',
@@ -29,7 +27,6 @@ lc_compliance = executable('lc-compliance', lc_compliance_sources,
dependencies : [
libatomic,
libcamera_public,
- libevent,
libgtest,
],
include_directories : lc_compliance_includes,
@@ -3,12 +3,7 @@
opt_cam = get_option('cam')
opt_lc_compliance = get_option('lc-compliance')
-# libevent is needed by cam and lc-compliance. As they are both feature options,
-# they can't be combined with simple boolean logic.
libevent = dependency('libevent_pthreads', required : opt_cam)
-if not libevent.found()
- libevent = dependency('libevent_pthreads', required : opt_lc_compliance)
-endif
libtiff = dependency('libtiff-4', required : false)
libevent is only used as a way to notify the main thread from the CameraManager thread when the capture should be terminated. Not only is libevent not really necessary and instead can be replaced with a simple combination of C++ STL parts, the current usage is prone to race conditions. Specifically, the camera's `queueRequest()` might complete the request before the main thread could set the `loop_` member variable and synchronize with the thread. This is a data race, practically resulting in a nullptr or dangling pointer dereference. This can easily be triggered by inserting a sufficiently large timeout before `loop_ = new EventLoop;`: [46:43:40.529620351] [671833] DEBUG Request request.cpp:129 Request(4:C:0/1:0) ../src/apps/lc-compliance/helpers/capture.cpp:140:14: runtime error: member call on null pointer of type 'struct EventLoop' 0 0x632c3e82f81a in CaptureBalanced::requestComplete(libcamera::Request*) ../src/apps/lc-compliance/helpers/capture.cpp:140 Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> --- README.rst | 2 +- src/apps/lc-compliance/helpers/capture.cpp | 23 ++++++------ src/apps/lc-compliance/helpers/capture.h | 42 +++++++++++++++++++--- src/apps/lc-compliance/meson.build | 7 ++-- src/apps/meson.build | 5 --- 5 files changed, 53 insertions(+), 26 deletions(-)