| Message ID | 20210722203658.3588263-2-djrscally@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Daniel, Thanks for the patch. On 7/23/21 2:06 AM, Daniel Scally wrote: > Add camera sensor properties for the OV8865 sensor. This is the world > facing camera on most MS Surface platforms. > > Signed-off-by: Daniel Scally <djrscally@gmail.com> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/libcamera/camera_sensor_properties.cpp | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > index 7d8ba9e9..e1b6416b 100644 > --- a/src/libcamera/camera_sensor_properties.cpp > +++ b/src/libcamera/camera_sensor_properties.cpp > @@ -102,6 +102,20 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > */ > }, > } }, > + { "ov8865", { > + .unitCellSize = { 1400, 1400 }, > + .testPatternModes = { > + { 0, controls::draft::TestPatternModeOff }, > + { 2, controls::draft::TestPatternModeColorBars }, > + /* > + * No corresponding test pattern mode for: > + * 1: "Random data" > + * 3: "Colour Bars with Rolling Bar" > + * 4: "Color squares" > + * 5: "Color squares with rolling bar" > + */ > + }, > + } }, > }; > > const auto it = sensorProps.find(sensor);
Hi Dan, Thank you for the patch. On Thu, Jul 22, 2021 at 09:36:58PM +0100, Daniel Scally wrote: > Add camera sensor properties for the OV8865 sensor. This is the world > facing camera on most MS Surface platforms. > > Signed-off-by: Daniel Scally <djrscally@gmail.com> > --- > src/libcamera/camera_sensor_properties.cpp | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > index 7d8ba9e9..e1b6416b 100644 > --- a/src/libcamera/camera_sensor_properties.cpp > +++ b/src/libcamera/camera_sensor_properties.cpp > @@ -102,6 +102,20 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > */ > }, > } }, > + { "ov8865", { > + .unitCellSize = { 1400, 1400 }, > + .testPatternModes = { > + { 0, controls::draft::TestPatternModeOff }, > + { 2, controls::draft::TestPatternModeColorBars }, Would you be able to capture an image with this test pattern ? The color bars pattern has a specific definition, identical to the one in the CCS specification. I'd like to double-check that it matches. > + /* > + * No corresponding test pattern mode for: > + * 1: "Random data" > + * 3: "Colour Bars with Rolling Bar" > + * 4: "Color squares" > + * 5: "Color squares with rolling bar" > + */ > + }, > + } }, > }; > > const auto it = sensorProps.find(sensor);
Hi Laurent On 24/07/2021 23:57, Laurent Pinchart wrote: > Hi Dan, > > Thank you for the patch. > > On Thu, Jul 22, 2021 at 09:36:58PM +0100, Daniel Scally wrote: >> Add camera sensor properties for the OV8865 sensor. This is the world >> facing camera on most MS Surface platforms. >> >> Signed-off-by: Daniel Scally <djrscally@gmail.com> >> --- >> src/libcamera/camera_sensor_properties.cpp | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp >> index 7d8ba9e9..e1b6416b 100644 >> --- a/src/libcamera/camera_sensor_properties.cpp >> +++ b/src/libcamera/camera_sensor_properties.cpp >> @@ -102,6 +102,20 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen >> */ >> }, >> } }, >> + { "ov8865", { >> + .unitCellSize = { 1400, 1400 }, >> + .testPatternModes = { >> + { 0, controls::draft::TestPatternModeOff }, >> + { 2, controls::draft::TestPatternModeColorBars }, > Would you be able to capture an image with this test pattern ? The color > bars pattern has a specific definition, identical to the one in the CCS > specification. I'd like to double-check that it matches. Sure, this is the output: https://i.imgur.com/Lna5L47.png ... guess it's more like "coloured columns" than bars. On that note, just noticed I spelled colour the UK way in the comment for item 3. > >> + /* >> + * No corresponding test pattern mode for: >> + * 1: "Random data" >> + * 3: "Colour Bars with Rolling Bar" >> + * 4: "Color squares" >> + * 5: "Color squares with rolling bar" >> + */ >> + }, >> + } }, >> }; >> >> const auto it = sensorProps.find(sensor);
On Sun, Jul 25, 2021 at 12:24:30AM +0100, Daniel Scally wrote: > On 24/07/2021 23:57, Laurent Pinchart wrote: > > On Thu, Jul 22, 2021 at 09:36:58PM +0100, Daniel Scally wrote: > >> Add camera sensor properties for the OV8865 sensor. This is the world > >> facing camera on most MS Surface platforms. > >> > >> Signed-off-by: Daniel Scally <djrscally@gmail.com> > >> --- > >> src/libcamera/camera_sensor_properties.cpp | 14 ++++++++++++++ > >> 1 file changed, 14 insertions(+) > >> > >> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > >> index 7d8ba9e9..e1b6416b 100644 > >> --- a/src/libcamera/camera_sensor_properties.cpp > >> +++ b/src/libcamera/camera_sensor_properties.cpp > >> @@ -102,6 +102,20 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > >> */ > >> }, > >> } }, > >> + { "ov8865", { > >> + .unitCellSize = { 1400, 1400 }, > >> + .testPatternModes = { > >> + { 0, controls::draft::TestPatternModeOff }, > >> + { 2, controls::draft::TestPatternModeColorBars }, > > Would you be able to capture an image with this test pattern ? The color > > bars pattern has a specific definition, identical to the one in the CCS > > specification. I'd like to double-check that it matches. > > > Sure, this is the output: https://i.imgur.com/Lna5L47.png ... guess it's > more like "coloured columns" than bars. Thank you. This doesn't seem to match what is expected by controls::draft::TestPatternModeColorBars, so I'd leave it out (you can check the MIPI CCS specification if you want to see what the color bars pattern is supposed to look like). > On that note, just noticed I spelled colour the UK way in the comment > for item 3. I prefer the British spelling, but in APIs it seems to be a lost battle :-( > >> + /* > >> + * No corresponding test pattern mode for: > >> + * 1: "Random data" > >> + * 3: "Colour Bars with Rolling Bar" > >> + * 4: "Color squares" > >> + * 5: "Color squares with rolling bar" > >> + */ > >> + }, > >> + } }, > >> }; > >> > >> const auto it = sensorProps.find(sensor);
On Sun, Jul 25, 2021 at 02:30:23AM +0300, Laurent Pinchart wrote: > On Sun, Jul 25, 2021 at 12:24:30AM +0100, Daniel Scally wrote: > > On 24/07/2021 23:57, Laurent Pinchart wrote: > > > On Thu, Jul 22, 2021 at 09:36:58PM +0100, Daniel Scally wrote: > > >> Add camera sensor properties for the OV8865 sensor. This is the world > > >> facing camera on most MS Surface platforms. > > >> > > >> Signed-off-by: Daniel Scally <djrscally@gmail.com> > > >> --- > > >> src/libcamera/camera_sensor_properties.cpp | 14 ++++++++++++++ > > >> 1 file changed, 14 insertions(+) > > >> > > >> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > > >> index 7d8ba9e9..e1b6416b 100644 > > >> --- a/src/libcamera/camera_sensor_properties.cpp > > >> +++ b/src/libcamera/camera_sensor_properties.cpp > > >> @@ -102,6 +102,20 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > > >> */ > > >> }, > > >> } }, > > >> + { "ov8865", { > > >> + .unitCellSize = { 1400, 1400 }, > > >> + .testPatternModes = { > > >> + { 0, controls::draft::TestPatternModeOff }, > > >> + { 2, controls::draft::TestPatternModeColorBars }, > > > Would you be able to capture an image with this test pattern ? The color > > > bars pattern has a specific definition, identical to the one in the CCS > > > specification. I'd like to double-check that it matches. > > > > > > Sure, this is the output: https://i.imgur.com/Lna5L47.png ... guess it's > > more like "coloured columns" than bars. > > Thank you. This doesn't seem to match what is expected by > controls::draft::TestPatternModeColorBars, so I'd leave it out (you can > check the MIPI CCS specification if you want to see what the color bars > pattern is supposed to look like). Please ignore this. It's the fade-to-gray bars that is special. The controls::draft::TestPatternModeColorBars seems to match. The white and black bars are shorter though, which I assume is due to cropping somewhere in the pipeline ? > > On that note, just noticed I spelled colour the UK way in the comment > > for item 3. > > I prefer the British spelling, but in APIs it seems to be a lost battle > :-( I'll update that to match the string from the kernel driver, "Color bars with rolling bar". Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > >> + /* > > >> + * No corresponding test pattern mode for: > > >> + * 1: "Random data" > > >> + * 3: "Colour Bars with Rolling Bar" > > >> + * 4: "Color squares" > > >> + * 5: "Color squares with rolling bar" > > >> + */ > > >> + }, > > >> + } }, > > >> }; > > >> > > >> const auto it = sensorProps.find(sensor);
diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp index 7d8ba9e9..e1b6416b 100644 --- a/src/libcamera/camera_sensor_properties.cpp +++ b/src/libcamera/camera_sensor_properties.cpp @@ -102,6 +102,20 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen */ }, } }, + { "ov8865", { + .unitCellSize = { 1400, 1400 }, + .testPatternModes = { + { 0, controls::draft::TestPatternModeOff }, + { 2, controls::draft::TestPatternModeColorBars }, + /* + * No corresponding test pattern mode for: + * 1: "Random data" + * 3: "Colour Bars with Rolling Bar" + * 4: "Color squares" + * 5: "Color squares with rolling bar" + */ + }, + } }, }; const auto it = sensorProps.find(sensor);
Add camera sensor properties for the OV8865 sensor. This is the world facing camera on most MS Surface platforms. Signed-off-by: Daniel Scally <djrscally@gmail.com> --- src/libcamera/camera_sensor_properties.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+)