Re: [PATCH/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera
On 03/15/2015 08:07 PM, Gregor Jasny wrote: On 21/11/14 17:14, Jacek Anaszewski wrote: diff --git a/lib/Makefile.am b/lib/Makefile.am index 3a0e19c..56b3a9f 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -5,7 +5,12 @@ SUBDIRS = \ libv4l2rds \ libv4l-mplane +if WITH_V4LUTILS +SUBDIRS += \ + libv4l-exynos4-camera +endif Why do you depend on WITH_V4LUTILS for a libv4l plugin? This looks wrong. WITH_V4LUTILS is intended to only switch off the utilities in utils (see root Makefile.am). This is an issue to be resolved. I need to depend on WITH_V4LUTILS, because the plugin depends on utils' libraries (e.g. libmediactl.so and lib4lsubdev.so). On the other hand some utils depend on core libs. Temporarily the libv4-exynos4-camera plugin doesn't link against utils libraries but has their sources compiled-in to avoid cyclic dependencies. I submit this as a subject for discussion on how to adjust the build system to handle such a configuration. -- 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/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera
> diff --git a/lib/libv4l-exynos4-camera/Makefile.am > b/lib/libv4l-exynos4-camera/Makefile.am > new file mode 100644 > index 000..23c60c6 > --- /dev/null > +++ b/lib/libv4l-exynos4-camera/Makefile.am > @@ -0,0 +1,7 @@ > +if WITH_V4L_PLUGINS > +libv4l2plugin_LTLIBRARIES = libv4l-exynos4-camera.la > +endif > + > +libv4l_exynos4_camera_la_SOURCES = libv4l-exynos4-camera.c > ../../utils/media-ctl/libmediactl.c ../../utils/media-ctl/libv4l2subdev.c > ../../utils/media-ctl/libv4l2media_ioctl.c ../../utils/media-ctl/mediatext.c > +libv4l_exynos4_camera_la_CFLAGS = -fvisibility=hidden -std=gnu99 Please use $(CFLAG_VISIBILITY) instead of -fvisibility=hidden. Also c99 is default. If you don't need GNU extensions, please drop the -std=gnu99. Thanks, Gregor -- 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/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera
On 21/11/14 17:14, Jacek Anaszewski wrote: > diff --git a/lib/Makefile.am b/lib/Makefile.am > index 3a0e19c..56b3a9f 100644 > --- a/lib/Makefile.am > +++ b/lib/Makefile.am > @@ -5,7 +5,12 @@ SUBDIRS = \ > libv4l2rds \ > libv4l-mplane > > +if WITH_V4LUTILS > +SUBDIRS += \ > + libv4l-exynos4-camera > +endif Why do you depend on WITH_V4LUTILS for a libv4l plugin? This looks wrong. WITH_V4LUTILS is intended to only switch off the utilities in utils (see root Makefile.am). Thanks, Gregor -- 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/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera
Hi Sakari, On 11/27/2014 09:41 AM, Sakari Ailus wrote: Hi Jacek, On Fri, Nov 21, 2014 at 05:14:40PM +0100, Jacek Anaszewski wrote: The plugin provides support for the media device on Exynos4 SoC. It performs single plane <-> multi plane API conversion, video pipeline linking and takes care of automatic data format negotiation for the whole pipeline, after intercepting VIDIOC_S_FMT or VIDIOC_TRY_FMT ioctls. [...] + +static void *plugin_init(int fd) +{ + struct v4l2_capability cap; + struct exynos4_camera_plugin *plugin = NULL; + const char *sink_entity_name; + struct media_device *media; + struct media_entity *sink_entity; + char video_devname[32]; + int ret; [...] + ret = SYS_IOCTL(fd, VIDIOC_QUERYCAP, &cap); + if (ret < 0) { + V4L2_EXYNOS4_ERR("Failed to query video capabilities."); + return NULL; + } + + /* Check if this is Exynos4 media device */ + if (strcmp((char *) cap.driver, EXYNOS4_FIMC_DRV) && + strcmp((char *) cap.driver, EXYNOS4_FIMC_LITE_DRV) && + strcmp((char *) cap.driver, EXYNOS4_FIMC_IS_ISP_DRV)) { + V4L2_EXYNOS4_ERR("Not an Exynos4 media device."); + return NULL; + } + + /* Obtain the node name of the opened device */ + ret = media_get_devname_by_fd(fd, video_devname); + if (ret < 0) { + V4L2_EXYNOS4_ERR("Failed to get video device node name."); + return NULL; + } + + /* +* Create the representation of a media device +* containing the opened video device. +*/ + media = media_device_new_by_entity_devname(video_devname); + if (media == NULL) { + V4L2_EXYNOS4_ERR("Failed to create media device."); + return NULL; + } + +#ifdef DEBUG + media_debug_set_handler(media, (void (*)(void *, ...))fprintf, stdout); +#endif + + /* Get the entity representing the opened video device node */ + sink_entity = media_get_entity_by_devname(media, video_devname, strlen(video_devname)); Could you use the fd directly instead of translating that to the device node? fstat(2) gives you directly inode / device major + minor which you can then use to find the MC device. After trying to switch it as you requested I decided to stay by current implementation to avoid the need for translating fd to device node twice. If we changed: media_device_new_by_entity_devname -> media_device_new_by_entity_fd then media_device_new_by_entity_fd would have to call fstat to find out the entity node name. Nonetheless we would have to call fstat one more time to obtain sink_entity to be passed below to media_entity_get_name. Therefore, it is better to obtain devname once and use it for both creating media_device and obtaining sink_entity node name. + if (sink_entity == NULL) { + V4L2_EXYNOS4_ERR("Failed to get sinkd entity name."); + goto err_get_sink_entity; + } + + /* The last entity in the pipeline represents video device node */ + media_entity_set_fd(sink_entity, fd); + + sink_entity_name = media_entity_get_name(sink_entity); + + /* Check if video entity is of capture type, not m2m */ + if (!__capture_entity(sink_entity_name)) { + V4L2_EXYNOS4_ERR("Device not of capture type."); + goto err_get_sink_entity; + } -- 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/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera
Hi Sakari, Thanks for the review. On 11/27/2014 09:41 AM, Sakari Ailus wrote: Hi Jacek, On Fri, Nov 21, 2014 at 05:14:40PM +0100, Jacek Anaszewski wrote: The plugin provides support for the media device on Exynos4 SoC. It performs single plane <-> multi plane API conversion, video pipeline linking and takes care of automatic data format negotiation for the whole pipeline, after intercepting VIDIOC_S_FMT or VIDIOC_TRY_FMT ioctls. Signed-off-by: Jacek Anaszewski Acked-by: Kyungmin Park --- configure.ac |1 + lib/Makefile.am |7 +- lib/libv4l-exynos4-camera/Makefile.am |7 + lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c | 595 + 4 files changed, 609 insertions(+), 1 deletion(-) create mode 100644 lib/libv4l-exynos4-camera/Makefile.am create mode 100644 lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c diff --git a/configure.ac b/configure.ac index c9b0524..ae653b9 100644 --- a/configure.ac +++ b/configure.ac @@ -17,6 +17,7 @@ AC_CONFIG_FILES([Makefile lib/libdvbv5/Makefile lib/libv4l2rds/Makefile lib/libv4l-mplane/Makefile + lib/libv4l-exynos4-camera/Makefile utils/Makefile utils/libv4l2util/Makefile diff --git a/lib/Makefile.am b/lib/Makefile.am index 3a0e19c..56b3a9f 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -5,7 +5,12 @@ SUBDIRS = \ libv4l2rds \ libv4l-mplane +if WITH_V4LUTILS +SUBDIRS += \ + libv4l-exynos4-camera +endif + if LINUX_OS SUBDIRS += \ libdvbv5 -endif \ No newline at end of file +endif diff --git a/lib/libv4l-exynos4-camera/Makefile.am b/lib/libv4l-exynos4-camera/Makefile.am new file mode 100644 index 000..23c60c6 --- /dev/null +++ b/lib/libv4l-exynos4-camera/Makefile.am @@ -0,0 +1,7 @@ +if WITH_V4L_PLUGINS +libv4l2plugin_LTLIBRARIES = libv4l-exynos4-camera.la +endif + +libv4l_exynos4_camera_la_SOURCES = libv4l-exynos4-camera.c ../../utils/media-ctl/libmediactl.c ../../utils/media-ctl/libv4l2subdev.c ../../utils/media-ctl/libv4l2media_ioctl.c ../../utils/media-ctl/mediatext.c +libv4l_exynos4_camera_la_CFLAGS = -fvisibility=hidden -std=gnu99 +libv4l_exynos4_camera_la_LDFLAGS = -avoid-version -module -shared -export-dynamic -lpthread diff --git a/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c b/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c new file mode 100644 index 000..119c75c --- /dev/null +++ b/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c @@ -0,0 +1,595 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * Author: Jacek Anaszewski + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation; either version 2.1 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "../../utils/media-ctl/libv4l2media_ioctl.h" +#include "../../utils/media-ctl/mediactl.h" +#include "../../utils/media-ctl/mediatext.h" +#include "../../utils/media-ctl/v4l2subdev.h" +#include "libv4l-plugin.h" + +struct media_device; +struct media_entity; + +/* + * struct exynos4_camera_plugin - libv4l exynos4 camera plugin + * @media: media device comprising the vid_fd related video device + */ +struct exynos4_camera_plugin { + struct media_device *media; +}; + +#ifdef DEBUG +#define V4L2_EXYNOS4_DBG(format, ARG...)\ + printf("[%s:%d] [%s] " format " \n", __FILE__, __LINE__, __func__, ##ARG) +#else +#define V4L2_EXYNOS4_DBG(format, ARG...) +#endif + +#define V4L2_EXYNOS4_ERR(format, ARG...)\ + fprintf(stderr, "Libv4l Exynos4 camera plugin: "format "\n", ##ARG) + +#define V4L2_EXYNOS4_LOG(format, ARG...)\ + fprintf(stdout, "Libv4l Exynos4 camera plugin: "format "\n", ##ARG) + +#if HAVE_VISIBILITY +#define PLUGIN_PUBLIC __attribute__ ((visibility("default"))) +#else +#define PLUGIN_PUBLIC +#endif + +#define SYS_IOCTL(fd, cmd, arg) \ + syscall(SYS_ioctl, (int)(fd), (unsigned long)(cmd), (void *)(arg)) +#define SIMPLE_CONVERT_IOCTL(fd, cmd, arg, __struc) ({ \ + int __ret; \ + struct __struc *req = arg; \ + uint32_t type = req->type; \ + req->type = convert_type(type); \ + __ret = SYS_IOCTL(fd, cmd, arg);\ + req->type = type; \ + __ret; \ + }) + +#define
Re: [PATCH/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera
Hi Jacek, On Fri, Nov 21, 2014 at 05:14:40PM +0100, Jacek Anaszewski wrote: > The plugin provides support for the media device on Exynos4 SoC. > It performs single plane <-> multi plane API conversion, > video pipeline linking and takes care of automatic data format > negotiation for the whole pipeline, after intercepting > VIDIOC_S_FMT or VIDIOC_TRY_FMT ioctls. > > Signed-off-by: Jacek Anaszewski > Acked-by: Kyungmin Park > --- > configure.ac |1 + > lib/Makefile.am |7 +- > lib/libv4l-exynos4-camera/Makefile.am |7 + > lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c | 595 > + > 4 files changed, 609 insertions(+), 1 deletion(-) > create mode 100644 lib/libv4l-exynos4-camera/Makefile.am > create mode 100644 lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c > > diff --git a/configure.ac b/configure.ac > index c9b0524..ae653b9 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -17,6 +17,7 @@ AC_CONFIG_FILES([Makefile > lib/libdvbv5/Makefile > lib/libv4l2rds/Makefile > lib/libv4l-mplane/Makefile > + lib/libv4l-exynos4-camera/Makefile > > utils/Makefile > utils/libv4l2util/Makefile > diff --git a/lib/Makefile.am b/lib/Makefile.am > index 3a0e19c..56b3a9f 100644 > --- a/lib/Makefile.am > +++ b/lib/Makefile.am > @@ -5,7 +5,12 @@ SUBDIRS = \ > libv4l2rds \ > libv4l-mplane > > +if WITH_V4LUTILS > +SUBDIRS += \ > + libv4l-exynos4-camera > +endif > + > if LINUX_OS > SUBDIRS += \ > libdvbv5 > -endif > \ No newline at end of file > +endif > diff --git a/lib/libv4l-exynos4-camera/Makefile.am > b/lib/libv4l-exynos4-camera/Makefile.am > new file mode 100644 > index 000..23c60c6 > --- /dev/null > +++ b/lib/libv4l-exynos4-camera/Makefile.am > @@ -0,0 +1,7 @@ > +if WITH_V4L_PLUGINS > +libv4l2plugin_LTLIBRARIES = libv4l-exynos4-camera.la > +endif > + > +libv4l_exynos4_camera_la_SOURCES = libv4l-exynos4-camera.c > ../../utils/media-ctl/libmediactl.c ../../utils/media-ctl/libv4l2subdev.c > ../../utils/media-ctl/libv4l2media_ioctl.c ../../utils/media-ctl/mediatext.c > +libv4l_exynos4_camera_la_CFLAGS = -fvisibility=hidden -std=gnu99 > +libv4l_exynos4_camera_la_LDFLAGS = -avoid-version -module -shared > -export-dynamic -lpthread > diff --git a/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c > b/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c > new file mode 100644 > index 000..119c75c > --- /dev/null > +++ b/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c > @@ -0,0 +1,595 @@ > +/* > + * Copyright (c) 2014 Samsung Electronics Co., Ltd. > + * http://www.samsung.com > + * > + * Author: Jacek Anaszewski > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU Lesser General Public License as published > by > + * the Free Software Foundation; either version 2.1 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > + > +#include "../../utils/media-ctl/libv4l2media_ioctl.h" > +#include "../../utils/media-ctl/mediactl.h" > +#include "../../utils/media-ctl/mediatext.h" > +#include "../../utils/media-ctl/v4l2subdev.h" > +#include "libv4l-plugin.h" > + > +struct media_device; > +struct media_entity; > + > +/* > + * struct exynos4_camera_plugin - libv4l exynos4 camera plugin > + * @media: media device comprising the vid_fd related video device > + */ > +struct exynos4_camera_plugin { > + struct media_device *media; > +}; > + > +#ifdef DEBUG > +#define V4L2_EXYNOS4_DBG(format, ARG...)\ > + printf("[%s:%d] [%s] " format " \n", __FILE__, __LINE__, __func__, > ##ARG) > +#else > +#define V4L2_EXYNOS4_DBG(format, ARG...) > +#endif > + > +#define V4L2_EXYNOS4_ERR(format, ARG...)\ > + fprintf(stderr, "Libv4l Exynos4 camera plugin: "format "\n", ##ARG) > + > +#define V4L2_EXYNOS4_LOG(format, ARG...)\ > + fprintf(stdout, "Libv4l Exynos4 camera plugin: "format "\n", ##ARG) > + > +#if HAVE_VISIBILITY > +#define PLUGIN_PUBLIC __attribute__ ((visibility("default"))) > +#else > +#define PLUGIN_PUBLIC > +#endif > + > +#define SYS_IOCTL(fd, cmd, arg) \ > + syscall(SYS_ioctl, (int)(fd), (unsigned long)(cmd), (void *)(arg)) > +#define SIMPLE_CONVERT_IOCTL(fd, cmd, arg, __struc) ({ \ > + int __ret; \ > + struct __struc *req = arg; \ > + uint32_t type = req->type; \ > + req->type = convert_type(type); \ > + __ret = SYS_IOCTL(fd, cm