From patchwork Fri May 17 00:54:37 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 1206 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8CAF160103 for ; Fri, 17 May 2019 02:55:08 +0200 (CEST) X-Halon-ID: 69871728-783e-11e9-8d05-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (unknown [89.233.230.99]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 69871728-783e-11e9-8d05-005056917f90; Fri, 17 May 2019 02:55:06 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Fri, 17 May 2019 02:54:37 +0200 Message-Id: <20190517005447.27171-2-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> References: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 01/11] libcamera: Always check return value of MediaDevice::acquire() X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 May 2019 00:55:08 -0000 In preparation for adding more responsibility to MediaDevice::acquire() remove unneeded calls to acquire() and release(), and make sure all needed calls to acquire() are checked and acted on. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/ipu3/ipu3.cpp | 10 ++++------ src/libcamera/pipeline/uvcvideo.cpp | 3 ++- src/libcamera/pipeline/vimc.cpp | 3 ++- test/media_device/media_device_link_test.cpp | 6 +++++- test/v4l2_device/v4l2_device_test.cpp | 4 ---- test/v4l2_subdevice/v4l2_subdevice_test.cpp | 7 ------- 6 files changed, 13 insertions(+), 20 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 0041662514e1d7ca..75e878afad4e67a9 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -614,21 +614,19 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) imgu_dm.add("ipu3-imgu 1 viewfinder"); imgu_dm.add("ipu3-imgu 1 3a stat"); - /* - * It is safe to acquire both media devices at this point as - * DeviceEnumerator::search() skips the busy ones for us. - */ cio2MediaDev_ = enumerator->search(cio2_dm); if (!cio2MediaDev_) return false; - cio2MediaDev_->acquire(); + if (!cio2MediaDev_->acquire()) + return false; imguMediaDev_ = enumerator->search(imgu_dm); if (!imguMediaDev_) return false; - imguMediaDev_->acquire(); + if (!imguMediaDev_->acquire()) + return false; /* * Disable all links that are enabled by default on CIO2, as camera diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 5d2f1c98fa36380c..e40b052f4d877d5d 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -183,7 +183,8 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) if (!media_) return false; - media_->acquire(); + if (!media_->acquire()) + return false; std::unique_ptr data = utils::make_unique(this); diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index cdf43770a9bfc40f..7b6ebd4cc0a27e25 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -193,7 +193,8 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) if (!media_) return false; - media_->acquire(); + if (!media_->acquire()) + return false; std::unique_ptr data = utils::make_unique(this); diff --git a/test/media_device/media_device_link_test.cpp b/test/media_device/media_device_link_test.cpp index 58a55cdfee634c0b..484d3691310f78a0 100644 --- a/test/media_device/media_device_link_test.cpp +++ b/test/media_device/media_device_link_test.cpp @@ -51,7 +51,11 @@ class MediaDeviceLinkTest : public Test return TestSkip; } - dev_->acquire(); + if (!dev_->acquire()) { + cerr << "Unable to acquire media device " + << dev_->deviceNode() << endl; + return TestSkip; + } if (dev_->open()) { cerr << "Failed to open media device at " diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp index 4225291bbb6e23f0..833038d56ea4631d 100644 --- a/test/v4l2_device/v4l2_device_test.cpp +++ b/test/v4l2_device/v4l2_device_test.cpp @@ -46,8 +46,6 @@ int V4L2DeviceTest::init() if (!media_) return TestSkip; - media_->acquire(); - MediaEntity *entity = media_->getEntityByName("vivid-000-vid-cap"); if (!entity) return TestSkip; @@ -61,8 +59,6 @@ int V4L2DeviceTest::init() void V4L2DeviceTest::cleanup() { - media_->release(); - capture_->streamOff(); capture_->releaseBuffers(); capture_->close(); diff --git a/test/v4l2_subdevice/v4l2_subdevice_test.cpp b/test/v4l2_subdevice/v4l2_subdevice_test.cpp index 72d5f72543d29378..5810ef3c962fab8e 100644 --- a/test/v4l2_subdevice/v4l2_subdevice_test.cpp +++ b/test/v4l2_subdevice/v4l2_subdevice_test.cpp @@ -45,20 +45,16 @@ int V4L2SubdeviceTest::init() return TestSkip; } - media_->acquire(); - int ret = media_->open(); if (ret) { cerr << "Unable to open media device: " << media_->deviceNode() << ": " << strerror(ret) << endl; - media_->release(); return TestSkip; } MediaEntity *videoEntity = media_->getEntityByName("Scaler"); if (!videoEntity) { cerr << "Unable to find media entity 'Scaler'" << endl; - media_->release(); return TestFail; } @@ -67,7 +63,6 @@ int V4L2SubdeviceTest::init() if (ret) { cerr << "Unable to open video subdevice " << scaler_->entity()->deviceNode() << endl; - media_->release(); return TestSkip; } @@ -76,7 +71,5 @@ int V4L2SubdeviceTest::init() void V4L2SubdeviceTest::cleanup() { - media_->release(); - delete scaler_; } From patchwork Fri May 17 00:54:38 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 1207 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B451960103 for ; Fri, 17 May 2019 02:55:09 +0200 (CEST) X-Halon-ID: 6a1cdf6d-783e-11e9-8d05-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (unknown [89.233.230.99]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 6a1cdf6d-783e-11e9-8d05-005056917f90; Fri, 17 May 2019 02:55:08 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Fri, 17 May 2019 02:54:38 +0200 Message-Id: <20190517005447.27171-3-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> References: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 02/11] libcamera: media_device: Open and close media device inside populate() X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 May 2019 00:55:10 -0000 Remove the need for the caller to open and close the media device when populating the MediaDevice. This is done as an effort to make the usage of the MediaDevice less error prone and the interface stricter. The rework also revealed and fixes a potential memory leak in MediaDevice::populate() where resources would not be deleted if the second MEDIA_IOC_G_TOPOLOGY would fail. Signed-off-by: Niklas Söderlund Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/libcamera/device_enumerator.cpp | 8 +------- src/libcamera/media_device.cpp | 12 ++++++++++-- test/media_device/media_device_print_test.cpp | 6 ------ test/pipeline/ipu3/ipu3_pipeline_test.cpp | 5 ----- test/v4l2_subdevice/v4l2_subdevice_test.cpp | 10 +--------- 5 files changed, 12 insertions(+), 29 deletions(-) diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp index 259eec41776e2ec4..6fcbae76b64e3774 100644 --- a/src/libcamera/device_enumerator.cpp +++ b/src/libcamera/device_enumerator.cpp @@ -207,11 +207,7 @@ int DeviceEnumerator::addDevice(const std::string &deviceNode) { std::shared_ptr media = std::make_shared(deviceNode); - int ret = media->open(); - if (ret < 0) - return ret; - - ret = media->populate(); + int ret = media->populate(); if (ret < 0) { LOG(DeviceEnumerator, Info) << "Unable to populate media device " << deviceNode @@ -238,8 +234,6 @@ int DeviceEnumerator::addDevice(const std::string &deviceNode) return ret; } - media->close(); - LOG(DeviceEnumerator, Debug) << "Added device " << deviceNode << ": " << media->driver(); diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index 449571fb4b78fb94..c39775164fab7487 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -221,6 +221,10 @@ int MediaDevice::populate() clear(); + ret = open(); + if (ret) + return ret; + /* * Keep calling G_TOPOLOGY until the version number stays stable. */ @@ -237,7 +241,7 @@ int MediaDevice::populate() LOG(MediaDevice, Error) << "Failed to enumerate topology: " << strerror(-ret); - return ret; + goto done; } if (version == topology.topology_version) @@ -262,6 +266,10 @@ int MediaDevice::populate() populateLinks(topology)) valid_ = true; + ret = 0; +done: + close(); + delete[] ents; delete[] interfaces; delete[] pads; @@ -272,7 +280,7 @@ int MediaDevice::populate() return -EINVAL; } - return 0; + return ret; } /** diff --git a/test/media_device/media_device_print_test.cpp b/test/media_device/media_device_print_test.cpp index 3eef973939201b27..ceffd538e13fca73 100644 --- a/test/media_device/media_device_print_test.cpp +++ b/test/media_device/media_device_print_test.cpp @@ -124,10 +124,6 @@ int MediaDevicePrintTest::testMediaDevice(const string deviceNode) dev.close(); - ret = dev.open(); - if (ret) - return ret; - ret = dev.populate(); if (ret) return ret; @@ -135,8 +131,6 @@ int MediaDevicePrintTest::testMediaDevice(const string deviceNode) /* Print out the media graph. */ printMediaGraph(dev, cerr); - dev.close(); - return 0; } diff --git a/test/pipeline/ipu3/ipu3_pipeline_test.cpp b/test/pipeline/ipu3/ipu3_pipeline_test.cpp index 953f0233cde485cb..1d4cc4d4950b8391 100644 --- a/test/pipeline/ipu3/ipu3_pipeline_test.cpp +++ b/test/pipeline/ipu3/ipu3_pipeline_test.cpp @@ -71,11 +71,6 @@ int IPU3PipelineTest::init() return TestSkip; } - if (cio2->open()) { - cerr << "Failed to open media device " << cio2->deviceNode() << endl; - return TestFail; - } - /* * Camera sensor are connected to the CIO2 unit. * Count how many sensors are connected in the system diff --git a/test/v4l2_subdevice/v4l2_subdevice_test.cpp b/test/v4l2_subdevice/v4l2_subdevice_test.cpp index 5810ef3c962fab8e..562a638cb28ebfb2 100644 --- a/test/v4l2_subdevice/v4l2_subdevice_test.cpp +++ b/test/v4l2_subdevice/v4l2_subdevice_test.cpp @@ -45,13 +45,6 @@ int V4L2SubdeviceTest::init() return TestSkip; } - int ret = media_->open(); - if (ret) { - cerr << "Unable to open media device: " << media_->deviceNode() - << ": " << strerror(ret) << endl; - return TestSkip; - } - MediaEntity *videoEntity = media_->getEntityByName("Scaler"); if (!videoEntity) { cerr << "Unable to find media entity 'Scaler'" << endl; @@ -59,8 +52,7 @@ int V4L2SubdeviceTest::init() } scaler_ = new V4L2Subdevice(videoEntity); - ret = scaler_->open(); - if (ret) { + if (scaler_->open()) { cerr << "Unable to open video subdevice " << scaler_->entity()->deviceNode() << endl; return TestSkip; From patchwork Fri May 17 00:54:39 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 1208 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DFE8560C02 for ; Fri, 17 May 2019 02:55:10 +0200 (CEST) X-Halon-ID: 6ac92fa8-783e-11e9-8d05-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (unknown [89.233.230.99]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 6ac92fa8-783e-11e9-8d05-005056917f90; Fri, 17 May 2019 02:55:08 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Fri, 17 May 2019 02:54:39 +0200 Message-Id: <20190517005447.27171-4-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> References: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 03/11] libcamera: media_device: Only read device information in populate() X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 May 2019 00:55:11 -0000 There is no reason to reread the MEDIA_IOC_DEVICE_INFO information every time the media device is opened. Move it populate() where it will be read once together the other information about the media device. Signed-off-by: Niklas Söderlund Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/libcamera/media_device.cpp | 51 +++++++++++++++------------------- 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index c39775164fab7487..eef348e35eb6a97f 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -126,18 +126,11 @@ bool MediaDevice::acquire() */ /** - * \brief Open a media device and retrieve device information - * - * Before populating the media graph or performing any operation that interact - * with the device node associated with the media device, the device node must - * be opened. - * - * This function also retrieves media device information from the device node, - * which can be queried through driver(). - * - * If the device is already open the function returns -EBUSY. + * \brief Open the media device * * \return 0 on success or a negative error code otherwise + * \retval -EBUSY Media device already open + * \sa close() */ int MediaDevice::open() { @@ -156,20 +149,6 @@ int MediaDevice::open() } fd_ = ret; - struct media_device_info info = { }; - ret = ioctl(fd_, MEDIA_IOC_DEVICE_INFO, &info); - if (ret) { - ret = -errno; - LOG(MediaDevice, Error) - << "Failed to get media device info " - << ": " << strerror(-ret); - return ret; - } - - driver_ = info.driver; - model_ = info.model; - version_ = info.media_version; - return 0; } @@ -196,12 +175,13 @@ void MediaDevice::close() } /** - * \brief Populate the media graph with media objects + * \brief Populate the MediaDevice with device information and media objects * - * This function enumerates all media objects in the media device graph and - * creates their MediaObject representations. All entities, pads and links are - * stored as MediaEntity, MediaPad and MediaLink respectively, with cross- - * references between objects. Interfaces are not processed. + * This function retrieves the media device information and enumerates all + * media objects in the media device graph and creates their MediaObject + * representations. All entities, pads and links are stored as MediaEntity, + * MediaPad and MediaLink respectively, with cross-references between objects. + * Interfaces are not processed. * * Entities are stored in a separate list in the MediaDevice to ease lookup, * while pads are accessible from the entity they belong to and links from the @@ -225,6 +205,19 @@ int MediaDevice::populate() if (ret) return ret; + struct media_device_info info = {}; + ret = ioctl(fd_, MEDIA_IOC_DEVICE_INFO, &info); + if (ret) { + ret = -errno; + LOG(MediaDevice, Error) + << "Failed to get media device info " << strerror(-ret); + goto done; + } + + driver_ = info.driver; + model_ = info.model; + version_ = info.media_version; + /* * Keep calling G_TOPOLOGY until the version number stays stable. */ From patchwork Fri May 17 00:54:40 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 1209 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 992D461867 for ; Fri, 17 May 2019 02:55:11 +0200 (CEST) X-Halon-ID: 6b5ae3e8-783e-11e9-8d05-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (unknown [89.233.230.99]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 6b5ae3e8-783e-11e9-8d05-005056917f90; Fri, 17 May 2019 02:55:09 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Fri, 17 May 2019 02:54:40 +0200 Message-Id: <20190517005447.27171-5-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> References: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 04/11] libcamera: media_device: Handle media device fd in acquire() and release() X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 May 2019 00:55:11 -0000 To gain better control of when a file descriptor is open to the media device and reduce the work needed in pipeline handler implementations, handle the file descriptor in acquire() and release(). This changes the current behavior where a file descriptor is only open when requested by the pipeline handler to that one is always open as long a media device is acquired. This new behavior is desired to allow implementing exclusive access to a pipeline handler between processes. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/include/media_device.h | 2 +- src/libcamera/media_device.cpp | 29 +++++++------- src/libcamera/pipeline/ipu3/ipu3.cpp | 39 +++--------------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 42 +++++--------------- test/media_device/media_device_link_test.cpp | 7 ---- 5 files changed, 33 insertions(+), 86 deletions(-) diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h index 9f038093b2b22fc7..883985055eb419dd 100644 --- a/src/libcamera/include/media_device.h +++ b/src/libcamera/include/media_device.h @@ -27,7 +27,7 @@ public: ~MediaDevice(); bool acquire(); - void release() { acquired_ = false; } + void release(); bool busy() const { return acquired_; } int open(); diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index eef348e35eb6a97f..b1cc37e840a9fa9d 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -40,10 +40,9 @@ LOG_DEFINE_CATEGORY(MediaDevice) * instance. * * The instance is created with an empty media graph. Before performing any - * other operation, it must be opened with the open() function and the media - * graph populated by calling populate(). Instances of MediaEntity, MediaPad and - * MediaLink are created to model the media graph, and stored in a map indexed - * by object id. + * other operation, it must be populate by calling populate(). Instances of + * MediaEntity, MediaPad and MediaLink are created to model the media graph, + * and stored in a map indexed by object id. * * The graph is valid once successfully populated, as reported by the valid() * function. It can be queried to list all entities(), or entities can be @@ -51,12 +50,6 @@ LOG_DEFINE_CATEGORY(MediaDevice) * entity to entity through pads and links as exposed by the corresponding * classes. * - * An open media device will keep an open file handle for the underlying media - * controller device node. It can be closed at any time with a call to close(). - * This will not invalidate the media graph and all cached media objects remain - * valid and can be accessed normally. The device can then be later reopened if - * needed to perform other operations that interact with the device node. - * * Media device can be claimed for exclusive use with acquire(), released with * release() and tested with busy(). This mechanism is aimed at pipeline * managers to claim media devices they support during enumeration. @@ -66,8 +59,8 @@ LOG_DEFINE_CATEGORY(MediaDevice) * \brief Construct a MediaDevice * \param[in] deviceNode The media device node path * - * Once constructed the media device is invalid, and must be opened and - * populated with open() and populate() before the media graph can be queried. + * Once constructed the media device is invalid, and must be populated with + * populate() before the media graph can be queried. */ MediaDevice::MediaDevice(const std::string &deviceNode) : deviceNode_(deviceNode), fd_(-1), valid_(false), acquired_(false) @@ -92,7 +85,8 @@ MediaDevice::~MediaDevice() * busy() function. * * Once claimed the device shall be released by its user when not needed anymore - * by calling the release() function. + * by calling the release() function. Acquiring the media device opens a file + * descriptor to the device which is kept open until release() is called. * * Exclusive access is only guaranteed if all users of the media device abide by * the device claiming mechanism, as it isn't enforced by the media device @@ -107,15 +101,22 @@ bool MediaDevice::acquire() if (acquired_) return false; + if (open()) + return false; + acquired_ = true; return true; } /** - * \fn MediaDevice::release() * \brief Release a device previously claimed for exclusive use * \sa acquire(), busy() */ +void MediaDevice::release() +{ + close(); + acquired_ = false; +} /** * \fn MediaDevice::busy() diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 75e878afad4e67a9..8a6a0e2721955d15 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -631,23 +631,10 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) /* * Disable all links that are enabled by default on CIO2, as camera * creation enables all valid links it finds. - * - * Close the CIO2 media device after, as links are enabled and should - * not need to be changed after. */ - if (cio2MediaDev_->open()) + if (cio2MediaDev_->disableLinks()) return false; - if (cio2MediaDev_->disableLinks()) { - cio2MediaDev_->close(); - return false; - } - - if (imguMediaDev_->open()) { - cio2MediaDev_->close(); - return false; - } - /* * FIXME: enabled links in one ImgU instance interfere with capture * operations on the other one. This can be easily triggered by @@ -674,14 +661,10 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) */ ret = imguMediaDev_->disableLinks(); if (ret) - goto error; + return ret; ret = registerCameras(); -error: - cio2MediaDev_->close(); - imguMediaDev_->close(); - return ret == 0; } @@ -1139,29 +1122,19 @@ int ImgUDevice::enableLinks(bool enable) std::string inputName = name_ + " input"; int ret; - /* \todo Establish rules to handle media devices open/close. */ - ret = media_->open(); - if (ret) - return ret; - ret = linkSetup(inputName, 0, name_, PAD_INPUT, enable); if (ret) - goto done; + return ret; ret = linkSetup(name_, PAD_OUTPUT, outputName, 0, enable); if (ret) - goto done; + return ret; ret = linkSetup(name_, PAD_VF, viewfinderName, 0, enable); if (ret) - goto done; + return ret; - ret = linkSetup(name_, PAD_STAT, statName, 0, enable); - -done: - media_->close(); - - return ret; + return linkSetup(name_, PAD_STAT, statName, 0, enable); } /*------------------------------------------------------------------------------ diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 134d3df4e1eb2b9b..b94d742dd6ec33a4 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -148,10 +148,6 @@ int PipelineHandlerRkISP1::configureStreams(Camera *camera, */ const MediaPad *pad = dphy_->entity()->getPadByIndex(0); - ret = media_->open(); - if (ret < 0) - return ret; - for (MediaLink *link : pad->links()) { bool enable = link->source()->entity() == sensor->entity(); @@ -169,8 +165,6 @@ int PipelineHandlerRkISP1::configureStreams(Camera *camera, break; } - media_->close(); - if (ret < 0) return ret; @@ -352,7 +346,6 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) { const MediaPad *pad; - int ret; DeviceMatch dm("rkisp1"); dm.add("rkisp1-isp-subdev"); @@ -368,35 +361,27 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) media_->acquire(); - ret = media_->open(); - if (ret < 0) - return ret; - /* Create the V4L2 subdevices we will need. */ dphy_ = V4L2Subdevice::fromEntityName(media_.get(), "rockchip-sy-mipi-dphy"); - ret = dphy_->open(); - if (ret < 0) - goto done; + if (dphy_->open() < 0) + return false; isp_ = V4L2Subdevice::fromEntityName(media_.get(), "rkisp1-isp-subdev"); - ret = isp_->open(); - if (ret < 0) - goto done; + if (isp_->open() < 0) + return false; /* Locate and open the capture video node. */ video_ = V4L2Device::fromEntityName(media_.get(), "rkisp1_mainpath"); - ret = video_->open(); - if (ret < 0) - goto done; + if (video_->open() < 0) + return false; video_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady); /* Configure default links. */ - ret = initLinks(); - if (ret < 0) { + if (initLinks() < 0) { LOG(RkISP1, Error) << "Failed to setup links"; - goto done; + return false; } /* @@ -404,18 +389,13 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) * camera instance for each of them. */ pad = dphy_->entity()->getPadByIndex(0); - if (!pad) { - ret = -EINVAL; - goto done; - } + if (!pad) + return false; for (MediaLink *link : pad->links()) createCamera(link->source()->entity()); -done: - media_->close(); - - return ret == 0; + return true; } /* ----------------------------------------------------------------------------- diff --git a/test/media_device/media_device_link_test.cpp b/test/media_device/media_device_link_test.cpp index 484d3691310f78a0..334bb44a6fc14371 100644 --- a/test/media_device/media_device_link_test.cpp +++ b/test/media_device/media_device_link_test.cpp @@ -57,12 +57,6 @@ class MediaDeviceLinkTest : public Test return TestSkip; } - if (dev_->open()) { - cerr << "Failed to open media device at " - << dev_->deviceNode() << endl; - return TestFail; - } - return 0; } @@ -238,7 +232,6 @@ class MediaDeviceLinkTest : public Test void cleanup() { - dev_->close(); dev_->release(); } From patchwork Fri May 17 00:54:41 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 1210 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 518646186F for ; Fri, 17 May 2019 02:55:12 +0200 (CEST) X-Halon-ID: 6be6f0e0-783e-11e9-8d05-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (unknown [89.233.230.99]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 6be6f0e0-783e-11e9-8d05-005056917f90; Fri, 17 May 2019 02:55:10 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Fri, 17 May 2019 02:54:41 +0200 Message-Id: <20190517005447.27171-6-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> References: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 05/11] test: media_device: Create a common MediaDeviceTest base class X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 May 2019 00:55:12 -0000 Before adding more tests which will act on the vimc pipeline break out a common base from media_device_link_test.cpp which already acts on vimc. The new common base class will help reduce code duplication. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- test/media_device/media_device_link_test.cpp | 71 ++++++-------------- test/media_device/media_device_test.cpp | 36 ++++++++++ test/media_device/media_device_test.h | 34 ++++++++++ test/media_device/meson.build | 9 ++- 4 files changed, 99 insertions(+), 51 deletions(-) create mode 100644 test/media_device/media_device_test.cpp create mode 100644 test/media_device/media_device_test.h diff --git a/test/media_device/media_device_link_test.cpp b/test/media_device/media_device_link_test.cpp index 334bb44a6fc14371..60e502df8043f979 100644 --- a/test/media_device/media_device_link_test.cpp +++ b/test/media_device/media_device_link_test.cpp @@ -4,13 +4,10 @@ * * media_device_link_test.cpp - Tests link handling on VIMC media device */ + #include -#include -#include "device_enumerator.h" -#include "media_device.h" - -#include "test.h" +#include "media_device_test.h" using namespace libcamera; using namespace std; @@ -29,44 +26,21 @@ using namespace std; * loaded) the test is skipped. */ -class MediaDeviceLinkTest : public Test +class MediaDeviceLinkTest : public MediaDeviceTest { - int init() - { - enumerator = unique_ptr(DeviceEnumerator::create()); - if (!enumerator) { - cerr << "Failed to create device enumerator" << endl; - return TestFail; - } - - if (enumerator->enumerate()) { - cerr << "Failed to enumerate media devices" << endl; - return TestFail; - } - - DeviceMatch dm("vimc"); - dev_ = enumerator->search(dm); - if (!dev_) { - cerr << "No VIMC media device found: skip test" << endl; - return TestSkip; - } - - if (!dev_->acquire()) { - cerr << "Unable to acquire media device " - << dev_->deviceNode() << endl; - return TestSkip; - } - - return 0; - } - int run() { + if (!media_->acquire()) { + cerr << "Unable to acquire media device " + << media_->deviceNode() << endl; + return TestFail; + } + /* * First of all disable all links in the media graph to * ensure we start from a known state. */ - if (dev_->disableLinks()) { + if (media_->disableLinks()) { cerr << "Failed to disable all links in the media graph"; return TestFail; } @@ -76,26 +50,26 @@ class MediaDeviceLinkTest : public Test * different methods the media device offers. */ string linkName("'Debayer A':[1] -> 'Scaler':[0]'"); - MediaLink *link = dev_->link("Debayer A", 1, "Scaler", 0); + MediaLink *link = media_->link("Debayer A", 1, "Scaler", 0); if (!link) { cerr << "Unable to find link: " << linkName << " using lookup by name" << endl; return TestFail; } - MediaEntity *source = dev_->getEntityByName("Debayer A"); + MediaEntity *source = media_->getEntityByName("Debayer A"); if (!source) { cerr << "Unable to find entity: 'Debayer A'" << endl; return TestFail; } - MediaEntity *sink = dev_->getEntityByName("Scaler"); + MediaEntity *sink = media_->getEntityByName("Scaler"); if (!sink) { cerr << "Unable to find entity: 'Scaler'" << endl; return TestFail; } - MediaLink *link2 = dev_->link(source, 1, sink, 0); + MediaLink *link2 = media_->link(source, 1, sink, 0); if (!link2) { cerr << "Unable to find link: " << linkName << " using lookup by entity" << endl; @@ -108,7 +82,7 @@ class MediaDeviceLinkTest : public Test return TestFail; } - link2 = dev_->link(source->getPadByIndex(1), + link2 = media_->link(source->getPadByIndex(1), sink->getPadByIndex(0)); if (!link2) { cerr << "Unable to find link: " << linkName @@ -160,7 +134,7 @@ class MediaDeviceLinkTest : public Test /* Try to get a non existing link. */ linkName = "'Sensor A':[1] -> 'Scaler':[0]"; - link = dev_->link("Sensor A", 1, "Scaler", 0); + link = media_->link("Sensor A", 1, "Scaler", 0); if (link) { cerr << "Link lookup for " << linkName << " succeeded but link does not exist" @@ -170,7 +144,7 @@ class MediaDeviceLinkTest : public Test /* Now get an immutable link and try to disable it. */ linkName = "'Sensor A':[0] -> 'Raw Capture 0':[0]"; - link = dev_->link("Sensor A", 0, "Raw Capture 0", 0); + link = media_->link("Sensor A", 0, "Raw Capture 0", 0); if (!link) { cerr << "Unable to find link: " << linkName << " using lookup by name" << endl; @@ -195,7 +169,7 @@ class MediaDeviceLinkTest : public Test * after disabling all links in the media graph. */ linkName = "'Debayer B':[1] -> 'Scaler':[0]'"; - link = dev_->link("Debayer B", 1, "Scaler", 0); + link = media_->link("Debayer B", 1, "Scaler", 0); if (!link) { cerr << "Unable to find link: " << linkName << " using lookup by name" << endl; @@ -215,7 +189,7 @@ class MediaDeviceLinkTest : public Test return TestFail; } - if (dev_->disableLinks()) { + if (media_->disableLinks()) { cerr << "Failed to disable all links in the media graph"; return TestFail; } @@ -227,17 +201,14 @@ class MediaDeviceLinkTest : public Test return TestFail; } + return 0; } void cleanup() { - dev_->release(); + media_->release(); } - -private: - unique_ptr enumerator; - shared_ptr dev_; }; TEST_REGISTER(MediaDeviceLinkTest); diff --git a/test/media_device/media_device_test.cpp b/test/media_device/media_device_test.cpp new file mode 100644 index 0000000000000000..055069ad1c331cbb --- /dev/null +++ b/test/media_device/media_device_test.cpp @@ -0,0 +1,36 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * libcamera MediaDevice API tests + */ + +#include + +#include "media_device_test.h" + +using namespace libcamera; +using namespace std; + +int MediaDeviceTest::init() +{ + enumerator_ = unique_ptr(DeviceEnumerator::create()); + if (!enumerator_) { + cerr << "Failed to create device enumerator" << endl; + return TestFail; + } + + if (enumerator_->enumerate()) { + cerr << "Failed to enumerate media devices" << endl; + return TestFail; + } + + DeviceMatch dm("vimc"); + media_ = enumerator_->search(dm); + if (!media_) { + cerr << "No VIMC media device found: skip test" << endl; + return TestSkip; + } + + return TestPass; +} diff --git a/test/media_device/media_device_test.h b/test/media_device/media_device_test.h new file mode 100644 index 0000000000000000..cdbd14841d5ccca2 --- /dev/null +++ b/test/media_device/media_device_test.h @@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * media_device_test.h - libcamera media device test base class + */ +#ifndef __LIBCAMERA_MEDIADEVICE_TEST_H__ +#define __LIBCAMERA_MEDIADEVICE_TEST_H__ + +#include + +#include "device_enumerator.h" +#include "media_device.h" + +#include "test.h" + +using namespace libcamera; + +class MediaDeviceTest : public Test +{ +public: + MediaDeviceTest() + : media_(nullptr), enumerator_(nullptr) {} + +protected: + int init(); + + std::shared_ptr media_; + +private: + std::unique_ptr enumerator_; +}; + +#endif /* __LIBCAMERA_MEDIADEVICE_TEST_H__ */ diff --git a/test/media_device/meson.build b/test/media_device/meson.build index d91a022fab190abf..364b4ecf662077ac 100644 --- a/test/media_device/meson.build +++ b/test/media_device/meson.build @@ -1,11 +1,18 @@ +libmediadevicetest_sources = files([ + 'media_device_test.cpp', +]) + media_device_tests = [ ['media_device_print_test', 'media_device_print_test.cpp'], ['media_device_link_test', 'media_device_link_test.cpp'], ] +libmediadevicetest = static_library('libmediadevicetest', libmediadevicetest_sources, + include_directories : test_includes_internal) + foreach t : media_device_tests exe = executable(t[0], t[1], - link_with : test_libraries, + link_with : [test_libraries, libmediadevicetest], include_directories : test_includes_internal) test(t[0], exe, suite: 'media_device', is_parallel: false) From patchwork Fri May 17 00:54:42 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 1211 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D4ABB60103 for ; Fri, 17 May 2019 02:55:13 +0200 (CEST) X-Halon-ID: 6c598152-783e-11e9-8d05-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (unknown [89.233.230.99]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 6c598152-783e-11e9-8d05-005056917f90; Fri, 17 May 2019 02:55:11 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Fri, 17 May 2019 02:54:42 +0200 Message-Id: <20190517005447.27171-7-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> References: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 06/11] test: media_device: Add test for acquire() and release() X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 May 2019 00:55:14 -0000 The interfaces MediaDevice::{open,close}() are about to be made private, replace them with a test of MediaDevice::{acquire,release}() instead. The new test will implicitly tests the open() and close() methods as they are about to be move inside acquire() and release() which will remain public. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- test/media_device/media_device_acquire.cpp | 33 +++++++++++++++++++ test/media_device/media_device_print_test.cpp | 11 ------- test/media_device/meson.build | 1 + 3 files changed, 34 insertions(+), 11 deletions(-) create mode 100644 test/media_device/media_device_acquire.cpp diff --git a/test/media_device/media_device_acquire.cpp b/test/media_device/media_device_acquire.cpp new file mode 100644 index 0000000000000000..d1e3d74439e1c513 --- /dev/null +++ b/test/media_device/media_device_acquire.cpp @@ -0,0 +1,33 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * media_device_acquire.cpp- Test acquire/release of a MediaDevice + */ + +#include "media_device_test.h" + +using namespace libcamera; + +class MediaDeviceAcquire : public MediaDeviceTest +{ + int run() + { + if (!media_->acquire()) + return TestFail; + + if (media_->acquire()) + return TestFail; + + media_->release(); + + if (!media_->acquire()) + return TestFail; + + media_->release(); + + return TestPass; + } +}; + +TEST_REGISTER(MediaDeviceAcquire); diff --git a/test/media_device/media_device_print_test.cpp b/test/media_device/media_device_print_test.cpp index ceffd538e13fca73..30d929b8c76387a7 100644 --- a/test/media_device/media_device_print_test.cpp +++ b/test/media_device/media_device_print_test.cpp @@ -113,17 +113,6 @@ int MediaDevicePrintTest::testMediaDevice(const string deviceNode) MediaDevice dev(deviceNode); int ret; - /* Fuzzy open/close sequence. */ - ret = dev.open(); - if (ret) - return ret; - - ret = dev.open(); - if (!ret) - return ret; - - dev.close(); - ret = dev.populate(); if (ret) return ret; diff --git a/test/media_device/meson.build b/test/media_device/meson.build index 364b4ecf662077ac..05bf7c9b8de335d2 100644 --- a/test/media_device/meson.build +++ b/test/media_device/meson.build @@ -3,6 +3,7 @@ libmediadevicetest_sources = files([ ]) media_device_tests = [ + ['media_device_acquire', 'media_device_acquire.cpp'], ['media_device_print_test', 'media_device_print_test.cpp'], ['media_device_link_test', 'media_device_link_test.cpp'], ] From patchwork Fri May 17 00:54:43 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 1212 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DA45661865 for ; Fri, 17 May 2019 02:55:13 +0200 (CEST) X-Halon-ID: 6cc7ba6e-783e-11e9-8d05-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (unknown [89.233.230.99]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 6cc7ba6e-783e-11e9-8d05-005056917f90; Fri, 17 May 2019 02:55:12 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Fri, 17 May 2019 02:54:43 +0200 Message-Id: <20190517005447.27171-8-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> References: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 07/11] libcamera: media_device: Make open() and close() private X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 May 2019 00:55:14 -0000 All external callers to open() and close() have been refactored and there is no need to expose these functions anymore, make them private. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/include/media_device.h | 6 +- src/libcamera/media_device.cpp | 98 ++++++++++++++-------------- 2 files changed, 52 insertions(+), 52 deletions(-) diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h index 883985055eb419dd..e513a2fee1b91a1b 100644 --- a/src/libcamera/include/media_device.h +++ b/src/libcamera/include/media_device.h @@ -30,9 +30,6 @@ public: void release(); bool busy() const { return acquired_; } - int open(); - void close(); - int populate(); bool valid() const { return valid_; } @@ -62,6 +59,9 @@ private: bool valid_; bool acquired_; + int open(); + void close(); + std::map objects_; MediaObject *object(unsigned int id); bool addObject(MediaObject *object); diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index b1cc37e840a9fa9d..62c59e7e8fc0299f 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -126,55 +126,6 @@ void MediaDevice::release() * \sa acquire(), release() */ -/** - * \brief Open the media device - * - * \return 0 on success or a negative error code otherwise - * \retval -EBUSY Media device already open - * \sa close() - */ -int MediaDevice::open() -{ - if (fd_ != -1) { - LOG(MediaDevice, Error) << "MediaDevice already open"; - return -EBUSY; - } - - int ret = ::open(deviceNode_.c_str(), O_RDWR); - if (ret < 0) { - ret = -errno; - LOG(MediaDevice, Error) - << "Failed to open media device at " - << deviceNode_ << ": " << strerror(-ret); - return ret; - } - fd_ = ret; - - return 0; -} - -/** - * \brief Close the media device - * - * This function closes the media device node. It does not invalidate the media - * graph and all cached media objects remain valid and can be accessed normally. - * Once closed no operation interacting with the media device node can be - * performed until the device is opened again. - * - * Closing an already closed device is allowed and will not perform any - * operation. - * - * \sa open() - */ -void MediaDevice::close() -{ - if (fd_ == -1) - return; - - ::close(fd_); - fd_ = -1; -} - /** * \brief Populate the MediaDevice with device information and media objects * @@ -440,6 +391,55 @@ int MediaDevice::disableLinks() * driver unloading for most devices. The media device is passed as a parameter. */ +/** + * \brief Open the media device + * + * \return 0 on success or a negative error code otherwise + * \retval -EBUSY Media device already open + * \sa close() + */ +int MediaDevice::open() +{ + if (fd_ != -1) { + LOG(MediaDevice, Error) << "MediaDevice already open"; + return -EBUSY; + } + + int ret = ::open(deviceNode_.c_str(), O_RDWR); + if (ret < 0) { + ret = -errno; + LOG(MediaDevice, Error) + << "Failed to open media device at " + << deviceNode_ << ": " << strerror(-ret); + return ret; + } + fd_ = ret; + + return 0; +} + +/** + * \brief Close the media device + * + * This function closes the media device node. It does not invalidate the media + * graph and all cached media objects remain valid and can be accessed normally. + * Once closed no operation interacting with the media device node can be + * performed until the device is opened again. + * + * Closing an already closed device is allowed and will not perform any + * operation. + * + * \sa open() + */ +void MediaDevice::close() +{ + if (fd_ == -1) + return; + + ::close(fd_); + fd_ = -1; +} + /** * \var MediaDevice::objects_ * \brief Global map of media objects (entities, pads, links) keyed by their From patchwork Fri May 17 00:54:44 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 1213 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E267D61878 for ; Fri, 17 May 2019 02:55:14 +0200 (CEST) X-Halon-ID: 6d46c880-783e-11e9-8d05-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (unknown [89.233.230.99]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 6d46c880-783e-11e9-8d05-005056917f90; Fri, 17 May 2019 02:55:13 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Fri, 17 May 2019 02:54:44 +0200 Message-Id: <20190517005447.27171-9-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> References: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 08/11] libcamera: media_device: Add functions to lock device for other processes X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 May 2019 00:55:15 -0000 Add lock() and unlock() which are backed by lockf() and allow an instance of libcamera to claim exclusive access to a media device. These functions are the base of allowing multiple user of libcamera to coexist in the system without stepping on each other's toes. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/include/media_device.h | 4 ++ src/libcamera/media_device.cpp | 59 +++++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h index e513a2fee1b91a1b..7b88e2875d59dfc3 100644 --- a/src/libcamera/include/media_device.h +++ b/src/libcamera/include/media_device.h @@ -30,6 +30,9 @@ public: void release(); bool busy() const { return acquired_; } + bool lock(); + void unlock(); + int populate(); bool valid() const { return valid_; } @@ -58,6 +61,7 @@ private: int fd_; bool valid_; bool acquired_; + bool lockOwner_; int open(); void close(); diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index 62c59e7e8fc0299f..bd99deba098db4ec 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -63,7 +63,8 @@ LOG_DEFINE_CATEGORY(MediaDevice) * populate() before the media graph can be queried. */ MediaDevice::MediaDevice(const std::string &deviceNode) - : deviceNode_(deviceNode), fd_(-1), valid_(false), acquired_(false) + : deviceNode_(deviceNode), fd_(-1), valid_(false), acquired_(false), + lockOwner_(false) { } @@ -118,6 +119,62 @@ void MediaDevice::release() acquired_ = false; } +/** + * \brief Lock the device to prevent it from being used by other instances of + * libcamera + * + * Multiple instances of libcamera might be running on the same system, at the + * same time. To allow the different instances to coexist, system resources in + * the form of media devices must be accessible for enumerating the cameras + * they provide at all times, while still allowing an instance to lock a + * resource while it prepares to actively use a camera from the resource. + * + * This method shall not be called from a pipeline handler implementation + * directly, as the base PipelineHandler implementation handles this on the + * behalf of the specified implementation. + * + * \return True if the device could be locked, false otherwise + * \sa unlock() + */ +bool MediaDevice::lock() +{ + if (fd_ == -1) + return false; + + /* Do not allow nested locking in the same libcamera instance. */ + if (lockOwner_) + return false; + + if (lockf(fd_, F_TLOCK, 0)) + return false; + + lockOwner_ = true; + + return true; +} + +/** + * \brief Unlock the device and free it for use for libcamera instances + * + * This method shall not be called from a pipeline handler implementation + * directly, as the base PipelineHandler implementation handles this on the + * behalf of the specified implementation. + * + * \sa lock() + */ +void MediaDevice::unlock() +{ + if (fd_ == -1) + return; + + if (!lockOwner_) + return; + + lockOwner_ = false; + + lockf(fd_, F_ULOCK, 0); +} + /** * \fn MediaDevice::busy() * \brief Check if a device is in use From patchwork Fri May 17 00:54:45 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 1214 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6E14C61878 for ; Fri, 17 May 2019 02:55:15 +0200 (CEST) X-Halon-ID: 6daed103-783e-11e9-8d05-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (unknown [89.233.230.99]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 6daed103-783e-11e9-8d05-005056917f90; Fri, 17 May 2019 02:55:13 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Fri, 17 May 2019 02:54:45 +0200 Message-Id: <20190517005447.27171-10-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> References: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 09/11] libcamera: pipeline_handler: Keep track of MediaDevice X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 May 2019 00:55:16 -0000 Instead of requiring each pipeline handle implementation to keep track of calling release() on its media devices upon deletion, keep track of them in the base class. Add a helper that pipeline handlers shall use to acquire a media device instead of directly interacting with the DeviceEnumerator. This also means that pipeline handler implementations do no need to keep a shared_ptr<> reference to the media devices they store locally as the PipelineHandler base class will keep a shared_ptr<> reference to all media devices consumed for the entire lifetime of the pipeline handler implementation. Centrally keeping track of media devices will also be beneficial to implement exclusive access to pipelines across processes. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/include/pipeline_handler.h | 4 +++ src/libcamera/pipeline/ipu3/ipu3.cpp | 30 ++++++---------------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 16 ++++-------- src/libcamera/pipeline/uvcvideo.cpp | 25 ++++++------------ src/libcamera/pipeline/vimc.cpp | 20 +++------------ src/libcamera/pipeline_handler.cpp | 32 ++++++++++++++++++++++++ 6 files changed, 59 insertions(+), 68 deletions(-) diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 5830e53108faa105..8e6a136dd4907d9f 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -22,6 +22,7 @@ class Camera; class CameraConfiguration; class CameraManager; class DeviceEnumerator; +class DeviceMatch; class MediaDevice; class PipelineHandler; class Request; @@ -53,6 +54,8 @@ public: virtual ~PipelineHandler(); virtual bool match(DeviceEnumerator *enumerator) = 0; + MediaDevice *acquireMediaDevice(DeviceEnumerator *enumerator, + const DeviceMatch &dm); virtual CameraConfiguration streamConfiguration(Camera *camera, const std::vector &usages) = 0; @@ -84,6 +87,7 @@ private: void mediaDeviceDisconnected(MediaDevice *media); virtual void disconnect(); + std::vector> mediaDevices_; std::vector> cameras_; std::map> cameraData_; }; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 8a6a0e2721955d15..75a70e66eacc443a 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -149,7 +149,6 @@ class PipelineHandlerIPU3 : public PipelineHandler { public: PipelineHandlerIPU3(CameraManager *manager); - ~PipelineHandlerIPU3(); CameraConfiguration streamConfiguration(Camera *camera, @@ -201,8 +200,8 @@ private: ImgUDevice imgu0_; ImgUDevice imgu1_; - std::shared_ptr cio2MediaDev_; - std::shared_ptr imguMediaDev_; + MediaDevice *cio2MediaDev_; + MediaDevice *imguMediaDev_; }; PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager) @@ -210,15 +209,6 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager) { } -PipelineHandlerIPU3::~PipelineHandlerIPU3() -{ - if (cio2MediaDev_) - cio2MediaDev_->release(); - - if (imguMediaDev_) - imguMediaDev_->release(); -} - CameraConfiguration PipelineHandlerIPU3::streamConfiguration(Camera *camera, const std::vector &usages) @@ -614,20 +604,14 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) imgu_dm.add("ipu3-imgu 1 viewfinder"); imgu_dm.add("ipu3-imgu 1 3a stat"); - cio2MediaDev_ = enumerator->search(cio2_dm); + cio2MediaDev_ = acquireMediaDevice(enumerator, cio2_dm); if (!cio2MediaDev_) return false; - if (!cio2MediaDev_->acquire()) - return false; - - imguMediaDev_ = enumerator->search(imgu_dm); + imguMediaDev_ = acquireMediaDevice(enumerator, imgu_dm); if (!imguMediaDev_) return false; - if (!imguMediaDev_->acquire()) - return false; - /* * Disable all links that are enabled by default on CIO2, as camera * creation enables all valid links it finds. @@ -682,11 +666,11 @@ int PipelineHandlerIPU3::registerCameras() { int ret; - ret = imgu0_.init(imguMediaDev_.get(), 0); + ret = imgu0_.init(imguMediaDev_, 0); if (ret) return ret; - ret = imgu1_.init(imguMediaDev_.get(), 1); + ret = imgu1_.init(imguMediaDev_, 1); if (ret) return ret; @@ -705,7 +689,7 @@ int PipelineHandlerIPU3::registerCameras() }; CIO2Device *cio2 = &data->cio2_; - ret = cio2->init(cio2MediaDev_.get(), id); + ret = cio2->init(cio2MediaDev_, id); if (ret) continue; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index b94d742dd6ec33a4..b395405c9ddb7f17 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -81,7 +81,7 @@ private: int createCamera(MediaEntity *sensor); void bufferReady(Buffer *buffer); - std::shared_ptr media_; + MediaDevice *media_; V4L2Subdevice *dphy_; V4L2Subdevice *isp_; V4L2Device *video_; @@ -100,9 +100,6 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1() delete video_; delete isp_; delete dphy_; - - if (media_) - media_->release(); } /* ----------------------------------------------------------------------------- @@ -355,24 +352,21 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) dm.add("rkisp1-input-params"); dm.add("rockchip-sy-mipi-dphy"); - media_ = enumerator->search(dm); + media_ = acquireMediaDevice(enumerator, dm); if (!media_) return false; - media_->acquire(); - /* Create the V4L2 subdevices we will need. */ - dphy_ = V4L2Subdevice::fromEntityName(media_.get(), - "rockchip-sy-mipi-dphy"); + dphy_ = V4L2Subdevice::fromEntityName(media_, "rockchip-sy-mipi-dphy"); if (dphy_->open() < 0) return false; - isp_ = V4L2Subdevice::fromEntityName(media_.get(), "rkisp1-isp-subdev"); + isp_ = V4L2Subdevice::fromEntityName(media_, "rkisp1-isp-subdev"); if (isp_->open() < 0) return false; /* Locate and open the capture video node. */ - video_ = V4L2Device::fromEntityName(media_.get(), "rkisp1_mainpath"); + video_ = V4L2Device::fromEntityName(media_, "rkisp1_mainpath"); if (video_->open() < 0) return false; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index e40b052f4d877d5d..351712cfdc69594d 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -24,7 +24,6 @@ class PipelineHandlerUVC : public PipelineHandler { public: PipelineHandlerUVC(CameraManager *manager); - ~PipelineHandlerUVC(); CameraConfiguration streamConfiguration(Camera *camera, @@ -69,21 +68,13 @@ private: return static_cast( PipelineHandler::cameraData(camera)); } - - std::shared_ptr media_; }; PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) - : PipelineHandler(manager), media_(nullptr) + : PipelineHandler(manager) { } -PipelineHandlerUVC::~PipelineHandlerUVC() -{ - if (media_) - media_->release(); -} - CameraConfiguration PipelineHandlerUVC::streamConfiguration(Camera *camera, const std::vector &usages) @@ -177,19 +168,17 @@ int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) { + MediaDevice *media; DeviceMatch dm("uvcvideo"); - media_ = enumerator->search(dm); - if (!media_) - return false; - - if (!media_->acquire()) + media = acquireMediaDevice(enumerator, dm); + if (!media) return false; std::unique_ptr data = utils::make_unique(this); /* Locate and open the default video node. */ - for (MediaEntity *entity : media_->entities()) { + for (MediaEntity *entity : media->entities()) { if (entity->flags() & MEDIA_ENT_FL_DEFAULT) { data->video_ = new V4L2Device(entity); break; @@ -208,11 +197,11 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) /* Create and register the camera. */ std::set streams{ &data->stream_ }; - std::shared_ptr camera = Camera::create(this, media_->model(), streams); + std::shared_ptr camera = Camera::create(this, media->model(), streams); registerCamera(std::move(camera), std::move(data)); /* Enable hot-unplug notifications. */ - hotplugMediaDevice(media_.get()); + hotplugMediaDevice(media); return true; } diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 7b6ebd4cc0a27e25..737d6df67def59c7 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -24,7 +24,6 @@ class PipelineHandlerVimc : public PipelineHandler { public: PipelineHandlerVimc(CameraManager *manager); - ~PipelineHandlerVimc(); CameraConfiguration streamConfiguration(Camera *camera, @@ -69,21 +68,13 @@ private: return static_cast( PipelineHandler::cameraData(camera)); } - - std::shared_ptr media_; }; PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager) - : PipelineHandler(manager), media_(nullptr) + : PipelineHandler(manager) { } -PipelineHandlerVimc::~PipelineHandlerVimc() -{ - if (media_) - media_->release(); -} - CameraConfiguration PipelineHandlerVimc::streamConfiguration(Camera *camera, const std::vector &usages) @@ -189,17 +180,14 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) dm.add("RGB/YUV Input"); dm.add("Scaler"); - media_ = enumerator->search(dm); - if (!media_) - return false; - - if (!media_->acquire()) + MediaDevice *media = acquireMediaDevice(enumerator, dm); + if (!media) return false; std::unique_ptr data = utils::make_unique(this); /* Locate and open the capture video node. */ - data->video_ = new V4L2Device(media_->getEntityByName("Raw Capture 1")); + data->video_ = new V4L2Device(media->getEntityByName("Raw Capture 1")); if (data->video_->open()) return false; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 4ecd6c49efd8ca89..c92ee782701f57bf 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -11,6 +11,7 @@ #include #include +#include "device_enumerator.h" #include "log.h" #include "media_device.h" #include "utils.h" @@ -116,6 +117,8 @@ PipelineHandler::PipelineHandler(CameraManager *manager) PipelineHandler::~PipelineHandler() { + for (std::shared_ptr media : mediaDevices_) + media->release(); }; /** @@ -149,6 +152,35 @@ PipelineHandler::~PipelineHandler() * created, or false otherwise */ +/** + * \brief Search and acquire a MediDevice matching a device pattern + * \param[in] enumerator Enumerator containing all media devices in the system + * \param[in] dm Device match pattern + * + * Search the device \a enumerator for an available media device matching the + * device match pattern \a dm. Matching media device that have previously been + * acquired by MediaDevice::acquire() are not considered. If a match is found, + * the media device is acquired and returned. The caller shall not release the + * device explicitly, it will be automatically released when the pipeline + * handler is destroyed. + * + * \return A pointer to the matching MediaDevice, or nullptr if no match is found + */ +MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator, + const DeviceMatch &dm) +{ + std::shared_ptr media = enumerator->search(dm); + if (!media) + return nullptr; + + if (!media->acquire()) + return nullptr; + + mediaDevices_.push_back(media); + + return media.get(); +} + /** * \fn PipelineHandler::streamConfiguration() * \brief Retrieve a group of stream configurations for a specified camera From patchwork Fri May 17 00:54:46 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 1215 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id EDA1260E4C for ; Fri, 17 May 2019 02:55:15 +0200 (CEST) X-Halon-ID: 6e415324-783e-11e9-8d05-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (unknown [89.233.230.99]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 6e415324-783e-11e9-8d05-005056917f90; Fri, 17 May 2019 02:55:14 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Fri, 17 May 2019 02:54:46 +0200 Message-Id: <20190517005447.27171-11-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> References: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 10/11] libcamera: pipeline_handler: Add functions to lock a whole pipeline X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 May 2019 00:55:16 -0000 Add lock() and unlock() which are backed by the MediaDevice implementation and lock all media devices claimed by a pipeline handler instance. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/include/pipeline_handler.h | 3 ++ src/libcamera/pipeline_handler.cpp | 36 ++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 8e6a136dd4907d9f..9f5fe3d673e294c8 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -57,6 +57,9 @@ public: MediaDevice *acquireMediaDevice(DeviceEnumerator *enumerator, const DeviceMatch &dm); + bool lock(); + void unlock(); + virtual CameraConfiguration streamConfiguration(Camera *camera, const std::vector &usages) = 0; virtual int configureStreams(Camera *camera, const CameraConfiguration &config) = 0; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index c92ee782701f57bf..1eeaf4bb6dae8601 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -181,6 +181,42 @@ MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator, return media.get(); } +/** + * \brief Lock all media devices acquired by the pipeline + * + * This method shall not be called from pipeline handler implementation, as the + * Camera class handles locking directly. + * + * \return True if the devices could be locked, false otherwise + * \sa unlock() + * \sa MediaDevice::lock() + */ +bool PipelineHandler::lock() +{ + for (std::shared_ptr &media : mediaDevices_) { + if (!media->lock()) { + unlock(); + return false; + } + } + + return true; +} + +/** + * \brief Unlock all media devices acquired by the pipeline + * + * This method shall not be called from pipeline handler implementation, as the + * Camera class handles locking directly. + * + * \sa lock() + */ +void PipelineHandler::unlock() +{ + for (std::shared_ptr &media : mediaDevices_) + media->unlock(); +} + /** * \fn PipelineHandler::streamConfiguration() * \brief Retrieve a group of stream configurations for a specified camera From patchwork Fri May 17 00:54:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 1216 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C0D1B61869 for ; Fri, 17 May 2019 02:55:16 +0200 (CEST) X-Halon-ID: 6e93dc6b-783e-11e9-8d05-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (unknown [89.233.230.99]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 6e93dc6b-783e-11e9-8d05-005056917f90; Fri, 17 May 2019 02:55:15 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Fri, 17 May 2019 02:54:47 +0200 Message-Id: <20190517005447.27171-12-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> References: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 11/11] libcamera: camera: Lock the pipeline handler in acquire() X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 May 2019 00:55:17 -0000 To allow more than one application using libcamera simultaneously there can be no overlap between which cameras are in use by which user. As a camera is part of a pipeline handler and there might be shared resources between all cameras exposed by that pipeline handler it's not enough to to only lock access to a single camera, all cameras from that pipeline need to be tied to the same process. Allow for this by locking the whole pipeline when one of its cameras is acquired by the user. Other processes can still enumerate and list all cameras in the system but can't acquire a camera from a locked pipeline handler. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/camera.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index cb45bafe3fb1ff85..a83768fcde126c32 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -470,13 +470,17 @@ void Camera::disconnect() * not blocking, if the device has already been acquired (by the same or another * process) the -EBUSY error code is returned. * + * Acquiring a camera will limit usage of any other camera(s) provided by the + * same pipeline hander to the same instance of libcamera. The limit is in + * effect until all cameras from the pipeline handler are released(). Other + * instances of libcamera can still list and examine the cameras but will fail + * if they attempt to acquire() any of them. + * * Once exclusive access isn't needed anymore, the device should be released * with a call to the release() function. * * This function affects the state of the camera, see \ref camera_operation. * - * \todo Implement exclusive access across processes. - * * \return 0 on success or a negative error code otherwise * \retval -ENODEV The camera has been disconnected from the system * \retval -EBUSY The camera is not free and can't be acquired by the caller @@ -489,6 +493,12 @@ int Camera::acquire() if (!stateIs(CameraAvailable)) return -EBUSY; + if (!pipe_->lock()) { + LOG(Camera, Info) + << "Pipeline handler in use by another process"; + return -EBUSY; + } + state_ = CameraAcquired; return 0; @@ -510,6 +520,8 @@ int Camera::release() if (!stateBetween(CameraAvailable, CameraConfigured)) return -EBUSY; + pipe_->unlock(); + state_ = CameraAvailable; return 0;