Re: [PATCH/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera

2015-03-16 Thread Jacek Anaszewski

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

2015-03-15 Thread Gregor Jasny
> 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

2015-03-15 Thread Gregor Jasny
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

2015-02-27 Thread Jacek Anaszewski

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

2014-11-28 Thread Jacek Anaszewski

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

2014-11-27 Thread Sakari Ailus
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