Message ID | 20211206105032.13876-6-david.plowman@raspberrypi.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi David, On Mon, Dec 06, 2021 at 10:50:28AM +0000, David Plowman wrote: > This adds a ColorSpace field to the V4L2SubdeviceFormat so that we can > set and request particular color spaces from V4L2. > > This commit simply adds the field and fixes some occurrences of brace > initializers that would otherwise be broken. A subsequent commit will > pass and retrieve the value correctly to/from V4l2 itself. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > --- > include/libcamera/internal/v4l2_subdevice.h | 2 ++ > src/libcamera/camera_sensor.cpp | 2 ++ > src/libcamera/pipeline/ipu3/cio2.cpp | 7 +++---- > src/libcamera/pipeline/simple/simple.cpp | 8 ++++++-- > src/libcamera/v4l2_subdevice.cpp | 11 +++++++++++ > 5 files changed, 24 insertions(+), 6 deletions(-) > > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h > index a6873b67..358bf2b6 100644 > --- a/include/libcamera/internal/v4l2_subdevice.h > +++ b/include/libcamera/internal/v4l2_subdevice.h > @@ -14,6 +14,7 @@ > #include <libcamera/base/class.h> > #include <libcamera/base/log.h> > > +#include <libcamera/color_space.h> > #include <libcamera/geometry.h> > > #include "libcamera/internal/formats.h" > @@ -27,6 +28,7 @@ class MediaDevice; > struct V4L2SubdeviceFormat { > uint32_t mbus_code; > Size size; > + std::optional<ColorSpace> colorSpace; > > const std::string toString() const; > uint8_t bitsPerPixel() const; > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > index 4c142a58..14358333 100644 > --- a/src/libcamera/camera_sensor.cpp > +++ b/src/libcamera/camera_sensor.cpp > @@ -15,6 +15,7 @@ > #include <math.h> > #include <string.h> > > +#include <libcamera/color_space.h> > #include <libcamera/property_ids.h> > > #include <libcamera/base/utils.h> > @@ -586,6 +587,7 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu > V4L2SubdeviceFormat format{ > .mbus_code = bestCode, > .size = *bestSize, > + .colorSpace = ColorSpace::Raw, > }; > > return format; > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp > index 59dda56b..f4e8c663 100644 > --- a/src/libcamera/pipeline/ipu3/cio2.cpp > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp > @@ -322,10 +322,9 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector<unsigned int> > return {}; > } > > - V4L2SubdeviceFormat format{ > - .mbus_code = bestCode, > - .size = bestSize, > - }; > + V4L2SubdeviceFormat format{}; > + format.mbus_code = bestCode; > + format.size = bestSize; > > return format; > } > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 701fb4be..a3108fc0 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -457,7 +457,9 @@ int SimpleCameraData::init() > * formats on the video node. > */ > for (unsigned int code : sensor_->mbusCodes()) { > - V4L2SubdeviceFormat format{ code, sensor_->resolution() }; > + V4L2SubdeviceFormat format{}; > + format.mbus_code = code; > + format.size = sensor_->resolution(); > > ret = setupFormats(&format, V4L2Subdevice::TryFormat); > if (ret < 0) { > @@ -908,7 +910,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) > return ret; > > const SimpleCameraData::Configuration *pipeConfig = config->pipeConfig(); > - V4L2SubdeviceFormat format{ pipeConfig->code, data->sensor_->resolution() }; > + V4L2SubdeviceFormat format{}; > + format.mbus_code = pipeConfig->code; > + format.size = data->sensor_->resolution(); > > ret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat); > if (ret < 0) > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp > index 61e15b69..981645e0 100644 > --- a/src/libcamera/v4l2_subdevice.cpp > +++ b/src/libcamera/v4l2_subdevice.cpp > @@ -169,6 +169,17 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = { > * \brief The image size in pixels > */ > > +/** > + * \var V4L2SubdeviceFormat::colorSpace > + * \brief The color space of the pixels > + * > + * The color space of the image. When setting the format this may be > + * unset, in which case the driver gets to use its default color space. > + * If this value is unset after a call to validate(), then the color space > + * chosen by the driver could not be represented by the ColorSpace class > + * (and should probably be added). > + */ > + > /** > * \brief Assemble and return a string describing the format > * \return A string describing the V4L2SubdeviceFormat > -- > 2.20.1 >
Hi David, Thank you for the patch. On Mon, Dec 06, 2021 at 10:50:28AM +0000, David Plowman wrote: > This adds a ColorSpace field to the V4L2SubdeviceFormat so that we can > set and request particular color spaces from V4L2. > > This commit simply adds the field and fixes some occurrences of brace > initializers that would otherwise be broken. A subsequent commit will > pass and retrieve the value correctly to/from V4l2 itself. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > --- > include/libcamera/internal/v4l2_subdevice.h | 2 ++ > src/libcamera/camera_sensor.cpp | 2 ++ > src/libcamera/pipeline/ipu3/cio2.cpp | 7 +++---- > src/libcamera/pipeline/simple/simple.cpp | 8 ++++++-- > src/libcamera/v4l2_subdevice.cpp | 11 +++++++++++ > 5 files changed, 24 insertions(+), 6 deletions(-) > > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h > index a6873b67..358bf2b6 100644 > --- a/include/libcamera/internal/v4l2_subdevice.h > +++ b/include/libcamera/internal/v4l2_subdevice.h > @@ -14,6 +14,7 @@ > #include <libcamera/base/class.h> > #include <libcamera/base/log.h> > > +#include <libcamera/color_space.h> > #include <libcamera/geometry.h> > > #include "libcamera/internal/formats.h" > @@ -27,6 +28,7 @@ class MediaDevice; > struct V4L2SubdeviceFormat { > uint32_t mbus_code; > Size size; > + std::optional<ColorSpace> colorSpace; #include <optional> > > const std::string toString() const; > uint8_t bitsPerPixel() const; > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > index 4c142a58..14358333 100644 > --- a/src/libcamera/camera_sensor.cpp > +++ b/src/libcamera/camera_sensor.cpp > @@ -15,6 +15,7 @@ > #include <math.h> > #include <string.h> > > +#include <libcamera/color_space.h> > #include <libcamera/property_ids.h> > > #include <libcamera/base/utils.h> > @@ -586,6 +587,7 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu > V4L2SubdeviceFormat format{ > .mbus_code = bestCode, > .size = *bestSize, > + .colorSpace = ColorSpace::Raw, > }; > > return format; > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp > index 59dda56b..f4e8c663 100644 > --- a/src/libcamera/pipeline/ipu3/cio2.cpp > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp > @@ -322,10 +322,9 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector<unsigned int> > return {}; > } > > - V4L2SubdeviceFormat format{ > - .mbus_code = bestCode, > - .size = bestSize, > - }; > + V4L2SubdeviceFormat format{}; > + format.mbus_code = bestCode; > + format.size = bestSize; Nitpicking, I would have used the same code construct in both camera_sensor.cpp and cio2.cpp. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > return format; > } > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 701fb4be..a3108fc0 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -457,7 +457,9 @@ int SimpleCameraData::init() > * formats on the video node. > */ > for (unsigned int code : sensor_->mbusCodes()) { > - V4L2SubdeviceFormat format{ code, sensor_->resolution() }; > + V4L2SubdeviceFormat format{}; > + format.mbus_code = code; > + format.size = sensor_->resolution(); > > ret = setupFormats(&format, V4L2Subdevice::TryFormat); > if (ret < 0) { > @@ -908,7 +910,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) > return ret; > > const SimpleCameraData::Configuration *pipeConfig = config->pipeConfig(); > - V4L2SubdeviceFormat format{ pipeConfig->code, data->sensor_->resolution() }; > + V4L2SubdeviceFormat format{}; > + format.mbus_code = pipeConfig->code; > + format.size = data->sensor_->resolution(); > > ret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat); > if (ret < 0) > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp > index 61e15b69..981645e0 100644 > --- a/src/libcamera/v4l2_subdevice.cpp > +++ b/src/libcamera/v4l2_subdevice.cpp > @@ -169,6 +169,17 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = { > * \brief The image size in pixels > */ > > +/** > + * \var V4L2SubdeviceFormat::colorSpace > + * \brief The color space of the pixels > + * > + * The color space of the image. When setting the format this may be > + * unset, in which case the driver gets to use its default color space. > + * If this value is unset after a call to validate(), then the color space > + * chosen by the driver could not be represented by the ColorSpace class > + * (and should probably be added). > + */ > + > /** > * \brief Assemble and return a string describing the format > * \return A string describing the V4L2SubdeviceFormat
diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h index a6873b67..358bf2b6 100644 --- a/include/libcamera/internal/v4l2_subdevice.h +++ b/include/libcamera/internal/v4l2_subdevice.h @@ -14,6 +14,7 @@ #include <libcamera/base/class.h> #include <libcamera/base/log.h> +#include <libcamera/color_space.h> #include <libcamera/geometry.h> #include "libcamera/internal/formats.h" @@ -27,6 +28,7 @@ class MediaDevice; struct V4L2SubdeviceFormat { uint32_t mbus_code; Size size; + std::optional<ColorSpace> colorSpace; const std::string toString() const; uint8_t bitsPerPixel() const; diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index 4c142a58..14358333 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -15,6 +15,7 @@ #include <math.h> #include <string.h> +#include <libcamera/color_space.h> #include <libcamera/property_ids.h> #include <libcamera/base/utils.h> @@ -586,6 +587,7 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu V4L2SubdeviceFormat format{ .mbus_code = bestCode, .size = *bestSize, + .colorSpace = ColorSpace::Raw, }; return format; diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index 59dda56b..f4e8c663 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -322,10 +322,9 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector<unsigned int> return {}; } - V4L2SubdeviceFormat format{ - .mbus_code = bestCode, - .size = bestSize, - }; + V4L2SubdeviceFormat format{}; + format.mbus_code = bestCode; + format.size = bestSize; return format; } diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 701fb4be..a3108fc0 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -457,7 +457,9 @@ int SimpleCameraData::init() * formats on the video node. */ for (unsigned int code : sensor_->mbusCodes()) { - V4L2SubdeviceFormat format{ code, sensor_->resolution() }; + V4L2SubdeviceFormat format{}; + format.mbus_code = code; + format.size = sensor_->resolution(); ret = setupFormats(&format, V4L2Subdevice::TryFormat); if (ret < 0) { @@ -908,7 +910,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) return ret; const SimpleCameraData::Configuration *pipeConfig = config->pipeConfig(); - V4L2SubdeviceFormat format{ pipeConfig->code, data->sensor_->resolution() }; + V4L2SubdeviceFormat format{}; + format.mbus_code = pipeConfig->code; + format.size = data->sensor_->resolution(); ret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat); if (ret < 0) diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp index 61e15b69..981645e0 100644 --- a/src/libcamera/v4l2_subdevice.cpp +++ b/src/libcamera/v4l2_subdevice.cpp @@ -169,6 +169,17 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = { * \brief The image size in pixels */ +/** + * \var V4L2SubdeviceFormat::colorSpace + * \brief The color space of the pixels + * + * The color space of the image. When setting the format this may be + * unset, in which case the driver gets to use its default color space. + * If this value is unset after a call to validate(), then the color space + * chosen by the driver could not be represented by the ColorSpace class + * (and should probably be added). + */ + /** * \brief Assemble and return a string describing the format * \return A string describing the V4L2SubdeviceFormat
This adds a ColorSpace field to the V4L2SubdeviceFormat so that we can set and request particular color spaces from V4L2. This commit simply adds the field and fixes some occurrences of brace initializers that would otherwise be broken. A subsequent commit will pass and retrieve the value correctly to/from V4l2 itself. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> --- include/libcamera/internal/v4l2_subdevice.h | 2 ++ src/libcamera/camera_sensor.cpp | 2 ++ src/libcamera/pipeline/ipu3/cio2.cpp | 7 +++---- src/libcamera/pipeline/simple/simple.cpp | 8 ++++++-- src/libcamera/v4l2_subdevice.cpp | 11 +++++++++++ 5 files changed, 24 insertions(+), 6 deletions(-)