| Message ID | 20250724065256.75175-4-dan.scally@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Quoting Daniel Scally (2025-07-24 07:52:49) > Add a NONE entry to BayerFormat::Order enumeration to denote pixel > formats encoding bayer data but which do not correspond to a specific > order. > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > --- > include/libcamera/internal/bayer_format.h | 3 ++- > src/libcamera/bayer_format.cpp | 3 +++ > src/libcamera/sensor/camera_sensor_legacy.cpp | 6 ++++++ > 3 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h > index 5c14bb5f..9b6b5b58 100644 > --- a/include/libcamera/internal/bayer_format.h > +++ b/include/libcamera/internal/bayer_format.h > @@ -27,7 +27,8 @@ public: > GBRG = 1, > GRBG = 2, > RGGB = 3, > - MONO = 4 > + MONO = 4, > + NONE = 5 > }; > > enum class Packing : uint16_t { > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp > index 3dab91fc..89fe10ed 100644 > --- a/src/libcamera/bayer_format.cpp > +++ b/src/libcamera/bayer_format.cpp > @@ -49,6 +49,9 @@ namespace libcamera { > * \brief R then G on the first row, G then B on the second row. > * \var BayerFormat::MONO > * \brief Monochrome image data, there is no colour filter array. > + * \var BayerFormat::NONE > + * \brief Bayer-formatted data but without a specific order, which will have to > + * be discerned through other means. > */ > > /** > diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp > index 32989c19..232a8525 100644 > --- a/src/libcamera/sensor/camera_sensor_legacy.cpp > +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp > @@ -641,6 +641,12 @@ int CameraSensorLegacy::initProperties() > case BayerFormat::MONO: > cfa = properties::draft::MONO; > break; > + case BayerFormat::NONE: > + LOG(CameraSensor, Warning) Maybe this should be Error then ? > + << "Sensor declares no CFA pattern. This is extremely unlikely " > + << "and should be investigated as a likely error."; > + cfa = properties::draft::MONO; > + break; And as far as things go for things that can't happen it looks like a safe way to continue. If it could only represent a development time bug - we could make it Fatal ... but I think this is good. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > } > > properties_.set(properties::draft::ColorFilterArrangement, cfa); > -- > 2.30.2 >
Hi Kieran, Dan On Thu, Jul 24, 2025 at 08:53:31AM +0100, Kieran Bingham wrote: > Quoting Daniel Scally (2025-07-24 07:52:49) > > Add a NONE entry to BayerFormat::Order enumeration to denote pixel > > formats encoding bayer data but which do not correspond to a specific > > order. > > > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > > --- > > include/libcamera/internal/bayer_format.h | 3 ++- > > src/libcamera/bayer_format.cpp | 3 +++ > > src/libcamera/sensor/camera_sensor_legacy.cpp | 6 ++++++ > > 3 files changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h > > index 5c14bb5f..9b6b5b58 100644 > > --- a/include/libcamera/internal/bayer_format.h > > +++ b/include/libcamera/internal/bayer_format.h > > @@ -27,7 +27,8 @@ public: > > GBRG = 1, > > GRBG = 2, > > RGGB = 3, > > - MONO = 4 > > + MONO = 4, > > + NONE = 5 > > }; > > > > enum class Packing : uint16_t { > > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp > > index 3dab91fc..89fe10ed 100644 > > --- a/src/libcamera/bayer_format.cpp > > +++ b/src/libcamera/bayer_format.cpp > > @@ -49,6 +49,9 @@ namespace libcamera { > > * \brief R then G on the first row, G then B on the second row. > > * \var BayerFormat::MONO > > * \brief Monochrome image data, there is no colour filter array. > > + * \var BayerFormat::NONE > > + * \brief Bayer-formatted data but without a specific order, which will have to > > + * be discerned through other means. > > */ > > > > /** > > diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp > > index 32989c19..232a8525 100644 > > --- a/src/libcamera/sensor/camera_sensor_legacy.cpp > > +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp > > @@ -641,6 +641,12 @@ int CameraSensorLegacy::initProperties() > > case BayerFormat::MONO: > > cfa = properties::draft::MONO; > > break; > > + case BayerFormat::NONE: > > + LOG(CameraSensor, Warning) > > Maybe this should be Error then ? > > > + << "Sensor declares no CFA pattern. This is extremely unlikely " > > + << "and should be investigated as a likely error."; > > + cfa = properties::draft::MONO; > > + break; > > And as far as things go for things that can't happen it looks like a > safe way to continue. > > If it could only represent a development time bug - we could make it > Fatal ... but I think this is good. I think the reasoning is that generic RAW bayer formats will only be used by sensor drivers supporting the long-awaited RAW camera sensor model. If a "legacy" driver (one that doesn't register an INTERNAL pad) reports a generic RAW format, this is likely an error that shouldn't happen for a mainline sensor driver. However, downstream drivers can get this wrong, so I guess we should report this loudly, but not abort completely. Is s/Warning/Error a good compromise here ? > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > } > > > > properties_.set(properties::draft::ColorFilterArrangement, cfa); > > -- > > 2.30.2 > >
diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h index 5c14bb5f..9b6b5b58 100644 --- a/include/libcamera/internal/bayer_format.h +++ b/include/libcamera/internal/bayer_format.h @@ -27,7 +27,8 @@ public: GBRG = 1, GRBG = 2, RGGB = 3, - MONO = 4 + MONO = 4, + NONE = 5 }; enum class Packing : uint16_t { diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp index 3dab91fc..89fe10ed 100644 --- a/src/libcamera/bayer_format.cpp +++ b/src/libcamera/bayer_format.cpp @@ -49,6 +49,9 @@ namespace libcamera { * \brief R then G on the first row, G then B on the second row. * \var BayerFormat::MONO * \brief Monochrome image data, there is no colour filter array. + * \var BayerFormat::NONE + * \brief Bayer-formatted data but without a specific order, which will have to + * be discerned through other means. */ /** diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp index 32989c19..232a8525 100644 --- a/src/libcamera/sensor/camera_sensor_legacy.cpp +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp @@ -641,6 +641,12 @@ int CameraSensorLegacy::initProperties() case BayerFormat::MONO: cfa = properties::draft::MONO; break; + case BayerFormat::NONE: + LOG(CameraSensor, Warning) + << "Sensor declares no CFA pattern. This is extremely unlikely " + << "and should be investigated as a likely error."; + cfa = properties::draft::MONO; + break; } properties_.set(properties::draft::ColorFilterArrangement, cfa);
Add a NONE entry to BayerFormat::Order enumeration to denote pixel formats encoding bayer data but which do not correspond to a specific order. Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> --- include/libcamera/internal/bayer_format.h | 3 ++- src/libcamera/bayer_format.cpp | 3 +++ src/libcamera/sensor/camera_sensor_legacy.cpp | 6 ++++++ 3 files changed, 11 insertions(+), 1 deletion(-)