[libcamera-devel,v3,1/3] libcamera: controls: Define a default lens position behaviour
diff mbox series

Message ID 20230605091406.31757-2-naush@raspberrypi.com
State Accepted
Headers show
Series
  • Default lens behaviour
Related show

Commit Message

Naushir Patuck June 5, 2023, 9:14 a.m. UTC
Update the AfMode control description to explicitly define a
startup/default behaviour.

On startup, the camera will move the lens to the position given by the
default value of the LensPosition control if operating in manual focus
mode. Typically this would be the hyperfocal position of the lens.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
 src/libcamera/control_ids.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Jacopo Mondi June 6, 2023, 1:22 p.m. UTC | #1
Hi Naush

On Mon, Jun 05, 2023 at 10:14:04AM +0100, Naushir Patuck via libcamera-devel wrote:
> Update the AfMode control description to explicitly define a
> startup/default behaviour.
>
> On startup, the camera will move the lens to the position given by the
> default value of the LensPosition control if operating in manual focus
> mode. Typically this would be the hyperfocal position of the lens.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>  src/libcamera/control_ids.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> index adea5f90acc5..765168e539a8 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -408,6 +408,13 @@ controls:
>              LensPosition control.
>
>              In this mode the AfState will always report AfStateIdle.
> +
> +            If the camera is started in AfModeManual, it will move the focus
> +            lens to the position specified by the LensPosition control.
> +
> +            This mode is the recommended default value for the AfMode control.
> +            External cameras (as reported by the Location property set to
> +            CameraLocationExternal) may use a different default value.

Missing an additional blank line

Can be fixed when applying

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

>          - name: AfModeAuto
>            value: 1
>            description: |
> --
> 2.34.1
>
Naushir Patuck June 6, 2023, 1:40 p.m. UTC | #2
Hi Jacopo,

Thank you for your feedback!

On Tue, 6 Jun 2023 at 14:22, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote:
>
> Hi Naush
>
> On Mon, Jun 05, 2023 at 10:14:04AM +0100, Naushir Patuck via libcamera-devel wrote:
> > Update the AfMode control description to explicitly define a
> > startup/default behaviour.
> >
> > On startup, the camera will move the lens to the position given by the
> > default value of the LensPosition control if operating in manual focus
> > mode. Typically this would be the hyperfocal position of the lens.
> >
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > ---
> >  src/libcamera/control_ids.yaml | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > index adea5f90acc5..765168e539a8 100644
> > --- a/src/libcamera/control_ids.yaml
> > +++ b/src/libcamera/control_ids.yaml
> > @@ -408,6 +408,13 @@ controls:
> >              LensPosition control.
> >
> >              In this mode the AfState will always report AfStateIdle.
> > +
> > +            If the camera is started in AfModeManual, it will move the focus
> > +            lens to the position specified by the LensPosition control.
> > +
> > +            This mode is the recommended default value for the AfMode control.
> > +            External cameras (as reported by the Location property set to
> > +            CameraLocationExternal) may use a different default value.
>
> Missing an additional blank line

I wasn't sure about this one.  The documentation for enum values doesn't seem to
have a blank line between them so I left it out here as well.  Is this
not correct?

Regards,
Naush

>
> Can be fixed when applying
>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> >          - name: AfModeAuto
> >            value: 1
> >            description: |
> > --
> > 2.34.1
> >
Jacopo Mondi June 6, 2023, 2:02 p.m. UTC | #3
Hi Naush

On Tue, Jun 06, 2023 at 02:40:38PM +0100, Naushir Patuck via libcamera-devel wrote:
> Hi Jacopo,
>
> Thank you for your feedback!
>
> On Tue, 6 Jun 2023 at 14:22, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote:
> >
> > Hi Naush
> >
> > On Mon, Jun 05, 2023 at 10:14:04AM +0100, Naushir Patuck via libcamera-devel wrote:
> > > Update the AfMode control description to explicitly define a
> > > startup/default behaviour.
> > >
> > > On startup, the camera will move the lens to the position given by the
> > > default value of the LensPosition control if operating in manual focus
> > > mode. Typically this would be the hyperfocal position of the lens.
> > >
> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > > ---
> > >  src/libcamera/control_ids.yaml | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > > index adea5f90acc5..765168e539a8 100644
> > > --- a/src/libcamera/control_ids.yaml
> > > +++ b/src/libcamera/control_ids.yaml
> > > @@ -408,6 +408,13 @@ controls:
> > >              LensPosition control.
> > >
> > >              In this mode the AfState will always report AfStateIdle.
> > > +
> > > +            If the camera is started in AfModeManual, it will move the focus
> > > +            lens to the position specified by the LensPosition control.
> > > +
> > > +            This mode is the recommended default value for the AfMode control.
> > > +            External cameras (as reported by the Location property set to
> > > +            CameraLocationExternal) may use a different default value.
> >
> > Missing an additional blank line
>
> I wasn't sure about this one.  The documentation for enum values doesn't seem to
> have a blank line between them so I left it out here as well.  Is this
> not correct?

Nope, you're right! I saw a blank line in the proposal Laurent sent
and thought it got lost, but it is indeed intentional and correct your
way

Sorry for the noise!

>
> Regards,
> Naush
>
> >
> > Can be fixed when applying
> >
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> >
> > >          - name: AfModeAuto
> > >            value: 1
> > >            description: |
> > > --
> > > 2.34.1
> > >
Laurent Pinchart June 6, 2023, 3:20 p.m. UTC | #4
Hi Naush,

Thank you for the patch.

On Mon, Jun 05, 2023 at 10:14:04AM +0100, Naushir Patuck via libcamera-devel wrote:
> Update the AfMode control description to explicitly define a
> startup/default behaviour.
> 
> On startup, the camera will move the lens to the position given by the
> default value of the LensPosition control if operating in manual focus
> mode. Typically this would be the hyperfocal position of the lens.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/libcamera/control_ids.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> index adea5f90acc5..765168e539a8 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -408,6 +408,13 @@ controls:
>              LensPosition control.
>  
>              In this mode the AfState will always report AfStateIdle.
> +
> +            If the camera is started in AfModeManual, it will move the focus
> +            lens to the position specified by the LensPosition control.
> +
> +            This mode is the recommended default value for the AfMode control.
> +            External cameras (as reported by the Location property set to
> +            CameraLocationExternal) may use a different default value.
>          - name: AfModeAuto
>            value: 1
>            description: |

Patch
diff mbox series

diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
index adea5f90acc5..765168e539a8 100644
--- a/src/libcamera/control_ids.yaml
+++ b/src/libcamera/control_ids.yaml
@@ -408,6 +408,13 @@  controls:
             LensPosition control.
 
             In this mode the AfState will always report AfStateIdle.
+
+            If the camera is started in AfModeManual, it will move the focus
+            lens to the position specified by the LensPosition control.
+
+            This mode is the recommended default value for the AfMode control.
+            External cameras (as reported by the Location property set to
+            CameraLocationExternal) may use a different default value.
         - name: AfModeAuto
           value: 1
           description: |