Re: [PATCH v4l-utils v7 3/7] mediactl: Add media_entity_get_backlinks()
Hi Jacek, On Thu, Nov 24, 2016 at 03:00:46PM +0100, Jacek Anaszewski wrote: > Hi Sakari, > > Thanks for the review. It's taken way too long. :-( My apologies for that. > On 11/24/2016 01:40 PM, Sakari Ailus wrote: > >Hi Jacek, > > > >On Wed, Oct 12, 2016 at 04:35:18PM +0200, Jacek Anaszewski wrote: > >>Add a new graph helper useful for discovering video pipeline. > >> > >>Signed-off-by: Jacek Anaszewski> >>Acked-by: Kyungmin Park > >>--- > >> utils/media-ctl/libmediactl.c | 21 + > >> utils/media-ctl/mediactl.h| 15 +++ > >> 2 files changed, 36 insertions(+) > >> > >>diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c > >>index 91ed003..155b65f 100644 > >>--- a/utils/media-ctl/libmediactl.c > >>+++ b/utils/media-ctl/libmediactl.c > >>@@ -36,6 +36,7 @@ > >> #include > >> > >> #include > >>+#include > > > >Is there something that needs this one in the patch? > > MAJOR and MINOR macros. Ok. > > > > >> #include > >> > >> #include "mediactl.h" > >>@@ -172,6 +173,26 @@ const struct media_entity_desc > >>*media_entity_get_info(struct media_entity *entit > >>return >info; > >> } > >> > >>+int media_entity_get_backlinks(struct media_entity *entity, > >>+ struct media_link **backlinks, > >>+ unsigned int *num_backlinks) > >>+{ > >>+ unsigned int num_bklinks = 0; > >>+ int i; > >>+ > >>+ if (entity == NULL || backlinks == NULL || num_backlinks == NULL) > >>+ return -EINVAL; > >>+ > > > >If you have an interface that accesses a memory buffer of unknown size, you > >need to verify that the user has provided a buffer large enough. > > > >How about using the num_backlinks argument to provide the maximum size to > >the function, and passing the actual number to the user, the latter of which > >you already do? > > Sounds reasonable. > > >Alternatively, an iterator style API could be nice as well. Up to you. > > It would probably need an addition of some generic infrastructure. > I suppose that there is no such a feature in v4l-utils? You basically need to store a value for the framework to tell which entity is being worked on. So nothing too fancy. I guess Laurent would prefer the current interface but I let him answer that. -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4l-utils v7 3/7] mediactl: Add media_entity_get_backlinks()
Hi Sakari, Thanks for the review. On 11/24/2016 01:40 PM, Sakari Ailus wrote: Hi Jacek, On Wed, Oct 12, 2016 at 04:35:18PM +0200, Jacek Anaszewski wrote: Add a new graph helper useful for discovering video pipeline. Signed-off-by: Jacek AnaszewskiAcked-by: Kyungmin Park --- utils/media-ctl/libmediactl.c | 21 + utils/media-ctl/mediactl.h| 15 +++ 2 files changed, 36 insertions(+) diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c index 91ed003..155b65f 100644 --- a/utils/media-ctl/libmediactl.c +++ b/utils/media-ctl/libmediactl.c @@ -36,6 +36,7 @@ #include #include +#include Is there something that needs this one in the patch? MAJOR and MINOR macros. #include #include "mediactl.h" @@ -172,6 +173,26 @@ const struct media_entity_desc *media_entity_get_info(struct media_entity *entit return >info; } +int media_entity_get_backlinks(struct media_entity *entity, + struct media_link **backlinks, + unsigned int *num_backlinks) +{ + unsigned int num_bklinks = 0; + int i; + + if (entity == NULL || backlinks == NULL || num_backlinks == NULL) + return -EINVAL; + If you have an interface that accesses a memory buffer of unknown size, you need to verify that the user has provided a buffer large enough. How about using the num_backlinks argument to provide the maximum size to the function, and passing the actual number to the user, the latter of which you already do? Sounds reasonable. Alternatively, an iterator style API could be nice as well. Up to you. It would probably need an addition of some generic infrastructure. I suppose that there is no such a feature in v4l-utils? I wonder what Laurent thinks. + for (i = 0; i < entity->num_links; ++i) + if ((entity->links[i].flags & MEDIA_LNK_FL_ENABLED) && + (entity->links[i].sink->entity == entity)) + backlinks[num_bklinks++] = >links[i]; + + *num_backlinks = num_bklinks; + + return 0; +} + /* - * Open/close */ diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h index 336cbf9..b1f33cd 100644 --- a/utils/media-ctl/mediactl.h +++ b/utils/media-ctl/mediactl.h @@ -434,6 +434,20 @@ int media_parse_setup_link(struct media_device *media, int media_parse_setup_links(struct media_device *media, const char *p); /** + * @brief Get entity's enabled backlinks + * @param entity - media entity. + * @param backlinks - array of pointers to matching backlinks. + * @param num_backlinks - number of matching backlinks. + * + * Get links that are connected to the entity sink pads. + * + * @return 0 on success, or a negative error code on failure. + */ +int media_entity_get_backlinks(struct media_entity *entity, + struct media_link **backlinks, + unsigned int *num_backlinks); + +/** * @brief Get v4l2_subdev for the entity * @param entity - media entity * @@ -443,4 +457,5 @@ int media_parse_setup_links(struct media_device *media, const char *p); */ struct v4l2_subdev *media_entity_get_v4l2_subdev(struct media_entity *entity); + Unrelated change. #endif -- Best regards, Jacek Anaszewski -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4l-utils v7 3/7] mediactl: Add media_entity_get_backlinks()
Hi Jacek, On Wed, Oct 12, 2016 at 04:35:18PM +0200, Jacek Anaszewski wrote: > Add a new graph helper useful for discovering video pipeline. > > Signed-off-by: Jacek Anaszewski> Acked-by: Kyungmin Park > --- > utils/media-ctl/libmediactl.c | 21 + > utils/media-ctl/mediactl.h| 15 +++ > 2 files changed, 36 insertions(+) > > diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c > index 91ed003..155b65f 100644 > --- a/utils/media-ctl/libmediactl.c > +++ b/utils/media-ctl/libmediactl.c > @@ -36,6 +36,7 @@ > #include > > #include > +#include Is there something that needs this one in the patch? > #include > > #include "mediactl.h" > @@ -172,6 +173,26 @@ const struct media_entity_desc > *media_entity_get_info(struct media_entity *entit > return >info; > } > > +int media_entity_get_backlinks(struct media_entity *entity, > + struct media_link **backlinks, > + unsigned int *num_backlinks) > +{ > + unsigned int num_bklinks = 0; > + int i; > + > + if (entity == NULL || backlinks == NULL || num_backlinks == NULL) > + return -EINVAL; > + If you have an interface that accesses a memory buffer of unknown size, you need to verify that the user has provided a buffer large enough. How about using the num_backlinks argument to provide the maximum size to the function, and passing the actual number to the user, the latter of which you already do? Alternatively, an iterator style API could be nice as well. Up to you. I wonder what Laurent thinks. > + for (i = 0; i < entity->num_links; ++i) > + if ((entity->links[i].flags & MEDIA_LNK_FL_ENABLED) && > + (entity->links[i].sink->entity == entity)) > + backlinks[num_bklinks++] = >links[i]; > + > + *num_backlinks = num_bklinks; > + > + return 0; > +} > + > /* > - > * Open/close > */ > diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h > index 336cbf9..b1f33cd 100644 > --- a/utils/media-ctl/mediactl.h > +++ b/utils/media-ctl/mediactl.h > @@ -434,6 +434,20 @@ int media_parse_setup_link(struct media_device *media, > int media_parse_setup_links(struct media_device *media, const char *p); > > /** > + * @brief Get entity's enabled backlinks > + * @param entity - media entity. > + * @param backlinks - array of pointers to matching backlinks. > + * @param num_backlinks - number of matching backlinks. > + * > + * Get links that are connected to the entity sink pads. > + * > + * @return 0 on success, or a negative error code on failure. > + */ > +int media_entity_get_backlinks(struct media_entity *entity, > + struct media_link **backlinks, > + unsigned int *num_backlinks); > + > +/** > * @brief Get v4l2_subdev for the entity > * @param entity - media entity > * > @@ -443,4 +457,5 @@ int media_parse_setup_links(struct media_device *media, > const char *p); > */ > struct v4l2_subdev *media_entity_get_v4l2_subdev(struct media_entity > *entity); > > + Unrelated change. > #endif -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4l-utils v7 3/7] mediactl: Add media_entity_get_backlinks()
Add a new graph helper useful for discovering video pipeline. Signed-off-by: Jacek AnaszewskiAcked-by: Kyungmin Park --- utils/media-ctl/libmediactl.c | 21 + utils/media-ctl/mediactl.h| 15 +++ 2 files changed, 36 insertions(+) diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c index 91ed003..155b65f 100644 --- a/utils/media-ctl/libmediactl.c +++ b/utils/media-ctl/libmediactl.c @@ -36,6 +36,7 @@ #include #include +#include #include #include "mediactl.h" @@ -172,6 +173,26 @@ const struct media_entity_desc *media_entity_get_info(struct media_entity *entit return >info; } +int media_entity_get_backlinks(struct media_entity *entity, + struct media_link **backlinks, + unsigned int *num_backlinks) +{ + unsigned int num_bklinks = 0; + int i; + + if (entity == NULL || backlinks == NULL || num_backlinks == NULL) + return -EINVAL; + + for (i = 0; i < entity->num_links; ++i) + if ((entity->links[i].flags & MEDIA_LNK_FL_ENABLED) && + (entity->links[i].sink->entity == entity)) + backlinks[num_bklinks++] = >links[i]; + + *num_backlinks = num_bklinks; + + return 0; +} + /* - * Open/close */ diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h index 336cbf9..b1f33cd 100644 --- a/utils/media-ctl/mediactl.h +++ b/utils/media-ctl/mediactl.h @@ -434,6 +434,20 @@ int media_parse_setup_link(struct media_device *media, int media_parse_setup_links(struct media_device *media, const char *p); /** + * @brief Get entity's enabled backlinks + * @param entity - media entity. + * @param backlinks - array of pointers to matching backlinks. + * @param num_backlinks - number of matching backlinks. + * + * Get links that are connected to the entity sink pads. + * + * @return 0 on success, or a negative error code on failure. + */ +int media_entity_get_backlinks(struct media_entity *entity, + struct media_link **backlinks, + unsigned int *num_backlinks); + +/** * @brief Get v4l2_subdev for the entity * @param entity - media entity * @@ -443,4 +457,5 @@ int media_parse_setup_links(struct media_device *media, const char *p); */ struct v4l2_subdev *media_entity_get_v4l2_subdev(struct media_entity *entity); + #endif -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html