[libcamera-devel,1/2] libcamera: camera_sensor: Add OV5675 sensor properties
diff mbox series

Message ID 20220503155725.1168826-1-foss+libcamera@0leil.net
State Accepted
Commit d3d7bc433e317ba215df80282c7e81ed104a592b
Headers show
Series
  • [libcamera-devel,1/2] libcamera: camera_sensor: Add OV5675 sensor properties
Related show

Commit Message

Quentin Schulz May 3, 2022, 3:57 p.m. UTC 
From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

Add an entry to the sensor properties for OmniVision OV5675. Only the
first test pattern is included as the others that are exposed by the
kernel aren't supported by libcamera control yet.

Cc: Quentin Schulz <foss+libcamera@0leil.net>
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 src/libcamera/camera_sensor_properties.cpp | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Jacopo Mondi May 4, 2022, 6:15 a.m. UTC | #1 
Hi Quentin,

On Tue, May 03, 2022 at 05:57:24PM +0200, Quentin Schulz via libcamera-devel wrote:
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> Add an entry to the sensor properties for OmniVision OV5675. Only the
> first test pattern is included as the others that are exposed by the
> kernel aren't supported by libcamera control yet.
>
> Cc: Quentin Schulz <foss+libcamera@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>

Don't have a datasheet here, but the patch looks good.

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Looking at the driver these three additional test patterns are
available

	"Top-Bottom Darker Color Bar",
	"Right-Left Darker Color Bar",
	"Bottom-Top Darker Color Bar"

I can see at least 4 other mainline drivers which features the same
test patterns, it might be worth adding them to the list of values
supported by the TestPatternMode control.

Thanks
  j

> ---
>  src/libcamera/camera_sensor_properties.cpp | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp
> index 7a349012..235edca1 100644
> --- a/src/libcamera/camera_sensor_properties.cpp
> +++ b/src/libcamera/camera_sensor_properties.cpp
> @@ -123,6 +123,13 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				{ controls::draft::TestPatternModeColorBars, 1 },
>  			},
>  		} },
> +		{ "ov5675", {
> +			.unitCellSize = { 1120, 1120 },
> +			.testPatternModes = {
> +				{ controls::draft::TestPatternModeOff, 0 },
> +				{ controls::draft::TestPatternModeColorBars, 1 },
> +			},
> +		} },
>  		{ "ov5693", {
>  			.unitCellSize = { 1400, 1400 },
>  			.testPatternModes = {
> --
> 2.35.1
>
Quentin Schulz May 4, 2022, 9:06 a.m. UTC | #2 
Hi Jacopo,

On 5/4/22 08:15, Jacopo Mondi via libcamera-devel wrote:
> Hi Quentin,
> 
> On Tue, May 03, 2022 at 05:57:24PM +0200, Quentin Schulz via libcamera-devel wrote:
>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>
>> Add an entry to the sensor properties for OmniVision OV5675. Only the
>> first test pattern is included as the others that are exposed by the
>> kernel aren't supported by libcamera control yet.
>>
>> Cc: Quentin Schulz <foss+libcamera@0leil.net>
>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> 
> Don't have a datasheet here, but the patch looks good.
> 

https://www.ovt.com/wp-content/uploads/2022/01/OV5675-PB-v1.2-WEB.pdf

Product brief includes the unit cell size (but not much more unfortunately).

> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> Looking at the driver these three additional test patterns are
> available
> 
> 	"Top-Bottom Darker Color Bar",
> 	"Right-Left Darker Color Bar",
> 	"Bottom-Top Darker Color Bar"
> 
> I can see at least 4 other mainline drivers which features the same
> test patterns, it might be worth adding them to the list of values
> supported by the TestPatternMode control.
> 

Yes, 4 other OmniVision sensors seem to support this (maybe more if the 
drivers didn't include all test patterns).

I'm not planning to work on this right now. So if anyone feels like 
working on it, feel free to do so.

Cheers,
Quentin

> Thanks
>    j
> 
>> ---
>>   src/libcamera/camera_sensor_properties.cpp | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp
>> index 7a349012..235edca1 100644
>> --- a/src/libcamera/camera_sensor_properties.cpp
>> +++ b/src/libcamera/camera_sensor_properties.cpp
>> @@ -123,6 +123,13 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>>   				{ controls::draft::TestPatternModeColorBars, 1 },
>>   			},
>>   		} },
>> +		{ "ov5675", {
>> +			.unitCellSize = { 1120, 1120 },
>> +			.testPatternModes = {
>> +				{ controls::draft::TestPatternModeOff, 0 },
>> +				{ controls::draft::TestPatternModeColorBars, 1 },
>> +			},
>> +		} },
>>   		{ "ov5693", {
>>   			.unitCellSize = { 1400, 1400 },
>>   			.testPatternModes = {
>> --
>> 2.35.1
>>
Kieran Bingham May 11, 2022, 7:29 p.m. UTC | #3 
Quoting Quentin Schulz via libcamera-devel (2022-05-03 16:57:24)
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> 
> Add an entry to the sensor properties for OmniVision OV5675. Only the
> first test pattern is included as the others that are exposed by the
> kernel aren't supported by libcamera control yet.
> 
> Cc: Quentin Schulz <foss+libcamera@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>  src/libcamera/camera_sensor_properties.cpp | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp
> index 7a349012..235edca1 100644
> --- a/src/libcamera/camera_sensor_properties.cpp
> +++ b/src/libcamera/camera_sensor_properties.cpp
> @@ -123,6 +123,13 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>                                 { controls::draft::TestPatternModeColorBars, 1 },
>                         },
>                 } },
> +               { "ov5675", {
> +                       .unitCellSize = { 1120, 1120 },

Checks out.


> +                       .testPatternModes = {
> +                               { controls::draft::TestPatternModeOff, 0 },
> +                               { controls::draft::TestPatternModeColorBars, 1 },

Looks reasonable to start.

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

> +                       },
> +               } },
>                 { "ov5693", {
>                         .unitCellSize = { 1400, 1400 },
>                         .testPatternModes = {
> -- 
> 2.35.1
>

Patch
diff mbox series 

diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp
index 7a349012..235edca1 100644
--- a/src/libcamera/camera_sensor_properties.cpp
+++ b/src/libcamera/camera_sensor_properties.cpp
@@ -123,6 +123,13 @@  const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
 				{ controls::draft::TestPatternModeColorBars, 1 },
 			},
 		} },
+		{ "ov5675", {
+			.unitCellSize = { 1120, 1120 },
+			.testPatternModes = {
+				{ controls::draft::TestPatternModeOff, 0 },
+				{ controls::draft::TestPatternModeColorBars, 1 },
+			},
+		} },
 		{ "ov5693", {
 			.unitCellSize = { 1400, 1400 },
 			.testPatternModes = {