[libcamera-devel] [RFC v1 6/7] py: New event handling

Laurent Pinchart laurent.pinchart at ideasonboard.com
Fri Jun 24 16:44:21 CEST 2022


On Fri, Jun 24, 2022 at 11:53:43AM +0100, Kieran Bingham wrote:
> Quoting Tomi Valkeinen (2022-06-23 15:47:35)
> > At the moment the Python bindings only handle the requestCompleted
> > events. But we have others, bufferCompleted and disconnec from the
> 
> disconnec/disconnect
> 
> > Camera, and cameraAdded and cameraRemoved from the CameraManager.
> > 
> > This makes all those events available to the users.
> 
> \o/
> 
> > The get_ready_requests() method is now deprecated, but available to keep
> > backward compatibility.

I'd rather drop it. We're nowhere close to guaranteeing any API in the
Python bindings.

> Aha, once again - answering my questions in the previous patch ;-)
> Maybe I need to read patch series backwards ... hehe.
>  
> > The new event handling works as follows:
> > 
> > The user sets callbacks to the CameraManager or Cameras (e.g.
> > Camera.buffer_completed). When the event_fd informs of an event, the
> > user must call CameraManager.dispatch_events() which gets the queued
> > events and calls the relevant callbacks for each queued event.
> > 
> > Additionally there is CameraManager.discard_events() if the user does
> > not want to process the events but wants to clear the event queue (e.g.
> > after stopping the cameras or when exiting the app).

What is this for ? If the user doesn't set any handler for a specific
event type, I would expect the corresponding events to be discarded.
Anything else will be passed to the event handlers. If it's about
ensuring that the event queue is cleared after stop() if the use doesn't
call dispatch_events(), I'd rather clear the queue at stop() time, and
possible even call dispatch_events() in stop().

> Hrm... in C++ lifetimes, the end of stop() means everything internally
> is released, so I'm still weary that if we change that it could be
> problematic (i.e. if we have to 'rely' on an application doing clean
> up).
>  
> > I'm not very happy with this patch. It works fine, but there's a lot of
> > repetition of almost-the-same code. Perhaps some template magics might
> > reduce the repetition, but I also fear that it can easily make the code
> > more difficult to read.
> > 
> > TODO: Documentation.
> > 
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
> > ---
> >  src/py/libcamera/py_camera_manager.cpp | 291 +++++++++++++++++++++++--
> >  src/py/libcamera/py_camera_manager.h   |  41 +++-
> >  src/py/libcamera/py_main.cpp           |  87 +++++++-
> >  3 files changed, 397 insertions(+), 22 deletions(-)
> > 
> > diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp
> > index ba45f713..bbadb9ee 100644
> > --- a/src/py/libcamera/py_camera_manager.cpp
> > +++ b/src/py/libcamera/py_camera_manager.cpp
> > @@ -15,6 +15,37 @@ namespace py = pybind11;
> >  
> >  using namespace libcamera;
> >  
> > +struct CameraEvent {

I'd make this a class.

> > +       enum class EventType {

You can s/EventType/Type/ as you'll need to write CameraEvent::Type
anyway.

> > +               Undefined = 0,
> > +               CameraAdded,
> > +               CameraRemoved,
> > +               Disconnect,
> > +               RequestCompleted,
> > +               BufferCompleted,
> > +       };
> > +
> > +       CameraEvent(EventType type)
> > +               : type(type)

No variable shadowing warning ? Ah, we disable them. It would still be
good to avoid any possible issue here, and use type_ for the member.

> > +       {
> > +       }
> > +
> > +       EventType type;
> > +
> > +       std::shared_ptr<Camera> cam;
> > +
> > +       union {
> > +               struct {
> > +                       Request *req;
> > +                       FrameBuffer *fb;
> > +               } buf_completed;
> > +
> > +               struct {
> > +                       Request *req;
> > +               } req_completed;
> > +       };

As this stores pointers only, I think you could simplify it to just

	Request *request_;
	FrameBuffer *fb_;

and initialize the two pointers to nullptr in the constructor, to more
easily catch bugs.

> > +};
> > +
> >  PyCameraManager::PyCameraManager()
> >  {
> >         int fd = eventfd(0, 0);
> > @@ -56,33 +87,261 @@ py::list PyCameraManager::getCameras()
> >         return l;
> >  }
> >  
> > +/* DEPRECATED */

Let's drop it :-)

> >  std::vector<py::object> PyCameraManager::getReadyRequests(bool nonBlocking)
> >  {
> >         if (!nonBlocking || hasEvents())
> >                 readFd();
> >  
> > -       std::vector<Request *> v;
> > -       getRequests(v);
> > +       std::vector<CameraEvent> v;
> > +       getEvents(v);
> >  
> >         std::vector<py::object> ret;
> >  
> > -       for (Request *req : v) {
> > -               py::object o = py::cast(req);
> > -               /* Decrease the ref increased in Camera.queue_request() */
> > -               o.dec_ref();
> > -               ret.push_back(o);
> > +       for (const auto &ev : v) {
> > +               switch (ev.type) {
> > +               case CameraEvent::EventType::RequestCompleted: {
> > +                       Request *req = ev.req_completed.req;
> > +                       py::object o = py::cast(req);
> > +                       /* Decrease the ref increased in Camera.queue_request() */
> > +                       o.dec_ref();
> > +                       ret.push_back(o);
> > +               }
> > +               default:
> > +                       /* ignore */
> 
> Does this mean that events will get lost if the deprecated call gets
> used? (Ok, so it's deprecated, but will it cause 'bugs' ?)
> 
> 
> > +                       break;
> > +               }
> >         }
> >  
> >         return ret;
> >  }
> >  
> >  /* Note: Called from another thread */
> > -void PyCameraManager::handleRequestCompleted(Request *req)
> > +void PyCameraManager::handleBufferCompleted(std::shared_ptr<Camera> cam, Request *req, FrameBuffer *fb)
> > +{
> > +       CameraEvent ev(CameraEvent::EventType::BufferCompleted);
> > +       ev.cam = cam;
> > +       ev.buf_completed.req = req;
> > +       ev.buf_completed.fb = fb;
> > +
> > +       pushEvent(ev);
> > +       writeFd();
> 
> Should writeFd be part of pushEvent? then there should be one entry on
> the poll Fd for every event right? And forces them to be kept in sync...

Sounds like a good idea.

> > +}
> > +
> > +/* Note: Called from another thread */
> > +void PyCameraManager::handleRequestCompleted(std::shared_ptr<Camera> cam, Request *req)
> > +{
> > +       CameraEvent ev(CameraEvent::EventType::RequestCompleted);
> > +       ev.cam = cam;
> > +       ev.req_completed.req = req;
> > +
> > +       pushEvent(ev);
> > +       writeFd();
> > +}
> > +
> > +/* Note: Called from another thread */
> > +void PyCameraManager::handleDisconnected(std::shared_ptr<Camera> cam)
> >  {
> > -       pushRequest(req);
> > +       CameraEvent ev(CameraEvent::EventType::Disconnect);
> > +       ev.cam = cam;
> > +
> > +       pushEvent(ev);
> >         writeFd();
> >  }
> >  
> > +/* Note: Called from another thread */
> > +void PyCameraManager::handleCameraAdded(std::shared_ptr<Camera> cam)
> > +{
> > +       CameraEvent ev(CameraEvent::EventType::CameraAdded);
> > +       ev.cam = cam;
> > +
> > +       pushEvent(ev);
> > +       writeFd();
> > +}
> > +
> > +/* Note: Called from another thread */
> > +void PyCameraManager::handleCameraRemoved(std::shared_ptr<Camera> cam)
> > +{
> > +       CameraEvent ev(CameraEvent::EventType::CameraRemoved);
> > +       ev.cam = cam;
> > +
> > +       pushEvent(ev);
> > +       writeFd();
> > +}
> 
> Is that what you mean about code duplication? I think it's fine.
> 
> These could be templated out to specify the event type I guess ... but
> that might be more pain. Especially when the request completed has
> different parameters anyway.

Yes, generalizing this is probably not really worth it.

> > +
> > +void PyCameraManager::dispatchEvents(bool nonBlocking)

Can we make this unconditionally non-blocking ?

> > +{
> > +       if (!nonBlocking || hasEvents())
> > +               readFd();
> > +
> > +       std::vector<CameraEvent> v;

s/v/events/

> > +       getEvents(v);

	std::vector<CameraEvent> events = getEvents();

> > +
> > +       LOG(Python, Debug) << "Dispatch " << v.size() << " events";
> > +
> > +       for (const auto &ev : v) {

s/ev/event/

> > +               switch (ev.type) {
> > +               case CameraEvent::EventType::CameraAdded: {
> > +                       std::shared_ptr<Camera> cam = ev.cam;

As there's always a camera in all events, you could move it before the
switch.

> > +
> > +                       if (cameraAddedHandler_)
> > +                               cameraAddedHandler_(cam);
> > +
> > +                       break;
> > +               }
> > +               case CameraEvent::EventType::CameraRemoved: {
> > +                       std::shared_ptr<Camera> cam = ev.cam;
> > +
> > +                       if (cameraRemovedHandler_)
> > +                               cameraRemovedHandler_(cam);
> > +
> > +                       break;
> > +               }
> > +               case CameraEvent::EventType::BufferCompleted: {
> > +                       std::shared_ptr<Camera> cam = ev.cam;
> > +
> > +                       auto cb = getBufferCompleted(cam.get());
> > +

Extra blank line. Same below.

> > +                       if (cb)
> > +                               cb(cam, ev.buf_completed.req, ev.buf_completed.fb);
> > +
> > +                       break;
> > +               }
> > +               case CameraEvent::EventType::RequestCompleted: {
> > +                       std::shared_ptr<Camera> cam = ev.cam;
> > +
> > +                       auto cb = getRequestCompleted(cam.get());
> > +
> > +                       if (cb)
> > +                               cb(cam, ev.req_completed.req);
> > +
> > +                       /* Decrease the ref increased in Camera.queue_request() */
> > +                       py::object o = py::cast(ev.req_completed.req);
> > +                       o.dec_ref();
> > +
> > +                       break;
> > +               }
> > +               case CameraEvent::EventType::Disconnect: {
> > +                       std::shared_ptr<Camera> cam = ev.cam;
> > +
> > +                       auto cb = getDisconnected(cam.get());
> > +
> > +                       if (cb)
> > +                               cb(cam);
> > +
> > +                       break;
> > +               }
> > +               default:
> > +                       assert(false);
> 
> Can this be a python throw to generate a backtrace? Or a LOG(Python,
> Fatal)? 
> 
> A plain assert isn't very informative.
> 
> > +               }
> > +       }
> > +}
> > +
> > +void PyCameraManager::discardEvents()
> > +{
> > +       if (hasEvents())
> > +               readFd();
> > +
> > +       std::vector<CameraEvent> v;
> > +       getEvents(v);
> > +
> > +       LOG(Python, Debug) << "Discard " << v.size() << " events";
> > +
> > +       for (const auto &ev : v) {
> > +               if (ev.type != CameraEvent::EventType::RequestCompleted)
> > +                       continue;
> > +
> > +               std::shared_ptr<Camera> cam = ev.cam;
> > +
> > +               /* Decrease the ref increased in Camera.queue_request() */
> > +               py::object o = py::cast(ev.req_completed.req);
> > +               o.dec_ref();
> > +       }
> > +}
> > +
> > +std::function<void(std::shared_ptr<Camera>)> PyCameraManager::getCameraAdded() const
> > +{
> > +       return cameraAddedHandler_;
> > +}
> > +
> > +void PyCameraManager::setCameraAdded(std::function<void(std::shared_ptr<Camera>)> fun)
> > +{
> > +       if (cameraAddedHandler_)
> > +               cameraAdded.disconnect();
> > +
> > +       cameraAddedHandler_ = fun;
> > +
> > +       if (fun)
> 
> Really trivially, but I'd call fun fn or func. Otherwise I read this as
> "If you're having fun... connect the signal" ;-)

I'd go for func, we use that already in other places.

> > +               cameraAdded.connect(this, &PyCameraManager::handleCameraAdded);
> > +}
> > +
> > +std::function<void(std::shared_ptr<Camera>)> PyCameraManager::getCameraRemoved() const
> > +{
> > +       return cameraRemovedHandler_;
> > +}
> > +
> > +void PyCameraManager::setCameraRemoved(std::function<void(std::shared_ptr<Camera>)> fun)
> > +{
> > +       if (cameraRemovedHandler_)
> > +               cameraRemoved.disconnect();
> > +
> > +       cameraRemovedHandler_ = fun;
> > +
> > +       if (fun)
> > +               cameraRemoved.connect(this, &PyCameraManager::handleCameraRemoved);
> > +}
> > +
> > +std::function<void(std::shared_ptr<Camera>, Request *)> PyCameraManager::getRequestCompleted(Camera *cam)
> > +{
> > +       if (auto it = cameraRequestCompletedHandlers_.find(cam);
> > +           it != cameraRequestCompletedHandlers_.end())
> > +               return it->second;
> > +
> > +       return nullptr;
> > +}
> > +
> > +void PyCameraManager::setRequestCompleted(Camera *cam, std::function<void(std::shared_ptr<Camera>, Request *)> fun)
> > +{
> > +       if (fun)
> > +               cameraRequestCompletedHandlers_[cam] = fun;
> > +       else
> > +               cameraRequestCompletedHandlers_.erase(cam);
> > +}
> > +
> > +std::function<void(std::shared_ptr<Camera>, Request *, FrameBuffer *)> PyCameraManager::getBufferCompleted(Camera *cam)
> > +{
> > +       if (auto it = cameraBufferCompletedHandlers_.find(cam);
> > +           it != cameraBufferCompletedHandlers_.end())
> > +               return it->second;
> > +
> > +       return nullptr;
> > +}
> > +
> > +void PyCameraManager::setBufferCompleted(Camera *cam, std::function<void(std::shared_ptr<Camera>, Request *, FrameBuffer *)> fun)
> > +{
> > +       if (fun)
> > +               cameraBufferCompletedHandlers_[cam] = fun;
> > +       else
> > +               cameraBufferCompletedHandlers_.erase(cam);
> > +}
> > +
> > +std::function<void(std::shared_ptr<Camera>)> PyCameraManager::getDisconnected(Camera *cam)
> > +{
> > +       if (auto it = cameraDisconnectHandlers_.find(cam);
> > +           it != cameraDisconnectHandlers_.end())
> > +               return it->second;
> > +
> > +       return nullptr;
> > +}
> > +
> > +void PyCameraManager::setDisconnected(Camera *cam, std::function<void(std::shared_ptr<Camera>)> fun)
> > +{
> > +       if (fun)
> > +               cameraDisconnectHandlers_[cam] = fun;
> > +       else
> > +               cameraDisconnectHandlers_.erase(cam);
> > +}
> > +
> 
> Yes, I see, lots more boilerplate code duplication. Not a problem in
> it's own right, and if it's templateable then maybe that can be helpful.
> 
> >  void PyCameraManager::writeFd()
> >  {
> >         uint64_t v = 1;
> > @@ -104,16 +363,18 @@ void PyCameraManager::readFd()
> >                 throw std::system_error(errno, std::generic_category());
> >  }
> >  
> > -void PyCameraManager::pushRequest(Request *req)
> > +void PyCameraManager::pushEvent(const CameraEvent &ev)
> >  {
> > -       std::lock_guard guard(reqlist_mutex_);
> > -       reqList_.push_back(req);
> > +       std::lock_guard guard(reqlistMutex_);
> > +       cameraEvents_.push_back(ev);
> > +
> > +       LOG(Python, Debug) << "Queued events: " << cameraEvents_.size();
> >  }
> >  
> > -void PyCameraManager::getRequests(std::vector<Request *> &v)
> > +void PyCameraManager::getEvents(std::vector<CameraEvent> &v)
> >  {
> > -       std::lock_guard guard(reqlist_mutex_);
> > -       swap(v, reqList_);
> > +       std::lock_guard guard(reqlistMutex_);
> 
> Is this now supposed to be an eventListMutex_?
> 
> > +       swap(v, cameraEvents_);
> >  }
> >  
> >  bool PyCameraManager::hasEvents()
> > diff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h
> > index 2396d236..fd28291b 100644
> > --- a/src/py/libcamera/py_camera_manager.h
> > +++ b/src/py/libcamera/py_camera_manager.h
> > @@ -13,6 +13,8 @@
> >  
> >  using namespace libcamera;
> >  
> > +struct CameraEvent;
> > +
> >  class PyCameraManager : public CameraManager
> >  {
> >  public:
> > @@ -27,15 +29,46 @@ public:
> >  
> >         void handleRequestCompleted(Request *req);
> >  
> > +       void handleBufferCompleted(std::shared_ptr<Camera> cam, Request *req, FrameBuffer *fb);
> > +       void handleRequestCompleted(std::shared_ptr<Camera> cam, Request *req);
> > +       void handleDisconnected(std::shared_ptr<Camera> cam);
> > +       void handleCameraAdded(std::shared_ptr<Camera> cam);
> > +       void handleCameraRemoved(std::shared_ptr<Camera> cam);
> 
> In libcamera, we don't prefix event handlers with 'handle'. i.e.
> handleCameraRemoved would just be cameraRemoved.
> 
> Do you prefer it to make it distinct? or is it possible to align with
> the existing styles?
> 
> > +
> > +       void dispatchEvents(bool nonBlocking = false);
> > +       void discardEvents();
> > +
> > +       std::function<void(std::shared_ptr<Camera>)> getCameraAdded() const;
> > +       void setCameraAdded(std::function<void(std::shared_ptr<Camera>)> fun);
> > +
> > +       std::function<void(std::shared_ptr<Camera>)> getCameraRemoved() const;
> > +       void setCameraRemoved(std::function<void(std::shared_ptr<Camera>)> fun);
> > +
> > +       std::function<void(std::shared_ptr<Camera>, Request *)> getRequestCompleted(Camera *cam);
> > +       void setRequestCompleted(Camera *cam, std::function<void(std::shared_ptr<Camera>, Request *)> fun);
> > +
> > +       std::function<void(std::shared_ptr<Camera>, Request *, FrameBuffer *)> getBufferCompleted(Camera *cam);
> > +       void setBufferCompleted(Camera *cam, std::function<void(std::shared_ptr<Camera>, Request *, FrameBuffer *)> fun);
> > +
> > +       std::function<void(std::shared_ptr<Camera>)> getDisconnected(Camera *cam);
> > +       void setDisconnected(Camera *cam, std::function<void(std::shared_ptr<Camera>)> fun);
> 
> so many getters and setters. Can the underlying events be more directly
> accessibly or do they have to have the setters?

It would be nice to clean this up a bit indeed if possible. Nothing
urgent though.

> > +
> >  private:
> >         int eventFd_ = -1;
> > -       std::mutex reqlist_mutex_;
> > +       std::mutex reqlistMutex_;
> >         std::vector<Request *> reqList_;
> > +       std::vector<CameraEvent> cameraEvents_;
> > +
> > +       std::function<void(std::shared_ptr<Camera>)> cameraAddedHandler_;
> > +       std::function<void(std::shared_ptr<Camera>)> cameraRemovedHandler_;
> > +
> > +       std::map<Camera *, std::function<void(std::shared_ptr<Camera>, Request *, FrameBuffer *)>> cameraBufferCompletedHandlers_;
> > +       std::map<Camera *, std::function<void(std::shared_ptr<Camera>, Request *)>> cameraRequestCompletedHandlers_;
> > +       std::map<Camera *, std::function<void(std::shared_ptr<Camera>)>> cameraDisconnectHandlers_;
> >  
> >         void writeFd();
> >         void readFd();
> > -       void pushRequest(Request *req);
> > -       void getRequests(std::vector<Request *> &v);
> > -
> > +       void pushEvent(const CameraEvent &ev);
> > +       void getEvents(std::vector<CameraEvent> &v);
> >         bool hasEvents();
> >  };
> > diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
> > index ee4ecb9b..b6fd9a9d 100644
> > --- a/src/py/libcamera/py_main.cpp
> > +++ b/src/py/libcamera/py_main.cpp
> > @@ -103,8 +103,20 @@ PYBIND11_MODULE(_libcamera, m)
> >                 .def_property_readonly("cameras", &PyCameraManager::getCameras)
> >  
> >                 .def_property_readonly("event_fd", &PyCameraManager::eventFd)
> > +               /* DEPRECATED */
> >                 .def("get_ready_requests", &PyCameraManager::getReadyRequests,
> > -                    py::arg("nonblocking") = false);
> > +                    py::arg("nonblocking") = false)
> > +               .def("dispatch_events", &PyCameraManager::dispatchEvents,
> > +                    py::arg("nonblocking") = false)
> 
> Why are there newlines between camera_added and camera_removed, but not
> between get_ready_requests, dispatch_events, and discard_events? Is it
> grouping events? or should these be separated by newlines too?
> 
> > +               .def("discard_events", &PyCameraManager::discardEvents)
> > +
> > +               .def_property("camera_added",
> > +                             &PyCameraManager::getCameraAdded,
> > +                             &PyCameraManager::setCameraAdded)
> > +
> > +               .def_property("camera_removed",
> > +                             &PyCameraManager::getCameraRemoved,
> > +                             &PyCameraManager::setCameraRemoved);
> 
> For these chained code styles, if the ; was on a new line by
> itself, it wouldn't have to modify the previous property to add a new
> one?
> 
> Is that a good thing or worse code style for you ?
>   
> >         pyCamera
> >                 .def_property_readonly("id", &Camera::id)
> > @@ -117,9 +129,13 @@ PYBIND11_MODULE(_libcamera, m)
> >                         auto cm = gCameraManager.lock();
> >                         assert(cm);
> >  
> > -                       self.requestCompleted.connect(&self, [cm=std::move(cm)](Request *req) {
> > +                       /*
> > +                        * Note: We always subscribe requestComplete, as the bindings
> > +                        * use requestComplete event to decrement the Request refcount-
> > +                        */
> 
> Interesting, so there are multiple subscribers to the event? Is it still
> guaranteed to be referenced while the user code has the request? (I
> assume so, and that this is managing the internal refcnts while the
> object is inside libcamera).
> 
> 
> If the ordering is required, then this layer could subscribe to the
> event, and the be responsible for calling the event on the user code and
> cleaning up after it completes.
> 
> But if it already is fine, then it's a nice example of how multiple
> functions can run on the event handler.
> 
> >  
> > -                               cm->handleRequestCompleted(req);
> > +                       self.requestCompleted.connect(&self, [cm, camera=self.shared_from_this()](Request *req) {
> > +                               cm->handleRequestCompleted(camera, req);
> >                         });
> >  
> >                         ControlList controlList(self.controls());
> > @@ -148,6 +164,71 @@ PYBIND11_MODULE(_libcamera, m)
> >                         return 0;
> >                 })
> >  
> > +               .def_property("request_completed",
> > +               [](Camera &self) {
> > +                       auto cm = gCameraManager.lock();
> > +                       assert(cm);
> > +
> > +                       return cm->getRequestCompleted(&self);
> > +               },
> > +               [](Camera &self, std::function<void(std::shared_ptr<Camera>, Request *)> f) {
> > +                       auto cm = gCameraManager.lock();
> > +                       assert(cm);
> > +
> > +                       cm->setRequestCompleted(&self, f);
> > +
> > +                       /*
> > +                        * Note: We do not subscribe requestComplete here, as we
> > +                        * do that in the start() method.
> > +                        */
> > +               })
> 
> Ok - so this shows why they all need distince getters and setters ...
> 
> > +
> > +               .def_property("buffer_completed",
> > +               [](Camera &self) -> std::function<void(std::shared_ptr<Camera>, Request *, FrameBuffer *)> {
> > +                       auto cm = gCameraManager.lock();
> > +                       assert(cm);
> > +
> > +                       return cm->getBufferCompleted(&self);
> > +               },
> > +               [](Camera &self, std::function<void(std::shared_ptr<Camera>, Request *, FrameBuffer *)> f) {
> > +                       auto cm = gCameraManager.lock();
> > +                       assert(cm);
> > +
> > +                       cm->setBufferCompleted(&self, f);
> > +
> > +                       self.bufferCompleted.disconnect();
> 
> Can a comment explain why we're disconnecting this bufferCompleted? It's
> not obvious to me why it's there ... Is it an internal bufferCompleted
> handler for some specificness that I haven't seen yet?
> 
> 
> > +
> > +                       if (!f)
> > +                               return;
> > +
> > +                       self.bufferCompleted.connect(&self, [cm, camera=self.shared_from_this()](Request *req, FrameBuffer *fb) {
> > +                               cm->handleBufferCompleted(camera, req, fb);
> > +                       });
> > +               })
> > +
> > +               .def_property("disconnected",
> > +               [](Camera &self) -> std::function<void(std::shared_ptr<Camera>)> {
> > +                       auto cm = gCameraManager.lock();
> > +                       assert(cm);
> > +
> > +                       return cm->getDisconnected(&self);
> > +               },
> > +               [](Camera &self, std::function<void(std::shared_ptr<Camera>)> f) {
> > +                       auto cm = gCameraManager.lock();
> > +                       assert(cm);
> > +
> > +                       cm->setDisconnected(&self, f);
> > +
> > +                       self.disconnected.disconnect();
> 
> Same here. Are there internal events just to be able to run by default?
> 
> > +
> > +                       if (!f)
> > +                               return;
> 
> Should the self.disconnected be reconnected if f was null?
> 
> > +
> > +                       self.disconnected.connect(&self, [cm, camera=self.shared_from_this()]() {
> > +                               cm->handleDisconnected(camera);
> > +                       });
> > +               })
> > +
> >                 .def("__str__", [](Camera &self) {
> >                         return "<libcamera.Camera '" + self.id() + "'>";
> >                 })

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list