From patchwork Mon Apr 29 19:17:19 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: 1129 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DA55B60E59 for ; Mon, 29 Apr 2019 21:17:46 +0200 (CEST) X-Halon-ID: 775676e9-6ab3-11e9-8e2c-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from wyvern.station (unknown [37.182.44.227]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 775676e9-6ab3-11e9-8e2c-005056917f90; Mon, 29 Apr 2019 21:17:45 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Apr 2019 21:17:19 +0200 Message-Id: <20190429191729.29697-2-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> References: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 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: Mon, 29 Apr 2019 19:17:47 -0000 In preparation of adding more responsibility to MediaDevice::acquire() make sure all callers checks and acts properly to its return value. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart Reviewed-by: Kieran Bingham --- 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 | 3 ++- test/v4l2_subdevice/v4l2_subdevice_test.cpp | 6 +++++- 6 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index fbb37498ca8a85cd..e8b9cd8f889b7cf2 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -617,21 +617,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 2f9b5e0fdc08d089..d7ef37a8171f0eac 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -189,7 +189,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 f70b4d3c6bab4a84..25f0802c30d3d07b 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -199,7 +199,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..90e5f7a3ee0c0f2f 100644 --- a/test/v4l2_device/v4l2_device_test.cpp +++ b/test/v4l2_device/v4l2_device_test.cpp @@ -46,7 +46,8 @@ int V4L2DeviceTest::init() if (!media_) return TestSkip; - media_->acquire(); + if (!media_->acquire()) + return TestSkip; MediaEntity *entity = media_->getEntityByName("vivid-000-vid-cap"); if (!entity) diff --git a/test/v4l2_subdevice/v4l2_subdevice_test.cpp b/test/v4l2_subdevice/v4l2_subdevice_test.cpp index 72d5f72543d29378..4e16ab6cf3e5f874 100644 --- a/test/v4l2_subdevice/v4l2_subdevice_test.cpp +++ b/test/v4l2_subdevice/v4l2_subdevice_test.cpp @@ -45,7 +45,11 @@ int V4L2SubdeviceTest::init() return TestSkip; } - media_->acquire(); + if (!media_->acquire()) { + cerr << "Unable to acquire media device " + << media_->deviceNode() << endl; + return TestSkip; + } int ret = media_->open(); if (ret) { From patchwork Mon Apr 29 19:17:20 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: 1130 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 33FE160E5A for ; Mon, 29 Apr 2019 21:17:47 +0200 (CEST) X-Halon-ID: 7811e05c-6ab3-11e9-8e2c-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from wyvern.station (unknown [37.182.44.227]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 7811e05c-6ab3-11e9-8e2c-005056917f90; Mon, 29 Apr 2019 21:17:46 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Apr 2019 21:17:20 +0200 Message-Id: <20190429191729.29697-3-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> References: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 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: Mon, 29 Apr 2019 19:17:47 -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 if the second (or later) call to MEDIA_IOC_G_TOPOLOGY would fail. Signed-off-by: Niklas Söderlund Reviewed-by: Kieran Bingham --- 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_device/v4l2_device_test.cpp | 5 ----- test/v4l2_subdevice/v4l2_subdevice_test.cpp | 21 +------------------ 6 files changed, 12 insertions(+), 45 deletions(-) diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp index f6878b3d58b3f966..caf91dcff6efd564 100644 --- a/src/libcamera/device_enumerator.cpp +++ b/src/libcamera/device_enumerator.cpp @@ -205,11 +205,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 @@ -236,8 +232,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..4b3b8f1fa3e6aaad 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,8 +241,9 @@ int MediaDevice::populate() LOG(MediaDevice, Error) << "Failed to enumerate topology: " << strerror(-ret); - return ret; + goto done; } + ret = 0; if (version == topology.topology_version) break; @@ -262,6 +267,9 @@ int MediaDevice::populate() populateLinks(topology)) valid_ = true; +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_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp index 90e5f7a3ee0c0f2f..833038d56ea4631d 100644 --- a/test/v4l2_device/v4l2_device_test.cpp +++ b/test/v4l2_device/v4l2_device_test.cpp @@ -46,9 +46,6 @@ int V4L2DeviceTest::init() if (!media_) return TestSkip; - if (!media_->acquire()) - return TestSkip; - MediaEntity *entity = media_->getEntityByName("vivid-000-vid-cap"); if (!entity) return TestSkip; @@ -62,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 4e16ab6cf3e5f874..562a638cb28ebfb2 100644 --- a/test/v4l2_subdevice/v4l2_subdevice_test.cpp +++ b/test/v4l2_subdevice/v4l2_subdevice_test.cpp @@ -45,33 +45,16 @@ int V4L2SubdeviceTest::init() return TestSkip; } - if (!media_->acquire()) { - cerr << "Unable to acquire media device " - << media_->deviceNode() << endl; - return TestSkip; - } - - 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; } scaler_ = new V4L2Subdevice(videoEntity); - ret = scaler_->open(); - if (ret) { + if (scaler_->open()) { cerr << "Unable to open video subdevice " << scaler_->entity()->deviceNode() << endl; - media_->release(); return TestSkip; } @@ -80,7 +63,5 @@ int V4L2SubdeviceTest::init() void V4L2SubdeviceTest::cleanup() { - media_->release(); - delete scaler_; } From patchwork Mon Apr 29 19:17:21 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: 1131 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 BC2D560E5A for ; Mon, 29 Apr 2019 21:17:47 +0200 (CEST) X-Halon-ID: 789e38be-6ab3-11e9-8e2c-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from wyvern.station (unknown [37.182.44.227]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 789e38be-6ab3-11e9-8e2c-005056917f90; Mon, 29 Apr 2019 21:17:46 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Apr 2019 21:17:21 +0200 Message-Id: <20190429191729.29697-4-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> References: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 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: Mon, 29 Apr 2019 19:17:48 -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 --- 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 4b3b8f1fa3e6aaad..416a0d0c207ea72d 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 Mon Apr 29 19:17:22 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: 1132 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 3F12B60E5F for ; Mon, 29 Apr 2019 21:17:49 +0200 (CEST) X-Halon-ID: 792b594f-6ab3-11e9-8e2c-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from wyvern.station (unknown [37.182.44.227]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 792b594f-6ab3-11e9-8e2c-005056917f90; Mon, 29 Apr 2019 21:17:48 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Apr 2019 21:17:22 +0200 Message-Id: <20190429191729.29697-5-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> References: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 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: Mon, 29 Apr 2019 19:17:50 -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 --- 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 416a0d0c207ea72d..ca260c9ea991dd53 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 e8b9cd8f889b7cf2..c18bb0ec9fa6ba8a 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -634,23 +634,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 @@ -677,14 +664,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; } @@ -1145,29 +1128,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 9a63a68b81dd3fd4..b720abaa6043a95c 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -150,10 +150,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(); @@ -171,8 +167,6 @@ int PipelineHandlerRkISP1::configureStreams(Camera *camera, break; } - media_->close(); - if (ret < 0) return ret; @@ -356,7 +350,6 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) { const MediaPad *pad; - int ret; DeviceMatch dm("rkisp1"); dm.add("rkisp1-isp-subdev"); @@ -372,35 +365,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; } /* @@ -408,18 +393,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 Mon Apr 29 19:17:23 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: 1133 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B6D1A60E6D for ; Mon, 29 Apr 2019 21:17:50 +0200 (CEST) X-Halon-ID: 79e70ecd-6ab3-11e9-8e2c-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from wyvern.station (unknown [37.182.44.227]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 79e70ecd-6ab3-11e9-8e2c-005056917f90; Mon, 29 Apr 2019 21:17:49 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Apr 2019 21:17:23 +0200 Message-Id: <20190429191729.29697-6-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> References: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 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: Mon, 29 Apr 2019 19:17:51 -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 --- test/media_device/media_device_link_test.cpp | 73 +++++--------------- test/media_device/media_device_test.cpp | 41 +++++++++++ test/media_device/media_device_test.h | 35 ++++++++++ test/media_device/meson.build | 2 +- 4 files changed, 96 insertions(+), 55 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..26aedf0ea23ec709 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; } @@ -229,15 +203,6 @@ class MediaDeviceLinkTest : public Test return 0; } - - void cleanup() - { - dev_->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..710613eeb80b6de1 --- /dev/null +++ b/test/media_device/media_device_test.cpp @@ -0,0 +1,41 @@ +/* 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; +} + +void MediaDeviceTest::cleanup() +{ + media_->release(); +} diff --git a/test/media_device/media_device_test.h b/test/media_device/media_device_test.h new file mode 100644 index 0000000000000000..af5c89f4fff2f63e --- /dev/null +++ b/test/media_device/media_device_test.h @@ -0,0 +1,35 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * media_device_test.h - libcamera media devie 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(); + void cleanup(); + + 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..1005685409d99aa7 100644 --- a/test/media_device/meson.build +++ b/test/media_device/meson.build @@ -4,7 +4,7 @@ media_device_tests = [ ] foreach t : media_device_tests - exe = executable(t[0], t[1], + exe = executable(t[0], [t[1], 'media_device_test.cpp'], link_with : test_libraries, include_directories : test_includes_internal) From patchwork Mon Apr 29 19:17:24 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: 1134 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 3119660E5A for ; Mon, 29 Apr 2019 21:17:51 +0200 (CEST) X-Halon-ID: 7a902e5c-6ab3-11e9-8e2c-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from wyvern.station (unknown [37.182.44.227]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 7a902e5c-6ab3-11e9-8e2c-005056917f90; Mon, 29 Apr 2019 21:17:50 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Apr 2019 21:17:24 +0200 Message-Id: <20190429191729.29697-7-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> References: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 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: Mon, 29 Apr 2019 19:17:52 -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 --- test/media_device/media_device_basic.cpp | 41 +++++++++++++++++++ test/media_device/media_device_print_test.cpp | 11 ----- test/media_device/meson.build | 1 + 3 files changed, 42 insertions(+), 11 deletions(-) create mode 100644 test/media_device/media_device_basic.cpp diff --git a/test/media_device/media_device_basic.cpp b/test/media_device/media_device_basic.cpp new file mode 100644 index 0000000000000000..ba95dd0578fce22d --- /dev/null +++ b/test/media_device/media_device_basic.cpp @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * media_device_basic.cpp - Test basic functions of MediaDevice + */ + +#include "media_device_test.h" + +using namespace libcamera; + +class MediaDeviceBasic : public MediaDeviceTest +{ + int testAcquire() + { + if (!media_->acquire()) + return TestFail; + + if (media_->acquire()) + return TestFail; + + media_->release(); + + if (!media_->acquire()) + return TestFail; + + media_->release(); + + return TestPass; + } + + int run() + { + if (testAcquire() != TestPass) + return TestFail; + + return TestPass; + } +}; + +TEST_REGISTER(MediaDeviceBasic); 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 1005685409d99aa7..7a3266187abd806f 100644 --- a/test/media_device/meson.build +++ b/test/media_device/meson.build @@ -1,4 +1,5 @@ media_device_tests = [ + ['media_device_basic', 'media_device_basic.cpp'], ['media_device_print_test', 'media_device_print_test.cpp'], ['media_device_link_test', 'media_device_link_test.cpp'], ] From patchwork Mon Apr 29 19:17:25 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: 1135 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 A846060E5A for ; Mon, 29 Apr 2019 21:17:52 +0200 (CEST) X-Halon-ID: 7afec997-6ab3-11e9-8e2c-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from wyvern.station (unknown [37.182.44.227]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 7afec997-6ab3-11e9-8e2c-005056917f90; Mon, 29 Apr 2019 21:17:50 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Apr 2019 21:17:25 +0200 Message-Id: <20190429191729.29697-8-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> References: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 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: Mon, 29 Apr 2019 19:17:53 -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 --- 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 ca260c9ea991dd53..cd6a01ed452073e2 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 Mon Apr 29 19:17:26 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: 1136 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A7E58600EC for ; Mon, 29 Apr 2019 21:17:52 +0200 (CEST) X-Halon-ID: 7b8c4cd4-6ab3-11e9-8e2c-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from wyvern.station (unknown [37.182.44.227]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 7b8c4cd4-6ab3-11e9-8e2c-005056917f90; Mon, 29 Apr 2019 21:17:51 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Apr 2019 21:17:26 +0200 Message-Id: <20190429191729.29697-9-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> References: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 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: Mon, 29 Apr 2019 19:17:53 -0000 Add lock() and unlock() which backed by lockf() allows an instance of libcamera to claim exclusive access to a media device. These functions is the base to allow multiple users of libcamera to coexist in the system without stepping on each others toes. Signed-off-by: Niklas Söderlund --- src/libcamera/include/media_device.h | 4 +++ src/libcamera/media_device.cpp | 54 +++++++++++++++++++++++++++- 2 files changed, 57 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 cd6a01ed452073e2..9116330570d7b8c2 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,57 @@ void MediaDevice::release() acquired_ = false; } +/** + * \brief Lock the device for use 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 coexists 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; + + if (lockOwner_) + return true; + + 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; + + lockOwner_ = false; + + lockf(fd_, F_ULOCK, 0); +} + /** * \fn MediaDevice::busy() * \brief Check if a device is in use From patchwork Mon Apr 29 19:17:27 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: 1137 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 2E298600EC for ; Mon, 29 Apr 2019 21:17:54 +0200 (CEST) X-Halon-ID: 7c18d849-6ab3-11e9-8e2c-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from wyvern.station (unknown [37.182.44.227]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 7c18d849-6ab3-11e9-8e2c-005056917f90; Mon, 29 Apr 2019 21:17:52 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Apr 2019 21:17:27 +0200 Message-Id: <20190429191729.29697-10-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> References: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 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: Mon, 29 Apr 2019 19:17:54 -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 which 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 it stores 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 implementing pipeline exclusive access across processes. Signed-off-by: Niklas Söderlund --- src/libcamera/include/pipeline_handler.h | 4 +++ src/libcamera/pipeline/ipu3/ipu3.cpp | 30 +++++---------------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 15 ++++------- src/libcamera/pipeline/uvcvideo.cpp | 24 +++++------------ src/libcamera/pipeline/vimc.cpp | 22 +++++----------- src/libcamera/pipeline_handler.cpp | 33 ++++++++++++++++++++++++ 6 files changed, 62 insertions(+), 66 deletions(-) diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 5830e53108faa105..4c13496246c2251c 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 *tryAcquire(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 c18bb0ec9fa6ba8a..4e3b210530ebd92d 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) @@ -617,20 +607,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_ = tryAcquire(enumerator, cio2_dm); if (!cio2MediaDev_) return false; - if (!cio2MediaDev_->acquire()) - return false; - - imguMediaDev_ = enumerator->search(imgu_dm); + imguMediaDev_ = tryAcquire(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. @@ -685,11 +669,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; @@ -708,7 +692,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 b720abaa6043a95c..64cbe5d2eb594672 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(); } /* ----------------------------------------------------------------------------- @@ -359,24 +356,22 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) dm.add("rkisp1-input-params"); dm.add("rockchip-sy-mipi-dphy"); - media_ = enumerator->search(dm); + media_ = tryAcquire(enumerator, dm); if (!media_) return false; - media_->acquire(); - /* Create the V4L2 subdevices we will need. */ - dphy_ = V4L2Subdevice::fromEntityName(media_.get(), + 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 d7ef37a8171f0eac..c6ffa0e6b2aa9c77 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, @@ -70,20 +69,13 @@ private: 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) @@ -183,19 +175,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 = tryAcquire(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; @@ -214,11 +204,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 25f0802c30d3d07b..03898f935925b006 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) @@ -183,6 +174,8 @@ int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) { + MediaDevice *media; + DeviceMatch dm("vimc"); dm.add("Raw Capture 0"); @@ -195,17 +188,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()) + media = tryAcquire(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 345abca89ab83590..702b398a50a18085 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,36 @@ PipelineHandler::~PipelineHandler() * created, or false otherwise */ +/** + * \brief Try to acquire a MediaDevice from a device pattern + * \param[in] enumerator Enumerator containing all media devices in the system + * \param[in] dm Device match pattern + * + * Search in the enumerated media devices that are not already in use for a + * match described in \a dm. If a match is found acquire it and store it for + * release when the pipeline handler is destroyed. + * + * \return pointer to the matching MediaDevice, or nullptr if no match is found + */ +MediaDevice *PipelineHandler::tryAcquire(DeviceEnumerator *enumerator, + const DeviceMatch &dm) +{ + std::shared_ptr media; + + media = enumerator->search(dm); + if (!media) + goto out; + + if (!media->acquire()) { + media.reset(); + goto out; + } + + mediaDevices_.push_back(media); +out: + return media.get(); +} + /** * \fn PipelineHandler::streamConfiguration() * \brief Retrieve a group of stream configurations for a specified camera From patchwork Mon Apr 29 19:17:28 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: 1138 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 B474C60E74 for ; Mon, 29 Apr 2019 21:17:55 +0200 (CEST) X-Halon-ID: 7d035941-6ab3-11e9-8e2c-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from wyvern.station (unknown [37.182.44.227]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 7d035941-6ab3-11e9-8e2c-005056917f90; Mon, 29 Apr 2019 21:17:54 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Apr 2019 21:17:28 +0200 Message-Id: <20190429191729.29697-11-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> References: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 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: Mon, 29 Apr 2019 19:17:56 -0000 Add lock() and unlock() which backed by the MediaDevice implementation can will lock all media devices claimed by a pipeline handler instance. Signed-off-by: Niklas Söderlund --- src/libcamera/include/pipeline_handler.h | 3 ++ src/libcamera/pipeline_handler.cpp | 38 ++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 4c13496246c2251c..019c8f4751a8c2b2 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -57,6 +57,9 @@ public: MediaDevice *tryAcquire(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 702b398a50a18085..119d8980e6fbd75a 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -182,6 +182,44 @@ out: return media.get(); } +/** + * \brief Lock all media devices acquired by the pipeline + * + * This method shall not be called from a pipeline handler implementation + * directly, as the Camera handles this on the behalf of the specified + * implementation. + * + * \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 a pipeline handler implementation + * directly, as the Camera handles this on the behalf of the specified + * implementation. + * + * \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 Mon Apr 29 19:17:29 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: 1139 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 B3BC060E69 for ; Mon, 29 Apr 2019 21:17:55 +0200 (CEST) X-Halon-ID: 7d73c0d9-6ab3-11e9-8e2c-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from wyvern.station (unknown [37.182.44.227]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 7d73c0d9-6ab3-11e9-8e2c-005056917f90; Mon, 29 Apr 2019 21:17:55 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Apr 2019 21:17:29 +0200 Message-Id: <20190429191729.29697-12-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> References: <20190429191729.29697-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 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: Mon, 29 Apr 2019 19:17:56 -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 --- src/libcamera/camera.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index ef9e15be09ece319..95f652e5953b2016 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -489,6 +489,11 @@ int Camera::acquire() if (!stateIs(CameraAvailable)) return -EBUSY; + if (!pipe_->lock()) { + LOG(Camera, Info) << "Pipeline handler in use by other process"; + return -EBUSY; + } + state_ = CameraAcquired; return 0; @@ -510,6 +515,8 @@ int Camera::release() if (!stateBetween(CameraAvailable, CameraConfigured)) return -EBUSY; + pipe_->unlock(); + state_ = CameraAvailable; return 0;