Re: [PATCH 2/3] v4l: Add V4L2_SEL_TGT_NATIVE_SIZE selection target

2014-11-17 Thread Hans Verkuil
On 11/16/2014 05:40 PM, Sakari Ailus wrote:
 Hi Hans,
 
 Thank you for the review.
 
 On Fri, Nov 14, 2014 at 09:46:54AM +0100, Hans Verkuil wrote:
 On 11/09/2014 12:04 AM, Sakari Ailus wrote:
 The V4L2_SEL_TGT_NATIVE_SIZE target is used to denote e.g. the size of a
 sensor's pixel array.

 Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
 ---
  Documentation/DocBook/media/v4l/selections-common.xml |8 
  include/uapi/linux/v4l2-common.h  |2 ++
  2 files changed, 10 insertions(+)

 diff --git a/Documentation/DocBook/media/v4l/selections-common.xml 
 b/Documentation/DocBook/media/v4l/selections-common.xml
 index 7502f78..5fc833a 100644
 --- a/Documentation/DocBook/media/v4l/selections-common.xml
 +++ b/Documentation/DocBook/media/v4l/selections-common.xml
 @@ -63,6 +63,14 @@
 entryYes/entry
   /row
   row
 +   entryconstantV4L2_SEL_TGT_NATIVE_SIZE/constant/entry
 +   entry0x0003/entry
 +   entryThe native size of the device, e.g. a sensor's
 +   pixel array./entry

 You might want to state that top and left are always 0.
 
 Fixed. I also added a patch to fix this in the smiapp driver --- the values
 were uninitialised. :-P
 
 +   entryYes/entry
 +   entryYes/entry
 + /row
 + row
 entryconstantV4L2_SEL_TGT_COMPOSE/constant/entry
 entry0x0100/entry
 entryCompose rectangle. Used to configure scaling
 diff --git a/include/uapi/linux/v4l2-common.h 
 b/include/uapi/linux/v4l2-common.h
 index 2f6f8ca..1527398 100644
 --- a/include/uapi/linux/v4l2-common.h
 +++ b/include/uapi/linux/v4l2-common.h
 @@ -43,6 +43,8 @@
  #define V4L2_SEL_TGT_CROP_DEFAULT  0x0001
  /* Cropping bounds */
  #define V4L2_SEL_TGT_CROP_BOUNDS   0x0002
 +/* Native frame size */
 +#define V4L2_SEL_TGT_NATIVE_SIZE   0x0003
  /* Current composing area */
  #define V4L2_SEL_TGT_COMPOSE   0x0100
  /* Default composing area */


 I like this. This would also make it possible to set the 'canvas' size of an
 mem2mem device. Currently calling S_FMT for a mem2mem device cannot setup any
 scaler since there is no native size. Instead S_FMT effectively *sets* the 
 native
 size. The same is true for webcams with a scaler, which is why you added 
 this in
 the first place. Obviously for sensors this target is read-only, but for a 
 mem2mem
 device it can be writable as well.

 However, to make full use of this you also need to add input and output
 capabilities if the native size can be set:

  V4L2_IN_CAP_NATIVE_SIZE
  V4L2_OUT_CAP_NATIVE_SIZE
 
 Do you think this would require a capability flag, rather than just
 returning an error if the target is unsettable, as we otherwise already do
 if a selection target isn't supported? For the compound controls it's even
 easier, you just don't have a read-only flag set in the equivalent control.

No, I really want a capability flag here. Otherwise applications would have to
call ENUMINPUT *and* call G_SELECTION followed by S_SELECTION just to test if
it can be set. Besides, this is also a per-input capability, so you want to get
hold of the capabilities without having to do a S_INPUT first. I.e. you don't
want to have to do this:

// pseudo code just to give the idea
for (i = 0; i  max_input; i++) {
struct v4l2_selection sel = { NATIVE_SIZE };

ioctl(S_INPUT, i);
ioctl(G_SELECTION, sel);
if (ioctl(S_SELECTION, sel))
// does not support NATIVE_SIZE
}

 
 (see ENUMINPUT/ENUMOUTPUT)

 This would nicely fill in a hole in the V4L2 Spec.
 

Regards,

Hans
--
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 2/3] v4l: Add V4L2_SEL_TGT_NATIVE_SIZE selection target

2014-11-17 Thread Sakari Ailus
Hi Hans,

On Mon, Nov 17, 2014 at 10:34:37AM +0100, Hans Verkuil wrote:
 On 11/16/2014 05:40 PM, Sakari Ailus wrote:
  Hi Hans,
  
  Thank you for the review.
  
  On Fri, Nov 14, 2014 at 09:46:54AM +0100, Hans Verkuil wrote:
  On 11/09/2014 12:04 AM, Sakari Ailus wrote:
  The V4L2_SEL_TGT_NATIVE_SIZE target is used to denote e.g. the size of a
  sensor's pixel array.
 
  Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
  ---
   Documentation/DocBook/media/v4l/selections-common.xml |8 
   include/uapi/linux/v4l2-common.h  |2 ++
   2 files changed, 10 insertions(+)
 
  diff --git a/Documentation/DocBook/media/v4l/selections-common.xml 
  b/Documentation/DocBook/media/v4l/selections-common.xml
  index 7502f78..5fc833a 100644
  --- a/Documentation/DocBook/media/v4l/selections-common.xml
  +++ b/Documentation/DocBook/media/v4l/selections-common.xml
  @@ -63,6 +63,14 @@
entryYes/entry
  /row
  row
  + entryconstantV4L2_SEL_TGT_NATIVE_SIZE/constant/entry
  + entry0x0003/entry
  + entryThe native size of the device, e.g. a sensor's
  + pixel array./entry
 
  You might want to state that top and left are always 0.
  
  Fixed. I also added a patch to fix this in the smiapp driver --- the values
  were uninitialised. :-P
  
  + entryYes/entry
  + entryYes/entry
  +   /row
  +   row
entryconstantV4L2_SEL_TGT_COMPOSE/constant/entry
entry0x0100/entry
entryCompose rectangle. Used to configure scaling
  diff --git a/include/uapi/linux/v4l2-common.h 
  b/include/uapi/linux/v4l2-common.h
  index 2f6f8ca..1527398 100644
  --- a/include/uapi/linux/v4l2-common.h
  +++ b/include/uapi/linux/v4l2-common.h
  @@ -43,6 +43,8 @@
   #define V4L2_SEL_TGT_CROP_DEFAULT0x0001
   /* Cropping bounds */
   #define V4L2_SEL_TGT_CROP_BOUNDS 0x0002
  +/* Native frame size */
  +#define V4L2_SEL_TGT_NATIVE_SIZE 0x0003
   /* Current composing area */
   #define V4L2_SEL_TGT_COMPOSE 0x0100
   /* Default composing area */
 
 
  I like this. This would also make it possible to set the 'canvas' size of 
  an
  mem2mem device. Currently calling S_FMT for a mem2mem device cannot setup 
  any
  scaler since there is no native size. Instead S_FMT effectively *sets* the 
  native
  size. The same is true for webcams with a scaler, which is why you added 
  this in
  the first place. Obviously for sensors this target is read-only, but for a 
  mem2mem
  device it can be writable as well.
 
  However, to make full use of this you also need to add input and output
  capabilities if the native size can be set:
 
 V4L2_IN_CAP_NATIVE_SIZE
 V4L2_OUT_CAP_NATIVE_SIZE
  
  Do you think this would require a capability flag, rather than just
  returning an error if the target is unsettable, as we otherwise already do
  if a selection target isn't supported? For the compound controls it's even
  easier, you just don't have a read-only flag set in the equivalent control.
 
 No, I really want a capability flag here. Otherwise applications would have to
 call ENUMINPUT *and* call G_SELECTION followed by S_SELECTION just to test if
 it can be set. Besides, this is also a per-input capability, so you want to 
 get
 hold of the capabilities without having to do a S_INPUT first. I.e. you don't
 want to have to do this:
 
   // pseudo code just to give the idea
   for (i = 0; i  max_input; i++) {
   struct v4l2_selection sel = { NATIVE_SIZE };
 
   ioctl(S_INPUT, i);
   ioctl(G_SELECTION, sel);
   if (ioctl(S_SELECTION, sel))
   // does not support NATIVE_SIZE
   }

Fair enough. I'll add a patch for that.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: 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: [PATCH 2/3] v4l: Add V4L2_SEL_TGT_NATIVE_SIZE selection target

2014-11-16 Thread Sakari Ailus
Hi Hans,

Thank you for the review.

On Fri, Nov 14, 2014 at 09:46:54AM +0100, Hans Verkuil wrote:
 On 11/09/2014 12:04 AM, Sakari Ailus wrote:
  The V4L2_SEL_TGT_NATIVE_SIZE target is used to denote e.g. the size of a
  sensor's pixel array.
  
  Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
  ---
   Documentation/DocBook/media/v4l/selections-common.xml |8 
   include/uapi/linux/v4l2-common.h  |2 ++
   2 files changed, 10 insertions(+)
  
  diff --git a/Documentation/DocBook/media/v4l/selections-common.xml 
  b/Documentation/DocBook/media/v4l/selections-common.xml
  index 7502f78..5fc833a 100644
  --- a/Documentation/DocBook/media/v4l/selections-common.xml
  +++ b/Documentation/DocBook/media/v4l/selections-common.xml
  @@ -63,6 +63,14 @@
  entryYes/entry
/row
row
  +   entryconstantV4L2_SEL_TGT_NATIVE_SIZE/constant/entry
  +   entry0x0003/entry
  +   entryThe native size of the device, e.g. a sensor's
  +   pixel array./entry
 
 You might want to state that top and left are always 0.

Fixed. I also added a patch to fix this in the smiapp driver --- the values
were uninitialised. :-P

  +   entryYes/entry
  +   entryYes/entry
  + /row
  + row
  entryconstantV4L2_SEL_TGT_COMPOSE/constant/entry
  entry0x0100/entry
  entryCompose rectangle. Used to configure scaling
  diff --git a/include/uapi/linux/v4l2-common.h 
  b/include/uapi/linux/v4l2-common.h
  index 2f6f8ca..1527398 100644
  --- a/include/uapi/linux/v4l2-common.h
  +++ b/include/uapi/linux/v4l2-common.h
  @@ -43,6 +43,8 @@
   #define V4L2_SEL_TGT_CROP_DEFAULT  0x0001
   /* Cropping bounds */
   #define V4L2_SEL_TGT_CROP_BOUNDS   0x0002
  +/* Native frame size */
  +#define V4L2_SEL_TGT_NATIVE_SIZE   0x0003
   /* Current composing area */
   #define V4L2_SEL_TGT_COMPOSE   0x0100
   /* Default composing area */
  
 
 I like this. This would also make it possible to set the 'canvas' size of an
 mem2mem device. Currently calling S_FMT for a mem2mem device cannot setup any
 scaler since there is no native size. Instead S_FMT effectively *sets* the 
 native
 size. The same is true for webcams with a scaler, which is why you added this 
 in
 the first place. Obviously for sensors this target is read-only, but for a 
 mem2mem
 device it can be writable as well.
 
 However, to make full use of this you also need to add input and output
 capabilities if the native size can be set:
 
   V4L2_IN_CAP_NATIVE_SIZE
   V4L2_OUT_CAP_NATIVE_SIZE

Do you think this would require a capability flag, rather than just
returning an error if the target is unsettable, as we otherwise already do
if a selection target isn't supported? For the compound controls it's even
easier, you just don't have a read-only flag set in the equivalent control.

 (see ENUMINPUT/ENUMOUTPUT)
 
 This would nicely fill in a hole in the V4L2 Spec.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: 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: [PATCH 2/3] v4l: Add V4L2_SEL_TGT_NATIVE_SIZE selection target

2014-11-14 Thread Hans Verkuil
On 11/09/2014 12:04 AM, Sakari Ailus wrote:
 The V4L2_SEL_TGT_NATIVE_SIZE target is used to denote e.g. the size of a
 sensor's pixel array.
 
 Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
 ---
  Documentation/DocBook/media/v4l/selections-common.xml |8 
  include/uapi/linux/v4l2-common.h  |2 ++
  2 files changed, 10 insertions(+)
 
 diff --git a/Documentation/DocBook/media/v4l/selections-common.xml 
 b/Documentation/DocBook/media/v4l/selections-common.xml
 index 7502f78..5fc833a 100644
 --- a/Documentation/DocBook/media/v4l/selections-common.xml
 +++ b/Documentation/DocBook/media/v4l/selections-common.xml
 @@ -63,6 +63,14 @@
   entryYes/entry
 /row
 row
 + entryconstantV4L2_SEL_TGT_NATIVE_SIZE/constant/entry
 + entry0x0003/entry
 + entryThe native size of the device, e.g. a sensor's
 + pixel array./entry

You might want to state that top and left are always 0.

 + entryYes/entry
 + entryYes/entry
 +   /row
 +   row
   entryconstantV4L2_SEL_TGT_COMPOSE/constant/entry
   entry0x0100/entry
   entryCompose rectangle. Used to configure scaling
 diff --git a/include/uapi/linux/v4l2-common.h 
 b/include/uapi/linux/v4l2-common.h
 index 2f6f8ca..1527398 100644
 --- a/include/uapi/linux/v4l2-common.h
 +++ b/include/uapi/linux/v4l2-common.h
 @@ -43,6 +43,8 @@
  #define V4L2_SEL_TGT_CROP_DEFAULT0x0001
  /* Cropping bounds */
  #define V4L2_SEL_TGT_CROP_BOUNDS 0x0002
 +/* Native frame size */
 +#define V4L2_SEL_TGT_NATIVE_SIZE 0x0003
  /* Current composing area */
  #define V4L2_SEL_TGT_COMPOSE 0x0100
  /* Default composing area */
 

I like this. This would also make it possible to set the 'canvas' size of an
mem2mem device. Currently calling S_FMT for a mem2mem device cannot setup any
scaler since there is no native size. Instead S_FMT effectively *sets* the 
native
size. The same is true for webcams with a scaler, which is why you added this in
the first place. Obviously for sensors this target is read-only, but for a 
mem2mem
device it can be writable as well.

However, to make full use of this you also need to add input and output
capabilities if the native size can be set:

V4L2_IN_CAP_NATIVE_SIZE
V4L2_OUT_CAP_NATIVE_SIZE

(see ENUMINPUT/ENUMOUTPUT)

This would nicely fill in a hole in the V4L2 Spec.

Regards,

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


[PATCH 2/3] v4l: Add V4L2_SEL_TGT_NATIVE_SIZE selection target

2014-11-08 Thread Sakari Ailus
The V4L2_SEL_TGT_NATIVE_SIZE target is used to denote e.g. the size of a
sensor's pixel array.

Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
 Documentation/DocBook/media/v4l/selections-common.xml |8 
 include/uapi/linux/v4l2-common.h  |2 ++
 2 files changed, 10 insertions(+)

diff --git a/Documentation/DocBook/media/v4l/selections-common.xml 
b/Documentation/DocBook/media/v4l/selections-common.xml
index 7502f78..5fc833a 100644
--- a/Documentation/DocBook/media/v4l/selections-common.xml
+++ b/Documentation/DocBook/media/v4l/selections-common.xml
@@ -63,6 +63,14 @@
entryYes/entry
  /row
  row
+   entryconstantV4L2_SEL_TGT_NATIVE_SIZE/constant/entry
+   entry0x0003/entry
+   entryThe native size of the device, e.g. a sensor's
+   pixel array./entry
+   entryYes/entry
+   entryYes/entry
+ /row
+ row
entryconstantV4L2_SEL_TGT_COMPOSE/constant/entry
entry0x0100/entry
entryCompose rectangle. Used to configure scaling
diff --git a/include/uapi/linux/v4l2-common.h b/include/uapi/linux/v4l2-common.h
index 2f6f8ca..1527398 100644
--- a/include/uapi/linux/v4l2-common.h
+++ b/include/uapi/linux/v4l2-common.h
@@ -43,6 +43,8 @@
 #define V4L2_SEL_TGT_CROP_DEFAULT  0x0001
 /* Cropping bounds */
 #define V4L2_SEL_TGT_CROP_BOUNDS   0x0002
+/* Native frame size */
+#define V4L2_SEL_TGT_NATIVE_SIZE   0x0003
 /* Current composing area */
 #define V4L2_SEL_TGT_COMPOSE   0x0100
 /* Default composing area */
-- 
1.7.10.4

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