[2/2] ipa/simple: Add tuning file for IMX355
diff mbox series

Message ID 20241019184340.111785-2-robert.mader@collabora.com
State New
Headers show
Series
  • [1/2] libcamera: software_isp: Stop clearing context config and state again
Related show

Commit Message

Robert Mader Oct. 19, 2024, 6:43 p.m. UTC 
64 at 10 bits. The value was guessed from known values for similar
sensors and testing - on a Google Pixel 3a - suggest it's correct.

Adding this tuning file is partly motivated in order to serve as
example, as it's the first one for the simple IPA.

Signed-off-by: Robert Mader <robert.mader@collabora.com>
---
 src/ipa/simple/data/imx355.yaml | 11 +++++++++++
 src/ipa/simple/data/meson.build |  1 +
 2 files changed, 12 insertions(+)
 create mode 100644 src/ipa/simple/data/imx355.yaml

Comments

Milan Zamazal Oct. 21, 2024, 10:31 a.m. UTC | #1 
Robert Mader <robert.mader@collabora.com> writes:

> 64 at 10 bits. The value was guessed from known values for similar
> sensors and testing - on a Google Pixel 3a - suggest it's correct.
>
> Adding this tuning file is partly motivated in order to serve as
> example, as it's the first one for the simple IPA.
>
> Signed-off-by: Robert Mader <robert.mader@collabora.com>

Reviewed-by: Milan Zamazal <mzamazal@redhat.com>

> ---
>  src/ipa/simple/data/imx355.yaml | 11 +++++++++++
>  src/ipa/simple/data/meson.build |  1 +
>  2 files changed, 12 insertions(+)
>  create mode 100644 src/ipa/simple/data/imx355.yaml
>
> diff --git a/src/ipa/simple/data/imx355.yaml b/src/ipa/simple/data/imx355.yaml
> new file mode 100644
> index 00000000..f7d01b73
> --- /dev/null
> +++ b/src/ipa/simple/data/imx355.yaml
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: CC0-1.0
> +%YAML 1.1
> +---
> +version: 1
> +algorithms:
> +  - BlackLevel:
> +      blackLevel: 4096
> +  - Awb:
> +  - Lut:
> +  - Agc:
> +...
> diff --git a/src/ipa/simple/data/meson.build b/src/ipa/simple/data/meson.build
> index 92795ee4..6e690f82 100644
> --- a/src/ipa/simple/data/meson.build
> +++ b/src/ipa/simple/data/meson.build
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
>  conf_files = files([
> +    'imx355.yaml',
>      'uncalibrated.yaml',
>  ])
Laurent Pinchart Nov. 6, 2024, 12:30 p.m. UTC | #2 
Hi Robert,

Thank you for the patch.

On Sat, Oct 19, 2024 at 08:43:40PM +0200, Robert Mader wrote:
> 64 at 10 bits. The value was guessed from known values for similar
> sensors and testing - on a Google Pixel 3a - suggest it's correct.
> 
> Adding this tuning file is partly motivated in order to serve as
> example, as it's the first one for the simple IPA.
> 
> Signed-off-by: Robert Mader <robert.mader@collabora.com>
> ---
>  src/ipa/simple/data/imx355.yaml | 11 +++++++++++
>  src/ipa/simple/data/meson.build |  1 +
>  2 files changed, 12 insertions(+)
>  create mode 100644 src/ipa/simple/data/imx355.yaml
> 
> diff --git a/src/ipa/simple/data/imx355.yaml b/src/ipa/simple/data/imx355.yaml
> new file mode 100644
> index 00000000..f7d01b73
> --- /dev/null
> +++ b/src/ipa/simple/data/imx355.yaml
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: CC0-1.0
> +%YAML 1.1
> +---
> +version: 1
> +algorithms:
> +  - BlackLevel:
> +      blackLevel: 4096
> +  - Awb:
> +  - Lut:
> +  - Agc:
> +...

To be absolutely honest, I'm not very keen on adding a tuning file for a
sensor that is not supported in the IPA sensor helpers :-S

We're considering a tool to characterize the gain model of sensors, I
wonder if it could be extended to the black level as well. It would be
nicer if we could capture optical black lines.

> diff --git a/src/ipa/simple/data/meson.build b/src/ipa/simple/data/meson.build
> index 92795ee4..6e690f82 100644
> --- a/src/ipa/simple/data/meson.build
> +++ b/src/ipa/simple/data/meson.build
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
>  conf_files = files([
> +    'imx355.yaml',
>      'uncalibrated.yaml',
>  ])
>
Robert Mader Nov. 12, 2024, 11:52 a.m. UTC | #3 
FTR., using

gainType_ = AnalogueGainExponential;
gainConstants_.exp = { 1.0, expGainDb(0.3) };

as used by several other IMX sensors seems to work fine - I just don't 
have the bandwidth to prove it somehow. But if you'd consider accepting 
such a helper non the less, I'd be happy to submit a corresponding patch.

On 06.11.24 13:30, Laurent Pinchart wrote:
> Hi Robert,
>
> Thank you for the patch.
>
> On Sat, Oct 19, 2024 at 08:43:40PM +0200, Robert Mader wrote:
>> 64 at 10 bits. The value was guessed from known values for similar
>> sensors and testing - on a Google Pixel 3a - suggest it's correct.
>>
>> Adding this tuning file is partly motivated in order to serve as
>> example, as it's the first one for the simple IPA.
>>
>> Signed-off-by: Robert Mader<robert.mader@collabora.com>
>> ---
>>   src/ipa/simple/data/imx355.yaml | 11 +++++++++++
>>   src/ipa/simple/data/meson.build |  1 +
>>   2 files changed, 12 insertions(+)
>>   create mode 100644 src/ipa/simple/data/imx355.yaml
>>
>> diff --git a/src/ipa/simple/data/imx355.yaml b/src/ipa/simple/data/imx355.yaml
>> new file mode 100644
>> index 00000000..f7d01b73
>> --- /dev/null
>> +++ b/src/ipa/simple/data/imx355.yaml
>> @@ -0,0 +1,11 @@
>> +# SPDX-License-Identifier: CC0-1.0
>> +%YAML 1.1
>> +---
>> +version: 1
>> +algorithms:
>> +  - BlackLevel:
>> +      blackLevel: 4096
>> +  - Awb:
>> +  - Lut:
>> +  - Agc:
>> +...
> To be absolutely honest, I'm not very keen on adding a tuning file for a
> sensor that is not supported in the IPA sensor helpers :-S
>
> We're considering a tool to characterize the gain model of sensors, I
> wonder if it could be extended to the black level as well. It would be
> nicer if we could capture optical black lines.
>
>> diff --git a/src/ipa/simple/data/meson.build b/src/ipa/simple/data/meson.build
>> index 92795ee4..6e690f82 100644
>> --- a/src/ipa/simple/data/meson.build
>> +++ b/src/ipa/simple/data/meson.build
>> @@ -1,6 +1,7 @@
>>   # SPDX-License-Identifier: CC0-1.0
>>   
>>   conf_files = files([
>> +    'imx355.yaml',
>>       'uncalibrated.yaml',
>>   ])
>>
Laurent Pinchart Nov. 12, 2024, 6:19 p.m. UTC | #4 
On Tue, Nov 12, 2024 at 12:52:39PM +0100, Robert Mader wrote:
> FTR., using
> 
> gainType_ = AnalogueGainExponential;
> gainConstants_.exp = { 1.0, expGainDb(0.3) };
> 
> as used by several other IMX sensors seems to work fine - I just don't 
> have the bandwidth to prove it somehow. But if you'd consider accepting 
> such a helper non the less, I'd be happy to submit a corresponding patch.

I'd be fine with that as a first step.

> On 06.11.24 13:30, Laurent Pinchart wrote:
> > Hi Robert,
> >
> > Thank you for the patch.
> >
> > On Sat, Oct 19, 2024 at 08:43:40PM +0200, Robert Mader wrote:
> >> 64 at 10 bits. The value was guessed from known values for similar
> >> sensors and testing - on a Google Pixel 3a - suggest it's correct.
> >>
> >> Adding this tuning file is partly motivated in order to serve as
> >> example, as it's the first one for the simple IPA.
> >>
> >> Signed-off-by: Robert Mader<robert.mader@collabora.com>
> >> ---
> >>   src/ipa/simple/data/imx355.yaml | 11 +++++++++++
> >>   src/ipa/simple/data/meson.build |  1 +
> >>   2 files changed, 12 insertions(+)
> >>   create mode 100644 src/ipa/simple/data/imx355.yaml
> >>
> >> diff --git a/src/ipa/simple/data/imx355.yaml b/src/ipa/simple/data/imx355.yaml
> >> new file mode 100644
> >> index 00000000..f7d01b73
> >> --- /dev/null
> >> +++ b/src/ipa/simple/data/imx355.yaml
> >> @@ -0,0 +1,11 @@
> >> +# SPDX-License-Identifier: CC0-1.0
> >> +%YAML 1.1
> >> +---
> >> +version: 1
> >> +algorithms:
> >> +  - BlackLevel:
> >> +      blackLevel: 4096
> >> +  - Awb:
> >> +  - Lut:
> >> +  - Agc:
> >> +...
> > To be absolutely honest, I'm not very keen on adding a tuning file for a
> > sensor that is not supported in the IPA sensor helpers :-S
> >
> > We're considering a tool to characterize the gain model of sensors, I
> > wonder if it could be extended to the black level as well. It would be
> > nicer if we could capture optical black lines.
> >
> >> diff --git a/src/ipa/simple/data/meson.build b/src/ipa/simple/data/meson.build
> >> index 92795ee4..6e690f82 100644
> >> --- a/src/ipa/simple/data/meson.build
> >> +++ b/src/ipa/simple/data/meson.build
> >> @@ -1,6 +1,7 @@
> >>   # SPDX-License-Identifier: CC0-1.0
> >>   
> >>   conf_files = files([
> >> +    'imx355.yaml',
> >>       'uncalibrated.yaml',
> >>   ])
> >>

Patch
diff mbox series 

diff --git a/src/ipa/simple/data/imx355.yaml b/src/ipa/simple/data/imx355.yaml
new file mode 100644
index 00000000..f7d01b73
--- /dev/null
+++ b/src/ipa/simple/data/imx355.yaml
@@ -0,0 +1,11 @@ 
+# SPDX-License-Identifier: CC0-1.0
+%YAML 1.1
+---
+version: 1
+algorithms:
+  - BlackLevel:
+      blackLevel: 4096
+  - Awb:
+  - Lut:
+  - Agc:
+...
diff --git a/src/ipa/simple/data/meson.build b/src/ipa/simple/data/meson.build
index 92795ee4..6e690f82 100644
--- a/src/ipa/simple/data/meson.build
+++ b/src/ipa/simple/data/meson.build
@@ -1,6 +1,7 @@ 
 # SPDX-License-Identifier: CC0-1.0
 
 conf_files = files([
+    'imx355.yaml',
     'uncalibrated.yaml',
 ])