Message ID | 1476282922-11544-5-git-send-email-j.anaszewski@samsung.com (mailing list archive) |
---|---|
State | New |
Delegated to: | Sakari Ailus |
Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.84_2) (envelope-from <linux-media-owner@vger.kernel.org>) id 1buKoP-0001Pv-2S; Wed, 12 Oct 2016 14:47:17 +0000 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.84_2/mailfrontend-8) with esmtp id 1buKoM-00047y-jw; Wed, 12 Oct 2016 16:47:16 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933345AbcJLOrM (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Wed, 12 Oct 2016 10:47:12 -0400 Received: from mailout1.samsung.com ([203.254.224.24]:35138 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933274AbcJLOrL (ORCPT <rfc822;linux-media@vger.kernel.org>); Wed, 12 Oct 2016 10:47:11 -0400 Received: from epcpsbgm1new.samsung.com (epcpsbgm1 [203.254.230.26]) by mailout1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0OEX01FZBV7FY8A0@mailout1.samsung.com> for linux-media@vger.kernel.org; Wed, 12 Oct 2016 23:35:42 +0900 (KST) X-AuditID: cbfee61a-f79786d000004c78-32-57fe4a3e0a48 Received: from epmmp1.local.host ( [203.254.227.16]) by epcpsbgm1new.samsung.com (EPCPMTA) with SMTP id D0.E8.19576.E3A4EF75; Wed, 12 Oct 2016 23:35:42 +0900 (KST) Received: from AMDC2362.DIGITAL.local ([106.120.53.23]) by mmp1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTPA id <0OEX00HXUV70WM50@mmp1.samsung.com>; Wed, 12 Oct 2016 23:35:42 +0900 (KST) From: Jacek Anaszewski <j.anaszewski@samsung.com> To: linux-media@vger.kernel.org Cc: sakari.ailus@linux.intel.com, hverkuil@xs4all.nl, mchehab@kernel.org, m.szyprowski@samsung.com, s.nawrocki@samsung.com, Jacek Anaszewski <j.anaszewski@samsung.com> Subject: [PATCH v4l-utils v7 4/7] mediactl: Add media_device creation helpers Date: Wed, 12 Oct 2016 16:35:19 +0200 Message-id: <1476282922-11544-5-git-send-email-j.anaszewski@samsung.com> X-Mailer: git-send-email 1.9.1 In-reply-to: <1476282922-11544-1-git-send-email-j.anaszewski@samsung.com> References: <1476282922-11544-1-git-send-email-j.anaszewski@samsung.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrNLMWRmVeSWpSXmKPExsVy+t9jAV07r3/hBl+a1SxOTX7GZNF79Tmj Rc+GrawWa4/cZbdYtukPk8XhN+2sFp+2fGNyYPfYtKqTzWPeyUCPvi2rGD0+b5LzOPX1M3sA a5SbTUZqYkpqkUJqXnJ+SmZeuq1SaIibroWSQl5ibqqtUoSub0iQkkJZYk4pkGdkgAYcnAPc g5X07RLcMva16xR0mFa8uTmXrYGxX7uLkZNDQsBEYs6Cr8wQtpjEhXvr2boYuTiEBJYySpx+ PI8ZwvnJKHH4YS8TSBWbgKHEzxevwWwRAXmJJ7032EBsZoHVjBIXl6WA2MICvhLr3n5lBLFZ BFQl7v5bBVbPK+AhsfPOTqhtchInj01mBbE5BTwlPn1+CTZHCKjmzL5DrBMYeRcwMqxilEgt SC4oTkrPNcxLLdcrTswtLs1L10vOz93ECA7oZ1I7GA/ucj/EKMDBqMTDK/D1T7gQa2JZcWXu IUYJDmYlEd5gt3/hQrwpiZVVqUX58UWlOanFhxhNgQ6byCwlmpwPjLa8knhDE3MTc2MDC3NL SxMjJXHextnPwoUE0hNLUrNTUwtSi2D6mDg4pRoY801YLqRrxJopax41/GXNXGm4r2BX9Ien 4p9rdSZtv7Xf7Htg0b53Meuv9nOk1F2Yx+F9ukOTeVPmT/+Hn0weZngHlQQ9szG3D+koEpI8 waIzNeYh38FfnFM3rpsg/ttJt+L1tbMmGldrNIK5bxQ6zn/hsP7URq7FkntfzsjSCdd6rL2i 89BjJZbijERDLeai4kQAa6idmX4CAAA= X-MTR: 20000000000000000@CPGS Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2016.10.12.144216 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODY_SIZE_6000_6999 0, BODY_SIZE_7000_LESS 0, IN_REP_TO 0, LEGITIMATE_NEGATE 0, LEGITIMATE_SIGNS 0, MSG_THREAD 0, MULTIPLE_REAL_RCPTS 0, NO_URI_HTTPS 0, REFERENCES 0, SINGLE_URI_IN_BODY 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __CC_NAME 0, __CC_NAME_DIFF_FROM_ACC 0, __CC_REAL_NAMES 0, __CP_URI_IN_BODY 0, __HAS_CC_HDR 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __IN_REP_TO 0, __MIME_TEXT_ONLY 0, __MIME_TEXT_P 0, __MIME_TEXT_P1 0, __MULTIPLE_RCPTS_CC_X2 0, __REFERENCES 0, __SANE_MSGID 0, __SINGLE_URI_TEXT 0, __SUBJ_ALPHA_END 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_IN_BODY 0, __URI_NO_WWW 0, __URI_NS , __URI_WITH_PATH 0' |
Commit Message
Jacek Anaszewski
Oct. 12, 2016, 2:35 p.m. UTC
Add helper functions that allow for easy instantiation of media_device object basing on whether the media device contains v4l2 subdev with given file descriptor. Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com> Acked-by: Kyungmin Park <kyungmin.park@samsung.com> --- utils/media-ctl/libmediactl.c | 131 +++++++++++++++++++++++++++++++++++++++++- utils/media-ctl/mediactl.h | 27 +++++++++ 2 files changed, 156 insertions(+), 2 deletions(-)
Comments
Hi Jacek, Thanks for the patchset. On Wed, Oct 12, 2016 at 04:35:19PM +0200, Jacek Anaszewski wrote: > Add helper functions that allow for easy instantiation of media_device > object basing on whether the media device contains v4l2 subdev with > given file descriptor. Doesn't this work with video nodes as well? That's what you seem to be using it for later on. And I think that's actually more useful. The existing implementation uses udev to look up devices. Could you use libudev device enumeration API to find the media devices, and fall back to sysfs if udev doesn't work? There seems to be a reasonable-looking example here: <URL:http://stackoverflow.com/questions/25361042/how-to-list-usb-mass-storage-devices-programatically-using-libudev-in-linux> > > Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > utils/media-ctl/libmediactl.c | 131 +++++++++++++++++++++++++++++++++++++++++- > utils/media-ctl/mediactl.h | 27 +++++++++ > 2 files changed, 156 insertions(+), 2 deletions(-) > > diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c > index 155b65f..d347a40 100644 > --- a/utils/media-ctl/libmediactl.c > +++ b/utils/media-ctl/libmediactl.c > @@ -27,6 +27,7 @@ > #include <sys/sysmacros.h> > > #include <ctype.h> > +#include <dirent.h> > #include <errno.h> > #include <fcntl.h> > #include <stdbool.h> > @@ -440,8 +441,9 @@ static int media_get_devname_udev(struct udev *udev, > return -EINVAL; > > devnum = makedev(entity->info.v4l.major, entity->info.v4l.minor); > - media_dbg(entity->media, "looking up device: %u:%u\n", > - major(devnum), minor(devnum)); > + if (entity->media) > + media_dbg(entity->media, "looking up device: %u:%u\n", > + major(devnum), minor(devnum)); > device = udev_device_new_from_devnum(udev, 'c', devnum); > if (device) { > p = udev_device_get_devnode(device); > @@ -523,6 +525,7 @@ static int media_get_devname_sysfs(struct media_entity *entity) > return 0; > } > > + Unrelated change. > static int media_enum_entities(struct media_device *media) > { > struct media_entity *entity; > @@ -707,6 +710,92 @@ struct media_device *media_device_new(const char *devnode) > return media; > } > > +struct media_device *media_device_new_by_subdev_fd(int fd, struct media_entity **fd_entity) > +{ > + char video_devname[32], device_dir_path[256], media_dev_path[256], media_major_minor[10]; > + struct media_device *media = NULL; > + struct dirent *entry; > + struct media_entity tmp_entity; > + DIR *device_dir; > + struct udev *udev; > + char *p; > + int ret, i; > + > + if (fd_entity == NULL) > + return NULL; > + > + ret = media_get_devname_by_fd(fd, video_devname); > + if (ret < 0) > + return NULL; > + > + p = strrchr(video_devname, '/'); > + if (p == NULL) > + return NULL; > + > + ret = media_udev_open(&udev); > + if (ret < 0) > + return NULL; > + > + sprintf(device_dir_path, "/sys/class/video4linux/%s/device/", p + 1); > + > + device_dir = opendir(device_dir_path); > + if (device_dir == NULL) > + return NULL; > + > + while ((entry = readdir(device_dir))) { > + if (strncmp(entry->d_name, "media", 4)) Why 4? And isn't entry->d_name nul-terminated, so you could use strcmp()? > + continue; > + > + sprintf(media_dev_path, "%s%s/dev", device_dir_path, entry->d_name); > + > + fd = open(media_dev_path, O_RDONLY); > + if (fd < 0) > + continue; > + > + ret = read(fd, media_major_minor, sizeof(media_major_minor)); > + if (ret < 0) > + continue; > + > + sscanf(media_major_minor, "%d:%d", &tmp_entity.info.dev.major, &tmp_entity.info.dev.minor); This would be better split on two lines. > + > + /* Try to get the device name via udev */ > + if (media_get_devname_udev(udev, &tmp_entity)) { > + /* Fall back to get the device name via sysfs */ > + if (media_get_devname_sysfs(&tmp_entity)) > + continue; > + } > + > + media = media_device_new(tmp_entity.devname); > + if (media == NULL) > + continue; > + > + ret = media_device_enumerate(media); > + if (ret < 0) { > + media_dbg(media, "Failed to enumerate %s (%d)\n", > + tmp_entity.devname, ret); > + media_device_unref(media); > + media = NULL; > + continue; > + } > + > + /* Get the entity associated with given fd */ > + for (i = 0; i < media->entities_count; i++) { > + struct media_entity *entity = &media->entities[i]; > + > + if (!strcmp(entity->devname, video_devname)) { > + *fd_entity = &media->entities[i]; > + break; > + } > + } What if you exit the loop without finding the entity you were looking for? > + > + break; > + } > + > + media_udev_close(udev); > + > + return media; > +} > + > struct media_device *media_device_new_emulated(struct media_device_info *info) > { > struct media_device *media; > @@ -748,6 +837,44 @@ void media_device_unref(struct media_device *media) > free(media); > } > > +int media_get_devname_by_fd(int fd, char *node_name) > +{ > + struct udev *udev; > + struct media_entity tmp_entity; > + struct stat stat; > + int ret, ret_udev; > + > + if (node_name == NULL) > + return -EINVAL; > + > + ret = fstat(fd, &stat); > + if (ret < 0) > + return -errno; > + > + tmp_entity.info.v4l.major = MAJOR(stat.st_rdev); > + tmp_entity.info.v4l.minor = MINOR(stat.st_rdev); > + > + ret_udev = media_udev_open(&udev); > + if (ret_udev < 0) > + printf("Can't get udev context\n"); > + > + /* Try to get the device name via udev */ > + ret = media_get_devname_udev(udev, &tmp_entity); > + if (!ret) > + goto out; > + > + ret = media_get_devname_sysfs(&tmp_entity); > + if (ret < 0) > + goto err_get_devname; > + > +out: > + strncpy(node_name, tmp_entity.devname, sizeof(tmp_entity.devname)); This seems risky. How does the caller know the maximum size? I'd simply allocate the string, and document the caller is responsible for releasing it. > +err_get_devname: > + if (!ret_udev) > + media_udev_close(udev); > + return ret; > +} > + > int media_device_add_entity(struct media_device *media, > const struct media_entity_desc *desc, > const char *devnode) > diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h > index b1f33cd..580a25a 100644 > --- a/utils/media-ctl/mediactl.h > +++ b/utils/media-ctl/mediactl.h > @@ -76,6 +76,21 @@ struct media_device *media_device_new(const char *devnode); > struct media_device *media_device_new_emulated(struct media_device_info *info); > > /** > + * @brief Create a new media device contatning entity associated with v4l2 subdev fd. > + * @param fd - file descriptor of a v4l2 subdev. > + * @param fd_entity - media entity associated with the v4l2 subdev. > + * > + * Create a representation of the media device referenced by the v4l2-subdev. > + * The media device instance is initialized with enumerated entities and links. > + * > + * Media devices are reference-counted, see media_device_ref() and > + * media_device_unref() for more information. > + * > + * @return A pointer to the new media device or NULL if error occurred. > + */ > +struct media_device *media_device_new_by_subdev_fd(int fd, struct media_entity **fd_entity); I'd drop the "subdev_" part as both V4L2 device nodes and sub-devices work. If you wish to keep this V4L2 specific, I suggest ...by_v4l2_fd(). > + > +/** > * @brief Take a reference to the device. > * @param media - device instance. > * > @@ -231,6 +246,18 @@ const struct media_link *media_entity_get_link(struct media_entity *entity, > const char *media_entity_get_devname(struct media_entity *entity); > > /** > + * @brief Get the device node name by its file descriptor > + * @param fd - file descriptor of a device. > + * @param node_name - output device node name string. > + * > + * This function returns the full path and name to the device node corresponding > + * to the given file descriptor. > + * > + * @return 0 on success, or a negative error code on failure. > + */ > +int media_get_devname_by_fd(int fd, char *node_name); > + > +/** > * @brief Get the type of an entity. > * @param entity - the entity. > *
Hi Sakari, Thanks for the review. On 11/24/2016 01:17 PM, Sakari Ailus wrote: > Hi Jacek, > > Thanks for the patchset. > > On Wed, Oct 12, 2016 at 04:35:19PM +0200, Jacek Anaszewski wrote: >> Add helper functions that allow for easy instantiation of media_device >> object basing on whether the media device contains v4l2 subdev with >> given file descriptor. > > Doesn't this work with video nodes as well? That's what you seem to be using > it for later on. And I think that's actually more useful. Exactly, thanks for spotting this. s/v4l2 subdev/video device opened/ > > The existing implementation uses udev to look up devices. Could you use > libudev device enumeration API to find the media devices, and fall back to > sysfs if udev doesn't work? There seems to be a reasonable-looking example > here: > > <URL:http://stackoverflow.com/questions/25361042/how-to-list-usb-mass-storage-devices-programatically-using-libudev-in-linux> I'll check that, thanks. >> >> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com> >> Acked-by: Kyungmin Park <kyungmin.park@samsung.com> >> --- >> utils/media-ctl/libmediactl.c | 131 +++++++++++++++++++++++++++++++++++++++++- >> utils/media-ctl/mediactl.h | 27 +++++++++ >> 2 files changed, 156 insertions(+), 2 deletions(-) >> >> diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c >> index 155b65f..d347a40 100644 >> --- a/utils/media-ctl/libmediactl.c >> +++ b/utils/media-ctl/libmediactl.c >> @@ -27,6 +27,7 @@ >> #include <sys/sysmacros.h> >> >> #include <ctype.h> >> +#include <dirent.h> >> #include <errno.h> >> #include <fcntl.h> >> #include <stdbool.h> >> @@ -440,8 +441,9 @@ static int media_get_devname_udev(struct udev *udev, >> return -EINVAL; >> >> devnum = makedev(entity->info.v4l.major, entity->info.v4l.minor); >> - media_dbg(entity->media, "looking up device: %u:%u\n", >> - major(devnum), minor(devnum)); >> + if (entity->media) >> + media_dbg(entity->media, "looking up device: %u:%u\n", >> + major(devnum), minor(devnum)); >> device = udev_device_new_from_devnum(udev, 'c', devnum); >> if (device) { >> p = udev_device_get_devnode(device); >> @@ -523,6 +525,7 @@ static int media_get_devname_sysfs(struct media_entity *entity) >> return 0; >> } >> >> + > > Unrelated change. > >> static int media_enum_entities(struct media_device *media) >> { >> struct media_entity *entity; >> @@ -707,6 +710,92 @@ struct media_device *media_device_new(const char *devnode) >> return media; >> } >> >> +struct media_device *media_device_new_by_subdev_fd(int fd, struct media_entity **fd_entity) >> +{ >> + char video_devname[32], device_dir_path[256], media_dev_path[256], media_major_minor[10]; >> + struct media_device *media = NULL; >> + struct dirent *entry; >> + struct media_entity tmp_entity; >> + DIR *device_dir; >> + struct udev *udev; >> + char *p; >> + int ret, i; >> + >> + if (fd_entity == NULL) >> + return NULL; >> + >> + ret = media_get_devname_by_fd(fd, video_devname); >> + if (ret < 0) >> + return NULL; >> + >> + p = strrchr(video_devname, '/'); >> + if (p == NULL) >> + return NULL; >> + >> + ret = media_udev_open(&udev); >> + if (ret < 0) >> + return NULL; >> + >> + sprintf(device_dir_path, "/sys/class/video4linux/%s/device/", p + 1); >> + >> + device_dir = opendir(device_dir_path); >> + if (device_dir == NULL) >> + return NULL; >> + >> + while ((entry = readdir(device_dir))) { >> + if (strncmp(entry->d_name, "media", 4)) > > Why 4? And isn't entry->d_name nul-terminated, so you could use strcmp()? Media devices, as other devices, have numerical postfix, which is not of our interest. >> + continue; >> + >> + sprintf(media_dev_path, "%s%s/dev", device_dir_path, entry->d_name); >> + >> + fd = open(media_dev_path, O_RDONLY); >> + if (fd < 0) >> + continue; >> + >> + ret = read(fd, media_major_minor, sizeof(media_major_minor)); >> + if (ret < 0) >> + continue; >> + >> + sscanf(media_major_minor, "%d:%d", &tmp_entity.info.dev.major, &tmp_entity.info.dev.minor); > > This would be better split on two lines. OK. >> + >> + /* Try to get the device name via udev */ >> + if (media_get_devname_udev(udev, &tmp_entity)) { >> + /* Fall back to get the device name via sysfs */ >> + if (media_get_devname_sysfs(&tmp_entity)) >> + continue; >> + } >> + >> + media = media_device_new(tmp_entity.devname); >> + if (media == NULL) >> + continue; >> + >> + ret = media_device_enumerate(media); >> + if (ret < 0) { >> + media_dbg(media, "Failed to enumerate %s (%d)\n", >> + tmp_entity.devname, ret); >> + media_device_unref(media); >> + media = NULL; >> + continue; >> + } >> + >> + /* Get the entity associated with given fd */ >> + for (i = 0; i < media->entities_count; i++) { >> + struct media_entity *entity = &media->entities[i]; >> + >> + if (!strcmp(entity->devname, video_devname)) { >> + *fd_entity = &media->entities[i]; >> + break; >> + } >> + } > > What if you exit the loop without finding the entity you were looking for? Ah, right, this case is unhandled. Adding below condition should cover that: if (i == media->entities_count) media = NULL; >> + >> + break; This break should be removed and the one in the inner for loop above should be replaced with goto here. Are you OK with that? >> + } >> + >> + media_udev_close(udev); >> + >> + return media; >> +} >> + >> struct media_device *media_device_new_emulated(struct media_device_info *info) >> { >> struct media_device *media; >> @@ -748,6 +837,44 @@ void media_device_unref(struct media_device *media) >> free(media); >> } >> >> +int media_get_devname_by_fd(int fd, char *node_name) >> +{ >> + struct udev *udev; >> + struct media_entity tmp_entity; >> + struct stat stat; >> + int ret, ret_udev; >> + >> + if (node_name == NULL) >> + return -EINVAL; >> + >> + ret = fstat(fd, &stat); >> + if (ret < 0) >> + return -errno; >> + >> + tmp_entity.info.v4l.major = MAJOR(stat.st_rdev); >> + tmp_entity.info.v4l.minor = MINOR(stat.st_rdev); >> + >> + ret_udev = media_udev_open(&udev); >> + if (ret_udev < 0) >> + printf("Can't get udev context\n"); >> + >> + /* Try to get the device name via udev */ >> + ret = media_get_devname_udev(udev, &tmp_entity); >> + if (!ret) >> + goto out; >> + >> + ret = media_get_devname_sysfs(&tmp_entity); >> + if (ret < 0) >> + goto err_get_devname; >> + >> +out: >> + strncpy(node_name, tmp_entity.devname, sizeof(tmp_entity.devname)); > > This seems risky. How does the caller know the maximum size? I'd simply > allocate the string, and document the caller is responsible for releasing > it. OK. >> +err_get_devname: >> + if (!ret_udev) >> + media_udev_close(udev); >> + return ret; >> +} >> + >> int media_device_add_entity(struct media_device *media, >> const struct media_entity_desc *desc, >> const char *devnode) >> diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h >> index b1f33cd..580a25a 100644 >> --- a/utils/media-ctl/mediactl.h >> +++ b/utils/media-ctl/mediactl.h >> @@ -76,6 +76,21 @@ struct media_device *media_device_new(const char *devnode); >> struct media_device *media_device_new_emulated(struct media_device_info *info); >> >> /** >> + * @brief Create a new media device contatning entity associated with v4l2 subdev fd. >> + * @param fd - file descriptor of a v4l2 subdev. >> + * @param fd_entity - media entity associated with the v4l2 subdev. >> + * >> + * Create a representation of the media device referenced by the v4l2-subdev. >> + * The media device instance is initialized with enumerated entities and links. >> + * >> + * Media devices are reference-counted, see media_device_ref() and >> + * media_device_unref() for more information. >> + * >> + * @return A pointer to the new media device or NULL if error occurred. >> + */ >> +struct media_device *media_device_new_by_subdev_fd(int fd, struct media_entity **fd_entity); > > I'd drop the "subdev_" part as both V4L2 device nodes and sub-devices work. > > If you wish to keep this V4L2 specific, I suggest ...by_v4l2_fd(). Right, I will rename it. >> + >> +/** >> * @brief Take a reference to the device. >> * @param media - device instance. >> * >> @@ -231,6 +246,18 @@ const struct media_link *media_entity_get_link(struct media_entity *entity, >> const char *media_entity_get_devname(struct media_entity *entity); >> >> /** >> + * @brief Get the device node name by its file descriptor >> + * @param fd - file descriptor of a device. >> + * @param node_name - output device node name string. >> + * >> + * This function returns the full path and name to the device node corresponding >> + * to the given file descriptor. >> + * >> + * @return 0 on success, or a negative error code on failure. >> + */ >> +int media_get_devname_by_fd(int fd, char *node_name); >> + >> +/** >> * @brief Get the type of an entity. >> * @param entity - the entity. >> * >
Hi Jacek, On Thu, Nov 24, 2016 at 02:50:39PM +0100, Jacek Anaszewski wrote: > Hi Sakari, > > Thanks for the review. > > On 11/24/2016 01:17 PM, Sakari Ailus wrote: > >Hi Jacek, > > > >Thanks for the patchset. > > > >On Wed, Oct 12, 2016 at 04:35:19PM +0200, Jacek Anaszewski wrote: > >>Add helper functions that allow for easy instantiation of media_device > >>object basing on whether the media device contains v4l2 subdev with > >>given file descriptor. > > > >Doesn't this work with video nodes as well? That's what you seem to be using > >it for later on. And I think that's actually more useful. > > Exactly, thanks for spotting this. > > s/v4l2 subdev/video device opened/ > > > > >The existing implementation uses udev to look up devices. Could you use > >libudev device enumeration API to find the media devices, and fall back to > >sysfs if udev doesn't work? There seems to be a reasonable-looking example > >here: > > > ><URL:http://stackoverflow.com/questions/25361042/how-to-list-usb-mass-storage-devices-programatically-using-libudev-in-linux> > > I'll check that, thanks. > > >> > >>Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com> > >>Acked-by: Kyungmin Park <kyungmin.park@samsung.com> > >>--- > >> utils/media-ctl/libmediactl.c | 131 +++++++++++++++++++++++++++++++++++++++++- > >> utils/media-ctl/mediactl.h | 27 +++++++++ > >> 2 files changed, 156 insertions(+), 2 deletions(-) > >> > >>diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c > >>index 155b65f..d347a40 100644 > >>--- a/utils/media-ctl/libmediactl.c > >>+++ b/utils/media-ctl/libmediactl.c > >>@@ -27,6 +27,7 @@ > >> #include <sys/sysmacros.h> > >> > >> #include <ctype.h> > >>+#include <dirent.h> > >> #include <errno.h> > >> #include <fcntl.h> > >> #include <stdbool.h> > >>@@ -440,8 +441,9 @@ static int media_get_devname_udev(struct udev *udev, > >> return -EINVAL; > >> > >> devnum = makedev(entity->info.v4l.major, entity->info.v4l.minor); > >>- media_dbg(entity->media, "looking up device: %u:%u\n", > >>- major(devnum), minor(devnum)); > >>+ if (entity->media) > >>+ media_dbg(entity->media, "looking up device: %u:%u\n", > >>+ major(devnum), minor(devnum)); > >> device = udev_device_new_from_devnum(udev, 'c', devnum); > >> if (device) { > >> p = udev_device_get_devnode(device); > >>@@ -523,6 +525,7 @@ static int media_get_devname_sysfs(struct media_entity *entity) > >> return 0; > >> } > >> > >>+ > > > >Unrelated change. > > > >> static int media_enum_entities(struct media_device *media) > >> { > >> struct media_entity *entity; > >>@@ -707,6 +710,92 @@ struct media_device *media_device_new(const char *devnode) > >> return media; > >> } > >> > >>+struct media_device *media_device_new_by_subdev_fd(int fd, struct media_entity **fd_entity) > >>+{ > >>+ char video_devname[32], device_dir_path[256], media_dev_path[256], media_major_minor[10]; > >>+ struct media_device *media = NULL; > >>+ struct dirent *entry; > >>+ struct media_entity tmp_entity; > >>+ DIR *device_dir; > >>+ struct udev *udev; > >>+ char *p; > >>+ int ret, i; > >>+ > >>+ if (fd_entity == NULL) > >>+ return NULL; > >>+ > >>+ ret = media_get_devname_by_fd(fd, video_devname); > >>+ if (ret < 0) > >>+ return NULL; > >>+ > >>+ p = strrchr(video_devname, '/'); > >>+ if (p == NULL) > >>+ return NULL; > >>+ > >>+ ret = media_udev_open(&udev); > >>+ if (ret < 0) > >>+ return NULL; > >>+ > >>+ sprintf(device_dir_path, "/sys/class/video4linux/%s/device/", p + 1); > >>+ > >>+ device_dir = opendir(device_dir_path); > >>+ if (device_dir == NULL) > >>+ return NULL; > >>+ > >>+ while ((entry = readdir(device_dir))) { > >>+ if (strncmp(entry->d_name, "media", 4)) > > > >Why 4? And isn't entry->d_name nul-terminated, so you could use strcmp()? > > Media devices, as other devices, have numerical postfix, which is > not of our interest. Right. But still 5 would be the right number as we should also check the last "a". > > >>+ continue; > >>+ > >>+ sprintf(media_dev_path, "%s%s/dev", device_dir_path, entry->d_name); > >>+ > >>+ fd = open(media_dev_path, O_RDONLY); > >>+ if (fd < 0) > >>+ continue; > >>+ > >>+ ret = read(fd, media_major_minor, sizeof(media_major_minor)); > >>+ if (ret < 0) > >>+ continue; > >>+ > >>+ sscanf(media_major_minor, "%d:%d", &tmp_entity.info.dev.major, &tmp_entity.info.dev.minor); > > > >This would be better split on two lines. > > OK. > > >>+ > >>+ /* Try to get the device name via udev */ > >>+ if (media_get_devname_udev(udev, &tmp_entity)) { > >>+ /* Fall back to get the device name via sysfs */ > >>+ if (media_get_devname_sysfs(&tmp_entity)) > >>+ continue; > >>+ } > >>+ > >>+ media = media_device_new(tmp_entity.devname); > >>+ if (media == NULL) > >>+ continue; > >>+ > >>+ ret = media_device_enumerate(media); > >>+ if (ret < 0) { > >>+ media_dbg(media, "Failed to enumerate %s (%d)\n", > >>+ tmp_entity.devname, ret); > >>+ media_device_unref(media); > >>+ media = NULL; > >>+ continue; > >>+ } > >>+ > >>+ /* Get the entity associated with given fd */ > >>+ for (i = 0; i < media->entities_count; i++) { > >>+ struct media_entity *entity = &media->entities[i]; > >>+ > >>+ if (!strcmp(entity->devname, video_devname)) { > >>+ *fd_entity = &media->entities[i]; > >>+ break; > >>+ } > >>+ } > > > >What if you exit the loop without finding the entity you were looking for? > > Ah, right, this case is unhandled. > > Adding below condition should cover that: > > if (i == media->entities_count) > media = NULL; and media_device_unref()? You could have a label for handling that at the end of the loop basic block so you could implement handling of that just once to avoid such issues in the future. > > >>+ > >>+ break; > > This break should be removed and the one in the inner for loop above > should be replaced with goto here. Are you OK with that? Um, yeah. There are indeed two loops. In Perl you could get out nicely but in C we have to do something else. Two labels perhaps? > > >>+ } > >>+ > >>+ media_udev_close(udev); > >>+ > >>+ return media; > >>+} > >>+ > >> struct media_device *media_device_new_emulated(struct media_device_info *info) > >> { > >> struct media_device *media;
On 11/24/2016 03:32 PM, Sakari Ailus wrote: [...] >>>> + sprintf(device_dir_path, "/sys/class/video4linux/%s/device/", p + 1); >>>> + >>>> + device_dir = opendir(device_dir_path); >>>> + if (device_dir == NULL) >>>> + return NULL; >>>> + >>>> + while ((entry = readdir(device_dir))) { >>>> + if (strncmp(entry->d_name, "media", 4)) >>> >>> Why 4? And isn't entry->d_name nul-terminated, so you could use strcmp()? >> >> Media devices, as other devices, have numerical postfix, which is >> not of our interest. > > Right. But still 5 would be the right number as we should also check the > last "a". Of course, this needs to be fixed, thanks.
Hi Sakari, On 11/24/2016 01:17 PM, Sakari Ailus wrote: > Hi Jacek, > > Thanks for the patchset. > > On Wed, Oct 12, 2016 at 04:35:19PM +0200, Jacek Anaszewski wrote: >> Add helper functions that allow for easy instantiation of media_device >> object basing on whether the media device contains v4l2 subdev with >> given file descriptor. > > Doesn't this work with video nodes as well? That's what you seem to be using > it for later on. And I think that's actually more useful. > > The existing implementation uses udev to look up devices. Could you use > libudev device enumeration API to find the media devices, and fall back to > sysfs if udev doesn't work? There seems to be a reasonable-looking example > here: > > <URL:http://stackoverflow.com/questions/25361042/how-to-list-usb-mass-storage-devices-programatically-using-libudev-in-linux> Actually I am calling media_get_devname_udev() at first and falling back to sysfs similarly as it is accomplished in media_enum_entities(). Is there any specific reason for which I should use libudev device enumeration API in media_device_new_by_subdev_fd()? Best regards, Jacek Anaszewski >> >> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com> >> Acked-by: Kyungmin Park <kyungmin.park@samsung.com> >> --- >> utils/media-ctl/libmediactl.c | 131 +++++++++++++++++++++++++++++++++++++++++- >> utils/media-ctl/mediactl.h | 27 +++++++++ >> 2 files changed, 156 insertions(+), 2 deletions(-) >> >> diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c >> index 155b65f..d347a40 100644 >> --- a/utils/media-ctl/libmediactl.c >> +++ b/utils/media-ctl/libmediactl.c >> @@ -27,6 +27,7 @@ >> #include <sys/sysmacros.h> >> >> #include <ctype.h> >> +#include <dirent.h> >> #include <errno.h> >> #include <fcntl.h> >> #include <stdbool.h> >> @@ -440,8 +441,9 @@ static int media_get_devname_udev(struct udev *udev, >> return -EINVAL; >> >> devnum = makedev(entity->info.v4l.major, entity->info.v4l.minor); >> - media_dbg(entity->media, "looking up device: %u:%u\n", >> - major(devnum), minor(devnum)); >> + if (entity->media) >> + media_dbg(entity->media, "looking up device: %u:%u\n", >> + major(devnum), minor(devnum)); >> device = udev_device_new_from_devnum(udev, 'c', devnum); >> if (device) { >> p = udev_device_get_devnode(device); >> @@ -523,6 +525,7 @@ static int media_get_devname_sysfs(struct media_entity *entity) >> return 0; >> } >> >> + > > Unrelated change. > >> static int media_enum_entities(struct media_device *media) >> { >> struct media_entity *entity; >> @@ -707,6 +710,92 @@ struct media_device *media_device_new(const char *devnode) >> return media; >> } >> >> +struct media_device *media_device_new_by_subdev_fd(int fd, struct media_entity **fd_entity) >> +{ >> + char video_devname[32], device_dir_path[256], media_dev_path[256], media_major_minor[10]; >> + struct media_device *media = NULL; >> + struct dirent *entry; >> + struct media_entity tmp_entity; >> + DIR *device_dir; >> + struct udev *udev; >> + char *p; >> + int ret, i; >> + >> + if (fd_entity == NULL) >> + return NULL; >> + >> + ret = media_get_devname_by_fd(fd, video_devname); >> + if (ret < 0) >> + return NULL; >> + >> + p = strrchr(video_devname, '/'); >> + if (p == NULL) >> + return NULL; >> + >> + ret = media_udev_open(&udev); >> + if (ret < 0) >> + return NULL; >> + >> + sprintf(device_dir_path, "/sys/class/video4linux/%s/device/", p + 1); >> + >> + device_dir = opendir(device_dir_path); >> + if (device_dir == NULL) >> + return NULL; >> + >> + while ((entry = readdir(device_dir))) { >> + if (strncmp(entry->d_name, "media", 4)) > > Why 4? And isn't entry->d_name nul-terminated, so you could use strcmp()? > >> + continue; >> + >> + sprintf(media_dev_path, "%s%s/dev", device_dir_path, entry->d_name); >> + >> + fd = open(media_dev_path, O_RDONLY); >> + if (fd < 0) >> + continue; >> + >> + ret = read(fd, media_major_minor, sizeof(media_major_minor)); >> + if (ret < 0) >> + continue; >> + >> + sscanf(media_major_minor, "%d:%d", &tmp_entity.info.dev.major, &tmp_entity.info.dev.minor); > > This would be better split on two lines. > >> + >> + /* Try to get the device name via udev */ >> + if (media_get_devname_udev(udev, &tmp_entity)) { >> + /* Fall back to get the device name via sysfs */ >> + if (media_get_devname_sysfs(&tmp_entity)) >> + continue; >> + } >> + >> + media = media_device_new(tmp_entity.devname); >> + if (media == NULL) >> + continue; >> + >> + ret = media_device_enumerate(media); >> + if (ret < 0) { >> + media_dbg(media, "Failed to enumerate %s (%d)\n", >> + tmp_entity.devname, ret); >> + media_device_unref(media); >> + media = NULL; >> + continue; >> + } >> + >> + /* Get the entity associated with given fd */ >> + for (i = 0; i < media->entities_count; i++) { >> + struct media_entity *entity = &media->entities[i]; >> + >> + if (!strcmp(entity->devname, video_devname)) { >> + *fd_entity = &media->entities[i]; >> + break; >> + } >> + } > > What if you exit the loop without finding the entity you were looking for? > >> + >> + break; >> + } >> + >> + media_udev_close(udev); >> + >> + return media; >> +} >> + >> struct media_device *media_device_new_emulated(struct media_device_info *info) >> { >> struct media_device *media; >> @@ -748,6 +837,44 @@ void media_device_unref(struct media_device *media) >> free(media); >> } >> >> +int media_get_devname_by_fd(int fd, char *node_name) >> +{ >> + struct udev *udev; >> + struct media_entity tmp_entity; >> + struct stat stat; >> + int ret, ret_udev; >> + >> + if (node_name == NULL) >> + return -EINVAL; >> + >> + ret = fstat(fd, &stat); >> + if (ret < 0) >> + return -errno; >> + >> + tmp_entity.info.v4l.major = MAJOR(stat.st_rdev); >> + tmp_entity.info.v4l.minor = MINOR(stat.st_rdev); >> + >> + ret_udev = media_udev_open(&udev); >> + if (ret_udev < 0) >> + printf("Can't get udev context\n"); >> + >> + /* Try to get the device name via udev */ >> + ret = media_get_devname_udev(udev, &tmp_entity); >> + if (!ret) >> + goto out; >> + >> + ret = media_get_devname_sysfs(&tmp_entity); >> + if (ret < 0) >> + goto err_get_devname; >> + >> +out: >> + strncpy(node_name, tmp_entity.devname, sizeof(tmp_entity.devname)); > > This seems risky. How does the caller know the maximum size? I'd simply > allocate the string, and document the caller is responsible for releasing > it. > >> +err_get_devname: >> + if (!ret_udev) >> + media_udev_close(udev); >> + return ret; >> +} >> + >> int media_device_add_entity(struct media_device *media, >> const struct media_entity_desc *desc, >> const char *devnode) >> diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h >> index b1f33cd..580a25a 100644 >> --- a/utils/media-ctl/mediactl.h >> +++ b/utils/media-ctl/mediactl.h >> @@ -76,6 +76,21 @@ struct media_device *media_device_new(const char *devnode); >> struct media_device *media_device_new_emulated(struct media_device_info *info); >> >> /** >> + * @brief Create a new media device contatning entity associated with v4l2 subdev fd. >> + * @param fd - file descriptor of a v4l2 subdev. >> + * @param fd_entity - media entity associated with the v4l2 subdev. >> + * >> + * Create a representation of the media device referenced by the v4l2-subdev. >> + * The media device instance is initialized with enumerated entities and links. >> + * >> + * Media devices are reference-counted, see media_device_ref() and >> + * media_device_unref() for more information. >> + * >> + * @return A pointer to the new media device or NULL if error occurred. >> + */ >> +struct media_device *media_device_new_by_subdev_fd(int fd, struct media_entity **fd_entity); > > I'd drop the "subdev_" part as both V4L2 device nodes and sub-devices work. > > If you wish to keep this V4L2 specific, I suggest ...by_v4l2_fd(). > >> + >> +/** >> * @brief Take a reference to the device. >> * @param media - device instance. >> * >> @@ -231,6 +246,18 @@ const struct media_link *media_entity_get_link(struct media_entity *entity, >> const char *media_entity_get_devname(struct media_entity *entity); >> >> /** >> + * @brief Get the device node name by its file descriptor >> + * @param fd - file descriptor of a device. >> + * @param node_name - output device node name string. >> + * >> + * This function returns the full path and name to the device node corresponding >> + * to the given file descriptor. >> + * >> + * @return 0 on success, or a negative error code on failure. >> + */ >> +int media_get_devname_by_fd(int fd, char *node_name); >> + >> +/** >> * @brief Get the type of an entity. >> * @param entity - the entity. >> * > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Jacek, On Thu, Dec 08, 2016 at 11:04:20PM +0100, Jacek Anaszewski wrote: > Hi Sakari, > > On 11/24/2016 01:17 PM, Sakari Ailus wrote: > >Hi Jacek, > > > >Thanks for the patchset. > > > >On Wed, Oct 12, 2016 at 04:35:19PM +0200, Jacek Anaszewski wrote: > >>Add helper functions that allow for easy instantiation of media_device > >>object basing on whether the media device contains v4l2 subdev with > >>given file descriptor. > > > >Doesn't this work with video nodes as well? That's what you seem to be using > >it for later on. And I think that's actually more useful. > > > >The existing implementation uses udev to look up devices. Could you use > >libudev device enumeration API to find the media devices, and fall back to > >sysfs if udev doesn't work? There seems to be a reasonable-looking example > >here: > > > ><URL:http://stackoverflow.com/questions/25361042/how-to-list-usb-mass-storage-devices-programatically-using-libudev-in-linux> > > Actually I am calling media_get_devname_udev() at first and falling back > to sysfs similarly as it is accomplished in media_enum_entities(). > Is there any specific reason for which I should use libudev device > enumeration API in media_device_new_by_subdev_fd()? Yes. You rely on the API udev provides; the sysfs implementation is just a fallback in case udev isn't available in the system. I guess it'd mostly work but, for instance, you assume sysfs is found under /sys. The sysfs itself isn't one of the most stable APIs either. Udev is a simply better option when it's there.
Hhi Sakari, On 12/09/2016 12:05 AM, Sakari Ailus wrote: > Hi Jacek, > > On Thu, Dec 08, 2016 at 11:04:20PM +0100, Jacek Anaszewski wrote: >> Hi Sakari, >> >> On 11/24/2016 01:17 PM, Sakari Ailus wrote: >>> Hi Jacek, >>> >>> Thanks for the patchset. >>> >>> On Wed, Oct 12, 2016 at 04:35:19PM +0200, Jacek Anaszewski wrote: >>>> Add helper functions that allow for easy instantiation of media_device >>>> object basing on whether the media device contains v4l2 subdev with >>>> given file descriptor. >>> >>> Doesn't this work with video nodes as well? That's what you seem to be using >>> it for later on. And I think that's actually more useful. >>> >>> The existing implementation uses udev to look up devices. Could you use >>> libudev device enumeration API to find the media devices, and fall back to >>> sysfs if udev doesn't work? There seems to be a reasonable-looking example >>> here: >>> >>> <URL:http://stackoverflow.com/questions/25361042/how-to-list-usb-mass-storage-devices-programatically-using-libudev-in-linux> >> >> Actually I am calling media_get_devname_udev() at first and falling back >> to sysfs similarly as it is accomplished in media_enum_entities(). >> Is there any specific reason for which I should use libudev device >> enumeration API in media_device_new_by_subdev_fd()? > > Yes. You rely on the API udev provides; the sysfs implementation is just a > fallback in case udev isn't available in the system. I guess it'd mostly > work but, for instance, you assume sysfs is found under /sys. The sysfs > itself isn't one of the most stable APIs either. Udev is a simply better > option when it's there. Thanks for clarifying that. I'll check the libudev device enumeration API then.
diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c index 155b65f..d347a40 100644 --- a/utils/media-ctl/libmediactl.c +++ b/utils/media-ctl/libmediactl.c @@ -27,6 +27,7 @@ #include <sys/sysmacros.h> #include <ctype.h> +#include <dirent.h> #include <errno.h> #include <fcntl.h> #include <stdbool.h> @@ -440,8 +441,9 @@ static int media_get_devname_udev(struct udev *udev, return -EINVAL; devnum = makedev(entity->info.v4l.major, entity->info.v4l.minor); - media_dbg(entity->media, "looking up device: %u:%u\n", - major(devnum), minor(devnum)); + if (entity->media) + media_dbg(entity->media, "looking up device: %u:%u\n", + major(devnum), minor(devnum)); device = udev_device_new_from_devnum(udev, 'c', devnum); if (device) { p = udev_device_get_devnode(device); @@ -523,6 +525,7 @@ static int media_get_devname_sysfs(struct media_entity *entity) return 0; } + static int media_enum_entities(struct media_device *media) { struct media_entity *entity; @@ -707,6 +710,92 @@ struct media_device *media_device_new(const char *devnode) return media; } +struct media_device *media_device_new_by_subdev_fd(int fd, struct media_entity **fd_entity) +{ + char video_devname[32], device_dir_path[256], media_dev_path[256], media_major_minor[10]; + struct media_device *media = NULL; + struct dirent *entry; + struct media_entity tmp_entity; + DIR *device_dir; + struct udev *udev; + char *p; + int ret, i; + + if (fd_entity == NULL) + return NULL; + + ret = media_get_devname_by_fd(fd, video_devname); + if (ret < 0) + return NULL; + + p = strrchr(video_devname, '/'); + if (p == NULL) + return NULL; + + ret = media_udev_open(&udev); + if (ret < 0) + return NULL; + + sprintf(device_dir_path, "/sys/class/video4linux/%s/device/", p + 1); + + device_dir = opendir(device_dir_path); + if (device_dir == NULL) + return NULL; + + while ((entry = readdir(device_dir))) { + if (strncmp(entry->d_name, "media", 4)) + continue; + + sprintf(media_dev_path, "%s%s/dev", device_dir_path, entry->d_name); + + fd = open(media_dev_path, O_RDONLY); + if (fd < 0) + continue; + + ret = read(fd, media_major_minor, sizeof(media_major_minor)); + if (ret < 0) + continue; + + sscanf(media_major_minor, "%d:%d", &tmp_entity.info.dev.major, &tmp_entity.info.dev.minor); + + /* Try to get the device name via udev */ + if (media_get_devname_udev(udev, &tmp_entity)) { + /* Fall back to get the device name via sysfs */ + if (media_get_devname_sysfs(&tmp_entity)) + continue; + } + + media = media_device_new(tmp_entity.devname); + if (media == NULL) + continue; + + ret = media_device_enumerate(media); + if (ret < 0) { + media_dbg(media, "Failed to enumerate %s (%d)\n", + tmp_entity.devname, ret); + media_device_unref(media); + media = NULL; + continue; + } + + /* Get the entity associated with given fd */ + for (i = 0; i < media->entities_count; i++) { + struct media_entity *entity = &media->entities[i]; + + if (!strcmp(entity->devname, video_devname)) { + *fd_entity = &media->entities[i]; + break; + } + } + + break; + } + + media_udev_close(udev); + + return media; +} + struct media_device *media_device_new_emulated(struct media_device_info *info) { struct media_device *media; @@ -748,6 +837,44 @@ void media_device_unref(struct media_device *media) free(media); } +int media_get_devname_by_fd(int fd, char *node_name) +{ + struct udev *udev; + struct media_entity tmp_entity; + struct stat stat; + int ret, ret_udev; + + if (node_name == NULL) + return -EINVAL; + + ret = fstat(fd, &stat); + if (ret < 0) + return -errno; + + tmp_entity.info.v4l.major = MAJOR(stat.st_rdev); + tmp_entity.info.v4l.minor = MINOR(stat.st_rdev); + + ret_udev = media_udev_open(&udev); + if (ret_udev < 0) + printf("Can't get udev context\n"); + + /* Try to get the device name via udev */ + ret = media_get_devname_udev(udev, &tmp_entity); + if (!ret) + goto out; + + ret = media_get_devname_sysfs(&tmp_entity); + if (ret < 0) + goto err_get_devname; + +out: + strncpy(node_name, tmp_entity.devname, sizeof(tmp_entity.devname)); +err_get_devname: + if (!ret_udev) + media_udev_close(udev); + return ret; +} + int media_device_add_entity(struct media_device *media, const struct media_entity_desc *desc, const char *devnode) diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h index b1f33cd..580a25a 100644 --- a/utils/media-ctl/mediactl.h +++ b/utils/media-ctl/mediactl.h @@ -76,6 +76,21 @@ struct media_device *media_device_new(const char *devnode); struct media_device *media_device_new_emulated(struct media_device_info *info); /** + * @brief Create a new media device contatning entity associated with v4l2 subdev fd. + * @param fd - file descriptor of a v4l2 subdev. + * @param fd_entity - media entity associated with the v4l2 subdev. + * + * Create a representation of the media device referenced by the v4l2-subdev. + * The media device instance is initialized with enumerated entities and links. + * + * Media devices are reference-counted, see media_device_ref() and + * media_device_unref() for more information. + * + * @return A pointer to the new media device or NULL if error occurred. + */ +struct media_device *media_device_new_by_subdev_fd(int fd, struct media_entity **fd_entity); + +/** * @brief Take a reference to the device. * @param media - device instance. * @@ -231,6 +246,18 @@ const struct media_link *media_entity_get_link(struct media_entity *entity, const char *media_entity_get_devname(struct media_entity *entity); /** + * @brief Get the device node name by its file descriptor + * @param fd - file descriptor of a device. + * @param node_name - output device node name string. + * + * This function returns the full path and name to the device node corresponding + * to the given file descriptor. + * + * @return 0 on success, or a negative error code on failure. + */ +int media_get_devname_by_fd(int fd, char *node_name); + +/** * @brief Get the type of an entity. * @param entity - the entity. *