[libcamera-devel] [PATCH v4 02/15] py: New event handling
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Mar 12 16:44:30 CET 2023
Hi Tomi,
Thank you for the patch.
On Thu, Mar 09, 2023 at 04:25:48PM +0200, Tomi Valkeinen via libcamera-devel wrote:
> At the moment the Python bindings only handle the requestCompleted
> events. But we have others, bufferCompleted and disconnect from the
> Camera, and cameraAdded and cameraRemoved from the CameraManager.
>
> This patch makes all those events available to the users.
>
> The get_ready_requests() method is now deprecated, but available to keep
> backward compatibility.
>
> The new event handling works like get_ready_requests(), but instead of
> returning only Requests from requestCompleted events, it returns all
> event types using a new Event class.
>
> Additionally camera.stop() has been changed to return events for that
> camera. This serves two purposes: first, it removes the requestCompleted
> events from the event queue, thus preventing the old events being
> returned when the camera is started again, and second, it allows the
> user to process those events if it so wants.
>
> All other event types are always collected and returned, except
> bufferCompleted which needs to be enabled by setting the
> CameraManager.buffer_completed_active to True. This is to reduce the
> overhead a bit. It is not clear if this is a necessary optimization or
> not.
>
> TODO: Documentation.
Looks like we're converging on the API, so it's time to write
documentation :-)
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
> ---
> src/py/libcamera/py_camera_manager.cpp | 195 +++++++++++++++++++++++--
> src/py/libcamera/py_camera_manager.h | 66 ++++++++-
> src/py/libcamera/py_main.cpp | 44 +++++-
> 3 files changed, 281 insertions(+), 24 deletions(-)
>
> diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp
> index 9ccb7aad..7d6dded4 100644
> --- a/src/py/libcamera/py_camera_manager.cpp
> +++ b/src/py/libcamera/py_camera_manager.cpp
> @@ -5,6 +5,7 @@
>
> #include "py_camera_manager.h"
>
> +#include <algorithm>
> #include <errno.h>
> #include <memory>
> #include <sys/eventfd.h>
> @@ -35,11 +36,17 @@ PyCameraManager::PyCameraManager()
> if (ret)
> throw std::system_error(-ret, std::generic_category(),
> "Failed to start CameraManager");
> +
> + cameraManager_->cameraAdded.connect(this, &PyCameraManager::handleCameraAdded);
> + cameraManager_->cameraRemoved.connect(this, &PyCameraManager::handleCameraRemoved);
> }
>
> PyCameraManager::~PyCameraManager()
> {
> LOG(Python, Debug) << "~PyCameraManager()";
> +
> + cameraManager_->cameraAdded.disconnect();
> + cameraManager_->cameraRemoved.disconnect();
> }
>
> py::list PyCameraManager::cameras()
> @@ -60,6 +67,43 @@ py::list PyCameraManager::cameras()
> return l;
> }
>
> +PyCameraEvent PyCameraManager::convertEvent(const CameraEvent &event)
> +{
> + /*
> + * We need to set a keep-alive here so that the camera keeps the
> + * camera manager alive.
> + */
> + py::object py_cm = py::cast(this);
> + py::object py_cam = py::cast(event.camera_);
> + py::detail::keep_alive_impl(py_cam, py_cm);
> +
> + PyCameraEvent pyevent(event.type_, py_cam);
> +
> + switch (event.type_) {
> + case CameraEventType::CameraAdded:
> + case CameraEventType::CameraRemoved:
> + case CameraEventType::Disconnect:
> + /* No additional parameters to add */
> + break;
> +
> + case CameraEventType::BufferCompleted:
> + pyevent.request_ = py::cast(event.request_);
> + pyevent.fb_ = py::cast(event.fb_);
> + break;
> +
> + case CameraEventType::RequestCompleted:
> + pyevent.request_ = py::cast(event.request_);
> +
> + /* Decrease the ref increased in Camera.queue_request() */
> + pyevent.request_.dec_ref();
> +
> + break;
> + }
> +
> + return pyevent;
> +}
> +
> +/* DEPRECATED */
> std::vector<py::object> PyCameraManager::getReadyRequests()
> {
> int ret = readFd();
> @@ -72,21 +116,134 @@ std::vector<py::object> PyCameraManager::getReadyRequests()
>
> std::vector<py::object> py_reqs;
>
> - for (Request *request : getCompletedRequests()) {
> - py::object o = py::cast(request);
> - /* Decrease the ref increased in Camera.queue_request() */
> - o.dec_ref();
> - py_reqs.push_back(o);
> + for (const auto &ev : getEvents()) {
> + if (ev.type_ != CameraEventType::RequestCompleted)
> + continue;
> +
> + PyCameraEvent pyev = convertEvent(ev);
> + py_reqs.push_back(pyev.request_);
> }
>
> return py_reqs;
> }
>
> +std::vector<PyCameraEvent> PyCameraManager::getPyEvents()
> +{
> + int ret = readFd();
> +
> + if (ret == EAGAIN) {
> + LOG(Python, Debug) << "No events";
> + return {};
> + }
> +
> + if (ret != 0)
> + throw std::system_error(ret, std::generic_category());
> +
> + std::vector<CameraEvent> events = getEvents();
> +
> + LOG(Python, Debug) << "Got " << events.size() << " events";
> +
> + std::vector<PyCameraEvent> pyevents;
> + pyevents.reserve(events.size());
> +
> + std::transform(events.begin(), events.end(), std::back_inserter(pyevents),
> + [this](const CameraEvent &ev) {
> + return convertEvent(ev);
> + });
> +
> + return pyevents;
> +}
> +
> +static bool isCameraSpecificEvent(const CameraEvent &event, std::shared_ptr<Camera> &camera)
> +{
> + return event.camera_ == camera &&
> + (event.type_ == CameraEventType::RequestCompleted ||
> + event.type_ == CameraEventType::BufferCompleted ||
> + event.type_ == CameraEventType::Disconnect);
Could you check my latest reply to the v3 mail thread for this function
(message ID Yw5ki0Tmycz9Uk6A at pendragon.ideasonboard.com) ? I think we
haven't finished the discussion.
> +}
> +
> +std::vector<PyCameraEvent> PyCameraManager::getPyCameraEvents(std::shared_ptr<Camera> camera)
> +{
> + std::vector<CameraEvent> events;
> + size_t unhandled_size;
> +
> + {
> + MutexLocker guard(eventsMutex_);
> +
> + /*
> + * Collect events related to the given camera and remove them
> + * from the events_ vector.
> + */
> +
> + auto it = events_.begin();
> + while (it != events_.end()) {
> + if (isCameraSpecificEvent(*it, camera)) {
> + events.push_back(*it);
> + it = events_.erase(it);
Erasing elements in the middle of a vactor is quite inefficient. We've
discussed this in the review of v4. Could you confirm you have deviced
to not switch to a std::list, and not just forgotten about it ?
> + } else {
> + it++;
> + }
> + }
> +
> + unhandled_size = events_.size();
> + }
> +
> + /* Convert events to Python events */
> +
> + std::vector<PyCameraEvent> pyevents;
> +
> + for (const auto &event : events) {
> + PyCameraEvent pyev = convertEvent(event);
> + pyevents.push_back(pyev);
> + }
> +
> + LOG(Python, Debug) << "Got " << pyevents.size() << " camera events, "
> + << unhandled_size << " unhandled events left";
> +
> + return pyevents;
> +}
> +
> /* Note: Called from another thread */
> -void PyCameraManager::handleRequestCompleted(Request *req)
> +void PyCameraManager::handleBufferCompleted(std::shared_ptr<Camera> cam, Request *req, FrameBuffer *fb)
> {
> - pushRequest(req);
> - writeFd();
> + if (!bufferCompletedEventActive_)
> + return;
> +
> + CameraEvent ev(CameraEventType::BufferCompleted, cam, req, fb);
> +
> + pushEvent(ev);
> +}
> +
> +/* Note: Called from another thread */
> +void PyCameraManager::handleRequestCompleted(std::shared_ptr<Camera> cam, Request *req)
> +{
> + CameraEvent ev(CameraEventType::RequestCompleted, cam, req);
> +
> + pushEvent(ev);
> +}
> +
> +/* Note: Called from another thread */
> +void PyCameraManager::handleDisconnected(std::shared_ptr<Camera> cam)
> +{
> + CameraEvent ev(CameraEventType::Disconnect, cam);
> +
> + pushEvent(ev);
> +}
> +
> +/* Note: Called from another thread */
> +void PyCameraManager::handleCameraAdded(std::shared_ptr<Camera> cam)
> +{
> + CameraEvent ev(CameraEventType::CameraAdded, cam);
> +
> + pushEvent(ev);
> +}
> +
> +/* Note: Called from another thread */
> +void PyCameraManager::handleCameraRemoved(std::shared_ptr<Camera> cam)
> +{
> + CameraEvent ev(CameraEventType::CameraRemoved, cam);
> +
> + pushEvent(ev);
> }
>
> void PyCameraManager::writeFd()
> @@ -116,16 +273,24 @@ int PyCameraManager::readFd()
> return -EIO;
> }
>
> -void PyCameraManager::pushRequest(Request *req)
> +void PyCameraManager::pushEvent(const CameraEvent &ev)
> {
> - MutexLocker guard(completedRequestsMutex_);
> - completedRequests_.push_back(req);
> + {
> + MutexLocker guard(eventsMutex_);
> + events_.push_back(ev);
> + }
> +
> + writeFd();
> +
> + LOG(Python, Debug) << "Queued events: " << events_.size();
> }
>
> -std::vector<Request *> PyCameraManager::getCompletedRequests()
> +std::vector<CameraEvent> PyCameraManager::getEvents()
> {
> - std::vector<Request *> v;
> - MutexLocker guard(completedRequestsMutex_);
> - swap(v, completedRequests_);
> + std::vector<CameraEvent> v;
> +
> + MutexLocker guard(eventsMutex_);
> + swap(v, events_);
> +
> return v;
> }
> diff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h
> index 3525057d..757f6d8e 100644
> --- a/src/py/libcamera/py_camera_manager.h
> +++ b/src/py/libcamera/py_camera_manager.h
> @@ -13,6 +13,48 @@
>
> using namespace libcamera;
>
> +enum class CameraEventType {
> + CameraAdded,
> + CameraRemoved,
> + Disconnect,
> + RequestCompleted,
> + BufferCompleted,
> +};
> +
> +/*
> + * This event struct is used internally to queue the events we receive from
> + * other threads.
> + */
> +struct CameraEvent {
> + CameraEvent(CameraEventType type, std::shared_ptr<Camera> camera,
> + Request *request = nullptr, FrameBuffer *fb = nullptr)
> + : type_(type), camera_(camera), request_(request), fb_(fb)
> + {
> + }
> +
> + CameraEventType type_;
> + std::shared_ptr<Camera> camera_;
> + Request *request_;
> + FrameBuffer *fb_;
> +};
> +
> +/*
> + * This event struct is passed to Python. We need to use pybind11::object here
> + * instead of a C++ pointer so that we keep a ref to the Request, and a
> + * keep-alive from the camera to the camera manager.
> + */
> +struct PyCameraEvent {
> + PyCameraEvent(CameraEventType type, pybind11::object camera)
> + : type_(type), camera_(camera)
> + {
> + }
> +
> + CameraEventType type_;
> + pybind11::object camera_;
> + pybind11::object request_;
> + pybind11::object fb_;
> +};
> +
> class PyCameraManager
> {
> public:
> @@ -26,20 +68,30 @@ public:
>
> int eventFd() const { return eventFd_.get(); }
>
> - std::vector<pybind11::object> getReadyRequests();
> + std::vector<pybind11::object> getReadyRequests(); /* DEPRECATED */
> + std::vector<PyCameraEvent> getPyEvents();
> + std::vector<PyCameraEvent> getPyCameraEvents(std::shared_ptr<Camera> camera);
> +
> + 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);
>
> - void handleRequestCompleted(Request *req);
> + bool bufferCompletedEventActive_ = false;
>
> private:
> std::unique_ptr<CameraManager> cameraManager_;
>
> UniqueFD eventFd_;
> - libcamera::Mutex completedRequestsMutex_;
> - std::vector<Request *> completedRequests_
> - LIBCAMERA_TSA_GUARDED_BY(completedRequestsMutex_);
> + libcamera::Mutex eventsMutex_;
> + std::vector<CameraEvent> events_
> + LIBCAMERA_TSA_GUARDED_BY(eventsMutex_);
>
> void writeFd();
> int readFd();
> - void pushRequest(Request *req);
> - std::vector<Request *> getCompletedRequests();
> + void pushEvent(const CameraEvent &ev);
> + std::vector<CameraEvent> getEvents();
> +
> + PyCameraEvent convertEvent(const CameraEvent &event);
> };
> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
> index 1d4c23b3..0fffc030 100644
> --- a/src/py/libcamera/py_main.cpp
> +++ b/src/py/libcamera/py_main.cpp
> @@ -61,6 +61,7 @@ PYBIND11_MODULE(_libcamera, m)
> * https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-c-types-in-docstrings
> */
>
> + auto pyEvent = py::class_<PyCameraEvent>(m, "Event");
Alphabetical order ?
> auto pyCameraManager = py::class_<PyCameraManager>(m, "CameraManager");
> auto pyCamera = py::class_<Camera>(m, "Camera");
> auto pyCameraConfiguration = py::class_<CameraConfiguration>(m, "CameraConfiguration");
> @@ -93,6 +94,20 @@ PYBIND11_MODULE(_libcamera, m)
> m.def("log_set_level", &logSetLevel);
>
> /* Classes */
> +
Extra blank line ?
> + py::enum_<CameraEventType>(pyEvent, "Type")
> + .value("CameraAdded", CameraEventType::CameraAdded)
> + .value("CameraRemoved", CameraEventType::CameraRemoved)
> + .value("Disconnect", CameraEventType::Disconnect)
> + .value("RequestCompleted", CameraEventType::RequestCompleted)
> + .value("BufferCompleted", CameraEventType::BufferCompleted);
> +
> + pyEvent
> + .def_readonly("type", &PyCameraEvent::type_)
> + .def_readonly("camera", &PyCameraEvent::camera_)
> + .def_readonly("request", &PyCameraEvent::request_)
> + .def_readonly("fb", &PyCameraEvent::fb_);
> +
> pyCameraManager
> .def_static("singleton", []() {
> std::shared_ptr<PyCameraManager> cm = gCameraManager.lock();
> @@ -110,7 +125,13 @@ PYBIND11_MODULE(_libcamera, m)
> .def_property_readonly("cameras", &PyCameraManager::cameras)
>
> .def_property_readonly("event_fd", &PyCameraManager::eventFd)
> - .def("get_ready_requests", &PyCameraManager::getReadyRequests);
> +
> + /* DEPRECATED */
> + .def("get_ready_requests", &PyCameraManager::getReadyRequests)
> +
> + .def("get_events", &PyCameraManager::getPyEvents)
> +
> + .def_readwrite("buffer_completed_active", &PyCameraManager::bufferCompletedEventActive_);
>
> pyCamera
> .def_property_readonly("id", &Camera::id)
> @@ -133,7 +154,17 @@ PYBIND11_MODULE(_libcamera, m)
> auto cm = gCameraManager.lock();
> ASSERT(cm);
>
> - self.requestCompleted.connect(cm.get(), &PyCameraManager::handleRequestCompleted);
> + self.requestCompleted.connect(&self, [cm, camera=self.shared_from_this()](Request *req) {
> + cm->handleRequestCompleted(camera, req);
> + });
> +
> + self.bufferCompleted.connect(&self, [cm, camera=self.shared_from_this()](Request *req, FrameBuffer *fb) {
> + cm->handleBufferCompleted(camera, req, fb);
> + });
> +
> + self.disconnected.connect(&self, [cm, camera=self.shared_from_this()]() {
> + cm->handleDisconnected(camera);
> + });
>
> ControlList controlList(self.controls());
>
> @@ -154,10 +185,19 @@ PYBIND11_MODULE(_libcamera, m)
> int ret = self.stop();
>
> self.requestCompleted.disconnect();
> + self.bufferCompleted.disconnect();
> + self.disconnected.disconnect();
> +
> + auto cm = gCameraManager.lock();
> + ASSERT(cm);
> +
> + auto events = cm->getPyCameraEvents(self.shared_from_this());
>
> if (ret)
> throw std::system_error(-ret, std::generic_category(),
> "Failed to stop camera");
> +
> + return events;
> })
>
> .def("__str__", [](Camera &self) {
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list