Re: [media-ctl PATCH 1/1] libv4l2subdev: Add v4l2_subdev_enum_mbus_code()

2012-08-01 Thread Sakari Ailus
Hi Laurent,

On Tue, Jul 31, 2012 at 11:50:53PM +0200, Laurent Pinchart wrote:
 On Tuesday 31 July 2012 15:17:04 Sakari Ailus wrote:
  On Tue, Jul 31, 2012 at 01:38:41PM +0200, Laurent Pinchart wrote:
   On Tuesday 31 July 2012 01:16:00 Sakari Ailus wrote:
v4l2_subdev_enum_mbus_code() enumerates over supported media bus formats
on a pad.

Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---

 src/v4l2subdev.c |   23 +++
 src/v4l2subdev.h |   14 ++
 2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/src/v4l2subdev.c b/src/v4l2subdev.c
index d60bd7e..6b6df0a 100644
--- a/src/v4l2subdev.c
+++ b/src/v4l2subdev.c
@@ -58,6 +58,29 @@ void v4l2_subdev_close(struct media_entity *entity)

entity-fd = -1;
 
 }

+int v4l2_subdev_enum_mbus_code(struct media_entity *entity,
+  uint32_t *code, uint32_t pad, uint32_t 
index)
   
   I would use unsigned int for the pad and index arguments to match the
   other functions. We could then fix all of them in one go to use stdint
   types to match the kernel API types.
  
  I'm fine with that.
  
+{
+   struct v4l2_subdev_mbus_code_enum c;
+   int ret;
+
+   ret = v4l2_subdev_open(entity);
+   if (ret  0)
+   return ret;
+
+   memset(c, 0, sizeof(c));
+   c.pad = pad;
+   c.index = index;
+
+   ret = ioctl(entity-fd, VIDIOC_SUBDEV_ENUM_MBUS_CODE, c);
+   if (ret  0)
+   return -errno;
+
+   *code = c.code;
+
+   return 0;
+}
   
   What about a higher-level API that would enumerate all formats and return
   a list/array ?
  
  The information could be stored to media entities. We could add a V4L2
  subdev pointer to media entities, and have the information stored there the
  first time this function is called. How about that?
  
  On source pads the pixel code is obviously possibly dependent on the pixel
  code on the sink pad so I need to store mappings from sink pad pixel code to
  a list of source pad pixel code, but can it have other dependencies? None
  come to mind right now, though.
 
 I would make this function store the enumerated mbus codes in the media 
 entity, as the codes on a source pad could depend on the selected code on a 
 sink pad. I would instead make the function allocate an array, fill it with 

(After a short discussion it has been confirmed that  not should be added
somewhere to the above sentence.)

 media bus codes and return it. The caller would be responsible for freeing it 
 (possibly later, after storing it).

I like this option better as it forces the user to make a decision how long
to keep the enumeration around. Changing the sink pad's try format is one
thing that may invalidate the enumeration on a source pad, but that doesn't
rule out other reasons since they may be hardware dependent.

So I'll change it so.

Cheers,

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [media-ctl PATCH 1/1] libv4l2subdev: Add v4l2_subdev_enum_mbus_code()

2012-07-31 Thread Sakari Ailus
Hi Laurent,

On Tue, Jul 31, 2012 at 01:38:41PM +0200, Laurent Pinchart wrote:
 Thanks for the patch.

Thanks for the comments!

 On Tuesday 31 July 2012 01:16:00 Sakari Ailus wrote:
  v4l2_subdev_enum_mbus_code() enumerates over supported media bus formats on
  a pad.
  
  Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
  ---
   src/v4l2subdev.c |   23 +++
   src/v4l2subdev.h |   14 ++
   2 files changed, 37 insertions(+), 0 deletions(-)
  
  diff --git a/src/v4l2subdev.c b/src/v4l2subdev.c
  index d60bd7e..6b6df0a 100644
  --- a/src/v4l2subdev.c
  +++ b/src/v4l2subdev.c
  @@ -58,6 +58,29 @@ void v4l2_subdev_close(struct media_entity *entity)
  entity-fd = -1;
   }
  
  +int v4l2_subdev_enum_mbus_code(struct media_entity *entity,
  +  uint32_t *code, uint32_t pad, uint32_t index)
 
 I would use unsigned int for the pad and index arguments to match the other 
 functions. We could then fix all of them in one go to use stdint types to 
 match the kernel API types.

I'm fine with that.

  +{
  +   struct v4l2_subdev_mbus_code_enum c;
  +   int ret;
  +
  +   ret = v4l2_subdev_open(entity);
  +   if (ret  0)
  +   return ret;
  +
  +   memset(c, 0, sizeof(c));
  +   c.pad = pad;
  +   c.index = index;
  +
  +   ret = ioctl(entity-fd, VIDIOC_SUBDEV_ENUM_MBUS_CODE, c);
  +   if (ret  0)
  +   return -errno;
  +
  +   *code = c.code;
  +
  +   return 0;
  +}
 
 What about a higher-level API that would enumerate all formats and return a 
 list/array ?

The information could be stored to media entities. We could add a V4L2
subdev pointer to media entities, and have the information stored there the
first time this function is called. How about that?

On source pads the pixel code is obviously possibly dependent on the pixel
code on the sink pad so I need to store mappings from sink pad pixel code to
a list of source pad pixel code, but can it have other dependencies? None
come to mind right now, though.

Cheers,

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [media-ctl PATCH 1/1] libv4l2subdev: Add v4l2_subdev_enum_mbus_code()

2012-07-31 Thread Laurent Pinchart
Hi Sakari,

On Tuesday 31 July 2012 15:17:04 Sakari Ailus wrote:
 On Tue, Jul 31, 2012 at 01:38:41PM +0200, Laurent Pinchart wrote:
  On Tuesday 31 July 2012 01:16:00 Sakari Ailus wrote:
   v4l2_subdev_enum_mbus_code() enumerates over supported media bus formats
   on a pad.
   
   Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
   ---
   
src/v4l2subdev.c |   23 +++
src/v4l2subdev.h |   14 ++
2 files changed, 37 insertions(+), 0 deletions(-)
   
   diff --git a/src/v4l2subdev.c b/src/v4l2subdev.c
   index d60bd7e..6b6df0a 100644
   --- a/src/v4l2subdev.c
   +++ b/src/v4l2subdev.c
   @@ -58,6 +58,29 @@ void v4l2_subdev_close(struct media_entity *entity)
   
 entity-fd = -1;

}
   
   +int v4l2_subdev_enum_mbus_code(struct media_entity *entity,
   +uint32_t *code, uint32_t pad, uint32_t index)
  
  I would use unsigned int for the pad and index arguments to match the
  other functions. We could then fix all of them in one go to use stdint
  types to match the kernel API types.
 
 I'm fine with that.
 
   +{
   + struct v4l2_subdev_mbus_code_enum c;
   + int ret;
   +
   + ret = v4l2_subdev_open(entity);
   + if (ret  0)
   + return ret;
   +
   + memset(c, 0, sizeof(c));
   + c.pad = pad;
   + c.index = index;
   +
   + ret = ioctl(entity-fd, VIDIOC_SUBDEV_ENUM_MBUS_CODE, c);
   + if (ret  0)
   + return -errno;
   +
   + *code = c.code;
   +
   + return 0;
   +}
  
  What about a higher-level API that would enumerate all formats and return
  a list/array ?
 
 The information could be stored to media entities. We could add a V4L2
 subdev pointer to media entities, and have the information stored there the
 first time this function is called. How about that?
 
 On source pads the pixel code is obviously possibly dependent on the pixel
 code on the sink pad so I need to store mappings from sink pad pixel code to
 a list of source pad pixel code, but can it have other dependencies? None
 come to mind right now, though.

I would make this function store the enumerated mbus codes in the media 
entity, as the codes on a source pad could depend on the selected code on a 
sink pad. I would instead make the function allocate an array, fill it with 
media bus codes and return it. The caller would be responsible for freeing it 
(possibly later, after storing it).

-- 
Regards,

Laurent Pinchart

--
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


[media-ctl PATCH 1/1] libv4l2subdev: Add v4l2_subdev_enum_mbus_code()

2012-07-30 Thread Sakari Ailus
v4l2_subdev_enum_mbus_code() enumerates over supported media bus formats on
a pad.

Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
 src/v4l2subdev.c |   23 +++
 src/v4l2subdev.h |   14 ++
 2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/src/v4l2subdev.c b/src/v4l2subdev.c
index d60bd7e..6b6df0a 100644
--- a/src/v4l2subdev.c
+++ b/src/v4l2subdev.c
@@ -58,6 +58,29 @@ void v4l2_subdev_close(struct media_entity *entity)
entity-fd = -1;
 }
 
+int v4l2_subdev_enum_mbus_code(struct media_entity *entity,
+  uint32_t *code, uint32_t pad, uint32_t index)
+{
+   struct v4l2_subdev_mbus_code_enum c;
+   int ret;
+
+   ret = v4l2_subdev_open(entity);
+   if (ret  0)
+   return ret;
+
+   memset(c, 0, sizeof(c));
+   c.pad = pad;
+   c.index = index;
+
+   ret = ioctl(entity-fd, VIDIOC_SUBDEV_ENUM_MBUS_CODE, c);
+   if (ret  0)
+   return -errno;
+
+   *code = c.code;
+
+   return 0;
+}
+
 int v4l2_subdev_get_format(struct media_entity *entity,
struct v4l2_mbus_framefmt *format, unsigned int pad,
enum v4l2_subdev_format_whence which)
diff --git a/src/v4l2subdev.h b/src/v4l2subdev.h
index 5d55482..1cca7b9 100644
--- a/src/v4l2subdev.h
+++ b/src/v4l2subdev.h
@@ -22,6 +22,7 @@
 #ifndef __SUBDEV_H__
 #define __SUBDEV_H__
 
+#include stdint.h
 #include linux/v4l2-subdev.h
 
 struct media_entity;
@@ -47,6 +48,19 @@ int v4l2_subdev_open(struct media_entity *entity);
 void v4l2_subdev_close(struct media_entity *entity);
 
 /**
+ * @brief Enumerate mbus pixel codes.
+ * @param entity - subdev-device media entity.
+ * @param code - mbus pixel code
+ *
+ * Enumerate media bus pixel codes. This is just a wrapper for
+ * VIDIOC_SUBDEV_ENUM_MBUS_CODE IOCTL.
+ *
+ * @return 0 on success, or a negative error code on failure.
+ */
+int v4l2_subdev_enum_mbus_code(struct media_entity *entity,
+  uint32_t *code, uint32_t pad, uint32_t index);
+
+/**
  * @brief Retrieve the format on a pad.
  * @param entity - subdev-device media entity.
  * @param format - format to be filled.
-- 
1.7.2.5

--
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