{"id":129,"url":"https://patchwork.libcamera.org/api/patches/129/?format=json","web_url":"https://patchwork.libcamera.org/patch/129/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20190102004903.24190-3-laurent.pinchart@ideasonboard.com>","date":"2019-01-02T00:49:03","name":"[libcamera-devel,3/3] libcamera: device_enumerator: Use MediaDevice","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"f293b077650f9858a46a3ea604d3775c705435f6","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/129/mbox/","series":[{"id":47,"url":"https://patchwork.libcamera.org/api/series/47/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=47","date":"2019-01-02T00:49:01","name":"[libcamera-devel,1/3] libcamera: media_device: Add DeviceInfo features","version":1,"mbox":"https://patchwork.libcamera.org/series/47/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/129/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/129/checks/","tags":{},"headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 024B160B30\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  2 Jan 2019 01:48:09 +0100 (CET)","from avalon.bb.dnainternet.fi\n\t(dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 57564505\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  2 Jan 2019 01:48:08 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1546390088;\n\tbh=JCv/CXK7OO2ZNTfYo8WanUC1fqo8QVhawn1tOKZoSZc=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=HBdGmetstCehblNe1FtMPifu1Ahaq9yyoa3mG2yokBjukqBGNY2lUjPiagT0HubUw\n\tE+u0Uyqjr7jsKPtyBkeTljfdmaIGVejVQH9SlUQrS0ofuNLD6y0Epc34pa8heDwhex\n\tNXGk22Nl0Crqz5bBzebsGpJqFDe1AaJ43tjPTA4o=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Date":"Wed,  2 Jan 2019 02:49:03 +0200","Message-Id":"<20190102004903.24190-3-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.19.2","In-Reply-To":"<20190102004903.24190-1-laurent.pinchart@ideasonboard.com>","References":"<20190102004903.24190-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH 3/3] libcamera: device_enumerator: Use\n\tMediaDevice","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 02 Jan 2019 00:48:09 -0000"},"content":"From: Jacopo Mondi <jacopo@jmondi.org>\n\nReplace usage of the DeviceInfo class with MediaDevice in the\nDeviceEnumerator and remove the DeviceInfo class.\n\nSigned-off-by: Jacopo Mondi <jacopo@jmondi.org>\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/libcamera/device_enumerator.cpp       | 313 +++-------------------\n src/libcamera/include/device_enumerator.h |  33 +--\n src/libcamera/pipeline/vimc.cpp           |  17 +-\n 3 files changed, 50 insertions(+), 313 deletions(-)","diff":"diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp\nindex d5ba869f3457..7eef450e0c65 100644\n--- a/src/libcamera/device_enumerator.cpp\n+++ b/src/libcamera/device_enumerator.cpp\n@@ -13,6 +13,7 @@\n \n #include \"device_enumerator.h\"\n #include \"log.h\"\n+#include \"media_device.h\"\n \n /**\n  * \\file device_enumerator.h\n@@ -28,8 +29,8 @@\n  * for other parts of libcamera.\n  *\n  * The DeviceEnumerator can enumerate all or specific media devices in\n- * the system. When a new media device is added the enumerator gathers\n- * information about it and stores it in a DeviceInfo object.\n+ * the system. When a new media device is added the enumerator creates a\n+ * corresponding MediaDevice instance.\n  *\n  * The last functionality provided is the ability to search among the\n  * enumerate media devices for one matching information known to the\n@@ -42,135 +43,6 @@\n \n namespace libcamera {\n \n-/**\n- * \\class DeviceInfo\n- * \\brief Container of information for enumerated device\n- *\n- * The DeviceInfo class holds information about a media device. It provides\n- * methods to retrieve the information stored and to lookup entity names\n- * to device node paths. Furthermore it provides a scheme where a device\n- * can be acquired and released to indicate if the device is in use.\n- *\n- * \\todo Look into the possibility to replace this with a more complete MediaDevice model.\n- */\n-\n-/**\n- * \\brief Construct a container of device information\n- *\n- * \\param[in] devnode The path to the device node of the media device\n- * \\param[in] info Information retrieved from MEDIA_IOC_DEVICE_INFO IOCTL\n- * \\param[in] entities A map of media graph 'Entity name' -> 'devnode path'\n- *\n- * The caller is responsible to provide all information for the device.\n- */\n-DeviceInfo::DeviceInfo(const std::string &devnode, const struct media_device_info &info,\n-\t\t       const std::map<std::string, std::string> &entities)\n-\t: acquired_(false), devnode_(devnode), info_(info), entities_(entities)\n-{\n-\tfor (const auto &entity : entities_)\n-\t\tLOG(Info) << \"Device: \" << devnode_ << \" Entity: '\" << entity.first << \"' -> \" << entity.second;\n-}\n-\n-/**\n- * \\brief Claim a device for exclusive use\n- *\n- * Once a device is successfully acquired the caller is responsible to\n- * release it once it is done wit it.\n- *\n- * \\retval 0 Device claimed\n- * \\retval -EBUSY Device already claimed by someone else\n- */\n-int DeviceInfo::acquire()\n-{\n-\tif (acquired_)\n-\t\treturn -EBUSY;\n-\n-\tacquired_ = true;\n-\n-\treturn 0;\n-}\n-\n-/**\n- * \\brief Release a device from exclusive use\n- */\n-void DeviceInfo::release()\n-{\n-\tacquired_ = false;\n-}\n-\n-/**\n- * \\brief Check if a device is in use\n- *\n- * \\retval true Device is in use\n- * \\retval false Device is free\n- */\n-bool DeviceInfo::busy() const\n-{\n-\treturn acquired_;\n-}\n-\n-/**\n- * \\brief Retrieve the devnode to the media device\n- *\n- * \\return Path to the media device (example /dev/media0)\n- */\n-const std::string &DeviceInfo::devnode() const\n-{\n-\treturn devnode_;\n-}\n-\n-/**\n- * \\brief Retrieve the media device v4l2 information\n- *\n- * \\return v4l2 specific information structure\n- */\n-const struct media_device_info &DeviceInfo::info() const\n-{\n-\treturn info_;\n-}\n-\n-/**\n- * \\brief List all entities of the device\n- *\n- * List all media entities names from the media graph which are known\n- * and to which this instance can lookup the device node path.\n- *\n- * \\return List of strings\n- */\n-std::vector<std::string> DeviceInfo::entities() const\n-{\n-\tstd::vector<std::string> entities;\n-\n-\tfor (const auto &entity : entities_)\n-\t\tentities.push_back(entity.first);\n-\n-\treturn entities;\n-}\n-\n-/**\n- * \\brief Lookup a media entity name and retrieve its device node path\n- *\n- * \\param[in] name Entity name to lookup\n- * \\param[out] devnode Path to \\a name devnode if lookup is successful\n- *\n- * The caller is responsible to check the return code of the function\n- * to determine if the entity name could be looked up.\n- *\n- * \\return 0 on success none zero otherwise\n- */\n-int DeviceInfo::lookup(const std::string &name, std::string &devnode) const\n-{\n-\tauto it = entities_.find(name);\n-\n-\tif (it == entities_.end()) {\n-\t\tLOG(Error) << \"Trying to lookup entity '\" << name << \"' which does not exist\";\n-\t\treturn -ENODEV;\n-\t}\n-\n-\tdevnode = it->second;\n-\treturn 0;\n-}\n-\n /**\n  * \\class DeviceMatch\n  * \\brief Description of a media device search pattern\n@@ -205,25 +77,23 @@ void DeviceMatch::add(const std::string &entity)\n \n /**\n  * \\brief Compare a search pattern with a media device\n- *\n- * \\param[in] info Information about a enumerated media device\n+ * \\param[in] device The media device\n  *\n  * Matching is performed on the Linux device driver name and entity names\n  * from the media graph.\n  *\n- * \\retval true The device described in \\a info matches search pattern\n- * \\retval false The device described in \\a info do not match search pattern\n+ * \\return true if the media device matches the search pattern, false otherwise\n  */\n-bool DeviceMatch::match(const DeviceInfo *info) const\n+bool DeviceMatch::match(const MediaDevice *device) const\n {\n-\tif (driver_ != info->info().driver)\n+\tif (driver_ != device->driver())\n \t\treturn false;\n \n \tfor (const std::string &name : entities_) {\n \t\tbool found = false;\n \n-\t\tfor (const std::string &entity : info->entities()) {\n-\t\t\tif (name == entity) {\n+\t\tfor (const MediaEntity *entity : device->entities()) {\n+\t\t\tif (name == entity->name()) {\n \t\t\t\tfound = true;\n \t\t\t\tbreak;\n \t\t\t}\n@@ -281,7 +151,7 @@ DeviceEnumerator *DeviceEnumerator::create()\n \n DeviceEnumerator::~DeviceEnumerator()\n {\n-\tfor (DeviceInfo *dev : devices_) {\n+\tfor (MediaDevice *dev : devices_) {\n \t\tif (dev->busy())\n \t\t\tLOG(Error) << \"Removing device info while still in use\";\n \n@@ -300,171 +170,62 @@ DeviceEnumerator::~DeviceEnumerator()\n  */\n int DeviceEnumerator::addDevice(const std::string &devnode)\n {\n-\tint fd, ret;\n+\tMediaDevice *media = new MediaDevice(devnode);\n \n-\tstruct media_device_info info = {};\n-\tstd::map<std::string, std::string> entities;\n-\n-\tfd = open(devnode.c_str(), O_RDWR);\n-\tif (fd < 0) {\n-\t\tret = -errno;\n-\t\tLOG(Info) << \"Unable to open \" << devnode <<\n-\t\t\t  \" (\" << strerror(-ret) << \"), skipping\";\n+\tint ret = media->open();\n+\tif (ret < 0)\n \t\treturn ret;\n-\t}\n \n-\tret = readInfo(fd, info);\n-\tif (ret)\n-\t\tgoto out;\n-\n-\tret = readTopology(fd, entities);\n-\tif (ret)\n-\t\tgoto out;\n-\n-\tdevices_.push_back(new DeviceInfo(devnode, info, entities));\n-out:\n-\tclose(fd);\n-\n-\treturn ret;\n-}\n-\n-/**\n- * \\brief Fetch the MEDIA_IOC_DEVICE_INFO from media device\n- *\n- * \\param[in] fd File pointer to media device\n- * \\param[out] info Information retrieved from MEDIA_IOC_DEVICE_INFO IOCTL\n- *\n- * Opens the media device and quires its information.\n- *\n- * \\return 0 on success none zero otherwise\n- */\n-int DeviceEnumerator::readInfo(int fd, struct media_device_info &info)\n-{\n-\tint ret;\n-\n-\tret = ioctl(fd, MEDIA_IOC_DEVICE_INFO, &info);\n+\tret = media->populate();\n \tif (ret < 0) {\n-\t\tret = -errno;\n-\t\tLOG(Info) << \"Unable to read device info \" <<\n+\t\tLOG(Info) << \"Unable to populate media device \" << devnode <<\n \t\t\t  \" (\" << strerror(-ret) << \"), skipping\";\n \t\treturn ret;\n \t}\n \n-\treturn 0;\n-}\n-\n-/**\n- * \\brief Fetch the topology from media device\n- *\n- * \\param[in] fd File pointer to media device\n- * \\param[out] entities Map of entity names to device node paths\n- *\n- * The media graph is retrieved using MEDIA_IOC_G_TOPOLOGY and the\n- * result is transformed to a map where the entity name is the key\n- * and the filesystem path for that entity device node is the value.\n- *\n- * \\return 0 on success none zero otherwise\n- */\n-int DeviceEnumerator::readTopology(int fd, std::map<std::string, std::string> &entities)\n-{\n-\tstruct media_v2_topology topology;\n-\tstruct media_v2_entity *ents = nullptr;\n-\tstruct media_v2_interface *ifaces = nullptr;\n-\tstruct media_v2_link *links = nullptr;\n-\tint ret;\n-\n-\twhile (true) {\n-\t\ttopology = {};\n-\n-\t\tret = ioctl(fd, MEDIA_IOC_G_TOPOLOGY, &topology);\n-\t\tif (ret < 0)\n-\t\t\treturn -errno;\n-\n-\t\t__u64 version = topology.topology_version;\n-\n-\t\tents = new media_v2_entity[topology.num_entities]();\n-\t\tifaces = new media_v2_interface[topology.num_interfaces]();\n-\t\tlinks = new media_v2_link[topology.num_links]();\n-\t\ttopology.ptr_entities = reinterpret_cast<__u64>(ents);\n-\t\ttopology.ptr_interfaces = reinterpret_cast<__u64>(ifaces);\n-\t\ttopology.ptr_links = reinterpret_cast<__u64>(links);\n-\n-\t\tret = ioctl(fd, MEDIA_IOC_G_TOPOLOGY, &topology);\n-\t\tif (ret < 0) {\n-\t\t\tret = -errno;\n-\t\t\tgoto done;\n-\t\t}\n-\n-\t\tif (version == topology.topology_version)\n-\t\t\tbreak;\n-\n-\t\tdelete[] links;\n-\t\tdelete[] ifaces;\n-\t\tdelete[] ents;\n-\t}\n-\n-\tfor (unsigned int link_id = 0; link_id < topology.num_links; link_id++) {\n-\t\tunsigned int iface_id, ent_id;\n-\t\tstd::string devnode;\n-\n-\t\tif ((links[link_id].flags & MEDIA_LNK_FL_LINK_TYPE) !=\n-\t\t    MEDIA_LNK_FL_INTERFACE_LINK)\n+\t/* Associate entities to device node paths. */\n+\tfor (MediaEntity *entity : media->entities()) {\n+\t\tif (entity->major() == 0 && entity->minor() == 0)\n \t\t\tcontinue;\n \n-\t\tfor (iface_id = 0; iface_id < topology.num_interfaces; iface_id++)\n-\t\t\tif (links[link_id].source_id == ifaces[iface_id].id)\n-\t\t\t\tbreak;\n-\n-\t\tfor (ent_id = 0; ent_id < topology.num_entities; ent_id++)\n-\t\t\tif (links[link_id].sink_id == ents[ent_id].id)\n-\t\t\t\tbreak;\n-\n-\t\tif (ent_id >= topology.num_entities ||\n-\t\t    iface_id >= topology.num_interfaces)\n-\t\t\tcontinue;\n+\t\tstd::string devnode = lookupDevnode(entity->major(), entity->minor());\n+\t\tif (devnode.empty())\n+\t\t\treturn -EINVAL;\n \n-\t\tdevnode = lookupDevnode(ifaces[iface_id].devnode.major,\n-\t\t\t\t\tifaces[iface_id].devnode.minor);\n-\t\tif (devnode == \"\")\n-\t\t\tbreak;\n-\n-\t\tentities[ents[ent_id].name] = devnode;\n+\t\tret = entity->setDeviceNode(devnode);\n+\t\tif (ret)\n+\t\t\treturn ret;\n \t}\n-done:\n-\tdelete[] links;\n-\tdelete[] ifaces;\n-\tdelete[] ents;\n \n-\treturn ret;\n+\tdevices_.push_back(media);\n+\tmedia->close();\n+\n+\treturn 0;\n }\n \n /**\n  * \\brief Search available media devices for a pattern match\n  *\n- * \\param[in] dm search pattern\n+ * \\param[in] dm Search pattern\n  *\n- * Search the enumerated media devices who are not already in use\n+ * Search in the enumerated media devices that are not already in use\n  * for a match described in \\a dm. If a match is found and the caller\n- * intends to use it the caller is responsible to mark the DeviceInfo\n+ * intends to use it the caller is responsible to mark the MediaDevice\n  * object as in use and to release it when it's done with it.\n  *\n- * \\return pointer to the matching DeviceInfo, nullptr if no match is found\n+ * \\return pointer to the matching MediaDevice, nullptr if no match is found\n  */\n-DeviceInfo *DeviceEnumerator::search(DeviceMatch &dm) const\n+MediaDevice *DeviceEnumerator::search(DeviceMatch &dm) const\n {\n-\tDeviceInfo *info = nullptr;\n-\n-\tfor (DeviceInfo *dev : devices_) {\n+\tfor (MediaDevice *dev : devices_) {\n \t\tif (dev->busy())\n \t\t\tcontinue;\n \n-\t\tif (dm.match(dev)) {\n-\t\t\tinfo = dev;\n-\t\t\tbreak;\n-\t\t}\n+\t\tif (dm.match(dev))\n+\t\t\treturn dev;\n \t}\n \n-\treturn info;\n+\treturn nullptr;\n }\n \n /**\ndiff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h\nindex 24bca0e3fc32..0d104667323b 100644\n--- a/src/libcamera/include/device_enumerator.h\n+++ b/src/libcamera/include/device_enumerator.h\n@@ -15,29 +15,7 @@\n \n namespace libcamera {\n \n-class DeviceInfo\n-{\n-public:\n-\tDeviceInfo(const std::string &devnode, const struct media_device_info &info,\n-\t\t   const std::map<std::string, std::string> &entities);\n-\n-\tint acquire();\n-\tvoid release();\n-\tbool busy() const;\n-\n-\tconst std::string &devnode() const;\n-\tconst struct media_device_info &info() const;\n-\tstd::vector<std::string> entities() const;\n-\n-\tint lookup(const std::string &name, std::string &devnode) const;\n-\n-private:\n-\tbool acquired_;\n-\n-\tstd::string devnode_;\n-\tstruct media_device_info info_;\n-\tstd::map<std::string, std::string> entities_;\n-};\n+class MediaDevice;\n \n class DeviceMatch\n {\n@@ -46,7 +24,7 @@ public:\n \n \tvoid add(const std::string &entity);\n \n-\tbool match(const DeviceInfo *info) const;\n+\tbool match(const MediaDevice *device) const;\n \n private:\n \tstd::string driver_;\n@@ -63,16 +41,13 @@ public:\n \tvirtual int init() = 0;\n \tvirtual int enumerate() = 0;\n \n-\tDeviceInfo *search(DeviceMatch &dm) const;\n+\tMediaDevice *search(DeviceMatch &dm) const;\n \n protected:\n \tint addDevice(const std::string &devnode);\n \n private:\n-\tstd::vector<DeviceInfo *> devices_;\n-\n-\tint readInfo(int fd, struct media_device_info &info);\n-\tint readTopology(int fd, std::map<std::string, std::string> &entities);\n+\tstd::vector<MediaDevice *> devices_;\n \n \tvirtual std::string lookupDevnode(int major, int minor) = 0;\n };\ndiff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\nindex b1e2d32c8ba2..720d9c2031c9 100644\n--- a/src/libcamera/pipeline/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc.cpp\n@@ -8,6 +8,7 @@\n #include <libcamera/camera.h>\n \n #include \"device_enumerator.h\"\n+#include \"media_device.h\"\n #include \"pipeline_handler.h\"\n \n namespace libcamera {\n@@ -24,12 +25,12 @@ public:\n \tCamera *camera(unsigned int id) final;\n \n private:\n-\tDeviceInfo *info_;\n+\tMediaDevice *dev_;\n \tCamera *camera_;\n };\n \n PipeHandlerVimc::PipeHandlerVimc()\n-\t: info_(nullptr), camera_(nullptr)\n+\t: dev_(nullptr), camera_(nullptr)\n {\n }\n \n@@ -38,8 +39,8 @@ PipeHandlerVimc::~PipeHandlerVimc()\n \tif (camera_)\n \t\tcamera_->put();\n \n-\tif (info_)\n-\t\tinfo_->release();\n+\tif (dev_)\n+\t\tdev_->release();\n }\n \n unsigned int PipeHandlerVimc::count()\n@@ -69,15 +70,15 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n \tdm.add(\"RGB/YUV Input\");\n \tdm.add(\"Scaler\");\n \n-\tinfo_ = enumerator->search(dm);\n-\tif (!info_)\n+\tdev_ = enumerator->search(dm);\n+\tif (!dev_)\n \t\treturn false;\n \n-\tinfo_->acquire();\n+\tdev_->acquire();\n \n \t/*\n \t * NOTE: A more complete Camera implementation could\n-\t * be passed the DeviceInfo(s) it controls here or\n+\t * be passed the MediaDevice(s) it controls here or\n \t * a reference to the PipelineHandler. Which method\n \t * will be chosen depends on how the Camera\n \t * object is modeled.\n","prefixes":["libcamera-devel","3/3"]}