| Message ID | 20200318033200.3042855-7-niklas.soderlund@ragnatech.se |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, Thank you for the patch. On Wed, Mar 18, 2020 at 04:31:58AM +0100, Niklas Söderlund wrote: > Create a class to represent a pixel format. This is done to add support > for modifiers for the formats. So far no modifiers are added by any > pipeline handler, all plumbing to deal with them is however in place. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > * Changes since v2 > - Add isValid() > - Add operator uint32_t() > - Handle modifiers in operator<() > - Remove include for log.h > > * Changes since v1 > - Remove copy constructor and operator= > - Removed LOG_DEFINE_CATEGORY > - Updated documentation > - Improved toString() > > * Changes since RFC > - Drop table of translation from V4L2 to DRM fourcc and turn PixelFormat > into a more basic data container class. > --- > include/libcamera/pixelformats.h | 25 ++++++- > src/cam/main.cpp | 6 +- > src/gstreamer/gstlibcamera-utils.cpp | 8 +-- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- > src/libcamera/pipeline/uvcvideo.cpp | 5 +- > src/libcamera/pipeline/vimc.cpp | 2 +- > src/libcamera/pixelformats.cpp | 87 ++++++++++++++++++++++-- > src/libcamera/stream.cpp | 4 +- > src/libcamera/v4l2_videodevice.cpp | 5 +- > src/qcam/format_converter.cpp | 2 +- > src/v4l2/v4l2_camera_proxy.cpp | 4 +- > 11 files changed, 124 insertions(+), 26 deletions(-) > > diff --git a/include/libcamera/pixelformats.h b/include/libcamera/pixelformats.h > index 544363af7a8c6f05..eb40e55ac159505a 100644 > --- a/include/libcamera/pixelformats.h > +++ b/include/libcamera/pixelformats.h > @@ -7,13 +7,36 @@ > #ifndef __LIBCAMERA_PIXEL_FORMATS_H__ > #define __LIBCAMERA_PIXEL_FORMATS_H__ > > +#include <set> > #include <stdint.h> > +#include <string> > > #include <linux/drm_fourcc.h> > > namespace libcamera { > > -using PixelFormat = uint32_t; > +class PixelFormat > +{ > +public: > + PixelFormat(); > + PixelFormat(uint32_t fourcc, const std::set<uint64_t> &modifiers = {}); > + > + bool operator==(const PixelFormat &other) const; > + bool operator!=(const PixelFormat &other) const { return !(*this == other); } > + bool operator<(const PixelFormat &other) const; > + > + bool isValid() const { return fourcc_ != 0; } > + > + operator uint32_t() const { return fourcc_; } > + uint32_t fourcc() const { return fourcc_; } > + const std::set<uint64_t> &modifiers() const { return modifiers_; } > + > + std::string toString() const; > + > +private: > + uint32_t fourcc_; > + std::set<uint64_t> modifiers_; > +}; > > } /* namespace libcamera */ > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > index af516f1cbf23974a..f73e77f381779853 100644 > --- a/src/cam/main.cpp > +++ b/src/cam/main.cpp > @@ -253,7 +253,7 @@ int CamApp::prepareConfig() > > /* TODO: Translate 4CC string to ID. */ > if (opt.isSet("pixelformat")) > - cfg.pixelFormat = opt["pixelformat"]; > + cfg.pixelFormat = PixelFormat(opt["pixelformat"]); > } > } > > @@ -304,8 +304,8 @@ int CamApp::infoConfiguration() > > const StreamFormats &formats = cfg.formats(); > for (PixelFormat pixelformat : formats.pixelformats()) { > - std::cout << " * Pixelformat: 0x" << std::hex > - << std::setw(8) << pixelformat << " " > + std::cout << " * Pixelformat: " > + << pixelformat.toString() << " " > << formats.range(pixelformat).toString() > << std::endl; > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp > index 3b3973bcea3dc759..f21e94c3eef92737 100644 > --- a/src/gstreamer/gstlibcamera-utils.cpp > +++ b/src/gstreamer/gstlibcamera-utils.cpp > @@ -80,11 +80,11 @@ gst_libcamera_stream_formats_to_caps(const StreamFormats &formats) > GstCaps *caps = gst_caps_new_empty(); > > for (PixelFormat pixelformat : formats.pixelformats()) { > - g_autoptr(GstStructure) bare_s = bare_structure_from_fourcc(pixelformat); > + g_autoptr(GstStructure) bare_s = bare_structure_from_fourcc(pixelformat.fourcc()); With the uint32_t conversion operator, I think you wouldn't have to call .fourcc() explicitly here and in several locations below. It's of course up to you. > > if (!bare_s) { > GST_WARNING("Unsupported DRM format %" GST_FOURCC_FORMAT, > - GST_FOURCC_ARGS(pixelformat)); > + GST_FOURCC_ARGS(pixelformat.fourcc())); > continue; > } > > @@ -120,7 +120,7 @@ GstCaps * > gst_libcamera_stream_configuration_to_caps(const StreamConfiguration &stream_cfg) > { > GstCaps *caps = gst_caps_new_empty(); > - GstStructure *s = bare_structure_from_fourcc(stream_cfg.pixelFormat); > + GstStructure *s = bare_structure_from_fourcc(stream_cfg.pixelFormat.fourcc()); > > gst_structure_set(s, > "width", G_TYPE_INT, stream_cfg.size.width, > @@ -135,7 +135,7 @@ void > gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg, > GstCaps *caps) > { > - GstVideoFormat gst_format = drm_to_gst_format(stream_cfg.pixelFormat); > + GstVideoFormat gst_format = drm_to_gst_format(stream_cfg.pixelFormat.fourcc()); > > /* First fixate the caps using default configuration value. */ > g_assert(gst_caps_is_writable(caps)); > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index a95ae48ac8cfbc98..8a11deb814bc0bfb 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -789,7 +789,7 @@ int PipelineHandlerRkISP1::start(Camera *camera) > /* Inform IPA of stream configuration and sensor controls. */ > std::map<unsigned int, IPAStream> streamConfig; > streamConfig[0] = { > - .pixelFormat = data->stream_.configuration().pixelFormat, > + .pixelFormat = data->stream_.configuration().pixelFormat.fourcc(), Note for later, IPAStream needs to be extended with modifiers. Could you write this down in the tasks list ? > .size = data->stream_.configuration().size, > }; > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > index 1de091e0c0e57f7c..731149755728f209 100644 > --- a/src/libcamera/pipeline/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo.cpp > @@ -113,8 +113,9 @@ CameraConfiguration::Status UVCCameraConfiguration::validate() > if (iter == pixelFormats.end()) { > cfg.pixelFormat = pixelFormats.front(); > LOG(UVC, Debug) > - << "Adjusting pixel format from " << pixelFormat > - << " to " << cfg.pixelFormat; > + << "Adjusting pixel format from " > + << pixelFormat.toString() << " to " > + << cfg.pixelFormat.toString(); > status = Adjusted; > } > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index 04cad94e739e9ae9..2e2162b2bf4477c5 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -103,7 +103,7 @@ private: > > namespace { > > -constexpr std::array<PixelFormat, 3> pixelformats{ > +static const std::array<PixelFormat, 3> pixelformats{ > DRM_FORMAT_RGB888, > DRM_FORMAT_BGR888, > DRM_FORMAT_BGRA8888, > diff --git a/src/libcamera/pixelformats.cpp b/src/libcamera/pixelformats.cpp > index c03335400b709d9b..929578fed3e22c92 100644 > --- a/src/libcamera/pixelformats.cpp > +++ b/src/libcamera/pixelformats.cpp > @@ -15,14 +15,91 @@ > namespace libcamera { > > /** > - * \typedef PixelFormat > + * \class PixelFormat > * \brief libcamera image pixel format > * > * The PixelFormat type describes the format of images in the public libcamera > - * API. It stores a FourCC value as a 32-bit unsigned integer. The values are > - * defined in the Linux kernel DRM/KMS API (see linux/drm_fourcc.h). > - * > - * \todo Add support for format modifiers > + * API. It stores a FourCC value as a 32-bit unsigned integer and a set of > + * modifiers. The FourCC and modifiers values are defined in the Linux kernel > + * DRM/KMS API (see linux/drm_fourcc.h). > */ > > +PixelFormat::PixelFormat() This should be documented. /** * \brief Construct a PixelFormat with an invalid format * * PixelFormat instances constructed with the default constructor are * invalid, calling the isValid() function returns false. */ > + : fourcc_(0) > +{ > +} > + > +/** > + * \brief Construct a PixelFormat from a DRM FourCC and a set of modifiers > + * \param[in] fourcc A DRM FourCC > + * \param[in] modifiers A set of DRM FourCC modifiers > + */ > +PixelFormat::PixelFormat(uint32_t fourcc, const std::set<uint64_t> &modifiers) > + : fourcc_(fourcc), modifiers_(modifiers) > +{ > +} > + > +/** > + * \brief Compare pixel formats for equality > + * \return True if the two pixel formats are equal, false otherwise > + */ > +bool PixelFormat::operator==(const PixelFormat &other) const > +{ > + return fourcc_ == other.fourcc() && modifiers_ == other.modifiers_; > +} > + > +/** > + * \fn bool PixelFormat::operator!=(const PixelFormat &other) const > + * \brief Compare pixel formats for inequality > + * \return True if the two pixel formats are not equal, false otherwise > + */ > + > +/** > + * \brief Compare pixel formats for smaller than order > + * \return True if \a this is smaller than \a other, false otherwise > + */ > +bool PixelFormat::operator<(const PixelFormat &other) const > +{ > + if (fourcc_ < other.fourcc_) > + return true; > + if (fourcc_ > other.fourcc_) > + return false; > + return modifiers_ < modifiers_; > +} > + > +/** > + * \fn bool PixelFormat::isValid() const > + * \brief Check if the pixel format is valid > + * \return True if the pixel format has a non-zero value, false otherwise Maybe we should avoid talking about a non-zero value if we want to keep that an implementation detail ? /** * \fn bool PixelFormat::isValid() const * \brief Check if the pixel format is valid * * PixelFormat instances constructed with the default constructor are * invalid. Instances constructed with a FourCC defined in the DRM API * are valid. The behaviour is undefined otherwise. * * \return True if the pixel format is valid, false otherwise */ Up to you. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + */ > + > +/** > + * \fn PixelFormat::operator uint32_t() const > + * \brief Convert the the pixel format numerical value > + * \return The pixel format numerical value > + */ > + > +/** > + * \fn PixelFormat::fourcc() const > + * \brief Retrieve the pixel format FourCC > + * \return DRM FourCC > + */ > + > +/** > + * \fn PixelFormat::modifiers() const > + * \brief Retrieve the pixel format modifiers > + * \return Set of DRM modifiers > + */ > + > +/** > + * \brief Assemble and return a string describing the pixel format > + * \return A string describing the pixel format > + */ > +std::string PixelFormat::toString() const > +{ > + char str[11]; > + snprintf(str, 11, "0x%08x", fourcc_); > + return str; > +} > + > } /* namespace libcamera */ > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp > index 13789e9eb344f95c..0716de388bd81d80 100644 > --- a/src/libcamera/stream.cpp > +++ b/src/libcamera/stream.cpp > @@ -347,9 +347,7 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats) > */ > std::string StreamConfiguration::toString() const > { > - std::stringstream ss; > - ss << size.toString() << "-" << utils::hex(pixelFormat); > - return ss.str(); > + return size.toString() + "-" + pixelFormat.toString(); > } > > /** > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index 3869766046236f34..c8ba0f8cebedb91a 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -1647,7 +1647,7 @@ uint32_t V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat) > */ > uint32_t V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool multiplanar) > { > - switch (pixelFormat) { > + switch (pixelFormat.fourcc()) { > /* RGB formats. */ > case DRM_FORMAT_BGR888: > return V4L2_PIX_FMT_RGB24; > @@ -1691,8 +1691,7 @@ uint32_t V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool multiplanar > * class. Until we fix the logger, work around it. > */ > libcamera::_log(__FILE__, __LINE__, _LOG_CATEGORY(V4L2)(), LogError).stream() > - << "Unsupported V4L2 pixel format " > - << utils::hex(pixelFormat); > + << "Unsupported V4L2 pixel format " << pixelFormat.toString(); > return 0; > } > > diff --git a/src/qcam/format_converter.cpp b/src/qcam/format_converter.cpp > index 7b8ad77c42fe85d4..66d07025ac9578ca 100644 > --- a/src/qcam/format_converter.cpp > +++ b/src/qcam/format_converter.cpp > @@ -28,7 +28,7 @@ > int FormatConverter::configure(libcamera::PixelFormat format, unsigned int width, > unsigned int height) > { > - switch (format) { > + switch (format.fourcc()) { > case DRM_FORMAT_NV12: > formatFamily_ = NV; > horzSubSample_ = 2; > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp > index e349fbddc2a4d5a2..c6d1e5030b58b630 100644 > --- a/src/v4l2/v4l2_camera_proxy.cpp > +++ b/src/v4l2/v4l2_camera_proxy.cpp > @@ -533,7 +533,7 @@ struct PixelFormatInfo { > > namespace { > > -constexpr std::array<PixelFormatInfo, 13> pixelFormatInfo = {{ > +static const std::array<PixelFormatInfo, 13> pixelFormatInfo = {{ > /* RGB formats. */ > { DRM_FORMAT_RGB888, V4L2_PIX_FMT_BGR24, 1, {{ { 24, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} }, > { DRM_FORMAT_BGR888, V4L2_PIX_FMT_RGB24, 1, {{ { 24, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} }, > @@ -605,7 +605,7 @@ uint32_t V4L2CameraProxy::drmToV4L2(PixelFormat format) > return info.format == format; > }); > if (info == pixelFormatInfo.end()) > - return format; > + return format.fourcc(); > > return info->v4l2Format; > }
diff --git a/include/libcamera/pixelformats.h b/include/libcamera/pixelformats.h index 544363af7a8c6f05..eb40e55ac159505a 100644 --- a/include/libcamera/pixelformats.h +++ b/include/libcamera/pixelformats.h @@ -7,13 +7,36 @@ #ifndef __LIBCAMERA_PIXEL_FORMATS_H__ #define __LIBCAMERA_PIXEL_FORMATS_H__ +#include <set> #include <stdint.h> +#include <string> #include <linux/drm_fourcc.h> namespace libcamera { -using PixelFormat = uint32_t; +class PixelFormat +{ +public: + PixelFormat(); + PixelFormat(uint32_t fourcc, const std::set<uint64_t> &modifiers = {}); + + bool operator==(const PixelFormat &other) const; + bool operator!=(const PixelFormat &other) const { return !(*this == other); } + bool operator<(const PixelFormat &other) const; + + bool isValid() const { return fourcc_ != 0; } + + operator uint32_t() const { return fourcc_; } + uint32_t fourcc() const { return fourcc_; } + const std::set<uint64_t> &modifiers() const { return modifiers_; } + + std::string toString() const; + +private: + uint32_t fourcc_; + std::set<uint64_t> modifiers_; +}; } /* namespace libcamera */ diff --git a/src/cam/main.cpp b/src/cam/main.cpp index af516f1cbf23974a..f73e77f381779853 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -253,7 +253,7 @@ int CamApp::prepareConfig() /* TODO: Translate 4CC string to ID. */ if (opt.isSet("pixelformat")) - cfg.pixelFormat = opt["pixelformat"]; + cfg.pixelFormat = PixelFormat(opt["pixelformat"]); } } @@ -304,8 +304,8 @@ int CamApp::infoConfiguration() const StreamFormats &formats = cfg.formats(); for (PixelFormat pixelformat : formats.pixelformats()) { - std::cout << " * Pixelformat: 0x" << std::hex - << std::setw(8) << pixelformat << " " + std::cout << " * Pixelformat: " + << pixelformat.toString() << " " << formats.range(pixelformat).toString() << std::endl; diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp index 3b3973bcea3dc759..f21e94c3eef92737 100644 --- a/src/gstreamer/gstlibcamera-utils.cpp +++ b/src/gstreamer/gstlibcamera-utils.cpp @@ -80,11 +80,11 @@ gst_libcamera_stream_formats_to_caps(const StreamFormats &formats) GstCaps *caps = gst_caps_new_empty(); for (PixelFormat pixelformat : formats.pixelformats()) { - g_autoptr(GstStructure) bare_s = bare_structure_from_fourcc(pixelformat); + g_autoptr(GstStructure) bare_s = bare_structure_from_fourcc(pixelformat.fourcc()); if (!bare_s) { GST_WARNING("Unsupported DRM format %" GST_FOURCC_FORMAT, - GST_FOURCC_ARGS(pixelformat)); + GST_FOURCC_ARGS(pixelformat.fourcc())); continue; } @@ -120,7 +120,7 @@ GstCaps * gst_libcamera_stream_configuration_to_caps(const StreamConfiguration &stream_cfg) { GstCaps *caps = gst_caps_new_empty(); - GstStructure *s = bare_structure_from_fourcc(stream_cfg.pixelFormat); + GstStructure *s = bare_structure_from_fourcc(stream_cfg.pixelFormat.fourcc()); gst_structure_set(s, "width", G_TYPE_INT, stream_cfg.size.width, @@ -135,7 +135,7 @@ void gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg, GstCaps *caps) { - GstVideoFormat gst_format = drm_to_gst_format(stream_cfg.pixelFormat); + GstVideoFormat gst_format = drm_to_gst_format(stream_cfg.pixelFormat.fourcc()); /* First fixate the caps using default configuration value. */ g_assert(gst_caps_is_writable(caps)); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index a95ae48ac8cfbc98..8a11deb814bc0bfb 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -789,7 +789,7 @@ int PipelineHandlerRkISP1::start(Camera *camera) /* Inform IPA of stream configuration and sensor controls. */ std::map<unsigned int, IPAStream> streamConfig; streamConfig[0] = { - .pixelFormat = data->stream_.configuration().pixelFormat, + .pixelFormat = data->stream_.configuration().pixelFormat.fourcc(), .size = data->stream_.configuration().size, }; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 1de091e0c0e57f7c..731149755728f209 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -113,8 +113,9 @@ CameraConfiguration::Status UVCCameraConfiguration::validate() if (iter == pixelFormats.end()) { cfg.pixelFormat = pixelFormats.front(); LOG(UVC, Debug) - << "Adjusting pixel format from " << pixelFormat - << " to " << cfg.pixelFormat; + << "Adjusting pixel format from " + << pixelFormat.toString() << " to " + << cfg.pixelFormat.toString(); status = Adjusted; } diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 04cad94e739e9ae9..2e2162b2bf4477c5 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -103,7 +103,7 @@ private: namespace { -constexpr std::array<PixelFormat, 3> pixelformats{ +static const std::array<PixelFormat, 3> pixelformats{ DRM_FORMAT_RGB888, DRM_FORMAT_BGR888, DRM_FORMAT_BGRA8888, diff --git a/src/libcamera/pixelformats.cpp b/src/libcamera/pixelformats.cpp index c03335400b709d9b..929578fed3e22c92 100644 --- a/src/libcamera/pixelformats.cpp +++ b/src/libcamera/pixelformats.cpp @@ -15,14 +15,91 @@ namespace libcamera { /** - * \typedef PixelFormat + * \class PixelFormat * \brief libcamera image pixel format * * The PixelFormat type describes the format of images in the public libcamera - * API. It stores a FourCC value as a 32-bit unsigned integer. The values are - * defined in the Linux kernel DRM/KMS API (see linux/drm_fourcc.h). - * - * \todo Add support for format modifiers + * API. It stores a FourCC value as a 32-bit unsigned integer and a set of + * modifiers. The FourCC and modifiers values are defined in the Linux kernel + * DRM/KMS API (see linux/drm_fourcc.h). */ +PixelFormat::PixelFormat() + : fourcc_(0) +{ +} + +/** + * \brief Construct a PixelFormat from a DRM FourCC and a set of modifiers + * \param[in] fourcc A DRM FourCC + * \param[in] modifiers A set of DRM FourCC modifiers + */ +PixelFormat::PixelFormat(uint32_t fourcc, const std::set<uint64_t> &modifiers) + : fourcc_(fourcc), modifiers_(modifiers) +{ +} + +/** + * \brief Compare pixel formats for equality + * \return True if the two pixel formats are equal, false otherwise + */ +bool PixelFormat::operator==(const PixelFormat &other) const +{ + return fourcc_ == other.fourcc() && modifiers_ == other.modifiers_; +} + +/** + * \fn bool PixelFormat::operator!=(const PixelFormat &other) const + * \brief Compare pixel formats for inequality + * \return True if the two pixel formats are not equal, false otherwise + */ + +/** + * \brief Compare pixel formats for smaller than order + * \return True if \a this is smaller than \a other, false otherwise + */ +bool PixelFormat::operator<(const PixelFormat &other) const +{ + if (fourcc_ < other.fourcc_) + return true; + if (fourcc_ > other.fourcc_) + return false; + return modifiers_ < modifiers_; +} + +/** + * \fn bool PixelFormat::isValid() const + * \brief Check if the pixel format is valid + * \return True if the pixel format has a non-zero value, false otherwise + */ + +/** + * \fn PixelFormat::operator uint32_t() const + * \brief Convert the the pixel format numerical value + * \return The pixel format numerical value + */ + +/** + * \fn PixelFormat::fourcc() const + * \brief Retrieve the pixel format FourCC + * \return DRM FourCC + */ + +/** + * \fn PixelFormat::modifiers() const + * \brief Retrieve the pixel format modifiers + * \return Set of DRM modifiers + */ + +/** + * \brief Assemble and return a string describing the pixel format + * \return A string describing the pixel format + */ +std::string PixelFormat::toString() const +{ + char str[11]; + snprintf(str, 11, "0x%08x", fourcc_); + return str; +} + } /* namespace libcamera */ diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index 13789e9eb344f95c..0716de388bd81d80 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -347,9 +347,7 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats) */ std::string StreamConfiguration::toString() const { - std::stringstream ss; - ss << size.toString() << "-" << utils::hex(pixelFormat); - return ss.str(); + return size.toString() + "-" + pixelFormat.toString(); } /** diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 3869766046236f34..c8ba0f8cebedb91a 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1647,7 +1647,7 @@ uint32_t V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat) */ uint32_t V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool multiplanar) { - switch (pixelFormat) { + switch (pixelFormat.fourcc()) { /* RGB formats. */ case DRM_FORMAT_BGR888: return V4L2_PIX_FMT_RGB24; @@ -1691,8 +1691,7 @@ uint32_t V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool multiplanar * class. Until we fix the logger, work around it. */ libcamera::_log(__FILE__, __LINE__, _LOG_CATEGORY(V4L2)(), LogError).stream() - << "Unsupported V4L2 pixel format " - << utils::hex(pixelFormat); + << "Unsupported V4L2 pixel format " << pixelFormat.toString(); return 0; } diff --git a/src/qcam/format_converter.cpp b/src/qcam/format_converter.cpp index 7b8ad77c42fe85d4..66d07025ac9578ca 100644 --- a/src/qcam/format_converter.cpp +++ b/src/qcam/format_converter.cpp @@ -28,7 +28,7 @@ int FormatConverter::configure(libcamera::PixelFormat format, unsigned int width, unsigned int height) { - switch (format) { + switch (format.fourcc()) { case DRM_FORMAT_NV12: formatFamily_ = NV; horzSubSample_ = 2; diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp index e349fbddc2a4d5a2..c6d1e5030b58b630 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -533,7 +533,7 @@ struct PixelFormatInfo { namespace { -constexpr std::array<PixelFormatInfo, 13> pixelFormatInfo = {{ +static const std::array<PixelFormatInfo, 13> pixelFormatInfo = {{ /* RGB formats. */ { DRM_FORMAT_RGB888, V4L2_PIX_FMT_BGR24, 1, {{ { 24, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} }, { DRM_FORMAT_BGR888, V4L2_PIX_FMT_RGB24, 1, {{ { 24, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} }, @@ -605,7 +605,7 @@ uint32_t V4L2CameraProxy::drmToV4L2(PixelFormat format) return info.format == format; }); if (info == pixelFormatInfo.end()) - return format; + return format.fourcc(); return info->v4l2Format; }
Create a class to represent a pixel format. This is done to add support for modifiers for the formats. So far no modifiers are added by any pipeline handler, all plumbing to deal with them is however in place. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- * Changes since v2 - Add isValid() - Add operator uint32_t() - Handle modifiers in operator<() - Remove include for log.h * Changes since v1 - Remove copy constructor and operator= - Removed LOG_DEFINE_CATEGORY - Updated documentation - Improved toString() * Changes since RFC - Drop table of translation from V4L2 to DRM fourcc and turn PixelFormat into a more basic data container class. --- include/libcamera/pixelformats.h | 25 ++++++- src/cam/main.cpp | 6 +- src/gstreamer/gstlibcamera-utils.cpp | 8 +-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- src/libcamera/pipeline/uvcvideo.cpp | 5 +- src/libcamera/pipeline/vimc.cpp | 2 +- src/libcamera/pixelformats.cpp | 87 ++++++++++++++++++++++-- src/libcamera/stream.cpp | 4 +- src/libcamera/v4l2_videodevice.cpp | 5 +- src/qcam/format_converter.cpp | 2 +- src/v4l2/v4l2_camera_proxy.cpp | 4 +- 11 files changed, 124 insertions(+), 26 deletions(-)