[libcamera-devel] [PATCH v2 4/6] libcamera: formats: Expose PixelFormatInfo as an internal API
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Apr 30 13:59:40 CEST 2020
Hi Kieran,
On Thu, Apr 30, 2020 at 10:53:40AM +0100, Kieran Bingham wrote:
> On 30/04/2020 04:07, Laurent Pinchart wrote:
> > To prepare for storing more information about pixel formats in
> > PixelFormatInfo, move the class to formats.cpp and document it. The
> > pixel formats database is moved to the same file, and a new static
> > function is added to PixelFormatInfo to retrieve a PixelFormatInfo for a
> > PixelFormat.
>
> Perfect, you addressed my comments before you read them.
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> Comment lower, Are multiplanar formats broken/unsupported?
>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > src/libcamera/formats.cpp | 126 +++++++++++++++++++++++++++++
> > src/libcamera/include/formats.h | 15 ++++
> > src/libcamera/v4l2_pixelformat.cpp | 75 +----------------
> > 3 files changed, 144 insertions(+), 72 deletions(-)
> >
> > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> > index 5f6552a4e06c..4a351020b0d9 100644
> > --- a/src/libcamera/formats.cpp
> > +++ b/src/libcamera/formats.cpp
> > @@ -9,6 +9,8 @@
> >
> > #include <errno.h>
> >
> > +#include "log.h"
> > +
> > /**
> > * \file formats.h
> > * \brief Types and helper methods to handle libcamera image formats
> > @@ -16,6 +18,8 @@
> >
> > namespace libcamera {
> >
> > +LOG_DEFINE_CATEGORY(Formats)
> > +
> > /**
> > * \class ImageFormats
> > * \brief Describe V4L2Device and V4L2SubDevice image formats
> > @@ -104,4 +108,126 @@ const std::map<unsigned int, std::vector<SizeRange>> &ImageFormats::data() const
> > return data_;
> > }
> >
> > +/**
> > + * \class PixelFormatInfo
> > + * \brief Information about pixel formats
> > + *
> > + * The PixelFormatInfo class groups together information describing a pixel
> > + * format. It facilitates handling of pixel formats by providing data commonly
> > + * used in pipeline handlers.
> > + *
> > + * \var PixelFormatInfo::format
> > + * \brief The PixelFormat described by this instance
> > + *
> > + * \var PixelFormatInfo::v4l2Format
> > + * \brief The V4L2 pixel format corresponding to the PixelFormat
> > + */
> > +
> > +namespace {
> > +
> > +const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
> > + /* RGB formats. */
> > + { PixelFormat(DRM_FORMAT_BGR888), {
> > + .format = PixelFormat(DRM_FORMAT_BGR888),
> > + .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGB24),
> > + } },
> > + { PixelFormat(DRM_FORMAT_RGB888), {
> > + .format = PixelFormat(DRM_FORMAT_RGB888),
> > + .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_BGR24),
> > + } },
> > + { PixelFormat(DRM_FORMAT_ABGR8888), {
> > + .format = PixelFormat(DRM_FORMAT_ABGR8888),
> > + .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGBA32),
> > + } },
> > + { PixelFormat(DRM_FORMAT_ARGB8888), {
> > + .format = PixelFormat(DRM_FORMAT_ARGB8888),
> > + .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_ABGR32),
> > + } },
> > + { PixelFormat(DRM_FORMAT_BGRA8888), {
> > + .format = PixelFormat(DRM_FORMAT_BGRA8888),
> > + .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_ARGB32),
> > + } },
> > + { PixelFormat(DRM_FORMAT_RGBA8888), {
> > + .format = PixelFormat(DRM_FORMAT_RGBA8888),
> > + .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_BGRA32),
> > + } },
> > +
> > + /* YUV packed formats. */
> > + { PixelFormat(DRM_FORMAT_YUYV), {
> > + .format = PixelFormat(DRM_FORMAT_YUYV),
> > + .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YUYV),
> > + } },
> > + { PixelFormat(DRM_FORMAT_YVYU), {
> > + .format = PixelFormat(DRM_FORMAT_YVYU),
> > + .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YVYU),
> > + } },
> > + { PixelFormat(DRM_FORMAT_UYVY), {
> > + .format = PixelFormat(DRM_FORMAT_UYVY),
> > + .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_UYVY),
> > + } },
> > + { PixelFormat(DRM_FORMAT_VYUY), {
> > + .format = PixelFormat(DRM_FORMAT_VYUY),
> > + .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_VYUY),
> > + } },
> > +
> > + /* YUV planar formats. */
> > + { PixelFormat(DRM_FORMAT_NV16), {
> > + .format = PixelFormat(DRM_FORMAT_NV16),
> > + .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV16),
> > + } },
> > + { PixelFormat(DRM_FORMAT_NV61), {
> > + .format = PixelFormat(DRM_FORMAT_NV61),
> > + .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV61),
> > + } },
> > + { PixelFormat(DRM_FORMAT_NV12), {
> > + .format = PixelFormat(DRM_FORMAT_NV12),
> > + .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV12),
> > + } },
> > + { PixelFormat(DRM_FORMAT_NV21), {
> > + .format = PixelFormat(DRM_FORMAT_NV21),
> > + .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV21),
> > + } },
> > +
> > + /* Greyscale formats. */
> > + { PixelFormat(DRM_FORMAT_R8), {
> > + .format = PixelFormat(DRM_FORMAT_R8),
> > + .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_GREY),
> > + } },
> > +
> > + /* Compressed formats. */
> > + { PixelFormat(DRM_FORMAT_MJPEG), {
> > + .format = PixelFormat(DRM_FORMAT_MJPEG),
> > + .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),
> > + } },
> > +};
> > +
> > +} /* namespace */
> > +
> > +/**
> > + * \fn bool PixelFormatInfo::isValid() const
> > + * \brief Check if the pixel format info is valid
> > + * \return True if the pixel format info is valid, false otherwise
> > + */
> > +
> > +/**
> > + * \brief Retrieve information about a pixel format
> > + * \param[in] format The pixel format
> > + * \return The PixelFormatInfo describing the \a format if known, or an invalid
> > + * PixelFormatInfo otherwise
> > + */
> > +const PixelFormatInfo &PixelFormatInfo::info(const PixelFormat &format)
> > +{
> > + static const PixelFormatInfo invalid{};
> > +
> > + const auto iter = pixelFormatInfo.find(format);
> > + if (iter == pixelFormatInfo.end()) {
> > + LOG(Formats, Warning)
> > + << "Unsupported pixel format "
> > + << format.toString();
> > + return invalid;
> > + }
> > +
> > + return iter->second;
> > +}
> > +
> > } /* namespace libcamera */
> > diff --git a/src/libcamera/include/formats.h b/src/libcamera/include/formats.h
> > index f43bc8c004f6..560df07c4451 100644
> > --- a/src/libcamera/include/formats.h
> > +++ b/src/libcamera/include/formats.h
> > @@ -12,6 +12,9 @@
> > #include <vector>
> >
> > #include <libcamera/geometry.h>
> > +#include <libcamera/pixelformats.h>
> > +
> > +#include "v4l2_pixelformat.h"
> >
> > namespace libcamera {
> >
> > @@ -29,6 +32,18 @@ private:
> > std::map<unsigned int, std::vector<SizeRange>> data_;
> > };
> >
> > +class PixelFormatInfo
> > +{
> > +public:
> > + bool isValid() const { return format.isValid(); }
> > +
> > + static const PixelFormatInfo &info(const PixelFormat &format);
> > +
> > + /* \todo Add support for non-contiguous memory planes */
> > + PixelFormat format;
> > + V4L2PixelFormat v4l2Format;
> > +};
> > +
> > } /* namespace libcamera */
> >
> > #endif /* __LIBCAMERA_FORMATS_H__ */
> > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> > index e1c96b9862c3..580c0fc9d983 100644
> > --- a/src/libcamera/v4l2_pixelformat.cpp
> > +++ b/src/libcamera/v4l2_pixelformat.cpp
> > @@ -16,6 +16,7 @@
> >
> > #include <libcamera/pixelformats.h>
> >
> > +#include "formats.h"
> > #include "log.h"
> >
> > /**
> > @@ -43,71 +44,6 @@ LOG_DECLARE_CATEGORY(V4L2)
> >
> > namespace {
> >
> > -struct PixelFormatInfo {
> > - /* \todo Add support for non-contiguous memory planes */
> > - V4L2PixelFormat v4l2Format;
> > -};
> > -
> > -const std::map<PixelFormat, PixelFormatInfo> pf2vpf{
> > - /* RGB formats. */
> > - { PixelFormat(DRM_FORMAT_BGR888), {
> > - .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGB24),
> > - } },
> > - { PixelFormat(DRM_FORMAT_RGB888), {
> > - .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_BGR24),
> > - } },
> > - { PixelFormat(DRM_FORMAT_ABGR8888), {
> > - .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGBA32),
> > - } },
> > - { PixelFormat(DRM_FORMAT_ARGB8888), {
> > - .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_ABGR32),
> > - } },
> > - { PixelFormat(DRM_FORMAT_BGRA8888), {
> > - .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_ARGB32),
> > - } },
> > - { PixelFormat(DRM_FORMAT_RGBA8888), {
> > - .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_BGRA32),
> > - } },
> > -
> > - /* YUV packed formats. */
> > - { PixelFormat(DRM_FORMAT_YUYV), {
> > - .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YUYV),
> > - } },
> > - { PixelFormat(DRM_FORMAT_YVYU), {
> > - .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YVYU),
> > - } },
> > - { PixelFormat(DRM_FORMAT_UYVY), {
> > - .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_UYVY),
> > - } },
> > - { PixelFormat(DRM_FORMAT_VYUY), {
> > - .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_VYUY),
> > - } },
> > -
> > - /* YUV planar formats. */
> > - { PixelFormat(DRM_FORMAT_NV16), {
> > - .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV16),
> > - } },
> > - { PixelFormat(DRM_FORMAT_NV61), {
> > - .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV61),
> > - } },
> > - { PixelFormat(DRM_FORMAT_NV12), {
> > - .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV12),
> > - } },
> > - { PixelFormat(DRM_FORMAT_NV21), {
> > - .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV21),
> > - } },
> > -
> > - /* Greyscale formats. */
> > - { PixelFormat(DRM_FORMAT_R8), {
> > - .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_GREY),
> > - } },
> > -
> > - /* Compressed formats. */
> > - { PixelFormat(DRM_FORMAT_MJPEG), {
> > - .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),
> > - } },
> > -};
> > -
> > const std::map<V4L2PixelFormat, PixelFormat> vpf2pf{
> > /* RGB formats. */
> > { V4L2PixelFormat(V4L2_PIX_FMT_RGB24), PixelFormat(DRM_FORMAT_BGR888) },
> > @@ -233,15 +169,10 @@ PixelFormat V4L2PixelFormat::toPixelFormat() const
> > V4L2PixelFormat V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat,
> > bool multiplanar)
> > {
> > - const auto iter = pf2vpf.find(pixelFormat);
> > - if (iter == pf2vpf.end()) {
> > - LOG(V4L2, Warning)
> > - << "Unsupported pixel format "
> > - << pixelFormat.toString();
> > + const PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);
> > + if (!info.isValid())
> > return V4L2PixelFormat();
> > - }
> >
> > - const PixelFormatInfo &info = iter->second;
>
> Isn't this failing to take bool multiplanar into consideration?
It does, because we don't support it yet :-) That's the case today
already, so it's not a regression. The todo is moved to PixelFormatInfo
to track this.
> > return info.v4l2Format;
> > }
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list