[libcamera-devel,2/2,DNI/RFC] pipeline: uvcvideo: Only open devices upon acquire
diff mbox series

Message ID 20221116001724.3938045-3-kieran.bingham@ideasonboard.com
State New
Headers show
Series
  • Support resource acquisition at 'acquire()'
Related show

Commit Message

Kieran Bingham Nov. 16, 2022, 12:17 a.m. UTC 
Keeping UVC Video device nodes open will maintain an increased use count
for that device and ensure that it remains powered by the USB subsystem.

While newer kernels may improve and reduce the power consumption of UVC
camera devices with open but non-streaming video nodes, ensure that our
UVC pipeline handler does not keep the video node open when it is not
directly acquired by an application.

This allows camera daemons such as Pipewire to be able to maintain a
list of available Camera devices, without acquiring resources when
before an application has requested the camera directly.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

---
Marked as DNI/RFC for two big reasons:

 - 1: We might simply not want to do this, and want to leave it to the
   kernel to maintain better power consumption. Though keeping video
   nodes open when not actually in use or needed does still seem wasteful
   of a file descriptor resource.

 - 2: This doesn't work yet. Closing the device during init() and then
   reopening immediately works, but if I postpone reopening the device
   until the acquire() call, I can see that buffers are queued to the video
   device, but no completion event are ever received.
   - It's not yet clear if this is a fault in V4L2 or if the
     thread/event mechanism in libcamera is failing.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 55 ++++++++++++++++----
 1 file changed, 44 insertions(+), 11 deletions(-)

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
index 277465b72164..01cda2b12bc1 100644
--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
@@ -53,7 +53,7 @@  public:
 	std::map<PixelFormat, std::vector<SizeRange>> formats_;
 
 private:
-	bool generateId();
+	bool generateId(V4L2VideoDevice *video);
 
 	std::string id_;
 };
@@ -74,6 +74,9 @@  class PipelineHandlerUVC : public PipelineHandler
 public:
 	PipelineHandlerUVC(CameraManager *manager);
 
+	bool acquireDevice(Camera *camera);
+	void releaseDevice(Camera *camera);
+
 	std::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,
 		const StreamRoles &roles) override;
 	int configure(Camera *camera, CameraConfiguration *config) override;
@@ -178,6 +181,28 @@  PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)
 {
 }
 
+bool PipelineHandlerUVC::acquireDevice(Camera *camera)
+{
+	UVCCameraData *data = cameraData(camera);
+
+	int ret = data->video_->open();
+	if (ret)
+		return false;
+
+	data->video_->bufferReady.connect(data, &UVCCameraData::bufferReady);
+
+	return true;
+}
+
+void PipelineHandlerUVC::releaseDevice(Camera *camera)
+{
+	UVCCameraData *data = cameraData(camera);
+
+	data->video_->bufferReady.disconnect();
+
+	data->video_->close();
+}
+
 std::unique_ptr<CameraConfiguration>
 PipelineHandlerUVC::generateConfiguration(Camera *camera,
 	const StreamRoles &roles)
@@ -426,16 +451,15 @@  int UVCCameraData::init(MediaDevice *media)
 		return -ENODEV;
 	}
 
+	std::unique_ptr<V4L2VideoDevice> video = std::make_unique<V4L2VideoDevice>(*entity);
+
 	/* Create and open the video device. */
-	video_ = std::make_unique<V4L2VideoDevice>(*entity);
-	ret = video_->open();
+	ret = video->open();
 	if (ret)
 		return ret;
 
-	video_->bufferReady.connect(this, &UVCCameraData::bufferReady);
-
 	/* Generate the camera ID. */
-	if (!generateId()) {
+	if (!generateId(video.get())) {
 		LOG(UVC, Error) << "Failed to generate camera ID";
 		return -EINVAL;
 	}
@@ -445,7 +469,7 @@  int UVCCameraData::init(MediaDevice *media)
 	 * resolution from the largest size it advertises.
 	 */
 	Size resolution;
-	for (const auto &format : video_->formats()) {
+	for (const auto &format : video->formats()) {
 		PixelFormat pixelFormat = format.first.toPixelFormat();
 		if (!pixelFormat.isValid())
 			continue;
@@ -485,7 +509,7 @@  int UVCCameraData::init(MediaDevice *media)
 	 * the _UPC.
 	 */
 	properties::LocationEnum location = properties::CameraLocationExternal;
-	std::ifstream file(video_->devicePath() + "/../removable");
+	std::ifstream file(video->devicePath() + "/../removable");
 	if (file.is_open()) {
 		std::string value;
 		std::getline(file, value);
@@ -503,7 +527,7 @@  int UVCCameraData::init(MediaDevice *media)
 	/* Initialise the supported controls. */
 	ControlInfoMap::Map ctrls;
 
-	for (const auto &ctrl : video_->controls()) {
+	for (const auto &ctrl : video->controls()) {
 		uint32_t cid = ctrl.first->id();
 		const ControlInfo &info = ctrl.second;
 
@@ -512,12 +536,21 @@  int UVCCameraData::init(MediaDevice *media)
 
 	controlInfo_ = ControlInfoMap(std::move(ctrls), controls::controls);
 
+	/*
+	 * Close the camera until it is acquired to prevent holding on to
+	 * resources that we do not own, or keeping devices powered when not in
+	 * use.
+	 */
+	video->close();
+
+	video_ = std::move(video);
+
 	return 0;
 }
 
-bool UVCCameraData::generateId()
+bool UVCCameraData::generateId(V4L2VideoDevice *video)
 {
-	const std::string path = video_->devicePath();
+	const std::string path = video->devicePath();
 
 	/* Create a controller ID from first device described in firmware. */
 	std::string controllerId;