From patchwork Tue Jan 1 21:23:25 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 118 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9EB6F60B31 for ; Tue, 1 Jan 2019 22:22:33 +0100 (CET) Received: from avalon.bb.dnainternet.fi (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8BA64505 for ; Tue, 1 Jan 2019 22:22:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1546377752; bh=nfgdTWa8yY4Udqvliy/qVwhEXGhtZ3ngjvmQJv3xGSU=; h=From:To:Subject:Date:From; b=mtUnx3Dm4lxqfIV5EwC18lI5f0zIpQvjEA7GQXEUgVsuDOi64bWIYXq6ao93iz05B hlXQomqXZWdLg/43NKLpRMxTh7dN7AaOLRFmiZ/5MPHEq5hOxR5eH6LK0FbSfbMIHc Vl680hRrr9oKZl29M4UBWS3hSZ616u5OciwUF108= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 1 Jan 2019 23:23:25 +0200 Message-Id: <20190101212328.18361-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/4] libcamera: mediadevice: Fix graph parsing error handling 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: Tue, 01 Jan 2019 21:22:34 -0000 Most errors recorded during graph parsing are logged but not propagated to the caller. Fix this and delete objects that are created but not successfully added to the graph to avoid memory leaks. As the error code returned from the addObject() and populate*() functions doesn't matter much, turn them into bool functions. Additionally, add a way to query whether the media graph was valid, and clear objects before populating the graph to avoid leaking them. Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham --- src/libcamera/include/media_device.h | 10 ++-- src/libcamera/media_device.cpp | 75 +++++++++++++++++----------- 2 files changed, 53 insertions(+), 32 deletions(-) diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h index bca7c9a19471..74a67c81398a 100644 --- a/src/libcamera/include/media_device.h +++ b/src/libcamera/include/media_device.h @@ -28,6 +28,7 @@ public: void close(); int populate(); + bool valid() const { return valid_; } const std::string driver() const { return driver_; } const std::string devnode() const { return devnode_; } @@ -37,18 +38,19 @@ private: std::string driver_; std::string devnode_; int fd_; + bool valid_; std::map objects_; MediaObject *object(unsigned int id); - int addObject(MediaObject *obj); + bool addObject(MediaObject *obj); void clear(); std::vector entities_; MediaEntity *getEntityByName(const std::string &name); - void populateEntities(const struct media_v2_topology &topology); - int populatePads(const struct media_v2_topology &topology); - int populateLinks(const struct media_v2_topology &topology); + bool populateEntities(const struct media_v2_topology &topology); + bool populatePads(const struct media_v2_topology &topology); + bool populateLinks(const struct media_v2_topology &topology); }; } /* namespace libcamera */ diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index d9a5196ac28c..fd5a31746075 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -56,7 +56,7 @@ namespace libcamera { * \param devnode The media device node path */ MediaDevice::MediaDevice(const std::string &devnode) - : devnode_(devnode), fd_(-1) + : devnode_(devnode), fd_(-1), valid_(false) { } @@ -99,6 +99,7 @@ void MediaDevice::clear() objects_.clear(); entities_.clear(); + valid_ = false; } /** @@ -163,18 +164,18 @@ void MediaDevice::close() * Add a new object to the global objects pool and fail if the object * has already been registered. */ -int MediaDevice::addObject(MediaObject *obj) +bool MediaDevice::addObject(MediaObject *obj) { if (objects_.find(obj->id()) != objects_.end()) { LOG(Error) << "Element with id " << obj->id() << " already enumerated."; - return -EEXIST; + return false; } objects_[obj->id()] = obj; - return 0; + return true; } /* @@ -201,7 +202,7 @@ MediaEntity *MediaDevice::getEntityByName(const std::string &name) return nullptr; } -int MediaDevice::populateLinks(const struct media_v2_topology &topology) +bool MediaDevice::populateLinks(const struct media_v2_topology &topology) { media_v2_link *mediaLinks = reinterpret_cast (topology.ptr_links); @@ -222,7 +223,7 @@ int MediaDevice::populateLinks(const struct media_v2_topology &topology) if (!source) { LOG(Error) << "Failed to find pad with id: " << source_id; - return -ENODEV; + return false; } unsigned int sink_id = mediaLinks[i].sink_id; @@ -231,20 +232,23 @@ int MediaDevice::populateLinks(const struct media_v2_topology &topology) if (!sink) { LOG(Error) << "Failed to find pad with id: " << sink_id; - return -ENODEV; + return false; } MediaLink *link = new MediaLink(&mediaLinks[i], source, sink); + if (!addObject(link)) { + delete link; + return false; + } + source->addLink(link); sink->addLink(link); - - addObject(link); } - return 0; + return true; } -int MediaDevice::populatePads(const struct media_v2_topology &topology) +bool MediaDevice::populatePads(const struct media_v2_topology &topology) { media_v2_pad *mediaPads = reinterpret_cast (topology.ptr_pads); @@ -258,16 +262,19 @@ int MediaDevice::populatePads(const struct media_v2_topology &topology) if (!mediaEntity) { LOG(Error) << "Failed to find entity with id: " << entity_id; - return -ENODEV; + return false; } MediaPad *pad = new MediaPad(&mediaPads[i], mediaEntity); - mediaEntity->addPad(pad); + if (!addObject(pad)) { + delete pad; + return false; + } - addObject(pad); + mediaEntity->addPad(pad); } - return 0; + return true; } /* @@ -275,17 +282,22 @@ int MediaDevice::populatePads(const struct media_v2_topology &topology) * reference in the MediaObject global pool and in the global vector of * entities. */ -void MediaDevice::populateEntities(const struct media_v2_topology &topology) +bool MediaDevice::populateEntities(const struct media_v2_topology &topology) { media_v2_entity *mediaEntities = reinterpret_cast (topology.ptr_entities); for (unsigned int i = 0; i < topology.num_entities; ++i) { MediaEntity *entity = new MediaEntity(&mediaEntities[i]); + if (!addObject(entity)) { + delete entity; + return false; + } - addObject(entity); entities_.push_back(entity); } + + return true; } /** @@ -311,6 +323,8 @@ int MediaDevice::populate() __u64 version = -1; int ret; + clear(); + /* * Keep calling G_TOPOLOGY until the version number stays stable. */ @@ -343,24 +357,29 @@ int MediaDevice::populate() } /* Populate entities, pads and links. */ - populateEntities(topology); - - ret = populatePads(topology); - if (ret) - goto error_free_objs; - - ret = populateLinks(topology); -error_free_objs: - if (ret) - clear(); + if (populateEntities(topology) && + populatePads(topology) && + populateLinks(topology)) + valid_ = true; delete[] links; delete[] ents; delete[] pads; - return ret; + if (!valid_) { + clear(); + return -EINVAL; + } + + return 0; } +/** + * \fn MediaDevice::valid() + * \brief Query whether the media graph is valid + * \return true if the media graph is valid, false otherwise + */ + /** * \var MediaDevice::objects_ * \brief Global map of media objects (entities, pads, links) keyed by their From patchwork Tue Jan 1 21:23:26 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 117 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A112860B33 for ; Tue, 1 Jan 2019 22:22:33 +0100 (CET) Received: from avalon.bb.dnainternet.fi (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id E02A3B81 for ; Tue, 1 Jan 2019 22:22:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1546377753; bh=Hw51POc7lSFpsCxFBLrFbyuQ3V0AtVZ2yuRFvdHOmHU=; h=From:To:Subject:Date:In-Reply-To:References:From; b=gCC3Gf1VHRskugqnCRwpbDMSEu9jBhibIBufwpFoFb8bhtGxGBarihAkhXPj1lM6a 83tmOcPVinnW95rDq+soGWmizs6CX0XdD4FK/wrfrmKqWzsrqVZYpHExXAaQNQwO5A QTUVcfsNGlJ0rmr6frmDPaL4flHqaqKLpPfrNRmw= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 1 Jan 2019 23:23:26 +0200 Message-Id: <20190101212328.18361-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190101212328.18361-1-laurent.pinchart@ideasonboard.com> References: <20190101212328.18361-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 2/4] libcamera: mediadevice: Make getEntityByName() public 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: Tue, 01 Jan 2019 21:22:34 -0000 The function is useful as a public API, make it public. Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham --- src/libcamera/include/media_device.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h index 74a67c81398a..fd1e12d1bbcb 100644 --- a/src/libcamera/include/media_device.h +++ b/src/libcamera/include/media_device.h @@ -32,7 +32,9 @@ public: const std::string driver() const { return driver_; } const std::string devnode() const { return devnode_; } + const std::vector &entities() const { return entities_; } + MediaEntity *getEntityByName(const std::string &name); private: std::string driver_; @@ -46,7 +48,6 @@ private: void clear(); std::vector entities_; - MediaEntity *getEntityByName(const std::string &name); bool populateEntities(const struct media_v2_topology &topology); bool populatePads(const struct media_v2_topology &topology); From patchwork Tue Jan 1 21:23:27 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 119 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 58A3160B35 for ; Tue, 1 Jan 2019 22:22:34 +0100 (CET) Received: from avalon.bb.dnainternet.fi (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 447E41173 for ; Tue, 1 Jan 2019 22:22:33 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1546377753; bh=sa3t6/qnse81eA4bkgYTY1VKOtsQY67ejkpXV05OzMA=; h=From:To:Subject:Date:In-Reply-To:References:From; b=DBZSMR1hzBkxICjkjGDp+AclGn07+LB6C0T+Vknpo4mAX8ZICUfEJ12w1VpAkPUw7 P9Pk4mEt2PGsOj+q8eg/N/vkCptcqgzdwsHdETCRlOYyYX2vW9W/yiQYidSXLkjpAg BaHxCozXf7yu8fSiEPxfbuiOp7uO2CPsm1siXF0c= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 1 Jan 2019 23:23:27 +0200 Message-Id: <20190101212328.18361-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190101212328.18361-1-laurent.pinchart@ideasonboard.com> References: <20190101212328.18361-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 3/4] libcamera: mediadevice: Reorder functions in declaration order 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: Tue, 01 Jan 2019 21:22:34 -0000 In order to simplify navigation in the .cpp file, order functions in the declaration order in the .h file. Signed-off-by: Laurent Pinchart Acked-by: Kieran Bingham --- src/libcamera/media_device.cpp | 356 ++++++++++++++++----------------- 1 file changed, 178 insertions(+), 178 deletions(-) diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index fd5a31746075..4c77d3787391 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -70,38 +70,6 @@ MediaDevice::~MediaDevice() clear(); } -/** - * \fn MediaDevice::driver() - * \brief Retrieve the media device driver name - * \return The name of the kernel driver that handles the MediaDevice - */ - -/** - * \fn MediaDevice::devnode() - * \brief Retrieve the media device device node path - * \return The MediaDevice devnode path - */ - -/** - * \brief Delete all media objects in the MediaDevice. - * - * Delete all MediaEntities; entities will then delete their pads, - * and each source pad will delete links. - * - * After this function has been called, the media graph will be unpopulated - * and its media objects deleted. The media device has to be populated - * before it could be used again. - */ -void MediaDevice::clear() -{ - for (auto const &o : objects_) - delete o.second; - - objects_.clear(); - entities_.clear(); - valid_ = false; -} - /** * \brief Open a media device and retrieve informations from it * @@ -154,152 +122,6 @@ void MediaDevice::close() fd_ = -1; } -/** - * \fn MediaDevice::entities() - * \brief Retrieve the list of entities in the media graph - * \return The list of MediaEntities registered in the MediaDevice - */ - -/* - * Add a new object to the global objects pool and fail if the object - * has already been registered. - */ -bool MediaDevice::addObject(MediaObject *obj) -{ - - if (objects_.find(obj->id()) != objects_.end()) { - LOG(Error) << "Element with id " << obj->id() - << " already enumerated."; - return false; - } - - objects_[obj->id()] = obj; - - return true; -} - -/* - * MediaObject pool lookup by id. - */ -MediaObject *MediaDevice::object(unsigned int id) -{ - auto it = objects_.find(id); - return (it == objects_.end()) ? nullptr : it->second; -} - -/** - * \brief Return the MediaEntity with name \a name - * \param name The entity name - * \return The entity with \a name - * \return nullptr if no entity with \a name is found - */ -MediaEntity *MediaDevice::getEntityByName(const std::string &name) -{ - for (MediaEntity *e : entities_) - if (e->name() == name) - return e; - - return nullptr; -} - -bool MediaDevice::populateLinks(const struct media_v2_topology &topology) -{ - media_v2_link *mediaLinks = reinterpret_cast - (topology.ptr_links); - - for (unsigned int i = 0; i < topology.num_links; ++i) { - /* - * Skip links between entities and interfaces: we only care - * about pad-2-pad links here. - */ - if ((mediaLinks[i].flags & MEDIA_LNK_FL_LINK_TYPE) == - MEDIA_LNK_FL_INTERFACE_LINK) - continue; - - /* Store references to source and sink pads in the link. */ - unsigned int source_id = mediaLinks[i].source_id; - MediaPad *source = dynamic_cast - (object(source_id)); - if (!source) { - LOG(Error) << "Failed to find pad with id: " - << source_id; - return false; - } - - unsigned int sink_id = mediaLinks[i].sink_id; - MediaPad *sink = dynamic_cast - (object(sink_id)); - if (!sink) { - LOG(Error) << "Failed to find pad with id: " - << sink_id; - return false; - } - - MediaLink *link = new MediaLink(&mediaLinks[i], source, sink); - if (!addObject(link)) { - delete link; - return false; - } - - source->addLink(link); - sink->addLink(link); - } - - return true; -} - -bool MediaDevice::populatePads(const struct media_v2_topology &topology) -{ - media_v2_pad *mediaPads = reinterpret_cast - (topology.ptr_pads); - - for (unsigned int i = 0; i < topology.num_pads; ++i) { - unsigned int entity_id = mediaPads[i].entity_id; - - /* Store a reference to this MediaPad in entity. */ - MediaEntity *mediaEntity = dynamic_cast - (object(entity_id)); - if (!mediaEntity) { - LOG(Error) << "Failed to find entity with id: " - << entity_id; - return false; - } - - MediaPad *pad = new MediaPad(&mediaPads[i], mediaEntity); - if (!addObject(pad)) { - delete pad; - return false; - } - - mediaEntity->addPad(pad); - } - - return true; -} - -/* - * For each entity in the media graph create a MediaEntity and store a - * reference in the MediaObject global pool and in the global vector of - * entities. - */ -bool MediaDevice::populateEntities(const struct media_v2_topology &topology) -{ - media_v2_entity *mediaEntities = reinterpret_cast - (topology.ptr_entities); - - for (unsigned int i = 0; i < topology.num_entities; ++i) { - MediaEntity *entity = new MediaEntity(&mediaEntities[i]); - if (!addObject(entity)) { - delete entity; - return false; - } - - entities_.push_back(entity); - } - - return true; -} - /** * \brief Populate the media graph with media objects * @@ -380,15 +202,193 @@ int MediaDevice::populate() * \return true if the media graph is valid, false otherwise */ +/** + * \fn MediaDevice::driver() + * \brief Retrieve the media device driver name + * \return The name of the kernel driver that handles the MediaDevice + */ + +/** + * \fn MediaDevice::devnode() + * \brief Retrieve the media device device node path + * \return The MediaDevice devnode path + */ + +/** + * \fn MediaDevice::entities() + * \brief Retrieve the list of entities in the media graph + * \return The list of MediaEntities registered in the MediaDevice + */ + +/** + * \brief Return the MediaEntity with name \a name + * \param name The entity name + * \return The entity with \a name + * \return nullptr if no entity with \a name is found + */ +MediaEntity *MediaDevice::getEntityByName(const std::string &name) +{ + for (MediaEntity *e : entities_) + if (e->name() == name) + return e; + + return nullptr; +} + /** * \var MediaDevice::objects_ * \brief Global map of media objects (entities, pads, links) keyed by their * object id. */ +/* + * MediaObject pool lookup by id. + */ +MediaObject *MediaDevice::object(unsigned int id) +{ + auto it = objects_.find(id); + return (it == objects_.end()) ? nullptr : it->second; +} + +/* + * Add a new object to the global objects pool and fail if the object + * has already been registered. + */ +bool MediaDevice::addObject(MediaObject *obj) +{ + + if (objects_.find(obj->id()) != objects_.end()) { + LOG(Error) << "Element with id " << obj->id() + << " already enumerated."; + return false; + } + + objects_[obj->id()] = obj; + + return true; +} + +/** + * \brief Delete all media objects in the MediaDevice. + * + * Delete all MediaEntities; entities will then delete their pads, + * and each source pad will delete links. + * + * After this function has been called, the media graph will be unpopulated + * and its media objects deleted. The media device has to be populated + * before it could be used again. + */ +void MediaDevice::clear() +{ + for (auto const &o : objects_) + delete o.second; + + objects_.clear(); + entities_.clear(); + valid_ = false; +} + /** * \var MediaDevice::entities_ * \brief Global list of media entities in the media graph */ +/* + * For each entity in the media graph create a MediaEntity and store a + * reference in the MediaObject global pool and in the global vector of + * entities. + */ +bool MediaDevice::populateEntities(const struct media_v2_topology &topology) +{ + media_v2_entity *mediaEntities = reinterpret_cast + (topology.ptr_entities); + + for (unsigned int i = 0; i < topology.num_entities; ++i) { + MediaEntity *entity = new MediaEntity(&mediaEntities[i]); + if (!addObject(entity)) { + delete entity; + return false; + } + + entities_.push_back(entity); + } + + return true; +} + +bool MediaDevice::populatePads(const struct media_v2_topology &topology) +{ + media_v2_pad *mediaPads = reinterpret_cast + (topology.ptr_pads); + + for (unsigned int i = 0; i < topology.num_pads; ++i) { + unsigned int entity_id = mediaPads[i].entity_id; + + /* Store a reference to this MediaPad in entity. */ + MediaEntity *mediaEntity = dynamic_cast + (object(entity_id)); + if (!mediaEntity) { + LOG(Error) << "Failed to find entity with id: " + << entity_id; + return false; + } + + MediaPad *pad = new MediaPad(&mediaPads[i], mediaEntity); + if (!addObject(pad)) { + delete pad; + return false; + } + + mediaEntity->addPad(pad); + } + + return true; +} + +bool MediaDevice::populateLinks(const struct media_v2_topology &topology) +{ + media_v2_link *mediaLinks = reinterpret_cast + (topology.ptr_links); + + for (unsigned int i = 0; i < topology.num_links; ++i) { + /* + * Skip links between entities and interfaces: we only care + * about pad-2-pad links here. + */ + if ((mediaLinks[i].flags & MEDIA_LNK_FL_LINK_TYPE) == + MEDIA_LNK_FL_INTERFACE_LINK) + continue; + + /* Store references to source and sink pads in the link. */ + unsigned int source_id = mediaLinks[i].source_id; + MediaPad *source = dynamic_cast + (object(source_id)); + if (!source) { + LOG(Error) << "Failed to find pad with id: " + << source_id; + return false; + } + + unsigned int sink_id = mediaLinks[i].sink_id; + MediaPad *sink = dynamic_cast + (object(sink_id)); + if (!sink) { + LOG(Error) << "Failed to find pad with id: " + << sink_id; + return false; + } + + MediaLink *link = new MediaLink(&mediaLinks[i], source, sink); + if (!addObject(link)) { + delete link; + return false; + } + + source->addLink(link); + sink->addLink(link); + } + + return true; +} + } /* namespace libcamera */ From patchwork Tue Jan 1 21:23:28 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 120 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9A40760B35 for ; Tue, 1 Jan 2019 22:22:34 +0100 (CET) Received: from avalon.bb.dnainternet.fi (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2C7C9505 for ; Tue, 1 Jan 2019 22:22:34 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1546377754; bh=BXCzdJJQ+hMAiaID4Un6OtGijZ8RxPniOI5D89cJDEw=; h=From:To:Subject:Date:In-Reply-To:References:From; b=Hy/zWKJGKDpwSvF3qUx8ZYpyBNBIXM2O96ic+e/xg5D0nB3mDEAlXCLh5fJzugciF Zoi+iSvsMbEEu5kr/4Ye8G77clhl0KyvrDcmAhZjYXuiG+MWXHkQGKVBd3tR9DzoA7 rp5h8XwsSj7adWJXj+2Md4dYtcnDJT/nUNqy8cDs= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 1 Jan 2019 23:23:28 +0200 Message-Id: <20190101212328.18361-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190101212328.18361-1-laurent.pinchart@ideasonboard.com> References: <20190101212328.18361-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 4/4] libcamera: mediadevice: Improve documentation 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: Tue, 01 Jan 2019 21:22:34 -0000 Improve the documentation of the media device operation, including how it handles the lifetime of media objects. Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham --- src/libcamera/include/media_device.h | 2 +- src/libcamera/media_device.cpp | 113 ++++++++++++++++----------- 2 files changed, 70 insertions(+), 45 deletions(-) diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h index fd1e12d1bbcb..d787be391882 100644 --- a/src/libcamera/include/media_device.h +++ b/src/libcamera/include/media_device.h @@ -44,7 +44,7 @@ private: std::map objects_; MediaObject *object(unsigned int id); - bool addObject(MediaObject *obj); + bool addObject(MediaObject *object); void clear(); std::vector entities_; diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index 4c77d3787391..2470c0be806e 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -32,37 +32,41 @@ namespace libcamera { * \brief The MediaDevice represents a Media Controller device with its full * graph of connected objects. * - * Media devices are created with an empty graph, which must be populated from + * A MediaDevice instance is associated with a media controller device node when + * created, and that association is kept for the lifetime of the 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. * - * The caller is responsible for opening the MediaDevice explicitly before - * operating on it, and shall close it when not needed anymore, as access - * to the MediaDevice is exclusive. + * Once successfully populated the graph is valid, as reported by the valid() + * function. It can be queried to list all entities(), or entities can be + * looked up by name with getEntityByName(). The graph can be traversed from + * entity to entity through pads and links as exposed by the corresponding + * classes. * - * A MediaDevice is created empty and gets populated by inspecting the media - * graph topology using the MEDIA_IOC_G_TOPOLOGY ioctls. Representation - * of each entity, pad and link described are created using MediaObject - * derived classes. - * - * All MediaObject are stored in a global pool, where they could be retrieved - * from by their globally unique id. - * - * References to MediaEntity registered in the graph are stored in a vector - * to allow easier by-name lookup, and the list of MediaEntities is accessible. + * 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 object remain + * valid and can be accessed normally. The device can then be later reopened if + * needed to perform other operations that interect with the device node. */ /** * \brief Construct a MediaDevice * \param devnode 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. */ MediaDevice::MediaDevice(const std::string &devnode) : devnode_(devnode), fd_(-1), valid_(false) { } -/** - * \brief Close the media device file descriptor and delete all object - */ MediaDevice::~MediaDevice() { if (fd_ != -1) @@ -71,11 +75,18 @@ MediaDevice::~MediaDevice() } /** - * \brief Open a media device and retrieve informations from it + * \brief Open a media device and retrieve device information * - * The function fails if the media device is already open or if either - * open or the media device information retrieval operations fail. - * \return 0 for success or a negative error number otherwise + * 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. + * + * \return 0 on success or a negative error code otherwise */ int MediaDevice::open() { @@ -108,10 +119,17 @@ int MediaDevice::open() } /** - * \brief Close the file descriptor associated with the media device. + * \brief Close the media device + * + * This function closes the media device node. It does not invalidate the media + * graph and all cached media object 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. * - * After this function has been called, for the MediaDevice to be operated on, - * the caller shall open it again. + * Closing an already closed device is allowed and will not performed any + * operation. + * + * \sa open() */ void MediaDevice::close() { @@ -130,9 +148,9 @@ void MediaDevice::close() * stored as MediaEntity, MediaPad and MediaLink respectively, with cross- * references between objects. Interfaces are not processed. * - * MediaEntities are stored in a global list in the MediaDevice itself to ease - * lookup, while MediaPads are accessible from the MediaEntity they belong - * to only and MediaLinks from the MediaPad they connect. + * 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 + * pad they connect. * * \return 0 on success, a negative error code otherwise */ @@ -241,8 +259,9 @@ MediaEntity *MediaDevice::getEntityByName(const std::string &name) * object id. */ -/* - * MediaObject pool lookup by id. +/** + * \brief Retrieve the media graph object specified by \a id + * \return The graph object, or nullptr if no object with \a id is found */ MediaObject *MediaDevice::object(unsigned int id) { @@ -250,33 +269,40 @@ MediaObject *MediaDevice::object(unsigned int id) return (it == objects_.end()) ? nullptr : it->second; } -/* - * Add a new object to the global objects pool and fail if the object - * has already been registered. +/** + * \brief Add a media object to the media graph + * + * If the \a object has a unique id it is added to the media graph, and its + * lifetime will be managed by the media device. Otherwise the object isn't + * added to the graph and the caller must delete it. + * + * \return true if the object was successfully added to the graph and false + * otherwise */ -bool MediaDevice::addObject(MediaObject *obj) +bool MediaDevice::addObject(MediaObject *object) { - if (objects_.find(obj->id()) != objects_.end()) { - LOG(Error) << "Element with id " << obj->id() + if (objects_.find(object->id()) != objects_.end()) { + LOG(Error) << "Element with id " << object->id() << " already enumerated."; return false; } - objects_[obj->id()] = obj; + objects_[object->id()] = object; return true; } /** - * \brief Delete all media objects in the MediaDevice. + * \brief Delete all graph objects in the media device + * + * Clear the media graph and delete all the objects it contains. After this + * function returns any previously obtained pointer to a media graph object + * becomes invalid. * - * Delete all MediaEntities; entities will then delete their pads, - * and each source pad will delete links. + * The media device graph state is reset to invalid when the graph is cleared. * - * After this function has been called, the media graph will be unpopulated - * and its media objects deleted. The media device has to be populated - * before it could be used again. + * \sa valid() */ void MediaDevice::clear() { @@ -295,8 +321,7 @@ void MediaDevice::clear() /* * For each entity in the media graph create a MediaEntity and store a - * reference in the MediaObject global pool and in the global vector of - * entities. + * reference in the media device objects map and entities list. */ bool MediaDevice::populateEntities(const struct media_v2_topology &topology) {