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 @@
> >         <entry>Yes</entry>
> >       </row>
> >       <row>
> > +       <entry><constant>V4L2_SEL_TGT_NATIVE_SIZE</constant></entry>
> > +       <entry>0x0003</entry>
> > +       <entry>The 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

> > +       <entry>Yes</entry>
> > +       <entry>Yes</entry>
> > +     </row>
> > +     <row>
> >         <entry><constant>V4L2_SEL_TGT_COMPOSE</constant></entry>
> >         <entry>0x0100</entry>
> >         <entry>Compose 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

Reply via email to