| Message ID | 1453133860-21571-3-git-send-email-j.anaszewski@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Jacek, Jacek Anaszewski wrote: > Make struct v4l2_subdev capable of aggregating v4l2-ctrl-redir > media device configuration entries. Added are also functions for > validating the config and checking whether a v4l2 sub-device > expects to receive ioctls related to the v4l2-control with given id. > > Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > utils/media-ctl/libv4l2subdev.c | 49 ++++++++++++++++++++++++++++++++++++++- > utils/media-ctl/v4l2subdev.h | 30 ++++++++++++++++++++++++ > 2 files changed, 78 insertions(+), 1 deletion(-) > > diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c > index 3977ce5..069ded6 100644 > --- a/utils/media-ctl/libv4l2subdev.c > +++ b/utils/media-ctl/libv4l2subdev.c > @@ -26,7 +26,6 @@ > #include <ctype.h> > #include <errno.h> > #include <fcntl.h> > -#include <stdbool.h> > #include <stdio.h> > #include <stdlib.h> > #include <string.h> > @@ -50,7 +49,15 @@ int v4l2_subdev_create(struct media_entity *entity) > > entity->sd->fd = -1; > > + entity->sd->v4l2_control_redir = malloc(sizeof(__u32)); > + if (entity->sd->v4l2_control_redir == NULL) > + goto err_v4l2_control_redir_alloc; > + > return 0; > + > +err_v4l2_control_redir_alloc: > + free(entity->sd); > + return -ENOMEM; > } > > int v4l2_subdev_create_with_fd(struct media_entity *entity, int fd) > @@ -870,3 +877,43 @@ enum v4l2_field v4l2_subdev_string_to_field(const char *string, > > return fields[i].field; > } > + > +int v4l2_subdev_validate_v4l2_ctrl(struct media_device *media, > + struct media_entity *entity, > + __u32 ctrl_id) > +{ > + struct v4l2_queryctrl queryctrl = {}; > + int ret; > + > + ret = v4l2_subdev_open(entity); > + if (ret < 0) > + return ret; > + > + queryctrl.id = ctrl_id; > + > + ret = ioctl(entity->sd->fd, VIDIOC_QUERYCTRL, &queryctrl); > + if (ret < 0) > + return ret; > + > + media_dbg(media, "Validated control \"%s\" (0x%8.8x) on entity %s\n", > + queryctrl.name, queryctrl.id, entity->info.name); > + > + return 0; > +} > + > +bool v4l2_subdev_has_v4l2_control_redir(struct media_device *media, > + struct media_entity *entity, > + int ctrl_id) > +{ > + struct v4l2_subdev *sd = entity->sd; > + int i; > + > + if (!sd) > + return false; > + > + for (i = 0; i < sd->v4l2_control_redir_num; ++i) > + if (sd->v4l2_control_redir[i] == ctrl_id) > + return true; > + > + return false; > +} > diff --git a/utils/media-ctl/v4l2subdev.h b/utils/media-ctl/v4l2subdev.h > index ba9b8c4..f395065 100644 > --- a/utils/media-ctl/v4l2subdev.h > +++ b/utils/media-ctl/v4l2subdev.h > @@ -23,12 +23,17 @@ > #define __SUBDEV_H__ > > #include <linux/v4l2-subdev.h> > +#include <stdbool.h> > > struct media_device; > struct media_entity; > +struct media_device; > > struct v4l2_subdev { > int fd; > + > + __u32 *v4l2_control_redir; > + unsigned int v4l2_control_redir_num; > }; > > /** > @@ -316,5 +321,30 @@ const char *v4l2_subdev_field_to_string(enum v4l2_field field); > */ > enum v4l2_field v4l2_subdev_string_to_field(const char *string, > unsigned int length); > +/** > + * @brief Validate v4l2 control for a sub-device > + * @param media - media device. > + * @param entity - subdev-device media entity. > + * @param ctrl_id - id of the v4l2 control to validate. > + * > + * Verify if the entity supports v4l2-control with given ctrl_id. > + * > + * @return 1 if the control is supported, 0 otherwise. > + */ > +int v4l2_subdev_validate_v4l2_ctrl(struct media_device *media, > + struct media_entity *entity, > + __u32 ctrl_id); > +/** > + * @brief Check if there was a v4l2_control redirection defined for the entity > + * @param media - media device. > + * @param entity - subdev-device media entity. > + * @param ctrl_id - v4l2 control identifier. > + * > + * Check if there was a v4l2-ctrl-redir entry defined for the entity. > + * > + * @return true if the entry exists, false otherwise > + */ > +bool v4l2_subdev_has_v4l2_control_redir(struct media_device *media, > + struct media_entity *entity, int ctrl_id); > > #endif > Where do you need this? If you have an application that's aware of V4L2 sub-devices (and thus multiple devices)), I'd expect it to set the controls on the sub-devices the controls are implemented in rather than rely on them being redirected. This would make perfect sense IMO when implementing plain V4L2 interface support on top of drivers that expose MC/V4L2 subdev/V4L2 APIs. But I wouldn't implement it in libv4l2subdev.
Hi Sakari, Thanks for the review. On 02/15/2016 11:58 AM, Sakari Ailus wrote: > Hi Jacek, > > Jacek Anaszewski wrote: >> Make struct v4l2_subdev capable of aggregating v4l2-ctrl-redir >> media device configuration entries. Added are also functions for >> validating the config and checking whether a v4l2 sub-device >> expects to receive ioctls related to the v4l2-control with given id. >> >> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com> >> Acked-by: Kyungmin Park <kyungmin.park@samsung.com> >> --- >> utils/media-ctl/libv4l2subdev.c | 49 ++++++++++++++++++++++++++++++++++++++- >> utils/media-ctl/v4l2subdev.h | 30 ++++++++++++++++++++++++ >> 2 files changed, 78 insertions(+), 1 deletion(-) >> >> diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c >> index 3977ce5..069ded6 100644 >> --- a/utils/media-ctl/libv4l2subdev.c >> +++ b/utils/media-ctl/libv4l2subdev.c >> @@ -26,7 +26,6 @@ >> #include <ctype.h> >> #include <errno.h> >> #include <fcntl.h> >> -#include <stdbool.h> >> #include <stdio.h> >> #include <stdlib.h> >> #include <string.h> >> @@ -50,7 +49,15 @@ int v4l2_subdev_create(struct media_entity *entity) >> >> entity->sd->fd = -1; >> >> + entity->sd->v4l2_control_redir = malloc(sizeof(__u32)); >> + if (entity->sd->v4l2_control_redir == NULL) >> + goto err_v4l2_control_redir_alloc; >> + >> return 0; >> + >> +err_v4l2_control_redir_alloc: >> + free(entity->sd); >> + return -ENOMEM; >> } >> >> int v4l2_subdev_create_with_fd(struct media_entity *entity, int fd) >> @@ -870,3 +877,43 @@ enum v4l2_field v4l2_subdev_string_to_field(const char *string, >> >> return fields[i].field; >> } >> + >> +int v4l2_subdev_validate_v4l2_ctrl(struct media_device *media, >> + struct media_entity *entity, >> + __u32 ctrl_id) >> +{ >> + struct v4l2_queryctrl queryctrl = {}; >> + int ret; >> + >> + ret = v4l2_subdev_open(entity); >> + if (ret < 0) >> + return ret; >> + >> + queryctrl.id = ctrl_id; >> + >> + ret = ioctl(entity->sd->fd, VIDIOC_QUERYCTRL, &queryctrl); >> + if (ret < 0) >> + return ret; >> + >> + media_dbg(media, "Validated control \"%s\" (0x%8.8x) on entity %s\n", >> + queryctrl.name, queryctrl.id, entity->info.name); >> + >> + return 0; >> +} >> + >> +bool v4l2_subdev_has_v4l2_control_redir(struct media_device *media, >> + struct media_entity *entity, >> + int ctrl_id) >> +{ >> + struct v4l2_subdev *sd = entity->sd; >> + int i; >> + >> + if (!sd) >> + return false; >> + >> + for (i = 0; i < sd->v4l2_control_redir_num; ++i) >> + if (sd->v4l2_control_redir[i] == ctrl_id) >> + return true; >> + >> + return false; >> +} >> diff --git a/utils/media-ctl/v4l2subdev.h b/utils/media-ctl/v4l2subdev.h >> index ba9b8c4..f395065 100644 >> --- a/utils/media-ctl/v4l2subdev.h >> +++ b/utils/media-ctl/v4l2subdev.h >> @@ -23,12 +23,17 @@ >> #define __SUBDEV_H__ >> >> #include <linux/v4l2-subdev.h> >> +#include <stdbool.h> >> >> struct media_device; >> struct media_entity; >> +struct media_device; >> >> struct v4l2_subdev { >> int fd; >> + >> + __u32 *v4l2_control_redir; >> + unsigned int v4l2_control_redir_num; >> }; >> >> /** >> @@ -316,5 +321,30 @@ const char *v4l2_subdev_field_to_string(enum v4l2_field field); >> */ >> enum v4l2_field v4l2_subdev_string_to_field(const char *string, >> unsigned int length); >> +/** >> + * @brief Validate v4l2 control for a sub-device >> + * @param media - media device. >> + * @param entity - subdev-device media entity. >> + * @param ctrl_id - id of the v4l2 control to validate. >> + * >> + * Verify if the entity supports v4l2-control with given ctrl_id. >> + * >> + * @return 1 if the control is supported, 0 otherwise. >> + */ >> +int v4l2_subdev_validate_v4l2_ctrl(struct media_device *media, >> + struct media_entity *entity, >> + __u32 ctrl_id); >> +/** >> + * @brief Check if there was a v4l2_control redirection defined for the entity >> + * @param media - media device. >> + * @param entity - subdev-device media entity. >> + * @param ctrl_id - v4l2 control identifier. >> + * >> + * Check if there was a v4l2-ctrl-redir entry defined for the entity. >> + * >> + * @return true if the entry exists, false otherwise >> + */ >> +bool v4l2_subdev_has_v4l2_control_redir(struct media_device *media, >> + struct media_entity *entity, int ctrl_id); >> >> #endif >> > > Where do you need this? It is used in v4l2_subdev_get_pipeline_entity_by_cid, which returns the first entity in the pipeline the control setting is to be redirected to. The v4l2_subdev_get_pipeline_entity_by_cid() is in turn required in libv4l2media_ioctl.c, in the functions that apply control settings to the pipeline. The actual sub-device to apply the settings on is being selected basing on the v4l2-ctrl-redir config entry. This is required in case more than one sub-device in the pipeline supports a control and we want to choose specific hardware implementation thereof. For example both S5C73M3 and fimc.0.capture sub-devices support "Color Effects", but the effects differ visually - e.g. Sepia realized by S5C73M3 is more "orange" than the one from fimc.0.capture. And we can set controls with GStreamer pipeline : gst-launch-1.0 v4l2src device=/dev/video1 extra-controls="c,color_effects=2" ! video/x-raw,width=960,height=920 ! fbdevsink > > If you have an application that's aware of V4L2 sub-devices (and thus > multiple devices)), I'd expect it to set the controls on the sub-devices > the controls are implemented in rather than rely on them being redirected. > > This would make perfect sense IMO when implementing plain V4L2 interface > support on top of drivers that expose MC/V4L2 subdev/V4L2 APIs. But I > wouldn't implement it in libv4l2subdev. >
Hi Jacek, Jacek Anaszewski wrote: > Hi Sakari, > > Thanks for the review. > > On 02/15/2016 11:58 AM, Sakari Ailus wrote: >> Hi Jacek, >> >> Jacek Anaszewski wrote: >>> Make struct v4l2_subdev capable of aggregating v4l2-ctrl-redir >>> media device configuration entries. Added are also functions for >>> validating the config and checking whether a v4l2 sub-device >>> expects to receive ioctls related to the v4l2-control with given id. >>> >>> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com> >>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com> >>> --- >>> utils/media-ctl/libv4l2subdev.c | 49 >>> ++++++++++++++++++++++++++++++++++++++- >>> utils/media-ctl/v4l2subdev.h | 30 ++++++++++++++++++++++++ >>> 2 files changed, 78 insertions(+), 1 deletion(-) >>> >>> diff --git a/utils/media-ctl/libv4l2subdev.c >>> b/utils/media-ctl/libv4l2subdev.c >>> index 3977ce5..069ded6 100644 >>> --- a/utils/media-ctl/libv4l2subdev.c >>> +++ b/utils/media-ctl/libv4l2subdev.c >>> @@ -26,7 +26,6 @@ >>> #include <ctype.h> >>> #include <errno.h> >>> #include <fcntl.h> >>> -#include <stdbool.h> >>> #include <stdio.h> >>> #include <stdlib.h> >>> #include <string.h> >>> @@ -50,7 +49,15 @@ int v4l2_subdev_create(struct media_entity *entity) >>> >>> entity->sd->fd = -1; >>> >>> + entity->sd->v4l2_control_redir = malloc(sizeof(__u32)); >>> + if (entity->sd->v4l2_control_redir == NULL) >>> + goto err_v4l2_control_redir_alloc; >>> + >>> return 0; >>> + >>> +err_v4l2_control_redir_alloc: >>> + free(entity->sd); >>> + return -ENOMEM; >>> } >>> >>> int v4l2_subdev_create_with_fd(struct media_entity *entity, int fd) >>> @@ -870,3 +877,43 @@ enum v4l2_field >>> v4l2_subdev_string_to_field(const char *string, >>> >>> return fields[i].field; >>> } >>> + >>> +int v4l2_subdev_validate_v4l2_ctrl(struct media_device *media, >>> + struct media_entity *entity, >>> + __u32 ctrl_id) >>> +{ >>> + struct v4l2_queryctrl queryctrl = {}; >>> + int ret; >>> + >>> + ret = v4l2_subdev_open(entity); >>> + if (ret < 0) >>> + return ret; >>> + >>> + queryctrl.id = ctrl_id; >>> + >>> + ret = ioctl(entity->sd->fd, VIDIOC_QUERYCTRL, &queryctrl); >>> + if (ret < 0) >>> + return ret; >>> + >>> + media_dbg(media, "Validated control \"%s\" (0x%8.8x) on entity >>> %s\n", >>> + queryctrl.name, queryctrl.id, entity->info.name); >>> + >>> + return 0; >>> +} >>> + >>> +bool v4l2_subdev_has_v4l2_control_redir(struct media_device *media, >>> + struct media_entity *entity, >>> + int ctrl_id) >>> +{ >>> + struct v4l2_subdev *sd = entity->sd; >>> + int i; >>> + >>> + if (!sd) >>> + return false; >>> + >>> + for (i = 0; i < sd->v4l2_control_redir_num; ++i) >>> + if (sd->v4l2_control_redir[i] == ctrl_id) >>> + return true; >>> + >>> + return false; >>> +} >>> diff --git a/utils/media-ctl/v4l2subdev.h b/utils/media-ctl/v4l2subdev.h >>> index ba9b8c4..f395065 100644 >>> --- a/utils/media-ctl/v4l2subdev.h >>> +++ b/utils/media-ctl/v4l2subdev.h >>> @@ -23,12 +23,17 @@ >>> #define __SUBDEV_H__ >>> >>> #include <linux/v4l2-subdev.h> >>> +#include <stdbool.h> >>> >>> struct media_device; >>> struct media_entity; >>> +struct media_device; >>> >>> struct v4l2_subdev { >>> int fd; >>> + >>> + __u32 *v4l2_control_redir; >>> + unsigned int v4l2_control_redir_num; >>> }; >>> >>> /** >>> @@ -316,5 +321,30 @@ const char *v4l2_subdev_field_to_string(enum >>> v4l2_field field); >>> */ >>> enum v4l2_field v4l2_subdev_string_to_field(const char *string, >>> unsigned int length); >>> +/** >>> + * @brief Validate v4l2 control for a sub-device >>> + * @param media - media device. >>> + * @param entity - subdev-device media entity. >>> + * @param ctrl_id - id of the v4l2 control to validate. >>> + * >>> + * Verify if the entity supports v4l2-control with given ctrl_id. >>> + * >>> + * @return 1 if the control is supported, 0 otherwise. >>> + */ >>> +int v4l2_subdev_validate_v4l2_ctrl(struct media_device *media, >>> + struct media_entity *entity, >>> + __u32 ctrl_id); >>> +/** >>> + * @brief Check if there was a v4l2_control redirection defined for >>> the entity >>> + * @param media - media device. >>> + * @param entity - subdev-device media entity. >>> + * @param ctrl_id - v4l2 control identifier. >>> + * >>> + * Check if there was a v4l2-ctrl-redir entry defined for the entity. >>> + * >>> + * @return true if the entry exists, false otherwise >>> + */ >>> +bool v4l2_subdev_has_v4l2_control_redir(struct media_device *media, >>> + struct media_entity *entity, int ctrl_id); >>> >>> #endif >>> >> >> Where do you need this? > > It is used in v4l2_subdev_get_pipeline_entity_by_cid, which returns the > first entity in the pipeline the control setting is to be redirected to. > The v4l2_subdev_get_pipeline_entity_by_cid() is in turn required in > libv4l2media_ioctl.c, in the functions that apply control settings to > the pipeline. The actual sub-device to apply the settings on is being > selected basing on the v4l2-ctrl-redir config entry. > > This is required in case more than one sub-device in the pipeline > supports a control and we want to choose specific hardware > implementation thereof. For example both S5C73M3 and fimc.0.capture > sub-devices support "Color Effects", but the effects differ visually - > e.g. Sepia realized by S5C73M3 is more "orange" than the one from > fimc.0.capture. Right. libv4l2subdev still deals with sub-devices, and I don't think this functionality belongs there. Instead, I'd put it in libv4l2media_ioctl. > > And we can set controls with GStreamer pipeline : > > gst-launch-1.0 v4l2src device=/dev/video1 > extra-controls="c,color_effects=2" ! video/x-raw,width=960,height=920 ! > fbdevsink > >> >> If you have an application that's aware of V4L2 sub-devices (and thus >> multiple devices)), I'd expect it to set the controls on the sub-devices >> the controls are implemented in rather than rely on them being >> redirected. >> >> This would make perfect sense IMO when implementing plain V4L2 interface >> support on top of drivers that expose MC/V4L2 subdev/V4L2 APIs. But I >> wouldn't implement it in libv4l2subdev. >> > >
diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c index 3977ce5..069ded6 100644 --- a/utils/media-ctl/libv4l2subdev.c +++ b/utils/media-ctl/libv4l2subdev.c @@ -26,7 +26,6 @@ #include <ctype.h> #include <errno.h> #include <fcntl.h> -#include <stdbool.h> #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -50,7 +49,15 @@ int v4l2_subdev_create(struct media_entity *entity) entity->sd->fd = -1; + entity->sd->v4l2_control_redir = malloc(sizeof(__u32)); + if (entity->sd->v4l2_control_redir == NULL) + goto err_v4l2_control_redir_alloc; + return 0; + +err_v4l2_control_redir_alloc: + free(entity->sd); + return -ENOMEM; } int v4l2_subdev_create_with_fd(struct media_entity *entity, int fd) @@ -870,3 +877,43 @@ enum v4l2_field v4l2_subdev_string_to_field(const char *string, return fields[i].field; } + +int v4l2_subdev_validate_v4l2_ctrl(struct media_device *media, + struct media_entity *entity, + __u32 ctrl_id) +{ + struct v4l2_queryctrl queryctrl = {}; + int ret; + + ret = v4l2_subdev_open(entity); + if (ret < 0) + return ret; + + queryctrl.id = ctrl_id; + + ret = ioctl(entity->sd->fd, VIDIOC_QUERYCTRL, &queryctrl); + if (ret < 0) + return ret; + + media_dbg(media, "Validated control \"%s\" (0x%8.8x) on entity %s\n", + queryctrl.name, queryctrl.id, entity->info.name); + + return 0; +} + +bool v4l2_subdev_has_v4l2_control_redir(struct media_device *media, + struct media_entity *entity, + int ctrl_id) +{ + struct v4l2_subdev *sd = entity->sd; + int i; + + if (!sd) + return false; + + for (i = 0; i < sd->v4l2_control_redir_num; ++i) + if (sd->v4l2_control_redir[i] == ctrl_id) + return true; + + return false; +} diff --git a/utils/media-ctl/v4l2subdev.h b/utils/media-ctl/v4l2subdev.h index ba9b8c4..f395065 100644 --- a/utils/media-ctl/v4l2subdev.h +++ b/utils/media-ctl/v4l2subdev.h @@ -23,12 +23,17 @@ #define __SUBDEV_H__ #include <linux/v4l2-subdev.h> +#include <stdbool.h> struct media_device; struct media_entity; +struct media_device; struct v4l2_subdev { int fd; + + __u32 *v4l2_control_redir; + unsigned int v4l2_control_redir_num; }; /** @@ -316,5 +321,30 @@ const char *v4l2_subdev_field_to_string(enum v4l2_field field); */ enum v4l2_field v4l2_subdev_string_to_field(const char *string, unsigned int length); +/** + * @brief Validate v4l2 control for a sub-device + * @param media - media device. + * @param entity - subdev-device media entity. + * @param ctrl_id - id of the v4l2 control to validate. + * + * Verify if the entity supports v4l2-control with given ctrl_id. + * + * @return 1 if the control is supported, 0 otherwise. + */ +int v4l2_subdev_validate_v4l2_ctrl(struct media_device *media, + struct media_entity *entity, + __u32 ctrl_id); +/** + * @brief Check if there was a v4l2_control redirection defined for the entity + * @param media - media device. + * @param entity - subdev-device media entity. + * @param ctrl_id - v4l2 control identifier. + * + * Check if there was a v4l2-ctrl-redir entry defined for the entity. + * + * @return true if the entry exists, false otherwise + */ +bool v4l2_subdev_has_v4l2_control_redir(struct media_device *media, + struct media_entity *entity, int ctrl_id); #endif