[libcamera-devel] [PATCH v1 2/5] cam: Use libevent to implement event loop
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Nov 16 15:52:08 CET 2020
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 at 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)
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list