From patchwork Sun Apr 14 01:34:56 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: 970 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 8CA2560DB4 for ; Sun, 14 Apr 2019 03:35:14 +0200 (CEST) X-Halon-ID: 8c76b016-5e55-11e9-8e2c-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 8c76b016-5e55-11e9-8e2c-005056917f90; Sun, 14 Apr 2019 03:35:13 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sun, 14 Apr 2019 03:34:56 +0200 Message-Id: <20190414013506.10515-2-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> References: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 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: Sun, 14 Apr 2019 01:35:14 -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 --- 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 ca09da753b908448..d8de6b3c36314fc0 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -451,21 +451,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 cd472cfadd86b7ba..5214bfd3097b8217 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -184,7 +184,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 c8bbe2a19847ba1e..e5e78ccedd59ae66 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -194,7 +194,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..4b8de3bee722e912 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 06582969eb45f658..21335efbad4d5668 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 Sun Apr 14 01:34:57 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: 971 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1138960DB4 for ; Sun, 14 Apr 2019 03:35:16 +0200 (CEST) X-Halon-ID: 8cd43bf5-5e55-11e9-8e2c-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 8cd43bf5-5e55-11e9-8e2c-005056917f90; Sun, 14 Apr 2019 03:35:15 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sun, 14 Apr 2019 03:34:57 +0200 Message-Id: <20190414013506.10515-3-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> References: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 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: Sun, 14 Apr 2019 01:35:16 -0000 Remove the need for the caller to open and close the media device when populating the MediaDeivce. This is done as an effort to make the usage of the MediaDevice less error prone and the interface stricter. Signed-off-by: Niklas Söderlund --- src/libcamera/device_enumerator.cpp | 8 +------- src/libcamera/media_device.cpp | 10 ++++++++-- test/media_device/media_device_print_test.cpp | 6 ------ test/pipeline/ipu3/ipu3_pipeline_test.cpp | 5 ----- 4 files changed, 9 insertions(+), 20 deletions(-) diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp index 58b81c354a706f03..6c08806d17dbfeec 100644 --- a/src/libcamera/device_enumerator.cpp +++ b/src/libcamera/device_enumerator.cpp @@ -212,11 +212,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 @@ -243,8 +239,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 1e9024bf97217bcf..ecb00a1d5abffe80 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -220,6 +220,10 @@ int MediaDevice::populate() clear(); + ret = open(); + if (ret) + return ret; + /* * Keep calling G_TOPOLOGY until the version number stays stable. */ @@ -236,7 +240,7 @@ int MediaDevice::populate() LOG(MediaDevice, Error) << "Failed to enumerate topology: " << strerror(-ret); - return ret; + goto err_out; } if (version == topology.topology_version) @@ -260,6 +264,8 @@ int MediaDevice::populate() populatePads(topology) && populateLinks(topology)) valid_ = true; +err_out: + close(); delete[] ents; delete[] interfaces; @@ -268,7 +274,7 @@ int MediaDevice::populate() if (!valid_) { clear(); - return -EINVAL; + return ret ? ret : -EINVAL; } return 0; 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 From patchwork Sun Apr 14 01:34:58 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: 972 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8C23E60DC0 for ; Sun, 14 Apr 2019 03:35:16 +0200 (CEST) X-Halon-ID: 8dc58b34-5e55-11e9-8e2c-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 8dc58b34-5e55-11e9-8e2c-005056917f90; Sun, 14 Apr 2019 03:35:15 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sun, 14 Apr 2019 03:34:58 +0200 Message-Id: <20190414013506.10515-4-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> References: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 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: Sun, 14 Apr 2019 01:35:17 -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: Laurent Pinchart --- src/libcamera/media_device.cpp | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index ecb00a1d5abffe80..54706bb73a7591d5 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -131,9 +131,6 @@ bool MediaDevice::acquire() * 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. * * \return 0 on success or a negative error code otherwise @@ -155,20 +152,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; } @@ -224,6 +207,20 @@ 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 err_out; + } + + driver_ = info.driver; + model_ = info.model; + version_ = info.media_version; + /* * Keep calling G_TOPOLOGY until the version number stays stable. */ From patchwork Sun Apr 14 01:34:59 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: 974 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 10DCE60DBF for ; Sun, 14 Apr 2019 03:35:18 +0200 (CEST) X-Halon-ID: 8e0c302b-5e55-11e9-8e2c-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 8e0c302b-5e55-11e9-8e2c-005056917f90; Sun, 14 Apr 2019 03:35:16 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sun, 14 Apr 2019 03:34:59 +0200 Message-Id: <20190414013506.10515-5-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> References: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 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: Sun, 14 Apr 2019 01:35:18 -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 | 33 ++++++++++---------- src/libcamera/pipeline/ipu3/ipu3.cpp | 25 +-------------- test/media_device/media_device_link_test.cpp | 7 ----- test/v4l2_subdevice/v4l2_subdevice_test.cpp | 10 +----- 5 files changed, 19 insertions(+), 58 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 54706bb73a7591d5..644c6143fb15e1fa 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -39,10 +39,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 @@ -50,12 +49,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. @@ -65,8 +58,8 @@ LOG_DEFINE_CATEGORY(MediaDevice) * \brief Construct a MediaDevice * \param 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) @@ -106,15 +99,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() @@ -127,10 +127,6 @@ 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. - * * If the device is already open the function returns -EBUSY. * * \return 0 on success or a negative error code otherwise @@ -201,6 +197,9 @@ int MediaDevice::populate() __u64 version = -1; int ret; + if (fd_ != -1) + return -EBUSY; + clear(); ret = open(); diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index d8de6b3c36314fc0..a87eff8dfec6d143 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -468,23 +468,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 @@ -516,9 +503,6 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) ret = registerCameras(); error: - cio2MediaDev_->close(); - imguMediaDev_->close(); - return ret == 0; } @@ -961,11 +945,6 @@ 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; @@ -981,8 +960,6 @@ int ImgUDevice::enableLinks(bool enable) ret = linkSetup(name_, PAD_STAT, statName, 0, enable); done: - media_->close(); - return ret; } diff --git a/test/media_device/media_device_link_test.cpp b/test/media_device/media_device_link_test.cpp index 4b8de3bee722e912..3989da43ca9ec30f 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(); } diff --git a/test/v4l2_subdevice/v4l2_subdevice_test.cpp b/test/v4l2_subdevice/v4l2_subdevice_test.cpp index 21335efbad4d5668..31f3465a7ba3a5b1 100644 --- a/test/v4l2_subdevice/v4l2_subdevice_test.cpp +++ b/test/v4l2_subdevice/v4l2_subdevice_test.cpp @@ -51,14 +51,6 @@ int V4L2SubdeviceTest::init() 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; @@ -67,7 +59,7 @@ int V4L2SubdeviceTest::init() } scaler_ = new V4L2Subdevice(videoEntity); - ret = scaler_->open(); + int ret = scaler_->open(); if (ret) { cerr << "Unable to open video subdevice " << scaler_->deviceNode() << endl; From patchwork Sun Apr 14 01:35:00 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: 973 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 1179760DC0 for ; Sun, 14 Apr 2019 03:35:18 +0200 (CEST) X-Halon-ID: 8e84d4c6-5e55-11e9-8e2c-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 8e84d4c6-5e55-11e9-8e2c-005056917f90; Sun, 14 Apr 2019 03:35:17 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sun, 14 Apr 2019 03:35:00 +0200 Message-Id: <20190414013506.10515-6-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> References: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 05/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: Sun, 14 Apr 2019 01:35:18 -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 | 78 +++++++------------ test/media_device/media_device_print_test.cpp | 11 --- 3 files changed, 32 insertions(+), 63 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 644c6143fb15e1fa..0138be526f886c90 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -124,55 +124,6 @@ void MediaDevice::release() * \sa acquire(), release() */ -/** - * \brief Open a media device and retrieve device information - * - * If the device is already open the function returns -EBUSY. - * - * \return 0 on success or a negative error code otherwise - */ -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 media graph with media objects * @@ -440,6 +391,35 @@ int MediaDevice::disableLinks() * driver unloading for most devices. The media device is passed as a parameter. */ +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; +} + +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 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; From patchwork Sun Apr 14 01:35:01 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: 975 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 99C1860DBF for ; Sun, 14 Apr 2019 03:35:18 +0200 (CEST) X-Halon-ID: 8ee54c97-5e55-11e9-8e2c-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 8ee54c97-5e55-11e9-8e2c-005056917f90; Sun, 14 Apr 2019 03:35:17 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sun, 14 Apr 2019 03:35:01 +0200 Message-Id: <20190414013506.10515-7-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> References: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 06/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: Sun, 14 Apr 2019 01:35:19 -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. 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 | 20 ++------------------ src/libcamera/pipeline/uvcvideo.cpp | 24 +++++++----------------- src/libcamera/pipeline/vimc.cpp | 22 ++++++---------------- src/libcamera/pipeline_handler.cpp | 22 ++++++++++++++++++++++ 5 files changed, 41 insertions(+), 51 deletions(-) diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index b6cbd3bae51b08dc..d995a7e56d90f706 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -21,6 +21,7 @@ class Camera; class CameraConfiguration; class CameraManager; class DeviceEnumerator; +class DeviceMatch; class MediaDevice; class PipelineHandler; class Request; @@ -52,6 +53,8 @@ public: virtual ~PipelineHandler(); virtual bool match(DeviceEnumerator *enumerator) = 0; + std::shared_ptr tryAcquire(DeviceEnumerator *enumerator, + const DeviceMatch &dm); virtual CameraConfiguration streamConfiguration(Camera *camera, const std::vector &usages) = 0; @@ -81,6 +84,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 a87eff8dfec6d143..77d14533e6bceb80 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -137,7 +137,6 @@ class PipelineHandlerIPU3 : public PipelineHandler { public: PipelineHandlerIPU3(CameraManager *manager); - ~PipelineHandlerIPU3(); CameraConfiguration streamConfiguration(Camera *camera, @@ -195,15 +194,6 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager) { } -PipelineHandlerIPU3::~PipelineHandlerIPU3() -{ - if (cio2MediaDev_) - cio2MediaDev_->release(); - - if (imguMediaDev_) - imguMediaDev_->release(); -} - CameraConfiguration PipelineHandlerIPU3::streamConfiguration(Camera *camera, const std::vector &usages) @@ -451,20 +441,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. diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 5214bfd3097b8217..43a6d59670299376 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, @@ -68,20 +67,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) @@ -178,19 +170,17 @@ int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) { + std::shared_ptr 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; @@ -209,11 +199,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.get()); return true; } diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index e5e78ccedd59ae66..a80331e1cd4d1e46 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, @@ -67,21 +66,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) @@ -178,6 +169,8 @@ int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) { + std::shared_ptr media; + DeviceMatch dm("vimc"); dm.add("Raw Capture 0"); @@ -190,17 +183,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 43550c0e0210b7b3..7f4035c008f95f91 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -9,6 +9,7 @@ #include #include +#include "device_enumerator.h" #include "log.h" #include "media_device.h" #include "pipeline_handler.h" @@ -115,6 +116,8 @@ PipelineHandler::PipelineHandler(CameraManager *manager) PipelineHandler::~PipelineHandler() { + for (std::shared_ptr media : mediaDevices_) + media->release(); }; /** @@ -148,6 +151,25 @@ PipelineHandler::~PipelineHandler() * created, or false otherwise */ +std::shared_ptr +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; +} + /** * \fn PipelineHandler::streamConfiguration() * \brief Retrieve a group of stream configurations for a specified camera From patchwork Sun Apr 14 01:35:02 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: 976 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1B13E60DBF for ; Sun, 14 Apr 2019 03:35:19 +0200 (CEST) X-Halon-ID: 8f554dd4-5e55-11e9-8e2c-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 8f554dd4-5e55-11e9-8e2c-005056917f90; Sun, 14 Apr 2019 03:35:18 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sun, 14 Apr 2019 03:35:02 +0200 Message-Id: <20190414013506.10515-8-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> References: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 07/11] [HACK] tests: disable media_device_link_test 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: Sun, 14 Apr 2019 01:35:19 -0000 The test depends on the ability to access acquire() and release() of MediaDevice. These members are about to be made private and will no longer be cacheable to the test. This needs to be solved as the test suite needs to be able to unit test classes properly. But for the purpose of this RFC lets disable it while it's figured out how to solve it. Not-Signed-off-by: Niklas Söderlund --- test/media_device/meson.build | 1 - 1 file changed, 1 deletion(-) diff --git a/test/media_device/meson.build b/test/media_device/meson.build index d91a022fab190abf..e4bedb79b572d655 100644 --- a/test/media_device/meson.build +++ b/test/media_device/meson.build @@ -1,6 +1,5 @@ media_device_tests = [ ['media_device_print_test', 'media_device_print_test.cpp'], - ['media_device_link_test', 'media_device_link_test.cpp'], ] foreach t : media_device_tests From patchwork Sun Apr 14 01:35:03 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: 977 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9017D60DCA for ; Sun, 14 Apr 2019 03:35:19 +0200 (CEST) X-Halon-ID: 8f958efd-5e55-11e9-8e2c-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 8f958efd-5e55-11e9-8e2c-005056917f90; Sun, 14 Apr 2019 03:35:18 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sun, 14 Apr 2019 03:35:03 +0200 Message-Id: <20190414013506.10515-9-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> References: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 08/11] libcamera: media_device: Restrict 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: Sun, 14 Apr 2019 01:35:20 -0000 The only valid call site for acquire() and release() are inside the PipelineHandler base class. Make them private to with a specific friend clause to block specific pipeline handler implementations from calling them. Signed-off-by: Niklas Söderlund --- src/libcamera/include/media_device.h | 6 +- src/libcamera/media_device.cpp | 85 +++++++++++---------- test/v4l2_device/v4l2_device_test.cpp | 5 -- test/v4l2_subdevice/v4l2_subdevice_test.cpp | 10 --- 4 files changed, 47 insertions(+), 59 deletions(-) diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h index e513a2fee1b91a1b..5d28a7bb8214ef4a 100644 --- a/src/libcamera/include/media_device.h +++ b/src/libcamera/include/media_device.h @@ -26,8 +26,6 @@ public: MediaDevice(const std::string &deviceNode); ~MediaDevice(); - bool acquire(); - void release(); bool busy() const { return acquired_; } int populate(); @@ -62,6 +60,10 @@ private: int open(); void close(); + friend class PipelineHandler; + bool acquire(); + void release(); + 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 0138be526f886c90..46931f52688f6c82 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -73,48 +73,6 @@ MediaDevice::~MediaDevice() clear(); } -/** - * \brief Claim a device for exclusive use - * - * The device claiming mechanism offers simple media device access arbitration - * between multiple users. When the media device is created, it is available to - * all users. Users can query the media graph to determine whether they can - * support the device and, if they do, claim the device for exclusive use. Other - * users are then expected to skip over media devices in use as reported by the - * busy() function. - * - * Once claimed the device shall be released by its user when not needed anymore - * by calling the release() function. - * - * 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 - * itself. - * - * \return true if the device was successfully claimed, or false if it was - * already in use - * \sa release(), busy() - */ -bool MediaDevice::acquire() -{ - if (acquired_) - return false; - - if (open()) - return false; - - acquired_ = true; - return true; -} - -/** - * \brief Release a device previously claimed for exclusive use - * \sa acquire(), busy() - */ -void MediaDevice::release() -{ - close(); - acquired_ = false; -} /** * \fn MediaDevice::busy() @@ -420,6 +378,49 @@ void MediaDevice::close() fd_ = -1; } +/** + * \brief Claim a device for exclusive use + * + * The device claiming mechanism offers simple media device access arbitration + * between multiple users. When the media device is created, it is available to + * all users. Users can query the media graph to determine whether they can + * support the device and, if they do, claim the device for exclusive use. Other + * users are then expected to skip over media devices in use as reported by the + * busy() function. + * + * Once claimed the device shall be released by its user when not needed anymore + * by calling the release() function. + * + * 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 + * itself. + * + * \return true if the device was successfully claimed, or false if it was + * already in use + * \sa release(), busy() + */ +bool MediaDevice::acquire() +{ + if (acquired_) + return false; + + if (open()) + return false; + + acquired_ = true; + return true; +} + +/** + * \brief Release a device previously claimed for exclusive use + * \sa acquire(), busy() + */ +void MediaDevice::release() +{ + close(); + acquired_ = false; +} + /** * \var MediaDevice::objects_ * \brief Global map of media objects (entities, pads, links) keyed by their 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 31f3465a7ba3a5b1..0180698840cc2751 100644 --- a/test/v4l2_subdevice/v4l2_subdevice_test.cpp +++ b/test/v4l2_subdevice/v4l2_subdevice_test.cpp @@ -45,16 +45,9 @@ int V4L2SubdeviceTest::init() return TestSkip; } - if (!media_->acquire()) { - cerr << "Unable to acquire media device: " - << media_->deviceNode() << endl; - return TestSkip; - } - MediaEntity *videoEntity = media_->getEntityByName("Scaler"); if (!videoEntity) { cerr << "Unable to find media entity 'Scaler'" << endl; - media_->release(); return TestFail; } @@ -63,7 +56,6 @@ int V4L2SubdeviceTest::init() if (ret) { cerr << "Unable to open video subdevice " << scaler_->deviceNode() << endl; - media_->release(); return TestSkip; } @@ -72,7 +64,5 @@ int V4L2SubdeviceTest::init() void V4L2SubdeviceTest::cleanup() { - media_->release(); - delete scaler_; } From patchwork Sun Apr 14 01:35:04 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: 978 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 1474E60DD8 for ; Sun, 14 Apr 2019 03:35:20 +0200 (CEST) X-Halon-ID: 8ff64ebe-5e55-11e9-8e2c-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 8ff64ebe-5e55-11e9-8e2c-005056917f90; Sun, 14 Apr 2019 03:35:19 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sun, 14 Apr 2019 03:35:04 +0200 Message-Id: <20190414013506.10515-10-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> References: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 09/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: Sun, 14 Apr 2019 01:35:21 -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 | 29 +++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h index 5d28a7bb8214ef4a..e2956549fa201ea3 100644 --- a/src/libcamera/include/media_device.h +++ b/src/libcamera/include/media_device.h @@ -64,6 +64,10 @@ private: bool acquire(); void release(); + bool lockOwner_; + bool lock(); + void unlock(); + 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 46931f52688f6c82..5cea4e63715125c8 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -62,7 +62,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) { } @@ -421,6 +422,32 @@ void MediaDevice::release() acquired_ = false; } +bool MediaDevice::lock() +{ + if (fd_ == -1) + return false; + + if (lockOwner_) + return true; + + if (lockf(fd_, F_TLOCK, 0)) + return false; + + lockOwner_ = true; + + return true; +} + +void MediaDevice::unlock() +{ + if (fd_ == -1) + return; + + lockOwner_ = false; + + lockf(fd_, F_ULOCK, 0); +} + /** * \var MediaDevice::objects_ * \brief Global map of media objects (entities, pads, links) keyed by their From patchwork Sun Apr 14 01:35:05 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: 979 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 87D6760DB9 for ; Sun, 14 Apr 2019 03:35:20 +0200 (CEST) X-Halon-ID: 90395d99-5e55-11e9-8e2c-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 90395d99-5e55-11e9-8e2c-005056917f90; Sun, 14 Apr 2019 03:35:19 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sun, 14 Apr 2019 03:35:05 +0200 Message-Id: <20190414013506.10515-11-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> References: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 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: Sun, 14 Apr 2019 01:35:21 -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 | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index d995a7e56d90f706..0e79b05035cc2abb 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -56,6 +56,9 @@ public: std::shared_ptr 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 7f4035c008f95f91..317b680214d91071 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -170,6 +170,24 @@ out: return media; } +bool PipelineHandler::lock() +{ + for (std::shared_ptr media : mediaDevices_) { + if (!media->lock()) { + unlock(); + return false; + } + } + + return true; +} + +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 Sun Apr 14 01:35:06 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: 980 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0DE6B60DCA for ; Sun, 14 Apr 2019 03:35:21 +0200 (CEST) X-Halon-ID: 907b0a98-5e55-11e9-8e2c-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 907b0a98-5e55-11e9-8e2c-005056917f90; Sun, 14 Apr 2019 03:35:20 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sun, 14 Apr 2019 03:35:06 +0200 Message-Id: <20190414013506.10515-12-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> References: <20190414013506.10515-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 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: Sun, 14 Apr 2019 01:35:21 -0000 To allow more then 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 needs to be tied to the same process. Allow for this by locking the whole pipeline when one of it's cameras are acquired by the user. Other processes can still enumerate and list all cameras in the system but cant progress a camera in a locked pipeline handler to the Acquired state. 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 bdf14b31d8eeba9c..b3860952efa6446f 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -495,6 +495,11 @@ int Camera::acquire() if (!stateIs(CameraAvailable)) return -EBUSY; + if (!pipe_->lock()) { + LOG(Camera, Info) << "Camera in use by other process"; + return -EBUSY; + } + state_ = CameraAcquired; return 0; @@ -516,6 +521,8 @@ int Camera::release() if (!stateBetween(CameraAvailable, CameraConfigured)) return -EBUSY; + pipe_->unlock(); + state_ = CameraAvailable; return 0;