| Message ID | 20230119155905.464995-3-mike.rudenko@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Mikhail On Thu, Jan 19, 2023 at 06:59:03PM +0300, Mikhail Rudenko via libcamera-devel wrote: > Add an entry to the sensor properties for Omnivision OV4689. > > Kernel supports three more types of color bars patterns, which we do > not expose now. > > Signed-off-by: Mikhail Rudenko <mike.rudenko@gmail.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 c3c2cace..ca8ef962 100644 > --- a/src/libcamera/camera_sensor_properties.cpp > +++ b/src/libcamera/camera_sensor_properties.cpp > @@ -130,6 +130,13 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { controls::draft::TestPatternModeColorBars, 1}, > }, > } }, > + { "ov4689", { > + .unitCellSize = { 2000, 2000 }, > + .testPatternModes = { > + { controls::draft::TestPatternModeOff, 0 }, > + { controls::draft::TestPatternModeColorBars, 1}, // TODO: more patterns There's are 3 more patters and they seems easy to add here. Any reason to skip them ? Thanks j > + }, > + } }, > { "ov5640", { > .unitCellSize = { 1400, 1400 }, > .testPatternModes = { > -- > 2.39.0 >
On 2023-01-19 at 18:11 +01, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > Hi Mikhail > > On Thu, Jan 19, 2023 at 06:59:03PM +0300, Mikhail Rudenko via libcamera-devel wrote: >> Add an entry to the sensor properties for Omnivision OV4689. >> >> Kernel supports three more types of color bars patterns, which we do >> not expose now. >> >> Signed-off-by: Mikhail Rudenko <mike.rudenko@gmail.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 c3c2cace..ca8ef962 100644 >> --- a/src/libcamera/camera_sensor_properties.cpp >> +++ b/src/libcamera/camera_sensor_properties.cpp >> @@ -130,6 +130,13 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen >> { controls::draft::TestPatternModeColorBars, 1}, >> }, >> } }, >> + { "ov4689", { >> + .unitCellSize = { 2000, 2000 }, >> + .testPatternModes = { >> + { controls::draft::TestPatternModeOff, 0 }, >> + { controls::draft::TestPatternModeColorBars, 1}, // TODO: more patterns > > There's are 3 more patters and they seems easy to add here. > Any reason to skip them ? All the patterns supported by the kernel driver are color bars variations. If the following is OK, I'll do it in v2: { controls::draft::TestPatternModeOff, 0 }, { controls::draft::TestPatternModeColorBars, 1}, { controls::draft::TestPatternModeColorBars, 2}, { controls::draft::TestPatternModeColorBars, 3}, { controls::draft::TestPatternModeColorBars, 4}, > Thanks > j > >> + }, >> + } }, >> { "ov5640", { >> .unitCellSize = { 1400, 1400 }, >> .testPatternModes = { >> -- >> 2.39.0 >> -- Best regards, Mikhail Rudenko
Hi Mikhail On Thu, Jan 19, 2023 at 08:43:46PM +0300, Mikhail Rudenko via libcamera-devel wrote: > > On 2023-01-19 at 18:11 +01, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > > > Hi Mikhail > > > > On Thu, Jan 19, 2023 at 06:59:03PM +0300, Mikhail Rudenko via libcamera-devel wrote: > >> Add an entry to the sensor properties for Omnivision OV4689. > >> > >> Kernel supports three more types of color bars patterns, which we do > >> not expose now. > >> > >> Signed-off-by: Mikhail Rudenko <mike.rudenko@gmail.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 c3c2cace..ca8ef962 100644 > >> --- a/src/libcamera/camera_sensor_properties.cpp > >> +++ b/src/libcamera/camera_sensor_properties.cpp > >> @@ -130,6 +130,13 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > >> { controls::draft::TestPatternModeColorBars, 1}, > >> }, > >> } }, > >> + { "ov4689", { > >> + .unitCellSize = { 2000, 2000 }, > >> + .testPatternModes = { > >> + { controls::draft::TestPatternModeOff, 0 }, > >> + { controls::draft::TestPatternModeColorBars, 1}, // TODO: more patterns > > > > There's are 3 more patters and they seems easy to add here. > > Any reason to skip them ? > > All the patterns supported by the kernel driver are color bars > variations. If the following is OK, I'll do it in v2: > > { controls::draft::TestPatternModeOff, 0 }, > { controls::draft::TestPatternModeColorBars, 1}, > { controls::draft::TestPatternModeColorBars, 2}, > { controls::draft::TestPatternModeColorBars, 3}, > { controls::draft::TestPatternModeColorBars, 4}, > We're here trying to index test patterns using the modes defined by MIPI CCS specifications from section 10.1. Looking at the sensor's test pattern it seems to me the additional "Color bar type 2" corresponds to the FadeToGray CCS test pattern mode. OV4689 CCS libcamera ColorBarType1 = 100% Color bars = TestPatternModeColorBars ColorBarType2 = Fade to gray bars = TestPatternModeColorBarsFadeToGray I cannot find any other CCS pattern corresponding to ColorBarType2 or 3. I would then change this to { "ov4689", { .unitCellSize = { 2000, 2000 }, .testPatternModes = { { controls::draft::TestPatternModeOff, 0 }, { controls::draft::TestPatternModeColorBars, 1}, { controls::draft::TestPatternModeColorBarsFadeToGray, 2}, /* * No corresponding test patterns in * MIPI CCS specification for sensor's * colorBarType2 and colorBarType3. */ }, } }, I can change it when applying with your ack, if no other comments. > > Thanks > > j > > > >> + }, > >> + } }, > >> { "ov5640", { > >> .unitCellSize = { 1400, 1400 }, > >> .testPatternModes = { > >> -- > >> 2.39.0 > >> > > -- > Best regards, > Mikhail Rudenko
Hi Jacopo, On 2023-01-20 at 09:17 +01, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > Hi Mikhail > > On Thu, Jan 19, 2023 at 08:43:46PM +0300, Mikhail Rudenko via libcamera-devel wrote: >> >> On 2023-01-19 at 18:11 +01, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: >> >> > Hi Mikhail >> > >> > On Thu, Jan 19, 2023 at 06:59:03PM +0300, Mikhail Rudenko via libcamera-devel wrote: >> >> Add an entry to the sensor properties for Omnivision OV4689. >> >> >> >> Kernel supports three more types of color bars patterns, which we do >> >> not expose now. >> >> >> >> Signed-off-by: Mikhail Rudenko <mike.rudenko@gmail.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 c3c2cace..ca8ef962 100644 >> >> --- a/src/libcamera/camera_sensor_properties.cpp >> >> +++ b/src/libcamera/camera_sensor_properties.cpp >> >> @@ -130,6 +130,13 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen >> >> { controls::draft::TestPatternModeColorBars, 1}, >> >> }, >> >> } }, >> >> + { "ov4689", { >> >> + .unitCellSize = { 2000, 2000 }, >> >> + .testPatternModes = { >> >> + { controls::draft::TestPatternModeOff, 0 }, >> >> + { controls::draft::TestPatternModeColorBars, 1}, // TODO: more patterns >> > >> > There's are 3 more patters and they seems easy to add here. >> > Any reason to skip them ? >> >> All the patterns supported by the kernel driver are color bars >> variations. If the following is OK, I'll do it in v2: >> >> { controls::draft::TestPatternModeOff, 0 }, >> { controls::draft::TestPatternModeColorBars, 1}, >> { controls::draft::TestPatternModeColorBars, 2}, >> { controls::draft::TestPatternModeColorBars, 3}, >> { controls::draft::TestPatternModeColorBars, 4}, >> > > We're here trying to index test patterns using the modes defined by MIPI CCS > specifications from section 10.1. Looking at the sensor's test pattern > it seems to me the additional "Color bar type 2" corresponds to the > FadeToGray CCS test pattern mode. > > > OV4689 CCS libcamera > ColorBarType1 = 100% Color bars = TestPatternModeColorBars > ColorBarType2 = Fade to gray bars = TestPatternModeColorBarsFadeToGray > > I cannot find any other CCS pattern corresponding to ColorBarType2 or > 3. > > I would then change this to > > { "ov4689", { > .unitCellSize = { 2000, 2000 }, > .testPatternModes = { > { controls::draft::TestPatternModeOff, 0 }, > { controls::draft::TestPatternModeColorBars, 1}, > { controls::draft::TestPatternModeColorBarsFadeToGray, 2}, > /* > * No corresponding test patterns in > * MIPI CCS specification for sensor's > * colorBarType2 and colorBarType3. > */ > }, > } }, This looks reasonable, thanks! I'll do so in v2 alongside a few other changes. Best regards, Mikhail > I can change it when applying with your ack, if no other comments. > > >> > Thanks >> > j >> > >> >> + }, >> >> + } }, >> >> { "ov5640", { >> >> .unitCellSize = { 1400, 1400 }, >> >> .testPatternModes = { >> >> -- >> >> 2.39.0 >> >>
diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp index c3c2cace..ca8ef962 100644 --- a/src/libcamera/camera_sensor_properties.cpp +++ b/src/libcamera/camera_sensor_properties.cpp @@ -130,6 +130,13 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen { controls::draft::TestPatternModeColorBars, 1}, }, } }, + { "ov4689", { + .unitCellSize = { 2000, 2000 }, + .testPatternModes = { + { controls::draft::TestPatternModeOff, 0 }, + { controls::draft::TestPatternModeColorBars, 1}, // TODO: more patterns + }, + } }, { "ov5640", { .unitCellSize = { 1400, 1400 }, .testPatternModes = {
Add an entry to the sensor properties for Omnivision OV4689. Kernel supports three more types of color bars patterns, which we do not expose now. Signed-off-by: Mikhail Rudenko <mike.rudenko@gmail.com> --- src/libcamera/camera_sensor_properties.cpp | 7 +++++++ 1 file changed, 7 insertions(+)