| Message ID | 20201113063815.10288-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
On 13/11/2020 06:38, Laurent Pinchart wrote: > To prepare for removal of the EventDispatcher from the libcamera public > API, switch to libevent to handle the event loop. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> This is missing an update to the README.rst to update the new package dependencies. And a small whitespace nit below. With those: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/cam/event_loop.cpp | 34 +++++++++++++++++++++++++++------- > src/cam/event_loop.h | 15 ++++++++------- > src/cam/main.cpp | 16 +++++----------- > src/cam/meson.build | 13 ++++++++++++- > 4 files changed, 52 insertions(+), 26 deletions(-) > > diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp > index e8ab861790ed..13f2583da0a1 100644 > --- a/src/cam/event_loop.cpp > +++ b/src/cam/event_loop.cpp > @@ -5,19 +5,34 @@ > * event_loop.cpp - cam - Event loop > */ > > -#include <libcamera/event_dispatcher.h> > - > #include "event_loop.h" > > -using namespace libcamera; > +#include <assert.h> > +#include <event2/event.h> > +#include <event2/thread.h> > > -EventLoop::EventLoop(EventDispatcher *dispatcher) > - : dispatcher_(dispatcher) > +EventLoop *EventLoop::instance_ = nullptr; > + > +EventLoop::EventLoop() > { > + assert(!instance_); > + > + evthread_use_pthreads(); > + event_ = event_base_new(); > + instance_ = this; > } > > EventLoop::~EventLoop() > { > + instance_ = nullptr; > + > + event_base_free(event_); > + libevent_global_shutdown(); > +} > + > +EventLoop *EventLoop::instance() > +{ > + return instance_; > } > > int EventLoop::exec() > @@ -26,7 +41,7 @@ int EventLoop::exec() > exit_.store(false, std::memory_order_release); > > while (!exit_.load(std::memory_order_acquire)) > - dispatcher_->processEvents(); > + event_base_loop(event_, EVLOOP_NO_EXIT_ON_EMPTY); > > return exitCode_; > } > @@ -35,5 +50,10 @@ void EventLoop::exit(int code) > { > exitCode_ = code; > exit_.store(true, std::memory_order_release); > - dispatcher_->interrupt(); > + interrupt(); > +} > + > +void EventLoop::interrupt() > +{ > + event_base_loopbreak(event_); > } > diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h > index 581c7cba2fc4..b1c6bd103080 100644 > --- a/src/cam/event_loop.h > +++ b/src/cam/event_loop.h > @@ -9,26 +9,27 @@ > > #include <atomic> > > -#include <libcamera/event_notifier.h> > - > -namespace libcamera { > -class EventDispatcher; > -} > +struct event_base; > > class EventLoop > { > public: > - EventLoop(libcamera::EventDispatcher *dispatcher); > + EventLoop(); > ~EventLoop(); > > + static EventLoop *instance(); > + > int exec(); > void exit(int code = 0); > > private: > - libcamera::EventDispatcher *dispatcher_; > + static EventLoop *instance_; > > + struct event_base *event_; > std::atomic<bool> exit_; > int exitCode_; > + > + void interrupt(); > }; > > #endif /* __CAM_EVENT_LOOP_H__ */ > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > index b3a8d94f5739..e01be63a7058 100644 > --- a/src/cam/main.cpp > +++ b/src/cam/main.cpp > @@ -52,7 +52,7 @@ private: > CameraManager *cm_; > std::shared_ptr<Camera> camera_; > std::unique_ptr<libcamera::CameraConfiguration> config_; > - EventLoop *loop_; > + EventLoop loop_; > > bool strictFormats_; > }; > @@ -60,7 +60,7 @@ private: > CamApp *CamApp::app_ = nullptr; > > CamApp::CamApp() > - : cm_(nullptr), camera_(nullptr), config_(nullptr), loop_(nullptr), > + : cm_(nullptr), camera_(nullptr), config_(nullptr), > strictFormats_(false) > { > CamApp::app_ = this; > @@ -134,16 +134,11 @@ int CamApp::init(int argc, char **argv) > std::cout << "Monitoring new hotplug and unplug events" << std::endl; > } > > - loop_ = new EventLoop(cm_->eventDispatcher()); > - > return 0; > } > > void CamApp::cleanup() > { > - delete loop_; > - loop_ = nullptr; > - > if (camera_) { > camera_->release(); > camera_.reset(); > @@ -166,8 +161,7 @@ int CamApp::exec() > > void CamApp::quit() > { > - if (loop_) > - loop_->exit(); > + loop_.exit(); > } > > int CamApp::parseOptions(int argc, char *argv[]) > @@ -366,13 +360,13 @@ int CamApp::run() > } > > if (options_.isSet(OptCapture)) { > - Capture capture(camera_, config_.get(), loop_); > + Capture capture(camera_, config_.get(), &loop_); > return capture.run(options_); > } > > if (options_.isSet(OptMonitor)) { > std::cout << "Press Ctrl-C to interrupt" << std::endl; > - ret = loop_->exec(); > + ret = loop_.exec(); > if (ret) > std::cout << "Failed to run monitor loop" << std::endl; > } > diff --git a/src/cam/meson.build b/src/cam/meson.build > index 89e124fbae2a..62003c58dad0 100644 > --- a/src/cam/meson.build > +++ b/src/cam/meson.build > @@ -1,5 +1,12 @@ > # SPDX-License-Identifier: CC0-1.0 > > +libevent = dependency('libevent_pthreads', required : false) > + > +if not libevent.found() > + warning('libevent_pthreads not found,\'cam\' application will not be compiled') missing a space after comma, > + subdir_done() > +endif > + > cam_sources = files([ > 'buffer_writer.cpp', > 'capture.cpp', > @@ -10,5 +17,9 @@ cam_sources = files([ > ]) > > cam = executable('cam', cam_sources, > - dependencies : [ libatomic, libcamera_dep ], > + dependencies : [ > + libatomic, > + libcamera_dep, > + libevent, > + ], > install : true) >
On Fri, Nov 13, 2020 at 09:59:02AM +0000, Kieran Bingham wrote: > On 13/11/2020 06:38, Laurent Pinchart wrote: > > To prepare for removal of the EventDispatcher from the libcamera public > > API, switch to libevent to handle the event loop. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > This is missing an update to the README.rst to update the new package > dependencies. diff --git a/README.rst b/README.rst index 5c4b6989f7cf..251291b77c62 100644 --- a/README.rst +++ b/README.rst @@ -78,6 +78,9 @@ for documentation: [optional] for gstreamer: [optional] libgstreamer1.0-dev libgstreamer-plugins-base1.0-dev +for cam: [optional] + libevent-dev + for qcam: [optional] qtbase5-dev libqt5core5a libqt5gui5 libqt5widgets5 qttools5-dev-tools libtiff-dev > And a small whitespace nit below. > > With those: > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > --- > > src/cam/event_loop.cpp | 34 +++++++++++++++++++++++++++------- > > src/cam/event_loop.h | 15 ++++++++------- > > src/cam/main.cpp | 16 +++++----------- > > src/cam/meson.build | 13 ++++++++++++- > > 4 files changed, 52 insertions(+), 26 deletions(-) > > > > diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp > > index e8ab861790ed..13f2583da0a1 100644 > > --- a/src/cam/event_loop.cpp > > +++ b/src/cam/event_loop.cpp > > @@ -5,19 +5,34 @@ > > * event_loop.cpp - cam - Event loop > > */ > > > > -#include <libcamera/event_dispatcher.h> > > - > > #include "event_loop.h" > > > > -using namespace libcamera; > > +#include <assert.h> > > +#include <event2/event.h> > > +#include <event2/thread.h> > > > > -EventLoop::EventLoop(EventDispatcher *dispatcher) > > - : dispatcher_(dispatcher) > > +EventLoop *EventLoop::instance_ = nullptr; > > + > > +EventLoop::EventLoop() > > { > > + assert(!instance_); > > + > > + evthread_use_pthreads(); > > + event_ = event_base_new(); > > + instance_ = this; > > } > > > > EventLoop::~EventLoop() > > { > > + instance_ = nullptr; > > + > > + event_base_free(event_); > > + libevent_global_shutdown(); > > +} > > + > > +EventLoop *EventLoop::instance() > > +{ > > + return instance_; > > } > > > > int EventLoop::exec() > > @@ -26,7 +41,7 @@ int EventLoop::exec() > > exit_.store(false, std::memory_order_release); > > > > while (!exit_.load(std::memory_order_acquire)) > > - dispatcher_->processEvents(); > > + event_base_loop(event_, EVLOOP_NO_EXIT_ON_EMPTY); > > > > return exitCode_; > > } > > @@ -35,5 +50,10 @@ void EventLoop::exit(int code) > > { > > exitCode_ = code; > > exit_.store(true, std::memory_order_release); > > - dispatcher_->interrupt(); > > + interrupt(); > > +} > > + > > +void EventLoop::interrupt() > > +{ > > + event_base_loopbreak(event_); > > } > > diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h > > index 581c7cba2fc4..b1c6bd103080 100644 > > --- a/src/cam/event_loop.h > > +++ b/src/cam/event_loop.h > > @@ -9,26 +9,27 @@ > > > > #include <atomic> > > > > -#include <libcamera/event_notifier.h> > > - > > -namespace libcamera { > > -class EventDispatcher; > > -} > > +struct event_base; > > > > class EventLoop > > { > > public: > > - EventLoop(libcamera::EventDispatcher *dispatcher); > > + EventLoop(); > > ~EventLoop(); > > > > + static EventLoop *instance(); > > + > > int exec(); > > void exit(int code = 0); > > > > private: > > - libcamera::EventDispatcher *dispatcher_; > > + static EventLoop *instance_; > > > > + struct event_base *event_; > > std::atomic<bool> exit_; > > int exitCode_; > > + > > + void interrupt(); > > }; > > > > #endif /* __CAM_EVENT_LOOP_H__ */ > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > > index b3a8d94f5739..e01be63a7058 100644 > > --- a/src/cam/main.cpp > > +++ b/src/cam/main.cpp > > @@ -52,7 +52,7 @@ private: > > CameraManager *cm_; > > std::shared_ptr<Camera> camera_; > > std::unique_ptr<libcamera::CameraConfiguration> config_; > > - EventLoop *loop_; > > + EventLoop loop_; > > > > bool strictFormats_; > > }; > > @@ -60,7 +60,7 @@ private: > > CamApp *CamApp::app_ = nullptr; > > > > CamApp::CamApp() > > - : cm_(nullptr), camera_(nullptr), config_(nullptr), loop_(nullptr), > > + : cm_(nullptr), camera_(nullptr), config_(nullptr), > > strictFormats_(false) > > { > > CamApp::app_ = this; > > @@ -134,16 +134,11 @@ int CamApp::init(int argc, char **argv) > > std::cout << "Monitoring new hotplug and unplug events" << std::endl; > > } > > > > - loop_ = new EventLoop(cm_->eventDispatcher()); > > - > > return 0; > > } > > > > void CamApp::cleanup() > > { > > - delete loop_; > > - loop_ = nullptr; > > - > > if (camera_) { > > camera_->release(); > > camera_.reset(); > > @@ -166,8 +161,7 @@ int CamApp::exec() > > > > void CamApp::quit() > > { > > - if (loop_) > > - loop_->exit(); > > + loop_.exit(); > > } > > > > int CamApp::parseOptions(int argc, char *argv[]) > > @@ -366,13 +360,13 @@ int CamApp::run() > > } > > > > if (options_.isSet(OptCapture)) { > > - Capture capture(camera_, config_.get(), loop_); > > + Capture capture(camera_, config_.get(), &loop_); > > return capture.run(options_); > > } > > > > if (options_.isSet(OptMonitor)) { > > std::cout << "Press Ctrl-C to interrupt" << std::endl; > > - ret = loop_->exec(); > > + ret = loop_.exec(); > > if (ret) > > std::cout << "Failed to run monitor loop" << std::endl; > > } > > diff --git a/src/cam/meson.build b/src/cam/meson.build > > index 89e124fbae2a..62003c58dad0 100644 > > --- a/src/cam/meson.build > > +++ b/src/cam/meson.build > > @@ -1,5 +1,12 @@ > > # SPDX-License-Identifier: CC0-1.0 > > > > +libevent = dependency('libevent_pthreads', required : false) > > + > > +if not libevent.found() > > + warning('libevent_pthreads not found,\'cam\' application will not be compiled') > > missing a space after comma, > > > + subdir_done() > > +endif > > + > > cam_sources = files([ > > 'buffer_writer.cpp', > > 'capture.cpp', > > @@ -10,5 +17,9 @@ cam_sources = files([ > > ]) > > > > cam = executable('cam', cam_sources, > > - dependencies : [ libatomic, libcamera_dep ], > > + dependencies : [ > > + libatomic, > > + libcamera_dep, > > + libevent, > > + ], > > install : true) > >
On 13/11/2020 10:08, Laurent Pinchart wrote: > On Fri, Nov 13, 2020 at 09:59:02AM +0000, Kieran Bingham wrote: >> On 13/11/2020 06:38, Laurent Pinchart wrote: >>> To prepare for removal of the EventDispatcher from the libcamera public >>> API, switch to libevent to handle the event loop. >>> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> >> This is missing an update to the README.rst to update the new package >> dependencies. > > diff --git a/README.rst b/README.rst > index 5c4b6989f7cf..251291b77c62 100644 > --- a/README.rst > +++ b/README.rst > @@ -78,6 +78,9 @@ for documentation: [optional] > for gstreamer: [optional] > libgstreamer1.0-dev libgstreamer-plugins-base1.0-dev > > +for cam: [optional] > + libevent-dev > + Looks good to me ;-) Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > for qcam: [optional] > qtbase5-dev libqt5core5a libqt5gui5 libqt5widgets5 qttools5-dev-tools libtiff-dev > >> And a small whitespace nit below. >> >> With those: >> >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> >>> --- >>> src/cam/event_loop.cpp | 34 +++++++++++++++++++++++++++------- >>> src/cam/event_loop.h | 15 ++++++++------- >>> src/cam/main.cpp | 16 +++++----------- >>> src/cam/meson.build | 13 ++++++++++++- >>> 4 files changed, 52 insertions(+), 26 deletions(-) >>> >>> diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp >>> index e8ab861790ed..13f2583da0a1 100644 >>> --- a/src/cam/event_loop.cpp >>> +++ b/src/cam/event_loop.cpp >>> @@ -5,19 +5,34 @@ >>> * event_loop.cpp - cam - Event loop >>> */ >>> >>> -#include <libcamera/event_dispatcher.h> >>> - >>> #include "event_loop.h" >>> >>> -using namespace libcamera; >>> +#include <assert.h> >>> +#include <event2/event.h> >>> +#include <event2/thread.h> >>> >>> -EventLoop::EventLoop(EventDispatcher *dispatcher) >>> - : dispatcher_(dispatcher) >>> +EventLoop *EventLoop::instance_ = nullptr; >>> + >>> +EventLoop::EventLoop() >>> { >>> + assert(!instance_); >>> + >>> + evthread_use_pthreads(); >>> + event_ = event_base_new(); >>> + instance_ = this; >>> } >>> >>> EventLoop::~EventLoop() >>> { >>> + instance_ = nullptr; >>> + >>> + event_base_free(event_); >>> + libevent_global_shutdown(); >>> +} >>> + >>> +EventLoop *EventLoop::instance() >>> +{ >>> + return instance_; >>> } >>> >>> int EventLoop::exec() >>> @@ -26,7 +41,7 @@ int EventLoop::exec() >>> exit_.store(false, std::memory_order_release); >>> >>> while (!exit_.load(std::memory_order_acquire)) >>> - dispatcher_->processEvents(); >>> + event_base_loop(event_, EVLOOP_NO_EXIT_ON_EMPTY); >>> >>> return exitCode_; >>> } >>> @@ -35,5 +50,10 @@ void EventLoop::exit(int code) >>> { >>> exitCode_ = code; >>> exit_.store(true, std::memory_order_release); >>> - dispatcher_->interrupt(); >>> + interrupt(); >>> +} >>> + >>> +void EventLoop::interrupt() >>> +{ >>> + event_base_loopbreak(event_); >>> } >>> diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h >>> index 581c7cba2fc4..b1c6bd103080 100644 >>> --- a/src/cam/event_loop.h >>> +++ b/src/cam/event_loop.h >>> @@ -9,26 +9,27 @@ >>> >>> #include <atomic> >>> >>> -#include <libcamera/event_notifier.h> >>> - >>> -namespace libcamera { >>> -class EventDispatcher; >>> -} >>> +struct event_base; >>> >>> class EventLoop >>> { >>> public: >>> - EventLoop(libcamera::EventDispatcher *dispatcher); >>> + EventLoop(); >>> ~EventLoop(); >>> >>> + static EventLoop *instance(); >>> + >>> int exec(); >>> void exit(int code = 0); >>> >>> private: >>> - libcamera::EventDispatcher *dispatcher_; >>> + static EventLoop *instance_; >>> >>> + struct event_base *event_; >>> std::atomic<bool> exit_; >>> int exitCode_; >>> + >>> + void interrupt(); >>> }; >>> >>> #endif /* __CAM_EVENT_LOOP_H__ */ >>> diff --git a/src/cam/main.cpp b/src/cam/main.cpp >>> index b3a8d94f5739..e01be63a7058 100644 >>> --- a/src/cam/main.cpp >>> +++ b/src/cam/main.cpp >>> @@ -52,7 +52,7 @@ private: >>> CameraManager *cm_; >>> std::shared_ptr<Camera> camera_; >>> std::unique_ptr<libcamera::CameraConfiguration> config_; >>> - EventLoop *loop_; >>> + EventLoop loop_; >>> >>> bool strictFormats_; >>> }; >>> @@ -60,7 +60,7 @@ private: >>> CamApp *CamApp::app_ = nullptr; >>> >>> CamApp::CamApp() >>> - : cm_(nullptr), camera_(nullptr), config_(nullptr), loop_(nullptr), >>> + : cm_(nullptr), camera_(nullptr), config_(nullptr), >>> strictFormats_(false) >>> { >>> CamApp::app_ = this; >>> @@ -134,16 +134,11 @@ int CamApp::init(int argc, char **argv) >>> std::cout << "Monitoring new hotplug and unplug events" << std::endl; >>> } >>> >>> - loop_ = new EventLoop(cm_->eventDispatcher()); >>> - >>> return 0; >>> } >>> >>> void CamApp::cleanup() >>> { >>> - delete loop_; >>> - loop_ = nullptr; >>> - >>> if (camera_) { >>> camera_->release(); >>> camera_.reset(); >>> @@ -166,8 +161,7 @@ int CamApp::exec() >>> >>> void CamApp::quit() >>> { >>> - if (loop_) >>> - loop_->exit(); >>> + loop_.exit(); >>> } >>> >>> int CamApp::parseOptions(int argc, char *argv[]) >>> @@ -366,13 +360,13 @@ int CamApp::run() >>> } >>> >>> if (options_.isSet(OptCapture)) { >>> - Capture capture(camera_, config_.get(), loop_); >>> + Capture capture(camera_, config_.get(), &loop_); >>> return capture.run(options_); >>> } >>> >>> if (options_.isSet(OptMonitor)) { >>> std::cout << "Press Ctrl-C to interrupt" << std::endl; >>> - ret = loop_->exec(); >>> + ret = loop_.exec(); >>> if (ret) >>> std::cout << "Failed to run monitor loop" << std::endl; >>> } >>> diff --git a/src/cam/meson.build b/src/cam/meson.build >>> index 89e124fbae2a..62003c58dad0 100644 >>> --- a/src/cam/meson.build >>> +++ b/src/cam/meson.build >>> @@ -1,5 +1,12 @@ >>> # SPDX-License-Identifier: CC0-1.0 >>> >>> +libevent = dependency('libevent_pthreads', required : false) >>> + >>> +if not libevent.found() >>> + warning('libevent_pthreads not found,\'cam\' application will not be compiled') >> >> missing a space after comma, >> >>> + subdir_done() >>> +endif >>> + >>> cam_sources = files([ >>> 'buffer_writer.cpp', >>> 'capture.cpp', >>> @@ -10,5 +17,9 @@ cam_sources = files([ >>> ]) >>> >>> cam = executable('cam', cam_sources, >>> - dependencies : [ libatomic, libcamera_dep ], >>> + dependencies : [ >>> + libatomic, >>> + libcamera_dep, >>> + libevent, >>> + ], >>> install : true) >>> >
Hi Laurent, Thanks for your work. On 2020-11-13 08:38:12 +0200, Laurent Pinchart wrote: > To prepare for removal of the EventDispatcher from the libcamera public > API, switch to libevent to handle the event loop. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> With the issue pointed out by Kieran fixed, Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/cam/event_loop.cpp | 34 +++++++++++++++++++++++++++------- > src/cam/event_loop.h | 15 ++++++++------- > src/cam/main.cpp | 16 +++++----------- > src/cam/meson.build | 13 ++++++++++++- > 4 files changed, 52 insertions(+), 26 deletions(-) > > diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp > index e8ab861790ed..13f2583da0a1 100644 > --- a/src/cam/event_loop.cpp > +++ b/src/cam/event_loop.cpp > @@ -5,19 +5,34 @@ > * event_loop.cpp - cam - Event loop > */ > > -#include <libcamera/event_dispatcher.h> > - > #include "event_loop.h" > > -using namespace libcamera; > +#include <assert.h> > +#include <event2/event.h> > +#include <event2/thread.h> > > -EventLoop::EventLoop(EventDispatcher *dispatcher) > - : dispatcher_(dispatcher) > +EventLoop *EventLoop::instance_ = nullptr; > + > +EventLoop::EventLoop() > { > + assert(!instance_); > + > + evthread_use_pthreads(); > + event_ = event_base_new(); > + instance_ = this; > } > > EventLoop::~EventLoop() > { > + instance_ = nullptr; > + > + event_base_free(event_); > + libevent_global_shutdown(); > +} > + > +EventLoop *EventLoop::instance() > +{ > + return instance_; > } > > int EventLoop::exec() > @@ -26,7 +41,7 @@ int EventLoop::exec() > exit_.store(false, std::memory_order_release); > > while (!exit_.load(std::memory_order_acquire)) > - dispatcher_->processEvents(); > + event_base_loop(event_, EVLOOP_NO_EXIT_ON_EMPTY); > > return exitCode_; > } > @@ -35,5 +50,10 @@ void EventLoop::exit(int code) > { > exitCode_ = code; > exit_.store(true, std::memory_order_release); > - dispatcher_->interrupt(); > + interrupt(); > +} > + > +void EventLoop::interrupt() > +{ > + event_base_loopbreak(event_); > } > diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h > index 581c7cba2fc4..b1c6bd103080 100644 > --- a/src/cam/event_loop.h > +++ b/src/cam/event_loop.h > @@ -9,26 +9,27 @@ > > #include <atomic> > > -#include <libcamera/event_notifier.h> > - > -namespace libcamera { > -class EventDispatcher; > -} > +struct event_base; > > class EventLoop > { > public: > - EventLoop(libcamera::EventDispatcher *dispatcher); > + EventLoop(); > ~EventLoop(); > > + static EventLoop *instance(); > + > int exec(); > void exit(int code = 0); > > private: > - libcamera::EventDispatcher *dispatcher_; > + static EventLoop *instance_; > > + struct event_base *event_; > std::atomic<bool> exit_; > int exitCode_; > + > + void interrupt(); > }; > > #endif /* __CAM_EVENT_LOOP_H__ */ > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > index b3a8d94f5739..e01be63a7058 100644 > --- a/src/cam/main.cpp > +++ b/src/cam/main.cpp > @@ -52,7 +52,7 @@ private: > CameraManager *cm_; > std::shared_ptr<Camera> camera_; > std::unique_ptr<libcamera::CameraConfiguration> config_; > - EventLoop *loop_; > + EventLoop loop_; > > bool strictFormats_; > }; > @@ -60,7 +60,7 @@ private: > CamApp *CamApp::app_ = nullptr; > > CamApp::CamApp() > - : cm_(nullptr), camera_(nullptr), config_(nullptr), loop_(nullptr), > + : cm_(nullptr), camera_(nullptr), config_(nullptr), > strictFormats_(false) > { > CamApp::app_ = this; > @@ -134,16 +134,11 @@ int CamApp::init(int argc, char **argv) > std::cout << "Monitoring new hotplug and unplug events" << std::endl; > } > > - loop_ = new EventLoop(cm_->eventDispatcher()); > - > return 0; > } > > void CamApp::cleanup() > { > - delete loop_; > - loop_ = nullptr; > - > if (camera_) { > camera_->release(); > camera_.reset(); > @@ -166,8 +161,7 @@ int CamApp::exec() > > void CamApp::quit() > { > - if (loop_) > - loop_->exit(); > + loop_.exit(); > } > > int CamApp::parseOptions(int argc, char *argv[]) > @@ -366,13 +360,13 @@ int CamApp::run() > } > > if (options_.isSet(OptCapture)) { > - Capture capture(camera_, config_.get(), loop_); > + Capture capture(camera_, config_.get(), &loop_); > return capture.run(options_); > } > > if (options_.isSet(OptMonitor)) { > std::cout << "Press Ctrl-C to interrupt" << std::endl; > - ret = loop_->exec(); > + ret = loop_.exec(); > if (ret) > std::cout << "Failed to run monitor loop" << std::endl; > } > diff --git a/src/cam/meson.build b/src/cam/meson.build > index 89e124fbae2a..62003c58dad0 100644 > --- a/src/cam/meson.build > +++ b/src/cam/meson.build > @@ -1,5 +1,12 @@ > # SPDX-License-Identifier: CC0-1.0 > > +libevent = dependency('libevent_pthreads', required : false) > + > +if not libevent.found() > + warning('libevent_pthreads not found,\'cam\' application will not be compiled') > + subdir_done() > +endif > + > cam_sources = files([ > 'buffer_writer.cpp', > 'capture.cpp', > @@ -10,5 +17,9 @@ cam_sources = files([ > ]) > > cam = executable('cam', cam_sources, > - dependencies : [ libatomic, libcamera_dep ], > + dependencies : [ > + libatomic, > + libcamera_dep, > + libevent, > + ], > install : true) > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
On 13/11/2020 06:38, Laurent Pinchart wrote: > To prepare for removal of the EventDispatcher from the libcamera public > API, switch to libevent to handle the event loop. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/cam/event_loop.cpp | 34 +++++++++++++++++++++++++++------- > src/cam/event_loop.h | 15 ++++++++------- > src/cam/main.cpp | 16 +++++----------- > src/cam/meson.build | 13 ++++++++++++- > 4 files changed, 52 insertions(+), 26 deletions(-) > > diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp > index e8ab861790ed..13f2583da0a1 100644 > --- a/src/cam/event_loop.cpp > +++ b/src/cam/event_loop.cpp > @@ -5,19 +5,34 @@ > * event_loop.cpp - cam - Event loop > */ > > -#include <libcamera/event_dispatcher.h> > - > #include "event_loop.h" > > -using namespace libcamera; > +#include <assert.h> > +#include <event2/event.h> > +#include <event2/thread.h> > > -EventLoop::EventLoop(EventDispatcher *dispatcher) > - : dispatcher_(dispatcher) > +EventLoop *EventLoop::instance_ = nullptr; > + > +EventLoop::EventLoop() > { > + assert(!instance_); > + > + evthread_use_pthreads(); > + event_ = event_base_new(); > + instance_ = this; > } > > EventLoop::~EventLoop() > { > + instance_ = nullptr; > + > + event_base_free(event_); > + libevent_global_shutdown(); > +} > + > +EventLoop *EventLoop::instance() > +{ > + return instance_; > } > > int EventLoop::exec() > @@ -26,7 +41,7 @@ int EventLoop::exec() > exit_.store(false, std::memory_order_release); > > while (!exit_.load(std::memory_order_acquire)) > - dispatcher_->processEvents(); > + event_base_loop(event_, EVLOOP_NO_EXIT_ON_EMPTY); > > return exitCode_; > } > @@ -35,5 +50,10 @@ void EventLoop::exit(int code) > { > exitCode_ = code; > exit_.store(true, std::memory_order_release); > - dispatcher_->interrupt(); > + interrupt(); > +} > + > +void EventLoop::interrupt() > +{ > + event_base_loopbreak(event_); > } > diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h > index 581c7cba2fc4..b1c6bd103080 100644 > --- a/src/cam/event_loop.h > +++ b/src/cam/event_loop.h > @@ -9,26 +9,27 @@ > > #include <atomic> > > -#include <libcamera/event_notifier.h> > - > -namespace libcamera { > -class EventDispatcher; > -} > +struct event_base; > > class EventLoop > { > public: > - EventLoop(libcamera::EventDispatcher *dispatcher); > + EventLoop(); > ~EventLoop(); > > + static EventLoop *instance(); > + > int exec(); > void exit(int code = 0); > > private: > - libcamera::EventDispatcher *dispatcher_; > + static EventLoop *instance_; > > + struct event_base *event_; > std::atomic<bool> exit_; > int exitCode_; It might not have been introduced by this patch, but it seems to have been highlighted during this rebuild. Could you check if this is a real issue or a false positive please? If you supply a patch to fix this issue, please add the following tag: Reported-by: Coverity CID=305971 Alternatively, if you believe it's a false positive, let me know and I'll close it. Hi, Please find the latest report on new defect(s) introduced to libcamera found with Coverity Scan. 1 new defect(s) introduced to libcamera found with Coverity Scan. 1 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan. New defect(s) Reported-by: Coverity Scan Showing 1 of 1 defect(s) ** CID 305971: Uninitialized members (UNINIT_CTOR) /home/linuxembedded/iob/libcamera/libcamera-daily/src/cam/event_loop.cpp: 23 in EventLoop::EventLoop()()
Hi Kieran, On Mon, Nov 16, 2020 at 11:52:47AM +0000, Kieran Bingham wrote: > On 13/11/2020 06:38, Laurent Pinchart wrote: > > To prepare for removal of the EventDispatcher from the libcamera public > > API, switch to libevent to handle the event loop. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/cam/event_loop.cpp | 34 +++++++++++++++++++++++++++------- > > src/cam/event_loop.h | 15 ++++++++------- > > src/cam/main.cpp | 16 +++++----------- > > src/cam/meson.build | 13 ++++++++++++- > > 4 files changed, 52 insertions(+), 26 deletions(-) > > > > diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp > > index e8ab861790ed..13f2583da0a1 100644 > > --- a/src/cam/event_loop.cpp > > +++ b/src/cam/event_loop.cpp > > @@ -5,19 +5,34 @@ > > * event_loop.cpp - cam - Event loop > > */ > > > > -#include <libcamera/event_dispatcher.h> > > - > > #include "event_loop.h" > > > > -using namespace libcamera; > > +#include <assert.h> > > +#include <event2/event.h> > > +#include <event2/thread.h> > > > > -EventLoop::EventLoop(EventDispatcher *dispatcher) > > - : dispatcher_(dispatcher) > > +EventLoop *EventLoop::instance_ = nullptr; > > + > > +EventLoop::EventLoop() > > { > > + assert(!instance_); > > + > > + evthread_use_pthreads(); > > + event_ = event_base_new(); > > + instance_ = this; > > } > > > > EventLoop::~EventLoop() > > { > > + instance_ = nullptr; > > + > > + event_base_free(event_); > > + libevent_global_shutdown(); > > +} > > + > > +EventLoop *EventLoop::instance() > > +{ > > + return instance_; > > } > > > > int EventLoop::exec() > > @@ -26,7 +41,7 @@ int EventLoop::exec() > > exit_.store(false, std::memory_order_release); > > > > while (!exit_.load(std::memory_order_acquire)) > > - dispatcher_->processEvents(); > > + event_base_loop(event_, EVLOOP_NO_EXIT_ON_EMPTY); > > > > return exitCode_; > > } > > @@ -35,5 +50,10 @@ void EventLoop::exit(int code) > > { > > exitCode_ = code; > > exit_.store(true, std::memory_order_release); > > - dispatcher_->interrupt(); > > + interrupt(); > > +} > > + > > +void EventLoop::interrupt() > > +{ > > + event_base_loopbreak(event_); > > } > > diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h > > index 581c7cba2fc4..b1c6bd103080 100644 > > --- a/src/cam/event_loop.h > > +++ b/src/cam/event_loop.h > > @@ -9,26 +9,27 @@ > > > > #include <atomic> > > > > -#include <libcamera/event_notifier.h> > > - > > -namespace libcamera { > > -class EventDispatcher; > > -} > > +struct event_base; > > > > class EventLoop > > { > > public: > > - EventLoop(libcamera::EventDispatcher *dispatcher); > > + EventLoop(); > > ~EventLoop(); > > > > + static EventLoop *instance(); > > + > > int exec(); > > void exit(int code = 0); > > > > private: > > - libcamera::EventDispatcher *dispatcher_; > > + static EventLoop *instance_; > > > > + struct event_base *event_; > > std::atomic<bool> exit_; > > int exitCode_; > > It might not have been introduced by this patch, but it seems to have > been highlighted during this rebuild. Could you check if this is a real > issue or a false positive please? > > > If you supply a patch to fix this issue, please add the following tag: > > Reported-by: Coverity CID=305971 > > Alternatively, if you believe it's a false positive, let me know and > I'll close it. I've noticed the report. The diagnostic is right, exitCode_ is not initialized in the constructor, but that's not an issue as it's initialized in all code paths that may potentially use it. > Hi, > > Please find the latest report on new defect(s) introduced to libcamera > found with Coverity Scan. > > 1 new defect(s) introduced to libcamera found with Coverity Scan. > 1 defect(s), reported by Coverity Scan earlier, were marked fixed in the > recent build analyzed by Coverity Scan. > > New defect(s) Reported-by: Coverity Scan > Showing 1 of 1 defect(s) > > > ** CID 305971: Uninitialized members (UNINIT_CTOR) > /home/linuxembedded/iob/libcamera/libcamera-daily/src/cam/event_loop.cpp: 23 > in EventLoop::EventLoop()() > > > ________________________________________________________________________________________________________ > *** CID 305971: Uninitialized members (UNINIT_CTOR) > /home/linuxembedded/iob/libcamera/libcamera-daily/src/cam/event_loop.cpp: 23 > in EventLoop::EventLoop()() > 17 { > 18 assert(!instance_); > 19 > 20 evthread_use_pthreads(); > 21 event_ = event_base_new(); > 22 instance_ = this; > >>> CID 305971: Uninitialized members (UNINIT_CTOR) > >>> Non-static class member "exitCode_" is not initialized in this > constructor nor in any functions that it calls. > 23 } > 24 > 25 EventLoop::~EventLoop() > 26 { > 27 instance_ = nullptr; > 28 > > > + > > + void interrupt(); > > }; > > > > #endif /* __CAM_EVENT_LOOP_H__ */ > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > > index b3a8d94f5739..e01be63a7058 100644 > > --- a/src/cam/main.cpp > > +++ b/src/cam/main.cpp > > @@ -52,7 +52,7 @@ private: > > CameraManager *cm_; > > std::shared_ptr<Camera> camera_; > > std::unique_ptr<libcamera::CameraConfiguration> config_; > > - EventLoop *loop_; > > + EventLoop loop_; > > > > bool strictFormats_; > > }; > > @@ -60,7 +60,7 @@ private: > > CamApp *CamApp::app_ = nullptr; > > > > CamApp::CamApp() > > - : cm_(nullptr), camera_(nullptr), config_(nullptr), loop_(nullptr), > > + : cm_(nullptr), camera_(nullptr), config_(nullptr), > > strictFormats_(false) > > { > > CamApp::app_ = this; > > @@ -134,16 +134,11 @@ int CamApp::init(int argc, char **argv) > > std::cout << "Monitoring new hotplug and unplug events" << std::endl; > > } > > > > - loop_ = new EventLoop(cm_->eventDispatcher()); > > - > > return 0; > > } > > > > void CamApp::cleanup() > > { > > - delete loop_; > > - loop_ = nullptr; > > - > > if (camera_) { > > camera_->release(); > > camera_.reset(); > > @@ -166,8 +161,7 @@ int CamApp::exec() > > > > void CamApp::quit() > > { > > - if (loop_) > > - loop_->exit(); > > + loop_.exit(); > > } > > > > int CamApp::parseOptions(int argc, char *argv[]) > > @@ -366,13 +360,13 @@ int CamApp::run() > > } > > > > if (options_.isSet(OptCapture)) { > > - Capture capture(camera_, config_.get(), loop_); > > + Capture capture(camera_, config_.get(), &loop_); > > return capture.run(options_); > > } > > > > if (options_.isSet(OptMonitor)) { > > std::cout << "Press Ctrl-C to interrupt" << std::endl; > > - ret = loop_->exec(); > > + ret = loop_.exec(); > > if (ret) > > std::cout << "Failed to run monitor loop" << std::endl; > > } > > diff --git a/src/cam/meson.build b/src/cam/meson.build > > index 89e124fbae2a..62003c58dad0 100644 > > --- a/src/cam/meson.build > > +++ b/src/cam/meson.build > > @@ -1,5 +1,12 @@ > > # SPDX-License-Identifier: CC0-1.0 > > > > +libevent = dependency('libevent_pthreads', required : false) > > + > > +if not libevent.found() > > + warning('libevent_pthreads not found,\'cam\' application will not be compiled') > > + subdir_done() > > +endif > > + > > cam_sources = files([ > > 'buffer_writer.cpp', > > 'capture.cpp', > > @@ -10,5 +17,9 @@ cam_sources = files([ > > ]) > > > > cam = executable('cam', cam_sources, > > - dependencies : [ libatomic, libcamera_dep ], > > + dependencies : [ > > + libatomic, > > + libcamera_dep, > > + libevent, > > + ], > > install : true) > >
On 16/11/2020 14:52, Laurent Pinchart wrote: > Hi Kieran, >>> >>> + struct event_base *event_; >>> std::atomic<bool> exit_; >>> int exitCode_; >> >> It might not have been introduced by this patch, but it seems to have >> been highlighted during this rebuild. Could you check if this is a real >> issue or a false positive please? >> >> >> If you supply a patch to fix this issue, please add the following tag: >> >> Reported-by: Coverity CID=305971 >> >> Alternatively, if you believe it's a false positive, let me know and >> I'll close it. > > I've noticed the report. The diagnostic is right, exitCode_ is not > initialized in the constructor, but that's not an issue as it's > initialized in all code paths that may potentially use it. That's fine, I've closed this one. Thanks -- Kieran > >> Hi, >> >> Please find the latest report on new defect(s) introduced to libcamera >> found with Coverity Scan. >> >> 1 new defect(s) introduced to libcamera found with Coverity Scan. >> 1 defect(s), reported by Coverity Scan earlier, were marked fixed in the >> recent build analyzed by Coverity Scan. >> >> New defect(s) Reported-by: Coverity Scan >> Showing 1 of 1 defect(s) >> >> >> ** CID 305971: Uninitialized members (UNINIT_CTOR) >> /home/linuxembedded/iob/libcamera/libcamera-daily/src/cam/event_loop.cpp: 23 >> in EventLoop::EventLoop()() >>
diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp index e8ab861790ed..13f2583da0a1 100644 --- a/src/cam/event_loop.cpp +++ b/src/cam/event_loop.cpp @@ -5,19 +5,34 @@ * event_loop.cpp - cam - Event loop */ -#include <libcamera/event_dispatcher.h> - #include "event_loop.h" -using namespace libcamera; +#include <assert.h> +#include <event2/event.h> +#include <event2/thread.h> -EventLoop::EventLoop(EventDispatcher *dispatcher) - : dispatcher_(dispatcher) +EventLoop *EventLoop::instance_ = nullptr; + +EventLoop::EventLoop() { + assert(!instance_); + + evthread_use_pthreads(); + event_ = event_base_new(); + instance_ = this; } EventLoop::~EventLoop() { + instance_ = nullptr; + + event_base_free(event_); + libevent_global_shutdown(); +} + +EventLoop *EventLoop::instance() +{ + return instance_; } int EventLoop::exec() @@ -26,7 +41,7 @@ int EventLoop::exec() exit_.store(false, std::memory_order_release); while (!exit_.load(std::memory_order_acquire)) - dispatcher_->processEvents(); + event_base_loop(event_, EVLOOP_NO_EXIT_ON_EMPTY); return exitCode_; } @@ -35,5 +50,10 @@ void EventLoop::exit(int code) { exitCode_ = code; exit_.store(true, std::memory_order_release); - dispatcher_->interrupt(); + interrupt(); +} + +void EventLoop::interrupt() +{ + event_base_loopbreak(event_); } diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h index 581c7cba2fc4..b1c6bd103080 100644 --- a/src/cam/event_loop.h +++ b/src/cam/event_loop.h @@ -9,26 +9,27 @@ #include <atomic> -#include <libcamera/event_notifier.h> - -namespace libcamera { -class EventDispatcher; -} +struct event_base; class EventLoop { public: - EventLoop(libcamera::EventDispatcher *dispatcher); + EventLoop(); ~EventLoop(); + static EventLoop *instance(); + int exec(); void exit(int code = 0); private: - libcamera::EventDispatcher *dispatcher_; + static EventLoop *instance_; + struct event_base *event_; std::atomic<bool> exit_; int exitCode_; + + void interrupt(); }; #endif /* __CAM_EVENT_LOOP_H__ */ diff --git a/src/cam/main.cpp b/src/cam/main.cpp index b3a8d94f5739..e01be63a7058 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -52,7 +52,7 @@ private: CameraManager *cm_; std::shared_ptr<Camera> camera_; std::unique_ptr<libcamera::CameraConfiguration> config_; - EventLoop *loop_; + EventLoop loop_; bool strictFormats_; }; @@ -60,7 +60,7 @@ private: CamApp *CamApp::app_ = nullptr; CamApp::CamApp() - : cm_(nullptr), camera_(nullptr), config_(nullptr), loop_(nullptr), + : cm_(nullptr), camera_(nullptr), config_(nullptr), strictFormats_(false) { CamApp::app_ = this; @@ -134,16 +134,11 @@ int CamApp::init(int argc, char **argv) std::cout << "Monitoring new hotplug and unplug events" << std::endl; } - loop_ = new EventLoop(cm_->eventDispatcher()); - return 0; } void CamApp::cleanup() { - delete loop_; - loop_ = nullptr; - if (camera_) { camera_->release(); camera_.reset(); @@ -166,8 +161,7 @@ int CamApp::exec() void CamApp::quit() { - if (loop_) - loop_->exit(); + loop_.exit(); } int CamApp::parseOptions(int argc, char *argv[]) @@ -366,13 +360,13 @@ int CamApp::run() } if (options_.isSet(OptCapture)) { - Capture capture(camera_, config_.get(), loop_); + Capture capture(camera_, config_.get(), &loop_); return capture.run(options_); } if (options_.isSet(OptMonitor)) { std::cout << "Press Ctrl-C to interrupt" << std::endl; - ret = loop_->exec(); + ret = loop_.exec(); if (ret) std::cout << "Failed to run monitor loop" << std::endl; } diff --git a/src/cam/meson.build b/src/cam/meson.build index 89e124fbae2a..62003c58dad0 100644 --- a/src/cam/meson.build +++ b/src/cam/meson.build @@ -1,5 +1,12 @@ # SPDX-License-Identifier: CC0-1.0 +libevent = dependency('libevent_pthreads', required : false) + +if not libevent.found() + warning('libevent_pthreads not found,\'cam\' application will not be compiled') + subdir_done() +endif + cam_sources = files([ 'buffer_writer.cpp', 'capture.cpp', @@ -10,5 +17,9 @@ cam_sources = files([ ]) cam = executable('cam', cam_sources, - dependencies : [ libatomic, libcamera_dep ], + dependencies : [ + libatomic, + libcamera_dep, + libevent, + ], install : true)
To prepare for removal of the EventDispatcher from the libcamera public API, switch to libevent to handle the event loop. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/cam/event_loop.cpp | 34 +++++++++++++++++++++++++++------- src/cam/event_loop.h | 15 ++++++++------- src/cam/main.cpp | 16 +++++----------- src/cam/meson.build | 13 ++++++++++++- 4 files changed, 52 insertions(+), 26 deletions(-)