[libcamera-devel,v10,3/3] ipa: raspberrypi: config: Update shutter speeds for imx219/477 and ov5647
diff mbox series

Message ID 20201218100626.205134-4-naush@raspberrypi.com
State Changes Requested
Delegated to: Laurent Pinchart
Headers show
Series
  • raspberrypi: FPS control
Related show

Commit Message

Naushir Patuck Dec. 18, 2020, 10:06 a.m. UTC 
Set the maximum shutter speed for the normal exposure profile to 66ms.
Set the maximum shutter speed for the sport exposure profile to 33ms.
Add a long exposure profile to allow shutter speeds of up to 120ms.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/ipa/raspberrypi/data/imx219.json | 15 +++++++++++++--
 src/ipa/raspberrypi/data/imx477.json | 15 +++++++++++++--
 src/ipa/raspberrypi/data/ov5647.json | 15 +++++++++++++--
 3 files changed, 39 insertions(+), 6 deletions(-)

Comments

Kieran Bingham Dec. 18, 2020, 1:07 p.m. UTC | #1 
Hi Naush,

On 18/12/2020 10:06, Naushir Patuck wrote:
> Set the maximum shutter speed for the normal exposure profile to 66ms.
> Set the maximum shutter speed for the sport exposure profile to 33ms.
> Add a long exposure profile to allow shutter speeds of up to 120ms.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> Tested-by: David Plowman <david.plowman@raspberrypi.com>

A little hard to parse some of that from the diff, but that's the mostly
the diff, and the changes below seem to match what is described above.

So on that basis:

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/ipa/raspberrypi/data/imx219.json | 15 +++++++++++++--
>  src/ipa/raspberrypi/data/imx477.json | 15 +++++++++++++--
>  src/ipa/raspberrypi/data/ov5647.json | 15 +++++++++++++--
>  3 files changed, 39 insertions(+), 6 deletions(-)
> 
> diff --git a/src/ipa/raspberrypi/data/imx219.json b/src/ipa/raspberrypi/data/imx219.json
> index 212f8b9a..1ec338be 100644
> --- a/src/ipa/raspberrypi/data/imx219.json
> +++ b/src/ipa/raspberrypi/data/imx219.json
> @@ -133,7 +133,7 @@
>              {
>                  "shutter":
>                  [
> -                    100, 10000, 30000, 30000, 30000
> +                    100, 10000, 30000, 60000, 66666
>                  ],
>                  "gain":
>                  [
> @@ -144,7 +144,18 @@
>              {
>                  "shutter":
>                  [
> -                    100, 5000, 10000, 20000, 30000
> +                    100, 5000, 10000, 20000, 33333
> +                ],
> +                "gain":
> +                [
> +                    1.0, 2.0, 4.0, 6.0, 6.0
> +                ]
> +            },
> +            "long":
> +            {
> +                "shutter":
> +                [
> +                    100, 10000, 30000, 60000, 120000
>                  ],
>                  "gain":
>                  [
> diff --git a/src/ipa/raspberrypi/data/imx477.json b/src/ipa/raspberrypi/data/imx477.json
> index 12fe2f41..9bee3f16 100644
> --- a/src/ipa/raspberrypi/data/imx477.json
> +++ b/src/ipa/raspberrypi/data/imx477.json
> @@ -133,7 +133,7 @@
>              {
>                  "shutter":
>                  [
> -                    100, 10000, 30000, 60000, 120000
> +                    100, 10000, 30000, 60000, 66666
>                  ],
>                  "gain":
>                  [
> @@ -144,7 +144,18 @@
>              {
>                  "shutter":
>                  [
> -                    100, 5000, 10000, 20000, 120000
> +                    100, 5000, 10000, 20000, 33333
> +                ],
> +                "gain":
> +                [
> +                    1.0, 2.0, 4.0, 6.0, 6.0
> +                ]
> +            },
> +            "long":
> +            {
> +                "shutter":
> +                [
> +                    100, 10000, 30000, 60000, 120000
>                  ],
>                  "gain":
>                  [
> diff --git a/src/ipa/raspberrypi/data/ov5647.json b/src/ipa/raspberrypi/data/ov5647.json
> index 3d8a7c8f..1a354f7c 100644
> --- a/src/ipa/raspberrypi/data/ov5647.json
> +++ b/src/ipa/raspberrypi/data/ov5647.json
> @@ -133,7 +133,7 @@
>              {
>                  "shutter":
>                  [
> -                    100, 10000, 30000, 30000, 30000
> +                    100, 10000, 30000, 60000, 66666
>                  ],
>                  "gain":
>                  [
> @@ -144,7 +144,18 @@
>              {
>                  "shutter":
>                  [
> -                    100, 5000, 10000, 20000, 30000
> +                    100, 5000, 10000, 20000, 33333
> +                ],
> +                "gain":
> +                [
> +                    1.0, 2.0, 4.0, 6.0, 6.0
> +                ]
> +            },
> +            "long":
> +            {
> +                "shutter":
> +                [
> +                    100, 10000, 30000, 60000, 120000
>                  ],
>                  "gain":
>                  [
>
Laurent Pinchart Dec. 20, 2020, 5:54 a.m. UTC | #2 
Hi Naush,

Thank you for the patch.

On Fri, Dec 18, 2020 at 10:06:27AM +0000, Naushir Patuck wrote:
> Set the maximum shutter speed for the normal exposure profile to 66ms.
> Set the maximum shutter speed for the sport exposure profile to 33ms.

It would be nice if we could add a sentence to explain why. The content
of the patch otherwise looks good.

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

> Add a long exposure profile to allow shutter speeds of up to 120ms.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> Tested-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  src/ipa/raspberrypi/data/imx219.json | 15 +++++++++++++--
>  src/ipa/raspberrypi/data/imx477.json | 15 +++++++++++++--
>  src/ipa/raspberrypi/data/ov5647.json | 15 +++++++++++++--
>  3 files changed, 39 insertions(+), 6 deletions(-)
> 
> diff --git a/src/ipa/raspberrypi/data/imx219.json b/src/ipa/raspberrypi/data/imx219.json
> index 212f8b9a..1ec338be 100644
> --- a/src/ipa/raspberrypi/data/imx219.json
> +++ b/src/ipa/raspberrypi/data/imx219.json
> @@ -133,7 +133,7 @@
>              {
>                  "shutter":
>                  [
> -                    100, 10000, 30000, 30000, 30000
> +                    100, 10000, 30000, 60000, 66666
>                  ],
>                  "gain":
>                  [
> @@ -144,7 +144,18 @@
>              {
>                  "shutter":
>                  [
> -                    100, 5000, 10000, 20000, 30000
> +                    100, 5000, 10000, 20000, 33333
> +                ],
> +                "gain":
> +                [
> +                    1.0, 2.0, 4.0, 6.0, 6.0
> +                ]
> +            },
> +            "long":
> +            {
> +                "shutter":
> +                [
> +                    100, 10000, 30000, 60000, 120000
>                  ],
>                  "gain":
>                  [
> diff --git a/src/ipa/raspberrypi/data/imx477.json b/src/ipa/raspberrypi/data/imx477.json
> index 12fe2f41..9bee3f16 100644
> --- a/src/ipa/raspberrypi/data/imx477.json
> +++ b/src/ipa/raspberrypi/data/imx477.json
> @@ -133,7 +133,7 @@
>              {
>                  "shutter":
>                  [
> -                    100, 10000, 30000, 60000, 120000
> +                    100, 10000, 30000, 60000, 66666
>                  ],
>                  "gain":
>                  [
> @@ -144,7 +144,18 @@
>              {
>                  "shutter":
>                  [
> -                    100, 5000, 10000, 20000, 120000
> +                    100, 5000, 10000, 20000, 33333
> +                ],
> +                "gain":
> +                [
> +                    1.0, 2.0, 4.0, 6.0, 6.0
> +                ]
> +            },
> +            "long":
> +            {
> +                "shutter":
> +                [
> +                    100, 10000, 30000, 60000, 120000
>                  ],
>                  "gain":
>                  [
> diff --git a/src/ipa/raspberrypi/data/ov5647.json b/src/ipa/raspberrypi/data/ov5647.json
> index 3d8a7c8f..1a354f7c 100644
> --- a/src/ipa/raspberrypi/data/ov5647.json
> +++ b/src/ipa/raspberrypi/data/ov5647.json
> @@ -133,7 +133,7 @@
>              {
>                  "shutter":
>                  [
> -                    100, 10000, 30000, 30000, 30000
> +                    100, 10000, 30000, 60000, 66666
>                  ],
>                  "gain":
>                  [
> @@ -144,7 +144,18 @@
>              {
>                  "shutter":
>                  [
> -                    100, 5000, 10000, 20000, 30000
> +                    100, 5000, 10000, 20000, 33333
> +                ],
> +                "gain":
> +                [
> +                    1.0, 2.0, 4.0, 6.0, 6.0
> +                ]
> +            },
> +            "long":
> +            {
> +                "shutter":
> +                [
> +                    100, 10000, 30000, 60000, 120000
>                  ],
>                  "gain":
>                  [
Naushir Patuck Dec. 21, 2020, 11:27 a.m. UTC | #3 
Hi Laurent,

Thank you for your review feedback.

On Sun, 20 Dec 2020 at 05:54, Laurent Pinchart <
laurent.pinchart@ideasonboard.com> wrote:

> Hi Naush,
>
> Thank you for the patch.
>
> On Fri, Dec 18, 2020 at 10:06:27AM +0000, Naushir Patuck wrote:
> > Set the maximum shutter speed for the normal exposure profile to 66ms.
> > Set the maximum shutter speed for the sport exposure profile to 33ms.
>
> It would be nice if we could add a sentence to explain why. The content
> of the patch otherwise looks good.
>

I can add a quick line of text to explain this a bit more in the next
revision.

Regards,
Naush



>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> > Add a long exposure profile to allow shutter speeds of up to 120ms.
> >
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> > Tested-by: David Plowman <david.plowman@raspberrypi.com>
> > ---
> >  src/ipa/raspberrypi/data/imx219.json | 15 +++++++++++++--
> >  src/ipa/raspberrypi/data/imx477.json | 15 +++++++++++++--
> >  src/ipa/raspberrypi/data/ov5647.json | 15 +++++++++++++--
> >  3 files changed, 39 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/data/imx219.json
> b/src/ipa/raspberrypi/data/imx219.json
> > index 212f8b9a..1ec338be 100644
> > --- a/src/ipa/raspberrypi/data/imx219.json
> > +++ b/src/ipa/raspberrypi/data/imx219.json
> > @@ -133,7 +133,7 @@
> >              {
> >                  "shutter":
> >                  [
> > -                    100, 10000, 30000, 30000, 30000
> > +                    100, 10000, 30000, 60000, 66666
> >                  ],
> >                  "gain":
> >                  [
> > @@ -144,7 +144,18 @@
> >              {
> >                  "shutter":
> >                  [
> > -                    100, 5000, 10000, 20000, 30000
> > +                    100, 5000, 10000, 20000, 33333
> > +                ],
> > +                "gain":
> > +                [
> > +                    1.0, 2.0, 4.0, 6.0, 6.0
> > +                ]
> > +            },
> > +            "long":
> > +            {
> > +                "shutter":
> > +                [
> > +                    100, 10000, 30000, 60000, 120000
> >                  ],
> >                  "gain":
> >                  [
> > diff --git a/src/ipa/raspberrypi/data/imx477.json
> b/src/ipa/raspberrypi/data/imx477.json
> > index 12fe2f41..9bee3f16 100644
> > --- a/src/ipa/raspberrypi/data/imx477.json
> > +++ b/src/ipa/raspberrypi/data/imx477.json
> > @@ -133,7 +133,7 @@
> >              {
> >                  "shutter":
> >                  [
> > -                    100, 10000, 30000, 60000, 120000
> > +                    100, 10000, 30000, 60000, 66666
> >                  ],
> >                  "gain":
> >                  [
> > @@ -144,7 +144,18 @@
> >              {
> >                  "shutter":
> >                  [
> > -                    100, 5000, 10000, 20000, 120000
> > +                    100, 5000, 10000, 20000, 33333
> > +                ],
> > +                "gain":
> > +                [
> > +                    1.0, 2.0, 4.0, 6.0, 6.0
> > +                ]
> > +            },
> > +            "long":
> > +            {
> > +                "shutter":
> > +                [
> > +                    100, 10000, 30000, 60000, 120000
> >                  ],
> >                  "gain":
> >                  [
> > diff --git a/src/ipa/raspberrypi/data/ov5647.json
> b/src/ipa/raspberrypi/data/ov5647.json
> > index 3d8a7c8f..1a354f7c 100644
> > --- a/src/ipa/raspberrypi/data/ov5647.json
> > +++ b/src/ipa/raspberrypi/data/ov5647.json
> > @@ -133,7 +133,7 @@
> >              {
> >                  "shutter":
> >                  [
> > -                    100, 10000, 30000, 30000, 30000
> > +                    100, 10000, 30000, 60000, 66666
> >                  ],
> >                  "gain":
> >                  [
> > @@ -144,7 +144,18 @@
> >              {
> >                  "shutter":
> >                  [
> > -                    100, 5000, 10000, 20000, 30000
> > +                    100, 5000, 10000, 20000, 33333
> > +                ],
> > +                "gain":
> > +                [
> > +                    1.0, 2.0, 4.0, 6.0, 6.0
> > +                ]
> > +            },
> > +            "long":
> > +            {
> > +                "shutter":
> > +                [
> > +                    100, 10000, 30000, 60000, 120000
> >                  ],
> >                  "gain":
> >                  [
>
> --
> Regards,
>
> Laurent Pinchart
>

Patch
diff mbox series 

diff --git a/src/ipa/raspberrypi/data/imx219.json b/src/ipa/raspberrypi/data/imx219.json
index 212f8b9a..1ec338be 100644
--- a/src/ipa/raspberrypi/data/imx219.json
+++ b/src/ipa/raspberrypi/data/imx219.json
@@ -133,7 +133,7 @@ 
             {
                 "shutter":
                 [
-                    100, 10000, 30000, 30000, 30000
+                    100, 10000, 30000, 60000, 66666
                 ],
                 "gain":
                 [
@@ -144,7 +144,18 @@ 
             {
                 "shutter":
                 [
-                    100, 5000, 10000, 20000, 30000
+                    100, 5000, 10000, 20000, 33333
+                ],
+                "gain":
+                [
+                    1.0, 2.0, 4.0, 6.0, 6.0
+                ]
+            },
+            "long":
+            {
+                "shutter":
+                [
+                    100, 10000, 30000, 60000, 120000
                 ],
                 "gain":
                 [
diff --git a/src/ipa/raspberrypi/data/imx477.json b/src/ipa/raspberrypi/data/imx477.json
index 12fe2f41..9bee3f16 100644
--- a/src/ipa/raspberrypi/data/imx477.json
+++ b/src/ipa/raspberrypi/data/imx477.json
@@ -133,7 +133,7 @@ 
             {
                 "shutter":
                 [
-                    100, 10000, 30000, 60000, 120000
+                    100, 10000, 30000, 60000, 66666
                 ],
                 "gain":
                 [
@@ -144,7 +144,18 @@ 
             {
                 "shutter":
                 [
-                    100, 5000, 10000, 20000, 120000
+                    100, 5000, 10000, 20000, 33333
+                ],
+                "gain":
+                [
+                    1.0, 2.0, 4.0, 6.0, 6.0
+                ]
+            },
+            "long":
+            {
+                "shutter":
+                [
+                    100, 10000, 30000, 60000, 120000
                 ],
                 "gain":
                 [
diff --git a/src/ipa/raspberrypi/data/ov5647.json b/src/ipa/raspberrypi/data/ov5647.json
index 3d8a7c8f..1a354f7c 100644
--- a/src/ipa/raspberrypi/data/ov5647.json
+++ b/src/ipa/raspberrypi/data/ov5647.json
@@ -133,7 +133,7 @@ 
             {
                 "shutter":
                 [
-                    100, 10000, 30000, 30000, 30000
+                    100, 10000, 30000, 60000, 66666
                 ],
                 "gain":
                 [
@@ -144,7 +144,18 @@ 
             {
                 "shutter":
                 [
-                    100, 5000, 10000, 20000, 30000
+                    100, 5000, 10000, 20000, 33333
+                ],
+                "gain":
+                [
+                    1.0, 2.0, 4.0, 6.0, 6.0
+                ]
+            },
+            "long":
+            {
+                "shutter":
+                [
+                    100, 10000, 30000, 60000, 120000
                 ],
                 "gain":
                 [