| Message ID | 20180316205512.GA6069@amd (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Pavel, I really don't want to add functions for this to libv4l2. That's just a quick hack. The real solution is to parse this from a config file. But that is a lot more work and it is something that needs to be designed properly. And that requires someone to put in the time and effort... Regards, Hans On 03/16/2018 09:55 PM, Pavel Machek wrote: > Hi! > > What about something like this? > > Pass map of controls and expects fds to libv4l2... > > +static struct v4l2_controls_map map = { > + .num_fds = 2, > + .num_controls = 3, > + .map = { [0] = { .control = 0x00980913, .fd = 1 }, > + [1] = { .control = V4L2_CID_EXPOSURE_ABSOLUTE, .fd = 1 }, > + [2] = { .control = V4L2_CID_FOCUS_ABSOLUTE, .fd = 2 }, > + > + }, > +}; > + > +static void open_chk(char *name, int fd) > +{ > + int f = open(name, O_RDWR); > + > + if (fd != f) { > + printf("Unexpected error %m opening %s\n", name); > + exit(1); > + } > +} > + > +static void cam_open_n900(struct dev_info *dev) > +{ > + struct v4l2_format *fmt = &dev->fmt; > + int fd; > + > + map.main_fd = open("/dev/video_ccdc", O_RDWR); > + > + open_chk("/dev/video_sensor", map.main_fd+1); > + open_chk("/dev/video_focus", map.main_fd+2); > + // open_chk("/dev/video_flash", map.main_fd+3); > + > + v4l2_open_pipeline(&map, 0); > + dev->fd = map.main_fd; > + > > ...so you can use it on complex devices. Tested on my N900. > > I guess later helper would be added that would parse some kind of > descritption file and do open_pipeline(). But.. lets solve that > next. In the first place, it would be nice to have libv4l2 usable on > complex devices. > > Best regards, > Pavel > Signed-off-by: Pavel Machek <pavel@ucw.cz> > > diff --git a/lib/include/libv4l2.h b/lib/include/libv4l2.h > index ea1870d..6220dfd 100644 > --- a/lib/include/libv4l2.h > +++ b/lib/include/libv4l2.h > @@ -109,6 +109,23 @@ LIBV4L_PUBLIC int v4l2_get_control(int fd, int cid); > (note the fd is left open in this case). */ > LIBV4L_PUBLIC int v4l2_fd_open(int fd, int v4l2_flags); > > +struct v4l2_control_map { > + unsigned long control; > + int fd; > +}; > + > +struct v4l2_controls_map { > + int main_fd; > + int num_fds; > + int num_controls; > + struct v4l2_control_map map[]; > +}; > + > +LIBV4L_PUBLIC int v4l2_open_pipeline(struct v4l2_controls_map *map, int v4l2_flags); > + > +LIBV4L_PUBLIC int v4l2_get_fd_for_control(int fd, unsigned long control); > + > + > #ifdef __cplusplus > } > #endif /* __cplusplus */ > diff --git a/lib/libv4l2/libv4l2-priv.h b/lib/libv4l2/libv4l2-priv.h > index 1924c91..ebe5dad 100644 > --- a/lib/libv4l2/libv4l2-priv.h > +++ b/lib/libv4l2/libv4l2-priv.h > @@ -104,6 +104,7 @@ struct v4l2_dev_info { > void *plugin_library; > void *dev_ops_priv; > const struct libv4l_dev_ops *dev_ops; > + struct v4l2_controls_map *map; > }; > > /* From v4l2-plugin.c */ > diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c > index 2db25d1..b3ae70b 100644 > --- a/lib/libv4l2/libv4l2.c > +++ b/lib/libv4l2/libv4l2.c > @@ -787,6 +787,8 @@ no_capture: > if (index >= devices_used) > devices_used = index + 1; > > + devices[index].map = NULL; > + > /* Note we always tell v4lconvert to optimize src fmt selection for > our default fps, the only exception is the app explicitly selecting > a frame rate using the S_PARM ioctl after a S_FMT */ > @@ -1056,12 +1058,39 @@ static int v4l2_s_fmt(int index, struct v4l2_format *dest_fmt) > return 0; > } > > +int v4l2_get_fd_for_control(int fd, unsigned long control) > +{ > + int index = v4l2_get_index(fd); > + struct v4l2_controls_map *map = devices[index].map; > + int lo = 0; > + int hi = map->num_controls; > + > + while (lo < hi) { > + int i = (lo + hi) / 2; > + if (map->map[i].control == control) { > + return map->map[i].fd + fd; > + } > + if (map->map[i].control > control) { > + hi = i; > + continue; > + } > + if (map->map[i].control < control) { > + lo = i+1; > + continue; > + } > + printf("Bad: impossible condition in binary search\n"); > + exit(1); > + } > + return fd; > +} > + > int v4l2_ioctl(int fd, unsigned long int request, ...) > { > void *arg; > va_list ap; > int result, index, saved_err; > - int is_capture_request = 0, stream_needs_locking = 0; > + int is_capture_request = 0, stream_needs_locking = 0, > + is_subdev_request = 0; > > va_start(ap, request); > arg = va_arg(ap, void *); > @@ -1076,18 +1105,20 @@ int v4l2_ioctl(int fd, unsigned long int request, ...) > ioctl, causing it to get sign extended, depending upon this behavior */ > request = (unsigned int)request; > > + /* FIXME */ > if (devices[index].convert == NULL) > goto no_capture_request; > > /* Is this a capture request and do we need to take the stream lock? */ > switch (request) { > - case VIDIOC_QUERYCAP: > case VIDIOC_QUERYCTRL: > case VIDIOC_G_CTRL: > case VIDIOC_S_CTRL: > case VIDIOC_G_EXT_CTRLS: > - case VIDIOC_TRY_EXT_CTRLS: > case VIDIOC_S_EXT_CTRLS: > + is_subdev_request = 1; > + case VIDIOC_QUERYCAP: > + case VIDIOC_TRY_EXT_CTRLS: > case VIDIOC_ENUM_FRAMESIZES: > case VIDIOC_ENUM_FRAMEINTERVALS: > is_capture_request = 1; > @@ -1151,10 +1182,15 @@ int v4l2_ioctl(int fd, unsigned long int request, ...) > } > > if (!is_capture_request) { > + int sub_fd; > no_capture_request: > + sub_fd = fd; > + if (is_subdev_request) { > + sub_fd = v4l2_get_fd_for_control(index, ((struct v4l2_queryctrl *) arg)->id); > + } > result = devices[index].dev_ops->ioctl( > devices[index].dev_ops_priv, > - fd, request, arg); > + sub_fd, request, arg); > saved_err = errno; > v4l2_log_ioctl(request, arg, result); > errno = saved_err; > @@ -1782,3 +1818,28 @@ int v4l2_get_control(int fd, int cid) > (qctrl.maximum - qctrl.minimum) / 2) / > (qctrl.maximum - qctrl.minimum); > } > + > + > +int v4l2_open_pipeline(struct v4l2_controls_map *map, int v4l2_flags) > +{ > + int index; > + int i; > + > + for (i=0; i<map->num_controls; i++) { > + printf("%lx %d\n", map->map[i].control, map->map[i].fd); > + if (map->map[i].fd <= 0) { > + printf("Bad fd in map\n"); > + return -1; > + } > + if (i>=1 && map->map[i].control <= map->map[i-1].control) { > + printf("Not sorted\n"); > + return -1; > + } > + } > + > + v4l2_fd_open(map->main_fd, v4l2_flags); > + index = v4l2_get_index(map->main_fd); > + devices[index].map = map; > + return 0; > +} > + > diff --git a/lib/libv4lconvert/control/libv4lcontrol.c b/lib/libv4lconvert/control/libv4lcontrol.c > index 1e784ed..1963e7e 100644 > --- a/lib/libv4lconvert/control/libv4lcontrol.c > +++ b/lib/libv4lconvert/control/libv4lcontrol.c > @@ -865,6 +865,7 @@ int v4lcontrol_vidioc_queryctrl(struct v4lcontrol_data *data, void *arg) > struct v4l2_queryctrl *ctrl = arg; > int retval; > uint32_t orig_id = ctrl->id; > + int fd; > > /* if we have an exact match return it */ > for (i = 0; i < V4LCONTROL_COUNT; i++) > @@ -874,8 +875,9 @@ int v4lcontrol_vidioc_queryctrl(struct v4lcontrol_data *data, void *arg) > return 0; > } > > + fd = v4l2_get_fd_for_control(data->fd, ctrl->id); > /* find out what the kernel driver would respond. */ > - retval = data->dev_ops->ioctl(data->dev_ops_priv, data->fd, > + retval = data->dev_ops->ioctl(data->dev_ops_priv, fd, > VIDIOC_QUERYCTRL, arg); > > if ((data->priv_flags & V4LCONTROL_SUPPORTS_NEXT_CTRL) && > @@ -905,6 +907,7 @@ int v4lcontrol_vidioc_g_ctrl(struct v4lcontrol_data *data, void *arg) > { > int i; > struct v4l2_control *ctrl = arg; > + int fd; > > for (i = 0; i < V4LCONTROL_COUNT; i++) > if ((data->controls & (1 << i)) && > @@ -913,7 +916,8 @@ int v4lcontrol_vidioc_g_ctrl(struct v4lcontrol_data *data, void *arg) > return 0; > } > > - return data->dev_ops->ioctl(data->dev_ops_priv, data->fd, > + fd = v4l2_get_fd_for_control(data->fd, ctrl->id); > + return data->dev_ops->ioctl(data->dev_ops_priv, fd, > VIDIOC_G_CTRL, arg); > } > > @@ -996,6 +1000,7 @@ int v4lcontrol_vidioc_s_ctrl(struct v4lcontrol_data *data, void *arg) > { > int i; > struct v4l2_control *ctrl = arg; > + int fd; > > for (i = 0; i < V4LCONTROL_COUNT; i++) > if ((data->controls & (1 << i)) && > @@ -1010,7 +1015,8 @@ int v4lcontrol_vidioc_s_ctrl(struct v4lcontrol_data *data, void *arg) > return 0; > } > > - return data->dev_ops->ioctl(data->dev_ops_priv, data->fd, > + fd = v4l2_get_fd_for_control(data->fd, ctrl->id); > + return data->dev_ops->ioctl(data->dev_ops_priv, fd, > VIDIOC_S_CTRL, arg); > } > > >
Hi! > I really don't want to add functions for this to libv4l2. That's just a > quick hack. The real solution is to parse this from a config > file. But No, this is not a quick hack. These are functions that will eventually be used by the config parser. (Oh and they allow me to use camera on complex hardware, but...). Hmm, I have mentioned that already. See quoted text below. > that is a lot more work and it is something that needs to be designed > properly. > > And that requires someone to put in the time and effort... Which is what I'm trying to do. But some cooperation from your side is needed, too. I acknowledged some kind of parser is needed. I can do that. Are you willing to cooperate? But I need your feedback on the parts below. We can bikeshed about the parser later. Do they look acceptable? Did I hook up right functions in acceptable way? If so, yes, I can proceed with parser. Best regards, Pavel > > ...so you can use it on complex devices. Tested on my N900. > > > > I guess later helper would be added that would parse some kind of > > descritption file and do open_pipeline(). But.. lets solve that > > next. In the first place, it would be nice to have libv4l2 usable on > > complex devices. > > > > Best regards, > > Pavel > > Signed-off-by: Pavel Machek <pavel@ucw.cz> > > > > diff --git a/lib/include/libv4l2.h b/lib/include/libv4l2.h > > index ea1870d..6220dfd 100644 > > --- a/lib/include/libv4l2.h > > +++ b/lib/include/libv4l2.h > > @@ -109,6 +109,23 @@ LIBV4L_PUBLIC int v4l2_get_control(int fd, int cid); > > (note the fd is left open in this case). */ > > LIBV4L_PUBLIC int v4l2_fd_open(int fd, int v4l2_flags); > > > > +struct v4l2_control_map { > > + unsigned long control; > > + int fd; > > +}; > > + > > +struct v4l2_controls_map { > > + int main_fd; > > + int num_fds; > > + int num_controls; > > + struct v4l2_control_map map[]; > > +}; > > + > > +LIBV4L_PUBLIC int v4l2_open_pipeline(struct v4l2_controls_map *map, int v4l2_flags); > > + > > +LIBV4L_PUBLIC int v4l2_get_fd_for_control(int fd, unsigned long control); > > + > > + > > #ifdef __cplusplus > > } > > #endif /* __cplusplus */ > > diff --git a/lib/libv4l2/libv4l2-priv.h b/lib/libv4l2/libv4l2-priv.h > > index 1924c91..ebe5dad 100644 > > --- a/lib/libv4l2/libv4l2-priv.h > > +++ b/lib/libv4l2/libv4l2-priv.h > > @@ -104,6 +104,7 @@ struct v4l2_dev_info { > > void *plugin_library; > > void *dev_ops_priv; > > const struct libv4l_dev_ops *dev_ops; > > + struct v4l2_controls_map *map; > > }; > > > > /* From v4l2-plugin.c */ > > diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c > > index 2db25d1..b3ae70b 100644 > > --- a/lib/libv4l2/libv4l2.c > > +++ b/lib/libv4l2/libv4l2.c > > @@ -787,6 +787,8 @@ no_capture: > > if (index >= devices_used) > > devices_used = index + 1; > > > > + devices[index].map = NULL; > > + > > /* Note we always tell v4lconvert to optimize src fmt selection for > > our default fps, the only exception is the app explicitly selecting > > a frame rate using the S_PARM ioctl after a S_FMT */ > > @@ -1056,12 +1058,39 @@ static int v4l2_s_fmt(int index, struct v4l2_format *dest_fmt) > > return 0; > > } > > > > +int v4l2_get_fd_for_control(int fd, unsigned long control) > > +{ > > + int index = v4l2_get_index(fd); > > + struct v4l2_controls_map *map = devices[index].map; > > + int lo = 0; > > + int hi = map->num_controls; > > + > > + while (lo < hi) { > > + int i = (lo + hi) / 2; > > + if (map->map[i].control == control) { > > + return map->map[i].fd + fd; > > + } > > + if (map->map[i].control > control) { > > + hi = i; > > + continue; > > + } > > + if (map->map[i].control < control) { > > + lo = i+1; > > + continue; > > + } > > + printf("Bad: impossible condition in binary search\n"); > > + exit(1); > > + } > > + return fd; > > +} > > + > > int v4l2_ioctl(int fd, unsigned long int request, ...) > > { > > void *arg; > > va_list ap; > > int result, index, saved_err; > > - int is_capture_request = 0, stream_needs_locking = 0; > > + int is_capture_request = 0, stream_needs_locking = 0, > > + is_subdev_request = 0; > > > > va_start(ap, request); > > arg = va_arg(ap, void *); > > @@ -1076,18 +1105,20 @@ int v4l2_ioctl(int fd, unsigned long int request, ...) > > ioctl, causing it to get sign extended, depending upon this behavior */ > > request = (unsigned int)request; > > > > + /* FIXME */ > > if (devices[index].convert == NULL) > > goto no_capture_request; > > > > /* Is this a capture request and do we need to take the stream lock? */ > > switch (request) { > > - case VIDIOC_QUERYCAP: > > case VIDIOC_QUERYCTRL: > > case VIDIOC_G_CTRL: > > case VIDIOC_S_CTRL: > > case VIDIOC_G_EXT_CTRLS: > > - case VIDIOC_TRY_EXT_CTRLS: > > case VIDIOC_S_EXT_CTRLS: > > + is_subdev_request = 1; > > + case VIDIOC_QUERYCAP: > > + case VIDIOC_TRY_EXT_CTRLS: > > case VIDIOC_ENUM_FRAMESIZES: > > case VIDIOC_ENUM_FRAMEINTERVALS: > > is_capture_request = 1; > > @@ -1151,10 +1182,15 @@ int v4l2_ioctl(int fd, unsigned long int request, ...) > > } > > > > if (!is_capture_request) { > > + int sub_fd; > > no_capture_request: > > + sub_fd = fd; > > + if (is_subdev_request) { > > + sub_fd = v4l2_get_fd_for_control(index, ((struct v4l2_queryctrl *) arg)->id); > > + } > > result = devices[index].dev_ops->ioctl( > > devices[index].dev_ops_priv, > > - fd, request, arg); > > + sub_fd, request, arg); > > saved_err = errno; > > v4l2_log_ioctl(request, arg, result); > > errno = saved_err; > > @@ -1782,3 +1818,28 @@ int v4l2_get_control(int fd, int cid) > > (qctrl.maximum - qctrl.minimum) / 2) / > > (qctrl.maximum - qctrl.minimum); > > } > > + > > + > > +int v4l2_open_pipeline(struct v4l2_controls_map *map, int v4l2_flags) > > +{ > > + int index; > > + int i; > > + > > + for (i=0; i<map->num_controls; i++) { > > + printf("%lx %d\n", map->map[i].control, map->map[i].fd); > > + if (map->map[i].fd <= 0) { > > + printf("Bad fd in map\n"); > > + return -1; > > + } > > + if (i>=1 && map->map[i].control <= map->map[i-1].control) { > > + printf("Not sorted\n"); > > + return -1; > > + } > > + } > > + > > + v4l2_fd_open(map->main_fd, v4l2_flags); > > + index = v4l2_get_index(map->main_fd); > > + devices[index].map = map; > > + return 0; > > +} > > + > > diff --git a/lib/libv4lconvert/control/libv4lcontrol.c b/lib/libv4lconvert/control/libv4lcontrol.c > > index 1e784ed..1963e7e 100644 > > --- a/lib/libv4lconvert/control/libv4lcontrol.c > > +++ b/lib/libv4lconvert/control/libv4lcontrol.c > > @@ -865,6 +865,7 @@ int v4lcontrol_vidioc_queryctrl(struct v4lcontrol_data *data, void *arg) > > struct v4l2_queryctrl *ctrl = arg; > > int retval; > > uint32_t orig_id = ctrl->id; > > + int fd; > > > > /* if we have an exact match return it */ > > for (i = 0; i < V4LCONTROL_COUNT; i++) > > @@ -874,8 +875,9 @@ int v4lcontrol_vidioc_queryctrl(struct v4lcontrol_data *data, void *arg) > > return 0; > > } > > > > + fd = v4l2_get_fd_for_control(data->fd, ctrl->id); > > /* find out what the kernel driver would respond. */ > > - retval = data->dev_ops->ioctl(data->dev_ops_priv, data->fd, > > + retval = data->dev_ops->ioctl(data->dev_ops_priv, fd, > > VIDIOC_QUERYCTRL, arg); > > > > if ((data->priv_flags & V4LCONTROL_SUPPORTS_NEXT_CTRL) && > > @@ -905,6 +907,7 @@ int v4lcontrol_vidioc_g_ctrl(struct v4lcontrol_data *data, void *arg) > > { > > int i; > > struct v4l2_control *ctrl = arg; > > + int fd; > > > > for (i = 0; i < V4LCONTROL_COUNT; i++) > > if ((data->controls & (1 << i)) && > > @@ -913,7 +916,8 @@ int v4lcontrol_vidioc_g_ctrl(struct v4lcontrol_data *data, void *arg) > > return 0; > > } > > > > - return data->dev_ops->ioctl(data->dev_ops_priv, data->fd, > > + fd = v4l2_get_fd_for_control(data->fd, ctrl->id); > > + return data->dev_ops->ioctl(data->dev_ops_priv, fd, > > VIDIOC_G_CTRL, arg); > > } > > > > @@ -996,6 +1000,7 @@ int v4lcontrol_vidioc_s_ctrl(struct v4lcontrol_data *data, void *arg) > > { > > int i; > > struct v4l2_control *ctrl = arg; > > + int fd; > > > > for (i = 0; i < V4LCONTROL_COUNT; i++) > > if ((data->controls & (1 << i)) && > > @@ -1010,7 +1015,8 @@ int v4lcontrol_vidioc_s_ctrl(struct v4lcontrol_data *data, void *arg) > > return 0; > > } > > > > - return data->dev_ops->ioctl(data->dev_ops_priv, data->fd, > > + fd = v4l2_get_fd_for_control(data->fd, ctrl->id); > > + return data->dev_ops->ioctl(data->dev_ops_priv, fd, > > VIDIOC_S_CTRL, arg); > > } > > > > > >
Em Mon, 19 Mar 2018 11:23:54 +0100 Pavel Machek <pavel@ucw.cz> escreveu: > Hi! > > > I really don't want to add functions for this to libv4l2. That's just a > > quick hack. The real solution is to parse this from a config > > file. But > > No, this is not a quick hack. These are functions that will eventually > be used by the config parser. (Oh and they allow me to use camera on > complex hardware, but...). > > Hmm, I have mentioned that already. See quoted text below. > > > that is a lot more work and it is something that needs to be designed > > properly. > > > > And that requires someone to put in the time and effort... > > Which is what I'm trying to do. But some cooperation from your side is > needed, too. I acknowledged some kind of parser is needed. I can > do that. Are you willing to cooperate? > > But I need your feedback on the parts below. We can bikeshed about the > parser later. > > Do they look acceptable? Did I hook up right functions in acceptable > way? > > If so, yes, I can proceed with parser. > > Best regards, > Pavel Pavel, I appreciate your efforts of adding support for mc-based devices to libv4l. I guess the main poin that Hans is pointing is that we should take extra care in order to avoid adding new symbols to libv4l ABI/API without being sure that they'll be needed in long term, as removing or changing the API is painful for app developers, and keeping it ABI compatible with apps compiled against previous versions of the library is very painful for us. The hole idea is that generic applications shouldn't notice if the device is using a mc-based device or not. Regards, Mauro
On 03/19/2018 11:47 AM, Mauro Carvalho Chehab wrote: > Em Mon, 19 Mar 2018 11:23:54 +0100 > Pavel Machek <pavel@ucw.cz> escreveu: > >> Hi! >> >>> I really don't want to add functions for this to libv4l2. That's just a >>> quick hack. The real solution is to parse this from a config >>> file. But >> >> No, this is not a quick hack. These are functions that will eventually >> be used by the config parser. (Oh and they allow me to use camera on >> complex hardware, but...). >> >> Hmm, I have mentioned that already. See quoted text below. >> >>> that is a lot more work and it is something that needs to be designed >>> properly. >>> >>> And that requires someone to put in the time and effort... >> >> Which is what I'm trying to do. But some cooperation from your side is >> needed, too. I acknowledged some kind of parser is needed. I can >> do that. Are you willing to cooperate? >> >> But I need your feedback on the parts below. We can bikeshed about the >> parser later. >> >> Do they look acceptable? Did I hook up right functions in acceptable >> way? >> >> If so, yes, I can proceed with parser. >> >> Best regards, >> Pavel > > > Pavel, > > I appreciate your efforts of adding support for mc-based devices to > libv4l. > > I guess the main poin that Hans is pointing is that we should take > extra care in order to avoid adding new symbols to libv4l ABI/API > without being sure that they'll be needed in long term, as removing > or changing the API is painful for app developers, and keeping it > ABI compatible with apps compiled against previous versions of the > library is very painful for us. Indeed. Sorry if I wasn't clear on that. > The hole idea is that generic applications shouldn't notice > if the device is using a mc-based device or not. What is needed IMHO is an RFC that explains how you want to solve this problem, what the parser would look like, how this would configure a complex pipeline for use with libv4l-using applications, etc. I.e., a full design. And once everyone agrees that that design is solid, then it needs to be implemented. I really want to work with you on this, but I am not looking for partial solutions. I think a proper solution is a fair amount of work. If you are willing to take that on, then that would be fantastic. Regards, Hans
Hi! > > Pavel, > > > > I appreciate your efforts of adding support for mc-based devices to > > libv4l. Thanks. > > I guess the main poin that Hans is pointing is that we should take > > extra care in order to avoid adding new symbols to libv4l ABI/API > > without being sure that they'll be needed in long term, as removing > > or changing the API is painful for app developers, and keeping it > > ABI compatible with apps compiled against previous versions of the > > library is very painful for us. > > Indeed. Sorry if I wasn't clear on that. Aha, ok, no, I did not get that. > > The hole idea is that generic applications shouldn't notice > > if the device is using a mc-based device or not. > > What is needed IMHO is an RFC that explains how you want to solve this > problem, what the parser would look like, how this would configure a > complex pipeline for use with libv4l-using applications, etc. > > I.e., a full design. > > And once everyone agrees that that design is solid, then it needs to be > implemented. > > I really want to work with you on this, but I am not looking for partial > solutions. Well, expecting design to be done for opensource development is a bit unusual :-). I really see two separate tasks 1) support for configuring pipeline. I believe this is best done out of libv4l2. It outputs description file, format below. Currently I have implemented this is in Python. File format is below. 2) support for running libv4l2 on mc-based devices. I'd like to do that. Description file would look like. (# comments would not be not part of file). V4L2MEDIADESC 3 # number of files to open /dev/video2 /dev/video6 /dev/video3 3 # number of controls to map. Controls not mentioned here go to # device 0 automatically. Sorted by control id. # Device 0 00980913 1 009a0003 1 009a000a 2 We can parse that easily without requiring external libraries. Sorted data allow us to do binary search. Best regards, Pavel
On 03/19/2018 01:00 PM, Pavel Machek wrote: > Hi! > >>> Pavel, >>> >>> I appreciate your efforts of adding support for mc-based devices to >>> libv4l. > > Thanks. > >>> I guess the main poin that Hans is pointing is that we should take >>> extra care in order to avoid adding new symbols to libv4l ABI/API >>> without being sure that they'll be needed in long term, as removing >>> or changing the API is painful for app developers, and keeping it >>> ABI compatible with apps compiled against previous versions of the >>> library is very painful for us. >> >> Indeed. Sorry if I wasn't clear on that. > > Aha, ok, no, I did not get that. > >>> The hole idea is that generic applications shouldn't notice >>> if the device is using a mc-based device or not. >> >> What is needed IMHO is an RFC that explains how you want to solve this >> problem, what the parser would look like, how this would configure a >> complex pipeline for use with libv4l-using applications, etc. >> >> I.e., a full design. >> >> And once everyone agrees that that design is solid, then it needs to be >> implemented. >> >> I really want to work with you on this, but I am not looking for partial >> solutions. > > Well, expecting design to be done for opensource development is a bit > unusual :-). Why? We have done that quite often in the past. Media is complex and you need to decide on a design up front. > I really see two separate tasks > > 1) support for configuring pipeline. I believe this is best done out > of libv4l2. It outputs description file, format below. Currently I > have implemented this is in Python. File format is below. You do need this, but why outside of libv4l2? I'm not saying I disagree with you, but you need to give reasons for that. > 2) support for running libv4l2 on mc-based devices. I'd like to do > that. > > Description file would look like. (# comments would not be not part of file). > > V4L2MEDIADESC > 3 # number of files to open > /dev/video2 > /dev/video6 > /dev/video3 This won't work. The video nodes numbers (or even names) can change. Instead these should be entity names from the media controller. > 3 # number of controls to map. Controls not mentioned here go to > # device 0 automatically. Sorted by control id. > # Device 0 > 00980913 1 > 009a0003 1 > 009a000a 2 You really don't need to specify the files to open. All you need is to specify the entity ID and the list of controls that you need. Then libv4l can just figure out which device node(s) to open for that. > > We can parse that easily without requiring external libraries. Sorted > data allow us to do binary search. But none of this addresses setting up the initial video pipeline or changing formats. We probably want to support that as well. For that matter: what is it exactly that we want to support? I.e. where do we draw the line? A good test platform for this (outside the N900) is the i.MX6 platform. Regards, Hans
Hi! > >> I really want to work with you on this, but I am not looking for partial > >> solutions. > > > > Well, expecting design to be done for opensource development is a bit > > unusual :-). > > Why? We have done that quite often in the past. Media is complex and you need > to decide on a design up front. > > I really see two separate tasks > > > > 1) support for configuring pipeline. I believe this is best done out > > of libv4l2. It outputs description file, format below. Currently I > > have implemented this is in Python. File format is below. > > You do need this, but why outside of libv4l2? I'm not saying I disagree > with you, but you need to give reasons for that. I'd prefer to do this in Python. There's a lot to configure there, and I'm not sure if libv4l2 is is right place for it. Anyway, design of 2) does not depend on this. > > 2) support for running libv4l2 on mc-based devices. I'd like to do > > that. > > > > Description file would look like. (# comments would not be not part of file). > > > > V4L2MEDIADESC > > 3 # number of files to open > > /dev/video2 > > /dev/video6 > > /dev/video3 > > This won't work. The video nodes numbers (or even names) can change. > Instead these should be entity names from the media controller. Yes, it will work. 1) will configure the pipeline, and prepare V4L2MEDIADESC file. The device names/numbers are stable after the system is booted. If these were entity names, v4l2_open() would have to go to /sys and search for corresponding files... which would be rather complex and slow. > > 3 # number of controls to map. Controls not mentioned here go to > > # device 0 automatically. Sorted by control id. > > # Device 0 > > 00980913 1 > > 009a0003 1 > > 009a000a 2 > > You really don't need to specify the files to open. All you need is to > specify the entity ID and the list of controls that you need. > > Then libv4l can just figure out which device node(s) to open for > that. Yes, but that would slow down v4l2_open() needlessly. I'd prefer to avoid that. > > We can parse that easily without requiring external libraries. Sorted > > data allow us to do binary search. > > But none of this addresses setting up the initial video pipeline or > changing formats. We probably want to support that as well. Well, maybe one day. But I don't believe we should attempt to support that today. Currently, there's no way to test that camera works on N900 with mainline v4l2... which is rather sad. Advanced use cases can come later. > For that matter: what is it exactly that we want to support? I.e. where do > we draw the line? I'd start with fixed format first. Python prepares pipeline, and provides V4L2MEDIADESC file libv4l2 can use. You can have that this week. I guess it would make sense to support "application says preffered resolution, libv4l2 attempts to set up some kind of pipeline to get that resolution", but yes, interface there will likely be quite complex. Media control is more than 5 years old now. libv4l2 is still completely uses on media control-based devices, and people are asking for controls propagation in the kernel to fix that. My proposol implements simple controls propagation in the userland. I believe we should do that. > A good test platform for this (outside the N900) is the i.MX6 platform. Do you have one? Pavel
Em Mon, 19 Mar 2018 13:15:22 +0100 Hans Verkuil <hverkuil@xs4all.nl> escreveu: > On 03/19/2018 01:00 PM, Pavel Machek wrote: > > Hi! > > > >>> Pavel, > >>> > >>> I appreciate your efforts of adding support for mc-based devices to > >>> libv4l. > > > > Thanks. > > > >>> I guess the main poin that Hans is pointing is that we should take > >>> extra care in order to avoid adding new symbols to libv4l ABI/API > >>> without being sure that they'll be needed in long term, as removing > >>> or changing the API is painful for app developers, and keeping it > >>> ABI compatible with apps compiled against previous versions of the > >>> library is very painful for us. > >> > >> Indeed. Sorry if I wasn't clear on that. > > > > Aha, ok, no, I did not get that. > > > >>> The hole idea is that generic applications shouldn't notice > >>> if the device is using a mc-based device or not. > >> > >> What is needed IMHO is an RFC that explains how you want to solve this > >> problem, what the parser would look like, how this would configure a > >> complex pipeline for use with libv4l-using applications, etc. > >> > >> I.e., a full design. > >> > >> And once everyone agrees that that design is solid, then it needs to be > >> implemented. > >> > >> I really want to work with you on this, but I am not looking for partial > >> solutions. > > > > Well, expecting design to be done for opensource development is a bit > > unusual :-). > > Why? We have done that quite often in the past. Media is complex and you need > to decide on a design up front. > > > I really see two separate tasks > > > > 1) support for configuring pipeline. I believe this is best done out > > of libv4l2. It outputs description file, format below. Currently I > > have implemented this is in Python. File format is below. > > You do need this, but why outside of libv4l2? I'm not saying I disagree > with you, but you need to give reasons for that. > > > 2) support for running libv4l2 on mc-based devices. I'd like to do > > that. > > > > Description file would look like. (# comments would not be not part of file). > > > > V4L2MEDIADESC > > 3 # number of files to open > > /dev/video2 > > /dev/video6 > > /dev/video3 "Easy" file formats usually means maintenance troubles as new things are needed, and makes worse for users to write it. You should take a look at lib/libdvbv5/dvb-file.c, mainly at fill_entry() and dvb_read_file(). It has a parser there for things like: [foo] bar = value and it doesn't require external libraries. I considered writing it as a small helper function, but it turns that parsing this format is so simple that can easily be added anywhere else. The advantage of such format is that it should be simple enough to add more stuff there, and intuitive enough for end users. > This won't work. The video nodes numbers (or even names) can change. Yes. That all depends on how udev is set on a given system. Btw, the logic should likely use libudev in order to get the devname. > Instead these should be entity names from the media controller. Agreed that it should use MC. Yet, IMHO, the best would be to use the entity function instead, as entity names might eventually change on newer versions of the Kernel. So, IMHO, entities should be described as: [entity entity1] name = foo function = bar (again, user may specify just name, just function or both) > > 3 # number of controls to map. Controls not mentioned here go to > > # device 0 automatically. Sorted by control id. > > # Device 0 > > 00980913 1 > > 009a0003 1 > > 009a000a 2 I would, instead, encode it as: [control white balance] control_id = 0x00980913 entity = foo_entity_name [control exposure] control_id = V4L2_CID_EXPOSURE_ABSOLUTE entity = bar_entity_name ... Allowing both hexadecimal values and control macro names (can easily parsed from the header file, as we already do for other things with "make sync"). Makes a way easier for people to understand what that means and to write such files. > You really don't need to specify the files to open. All you need is to > specify the entity ID and the list of controls that you need. > > Then libv4l can just figure out which device node(s) to open for that. The only file it could make sense to specify is the media controller device or the name of the media controller device, as more than one /dev/media? devnode could be present on a given system. I would actually support both, e. g.: [media controller] name = foo devname = bar Only "name" or "devname" would be required: - If "name" is blank, it would just use "devname"; - If "devname" is blank, it would open all /dev/media? devices until it matches "name"; - If both are given, it would open "devname" and check if "name" matches the media controller name, failing otherwise. > > > > We can parse that easily without requiring external libraries. Sorted > > data allow us to do binary search. > > But none of this addresses setting up the initial video pipeline or > changing formats. We probably want to support that as well. It should probably be easy to add a generic pipeline descriptor with a format like: [pipeline pipe1] link0 = SGRBG10 640x480: entity1:0 -> entity2:0[1] link1 = SGRBG10 640x480: entity2:2-> entity3:0[1] link2 = UYVY 640x480: entity3:1-> entity4:0[1] link3 = UYVY 640x480: entity4:1-> entity5:0[1] sink0 = UYVY 320x200: entity5:0[1] sink1 = UYVY 640x480: entity3:0[1] > > For that matter: what is it exactly that we want to support? I.e. where do > we draw the line? > > A good test platform for this (outside the N900) is the i.MX6 platform. > > Regards, > > Hans Thanks, Mauro
On 03/19/2018 01:48 PM, Pavel Machek wrote: > Hi! > >>>> I really want to work with you on this, but I am not looking for partial >>>> solutions. >>> >>> Well, expecting design to be done for opensource development is a bit >>> unusual :-). >> >> Why? We have done that quite often in the past. Media is complex and you need >> to decide on a design up front. > > > >>> I really see two separate tasks >>> >>> 1) support for configuring pipeline. I believe this is best done out >>> of libv4l2. It outputs description file, format below. Currently I >>> have implemented this is in Python. File format is below. >> >> You do need this, but why outside of libv4l2? I'm not saying I disagree >> with you, but you need to give reasons for that. > > I'd prefer to do this in Python. There's a lot to configure there, and > I'm not sure if libv4l2 is is right place for it. Anyway, design of 2) > does not depend on this. > >>> 2) support for running libv4l2 on mc-based devices. I'd like to do >>> that. >>> >>> Description file would look like. (# comments would not be not part of file). >>> >>> V4L2MEDIADESC >>> 3 # number of files to open >>> /dev/video2 >>> /dev/video6 >>> /dev/video3 >> >> This won't work. The video nodes numbers (or even names) can change. >> Instead these should be entity names from the media controller. > > Yes, it will work. 1) will configure the pipeline, and prepare > V4L2MEDIADESC file. The device names/numbers are stable after the > system is booted. No, they are not. Drivers can be unloaded and reloaded, thus changing the device nodes. The media device will give you the right major/minor numbers and with libudev you can find the corresponding device node. That's the right approach. > > If these were entity names, v4l2_open() would have to go to /sys and > search for corresponding files... which would be rather complex and > slow. > >>> 3 # number of controls to map. Controls not mentioned here go to >>> # device 0 automatically. Sorted by control id. >>> # Device 0 >>> 00980913 1 >>> 009a0003 1 >>> 009a000a 2 >> >> You really don't need to specify the files to open. All you need is to >> specify the entity ID and the list of controls that you need. >> >> Then libv4l can just figure out which device node(s) to open for >> that. > > Yes, but that would slow down v4l2_open() needlessly. I'd prefer to > avoid that. It should be quite fast. BTW, v4l2-ctl has code to find the media device node for a given video node. > >>> We can parse that easily without requiring external libraries. Sorted >>> data allow us to do binary search. >> >> But none of this addresses setting up the initial video pipeline or >> changing formats. We probably want to support that as well. > > Well, maybe one day. But I don't believe we should attempt to support > that today. But this should at least be thought about in the design. What is needed in the configuration file to support this? What are the consequences for how everything is set up? Otherwise we'd implement something only to discover later that it doesn't work for the more advanced scenarios. > Currently, there's no way to test that camera works on N900 with > mainline v4l2... which is rather sad. Advanced use cases can come later. > >> For that matter: what is it exactly that we want to support? I.e. where do >> we draw the line? > > I'd start with fixed format first. Python prepares pipeline, and > provides V4L2MEDIADESC file libv4l2 can use. You can have that this > week. I'm not sure I want python. An embedded system might not have python installed. Also, if we need to parse the configuration file in libv4l (and I am 90% certain that that is what we need to do), then you don't want to call a python script from there. > > I guess it would make sense to support "application says preffered > resolution, libv4l2 attempts to set up some kind of pipeline to get > that resolution", but yes, interface there will likely be quite > complex. > > Media control is more than 5 years old now. libv4l2 is still > completely uses on media control-based devices, and people are asking > for controls propagation in the kernel to fix that. My proposol > implements simple controls propagation in the userland. I believe we > should do that. Your proposal implements one specific use-case. One piece of a larger puzzle. We first need a proper design proposal and we can go from there. Now, I admit, all this takes time. And that's why this still hasn't been done after all those years. > >> A good test platform for this (outside the N900) is the i.MX6 platform. > > Do you have one? Yes. Although I would need to set it up, I still haven't done that. But Steve Longerbeam is probably the right person to test this for you. What I have been thinking of (although never in any real detail) is that we probably want to have an application that is run from udev when a media device is created and that reads a config file and does an initial pipeline configuration. So once that's setup you should be able to do: 'v4l2-ctl --stream-mmap' and get video. Next, libv4l will also read the same configuration file and use it to provide a compatibility layer so applications that use libv4l will work better with MC devices. Part of that is indeed control handling. Regards, Hans
Em Mon, 19 Mar 2018 13:48:55 +0100 Pavel Machek <pavel@ucw.cz> escreveu: > Hi! > > > >> I really want to work with you on this, but I am not looking for partial > > >> solutions. > > > > > > Well, expecting design to be done for opensource development is a bit > > > unusual :-). > > > > Why? We have done that quite often in the past. Media is complex and you need > > to decide on a design up front. > > > > > > I really see two separate tasks > > > > > > 1) support for configuring pipeline. I believe this is best done out > > > of libv4l2. It outputs description file, format below. Currently I > > > have implemented this is in Python. File format is below. > > > > You do need this, but why outside of libv4l2? I'm not saying I disagree > > with you, but you need to give reasons for that. > > I'd prefer to do this in Python. There's a lot to configure there, and > I'm not sure if libv4l2 is is right place for it. Anyway, design of 2) > does not depend on this. Yes, the functionality of parsing the pipeline can be done later, but, IMO, it should be integrated at the library. > > > 2) support for running libv4l2 on mc-based devices. I'd like to do > > > that. > > > > > > Description file would look like. (# comments would not be not part of file). > > > > > > V4L2MEDIADESC > > > 3 # number of files to open > > > /dev/video2 > > > /dev/video6 > > > /dev/video3 > > > > This won't work. The video nodes numbers (or even names) can change. > > Instead these should be entity names from the media controller. > > Yes, it will work. 1) will configure the pipeline, and prepare > V4L2MEDIADESC file. The device names/numbers are stable after the > system is booted. No, it doensn't. On more complex SoC chips (like Exynos) with multiple drivers for video capture, mpeg encoding/decoding, etc, the device node numbers change on every reboot, as they're probed by different drivers. So, it depends on what device driver is probed first. That's a list of such devices on an Exynos 5 hardware: $ v4l2-ctl --list-devices s5p-mfc-dec (platform:11000000.codec): /dev/video4 /dev/video5 s5p-jpeg encoder (platform:11f50000.jpeg): /dev/video0 /dev/video1 s5p-jpeg encoder (platform:11f60000.jpeg): /dev/video2 /dev/video3 exynos-gsc gscaler (platform:13e00000.video-scaler): /dev/video6 exynos-gsc gscaler (platform:13e10000.video-scaler): /dev/video7 (on every boot, the /dev/video? numbe changes). We need to use something using sysfs in order to get it right. That's what I did on some test scripts here: #!/bin/bash NEEDED=platform-11000000.codec-video-index0 DEV=$(ls -l /dev/v4l/by-path/$NEEDED|perl -ne ' print $1 if (m,/video(\d+),)') echo "jpeg scaler ($NEEDED) at: /dev/video$DEV" gst-launch-1.0 filesrc location=~/test.mov ! qtdemux ! h264parse ! v4l2video${DEV}dec ! videoconvert ! kmssink But that's because I was just too lazy to do the right thing (and this was for a test machine - so no real need for it to be stable enough). > If these were entity names, v4l2_open() would have to go to /sys and > search for corresponding files... which would be rather complex and > slow. It has to, but speed here is not the problem, as this happens only at open() time, and sysfs parsing is fast, as it is not stored on disk. > > > > We can parse that easily without requiring external libraries. Sorted > > > data allow us to do binary search. > > > > But none of this addresses setting up the initial video pipeline or > > changing formats. We probably want to support that as well. > > Well, maybe one day. But I don't believe we should attempt to support > that today. > > Currently, there's no way to test that camera works on N900 with > mainline v4l2... which is rather sad. Advanced use cases can come later. > > > For that matter: what is it exactly that we want to support? I.e. where do > > we draw the line? > > I'd start with fixed format first. Python prepares pipeline, and > provides V4L2MEDIADESC file libv4l2 can use. You can have that this > week. I don't see any problems with starting implementing it for controls, but we need to have a broad view when designing, in order to avoid breaking APIs and file formats. > I guess it would make sense to support "application says preffered > resolution, libv4l2 attempts to set up some kind of pipeline to get > that resolution", but yes, interface there will likely be quite > complex. I don't it will be that complex. A simple pipeline setting like what I proposed on the previous email: [pipeline pipe1] link0 = SGRBG10 640x480: entity1:0 -> entity2:0[1] link1 = SGRBG10 640x480: entity2:2-> entity3:0[1] link2 = UYVY 640x480: entity3:1-> entity4:0[1] link3 = UYVY 640x480: entity4:1-> entity5:0[1] sink0 = UYVY 320x200: entity5:0[1] sink1 = UYVY 640x480: entity3:0[1] Seems enough to let the library to know what to do if an app wants to get a pipeline with UYVY output at 320x200 or at 640x480 resolutions. Thanks, Mauro
Hi! > >>> V4L2MEDIADESC > >>> 3 # number of files to open > >>> /dev/video2 > >>> /dev/video6 > >>> /dev/video3 > >> > >> This won't work. The video nodes numbers (or even names) can change. > >> Instead these should be entity names from the media controller. > > > > Yes, it will work. 1) will configure the pipeline, and prepare > > V4L2MEDIADESC file. The device names/numbers are stable after the > > system is booted. > > No, they are not. Drivers can be unloaded and reloaded, thus changing > the device nodes. The media device will give you the right major/minor > numbers and with libudev you can find the corresponding device node. > That's the right approach. Well, yes, drivers can be unloaded and reloaded. But at that point pipeline will need to be set up again, so we'll get new V4L2MEDIADESC file. > > Yes, but that would slow down v4l2_open() needlessly. I'd prefer to > > avoid that. > > It should be quite fast. BTW, v4l2-ctl has code to find the media device > node for a given video node. Ok, I'll take a look, but I'd still prefer fast & simple. > >> For that matter: what is it exactly that we want to support? I.e. where do > >> we draw the line? > > > > I'd start with fixed format first. Python prepares pipeline, and > > provides V4L2MEDIADESC file libv4l2 can use. You can have that this > > week. > > I'm not sure I want python. An embedded system might not have python > installed. Also, if we need to parse the configuration file in libv4l > (and I am 90% certain that that is what we need to do), then you don't > want to call a python script from there. No, I don't propose calling python from libv4l2. I propose "separate component configures the pipeline, and writes V4L2MEDIADESC file libv4l2 then uses to map controls to devices". For me, that component is currently python. In embededded world, I guess they could hard-code the config, or write it in whatever language they prefer. > > Media control is more than 5 years old now. libv4l2 is still > > completely uses on media control-based devices, and people are asking > > for controls propagation in the kernel to fix that. My proposol > > implements simple controls propagation in the userland. I believe we > > should do that. > > Your proposal implements one specific use-case. One piece of a > larger puzzle. Well, rather important piece, because it allows the complex cameras to be used at all. > >> A good test platform for this (outside the N900) is the i.MX6 platform. > > > > Do you have one? > > Yes. Although I would need to set it up, I still haven't done that. Ok. > But Steve Longerbeam is probably the right person to test this for you. > > What I have been thinking of (although never in any real detail) is that we > probably want to have an application that is run from udev when a media device > is created and that reads a config file and does an initial pipeline configuration. > > So once that's setup you should be able to do: 'v4l2-ctl --stream-mmap' and > get video. > > Next, libv4l will also read the same configuration file and use it to provide > a compatibility layer so applications that use libv4l will work better with > MC devices. Part of that is indeed control handling. Yep, that would be nice. And yes, control handling is important for me. Pavel
Hi! > > > 2) support for running libv4l2 on mc-based devices. I'd like to do > > > that. > > > > > > Description file would look like. (# comments would not be not part of file). > > > > > > V4L2MEDIADESC > > > 3 # number of files to open > > > /dev/video2 > > > /dev/video6 > > > /dev/video3 > > "Easy" file formats usually means maintenance troubles as new > things are needed, and makes worse for users to write it. > You should take a look at lib/libdvbv5/dvb-file.c, mainly at > fill_entry() and dvb_read_file(). Well, file formats just need to be maintained. > > Instead these should be entity names from the media controller. > > Agreed that it should use MC. Yet, IMHO, the best would be to use > the entity function instead, as entity names might eventually > change on newer versions of the Kernel. Kernel interfaces are not supposed to be changing. > (again, user may specify just name, just function or both) > > > > 3 # number of controls to map. Controls not mentioned here go to > > > # device 0 automatically. Sorted by control id. > > > # Device 0 > > > 00980913 1 > > > 009a0003 1 > > > 009a000a 2 > > I would, instead, encode it as: > > [control white balance] > control_id = 0x00980913 > entity = foo_entity_name Ok, that's really overly complex. If futrue extensibility is concern we can do 0x00980913=whatever. > Allowing both hexadecimal values and control macro names (can easily parsed > from the header file, as we already do for other things with "make > sync"). "Easily" as in "more complex then rest of proposed code combined" :-(. > It should probably be easy to add a generic pipeline descriptor > with a format like: > > [pipeline pipe1] > link0 = SGRBG10 640x480: entity1:0 -> entity2:0[1] > link1 = SGRBG10 640x480: entity2:2-> entity3:0[1] > link2 = UYVY 640x480: entity3:1-> entity4:0[1] > link3 = UYVY 640x480: entity4:1-> entity5:0[1] > > sink0 = UYVY 320x200: entity5:0[1] > sink1 = UYVY 640x480: entity3:0[1] Well, first, this looks like very unsuitable for key=value file. Plus, it will not be easy to parse. .... and control map needs to be per-pipeline-configuration. Again, proposed Windows-style format can not easily do that. Best regards, Pavel
Hi! > So, IMHO, entities should be described as: > > [entity entity1] > name = foo > function = bar I don't really think windows-style config file is suitable here, as we have more than two "nested blocks". What about something like this? Note that I'd only implement the controls mapping for now... but it should be extensible later to setup mappings for the application. Best regards, Pavel #modes: 2 Driver name: OMAP 3 resizer Mode: 3000x1800 #devices: 2 0: et8ek8 sensor 1: OMAP3 resizer #controls: 2 0x4321a034: 1 0x4113aab0: 1 #links: 1 link: entity1: et8ek8 sensor:1 entity2: OMAP3 resizer:0 resolution1: 1024x768 resolution2: 1024x768 Mode: 1024x768 #devices: 2 0: et8ek8 sensor 1: OMAP3 resizer #controls: 2 0x4321a034: 1 0x4113aab0: 1 #links: 1 link: entity1: et8ek8 sensor:1 entity2: OMAP3 resizer:0 resolution1: 1024x768 resolution2: 1024x768
Em Tue, 15 May 2018 22:01:17 +0200 Pavel Machek <pavel@ucw.cz> escreveu: > Hi! > > > So, IMHO, entities should be described as: > > > > [entity entity1] > > name = foo > > function = bar > > I don't really think windows-style config file is suitable here, as we > have more than two "nested blocks". > > What about something like this? Note that I'd only implement the > controls mapping for now... but it should be extensible later to setup > mappings for the application. > > Best regards, > Pavel > > > #modes: 2 > Driver name: OMAP 3 resizer It probably makes sense to place the driver name before #modes. From what I understood, the "#foo: <number>" is a tag that makes the parser to expect for <number> of "foo". I would also add a first line at the beginning describing the version of the format, just in case we add more stuff that would require to change something at the format. Except for that, it seems ok. > Mode: 3000x1800 > #devices: 2 > 0: et8ek8 sensor > 1: OMAP3 resizer > #controls: 2 > 0x4321a034: 1 > 0x4113aab0: 1 > #links: 1 > link: > entity1: et8ek8 sensor:1 > entity2: OMAP3 resizer:0 > resolution1: 1024x768 > resolution2: 1024x768 > Mode: 1024x768 > #devices: 2 > 0: et8ek8 sensor > 1: OMAP3 resizer > #controls: 2 > 0x4321a034: 1 > 0x4113aab0: 1 > #links: 1 > link: > entity1: et8ek8 sensor:1 > entity2: OMAP3 resizer:0 > resolution1: 1024x768 > resolution2: 1024x768 > > > Thanks, Mauro
diff --git a/lib/include/libv4l2.h b/lib/include/libv4l2.h index ea1870d..6220dfd 100644 --- a/lib/include/libv4l2.h +++ b/lib/include/libv4l2.h @@ -109,6 +109,23 @@ LIBV4L_PUBLIC int v4l2_get_control(int fd, int cid); (note the fd is left open in this case). */ LIBV4L_PUBLIC int v4l2_fd_open(int fd, int v4l2_flags); +struct v4l2_control_map { + unsigned long control; + int fd; +}; + +struct v4l2_controls_map { + int main_fd; + int num_fds; + int num_controls; + struct v4l2_control_map map[]; +}; + +LIBV4L_PUBLIC int v4l2_open_pipeline(struct v4l2_controls_map *map, int v4l2_flags); + +LIBV4L_PUBLIC int v4l2_get_fd_for_control(int fd, unsigned long control); + + #ifdef __cplusplus } #endif /* __cplusplus */ diff --git a/lib/libv4l2/libv4l2-priv.h b/lib/libv4l2/libv4l2-priv.h index 1924c91..ebe5dad 100644 --- a/lib/libv4l2/libv4l2-priv.h +++ b/lib/libv4l2/libv4l2-priv.h @@ -104,6 +104,7 @@ struct v4l2_dev_info { void *plugin_library; void *dev_ops_priv; const struct libv4l_dev_ops *dev_ops; + struct v4l2_controls_map *map; }; /* From v4l2-plugin.c */ diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c index 2db25d1..b3ae70b 100644 --- a/lib/libv4l2/libv4l2.c +++ b/lib/libv4l2/libv4l2.c @@ -787,6 +787,8 @@ no_capture: if (index >= devices_used) devices_used = index + 1; + devices[index].map = NULL; + /* Note we always tell v4lconvert to optimize src fmt selection for our default fps, the only exception is the app explicitly selecting a frame rate using the S_PARM ioctl after a S_FMT */ @@ -1056,12 +1058,39 @@ static int v4l2_s_fmt(int index, struct v4l2_format *dest_fmt) return 0; } +int v4l2_get_fd_for_control(int fd, unsigned long control) +{ + int index = v4l2_get_index(fd); + struct v4l2_controls_map *map = devices[index].map; + int lo = 0; + int hi = map->num_controls; + + while (lo < hi) { + int i = (lo + hi) / 2; + if (map->map[i].control == control) { + return map->map[i].fd + fd; + } + if (map->map[i].control > control) { + hi = i; + continue; + } + if (map->map[i].control < control) { + lo = i+1; + continue; + } + printf("Bad: impossible condition in binary search\n"); + exit(1); + } + return fd; +} + int v4l2_ioctl(int fd, unsigned long int request, ...) { void *arg; va_list ap; int result, index, saved_err; - int is_capture_request = 0, stream_needs_locking = 0; + int is_capture_request = 0, stream_needs_locking = 0, + is_subdev_request = 0; va_start(ap, request); arg = va_arg(ap, void *); @@ -1076,18 +1105,20 @@ int v4l2_ioctl(int fd, unsigned long int request, ...) ioctl, causing it to get sign extended, depending upon this behavior */ request = (unsigned int)request; + /* FIXME */ if (devices[index].convert == NULL) goto no_capture_request; /* Is this a capture request and do we need to take the stream lock? */ switch (request) { - case VIDIOC_QUERYCAP: case VIDIOC_QUERYCTRL: case VIDIOC_G_CTRL: case VIDIOC_S_CTRL: case VIDIOC_G_EXT_CTRLS: - case VIDIOC_TRY_EXT_CTRLS: case VIDIOC_S_EXT_CTRLS: + is_subdev_request = 1; + case VIDIOC_QUERYCAP: + case VIDIOC_TRY_EXT_CTRLS: case VIDIOC_ENUM_FRAMESIZES: case VIDIOC_ENUM_FRAMEINTERVALS: is_capture_request = 1; @@ -1151,10 +1182,15 @@ int v4l2_ioctl(int fd, unsigned long int request, ...) } if (!is_capture_request) { + int sub_fd; no_capture_request: + sub_fd = fd; + if (is_subdev_request) { + sub_fd = v4l2_get_fd_for_control(index, ((struct v4l2_queryctrl *) arg)->id); + } result = devices[index].dev_ops->ioctl( devices[index].dev_ops_priv, - fd, request, arg); + sub_fd, request, arg); saved_err = errno; v4l2_log_ioctl(request, arg, result); errno = saved_err; @@ -1782,3 +1818,28 @@ int v4l2_get_control(int fd, int cid) (qctrl.maximum - qctrl.minimum) / 2) / (qctrl.maximum - qctrl.minimum); } + + +int v4l2_open_pipeline(struct v4l2_controls_map *map, int v4l2_flags) +{ + int index; + int i; + + for (i=0; i<map->num_controls; i++) { + printf("%lx %d\n", map->map[i].control, map->map[i].fd); + if (map->map[i].fd <= 0) { + printf("Bad fd in map\n"); + return -1; + } + if (i>=1 && map->map[i].control <= map->map[i-1].control) { + printf("Not sorted\n"); + return -1; + } + } + + v4l2_fd_open(map->main_fd, v4l2_flags); + index = v4l2_get_index(map->main_fd); + devices[index].map = map; + return 0; +} + diff --git a/lib/libv4lconvert/control/libv4lcontrol.c b/lib/libv4lconvert/control/libv4lcontrol.c index 1e784ed..1963e7e 100644 --- a/lib/libv4lconvert/control/libv4lcontrol.c +++ b/lib/libv4lconvert/control/libv4lcontrol.c @@ -865,6 +865,7 @@ int v4lcontrol_vidioc_queryctrl(struct v4lcontrol_data *data, void *arg) struct v4l2_queryctrl *ctrl = arg; int retval; uint32_t orig_id = ctrl->id; + int fd; /* if we have an exact match return it */ for (i = 0; i < V4LCONTROL_COUNT; i++) @@ -874,8 +875,9 @@ int v4lcontrol_vidioc_queryctrl(struct v4lcontrol_data *data, void *arg) return 0; } + fd = v4l2_get_fd_for_control(data->fd, ctrl->id); /* find out what the kernel driver would respond. */ - retval = data->dev_ops->ioctl(data->dev_ops_priv, data->fd, + retval = data->dev_ops->ioctl(data->dev_ops_priv, fd, VIDIOC_QUERYCTRL, arg); if ((data->priv_flags & V4LCONTROL_SUPPORTS_NEXT_CTRL) && @@ -905,6 +907,7 @@ int v4lcontrol_vidioc_g_ctrl(struct v4lcontrol_data *data, void *arg) { int i; struct v4l2_control *ctrl = arg; + int fd; for (i = 0; i < V4LCONTROL_COUNT; i++) if ((data->controls & (1 << i)) && @@ -913,7 +916,8 @@ int v4lcontrol_vidioc_g_ctrl(struct v4lcontrol_data *data, void *arg) return 0; } - return data->dev_ops->ioctl(data->dev_ops_priv, data->fd, + fd = v4l2_get_fd_for_control(data->fd, ctrl->id); + return data->dev_ops->ioctl(data->dev_ops_priv, fd, VIDIOC_G_CTRL, arg); } @@ -996,6 +1000,7 @@ int v4lcontrol_vidioc_s_ctrl(struct v4lcontrol_data *data, void *arg) { int i; struct v4l2_control *ctrl = arg; + int fd; for (i = 0; i < V4LCONTROL_COUNT; i++) if ((data->controls & (1 << i)) && @@ -1010,7 +1015,8 @@ int v4lcontrol_vidioc_s_ctrl(struct v4lcontrol_data *data, void *arg) return 0; } - return data->dev_ops->ioctl(data->dev_ops_priv, data->fd, + fd = v4l2_get_fd_for_control(data->fd, ctrl->id); + return data->dev_ops->ioctl(data->dev_ops_priv, fd, VIDIOC_S_CTRL, arg); }
Hi! What about something like this? Pass map of controls and expects fds to libv4l2... +static struct v4l2_controls_map map = { + .num_fds = 2, + .num_controls = 3, + .map = { [0] = { .control = 0x00980913, .fd = 1 }, + [1] = { .control = V4L2_CID_EXPOSURE_ABSOLUTE, .fd = 1 }, + [2] = { .control = V4L2_CID_FOCUS_ABSOLUTE, .fd = 2 }, + + }, +}; + +static void open_chk(char *name, int fd) +{ + int f = open(name, O_RDWR); + + if (fd != f) { + printf("Unexpected error %m opening %s\n", name); + exit(1); + } +} + +static void cam_open_n900(struct dev_info *dev) +{ + struct v4l2_format *fmt = &dev->fmt; + int fd; + + map.main_fd = open("/dev/video_ccdc", O_RDWR); + + open_chk("/dev/video_sensor", map.main_fd+1); + open_chk("/dev/video_focus", map.main_fd+2); + // open_chk("/dev/video_flash", map.main_fd+3); + + v4l2_open_pipeline(&map, 0); + dev->fd = map.main_fd; + ...so you can use it on complex devices. Tested on my N900. I guess later helper would be added that would parse some kind of descritption file and do open_pipeline(). But.. lets solve that next. In the first place, it would be nice to have libv4l2 usable on complex devices. Best regards, Pavel Signed-off-by: Pavel Machek <pavel@ucw.cz>