[libcamera-devel] [PATCH v2 3/8] libcamera: v4l2_pixelformat: Return a format list in fromPixelFormat()
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Jul 23 19:14:04 CEST 2022
Hi Jacopo,
Thank you for the patch.
On Sat, Jul 23, 2022 at 11:53:25AM +0200, Jacopo Mondi via libcamera-devel wrote:
> As we prepare to associate multiple V4L2 FourCC to a single libcamera
> format, make the V4L2PixelFormat::fromPixelFormat() return a list of
> formats and convert all its users to pick the first one.
>
> This change prepares to re-introduce V4L2VideoDevice::toV4L2PixelFormat()
> which will instead match the list of V4L2 FourCC against the formats
> supported by the video device.
I think it would be easier to first re-introduce
V4L2VideoDevice::toV4L2PixelFormat(), so that changing
V4L2PixelFormat::fromPixelFormat() will not have to update a large
number of callers.
> While at it, reword the V4L2PixelFormat::fromPixelFormat() documentation
> to distinguish between planar/multiplanar and contiguous/non-contiguous.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> include/libcamera/internal/v4l2_pixelformat.h | 6 ++++--
> src/libcamera/pipeline/ipu3/cio2.cpp | 2 +-
> src/libcamera/pipeline/ipu3/imgu.cpp | 2 +-
> .../pipeline/raspberrypi/raspberrypi.cpp | 12 +++++------
> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 6 +++---
> src/libcamera/pipeline/simple/converter.cpp | 8 ++++----
> src/libcamera/pipeline/simple/simple.cpp | 4 ++--
> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 6 +++---
> src/libcamera/pipeline/vimc/vimc.cpp | 8 ++++----
> src/libcamera/v4l2_pixelformat.cpp | 20 +++++++++----------
> src/v4l2/v4l2_camera_proxy.cpp | 6 +++---
> test/libtest/buffer_source.cpp | 2 +-
> 12 files changed, 42 insertions(+), 40 deletions(-)
>
> diff --git a/include/libcamera/internal/v4l2_pixelformat.h b/include/libcamera/internal/v4l2_pixelformat.h
> index fb2d5d0b93df..750f31fc1926 100644
> --- a/include/libcamera/internal/v4l2_pixelformat.h
> +++ b/include/libcamera/internal/v4l2_pixelformat.h
> @@ -11,6 +11,7 @@
> #include <ostream>
> #include <stdint.h>
> #include <string>
> +#include <vector>
>
> #include <linux/videodev2.h>
>
> @@ -44,8 +45,9 @@ public:
> const char *description() const;
>
> PixelFormat toPixelFormat() const;
> - static V4L2PixelFormat fromPixelFormat(const PixelFormat &pixelFormat,
> - bool multiplanar = false);
> + static std::vector<V4L2PixelFormat>
> + fromPixelFormat(const PixelFormat &pixelFormat,
> + bool multiplanar = false);
>
> private:
> uint32_t fourcc_;
> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> index 08e254f75eee..4a13d16973e9 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> @@ -203,7 +203,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)
> if (itInfo == mbusCodesToPixelFormat.end())
> return -EINVAL;
>
> - outputFormat->fourcc = V4L2PixelFormat::fromPixelFormat(itInfo->second);
> + outputFormat->fourcc = V4L2PixelFormat::fromPixelFormat(itInfo->second)[0];
> outputFormat->size = sensorFormat.size;
> outputFormat->planesCount = 1;
>
> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
> index 59305f85073c..251164001ffd 100644
> --- a/src/libcamera/pipeline/ipu3/imgu.cpp
> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp
> @@ -558,7 +558,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad,
> return 0;
>
> *outputFormat = {};
> - outputFormat->fourcc = V4L2PixelFormat::fromPixelFormat(formats::NV12);
> + outputFormat->fourcc = V4L2PixelFormat::fromPixelFormat(formats::NV12)[0];
> outputFormat->size = cfg.size;
> outputFormat->planesCount = 2;
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 9d5614152430..6e813bcb094a 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -96,7 +96,7 @@ V4L2DeviceFormat toV4L2DeviceFormat(const V4L2SubdeviceFormat &format,
> const PixelFormat pix = mbusCodeToPixelFormat(format.mbus_code, packingReq);
> V4L2DeviceFormat deviceFormat;
>
> - deviceFormat.fourcc = V4L2PixelFormat::fromPixelFormat(pix);
> + deviceFormat.fourcc = V4L2PixelFormat::fromPixelFormat(pix)[0];
> deviceFormat.size = format.size;
> deviceFormat.colorSpace = format.colorSpace;
> return deviceFormat;
> @@ -516,14 +516,14 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
>
> V4L2VideoDevice::Formats fmts = dev->formats();
>
> - if (fmts.find(V4L2PixelFormat::fromPixelFormat(cfgPixFmt)) == fmts.end()) {
> + if (fmts.find(V4L2PixelFormat::fromPixelFormat(cfgPixFmt)[0]) == fmts.end()) {
> /* If we cannot find a native format, use a default one. */
> cfgPixFmt = formats::NV12;
> status = Adjusted;
> }
>
> V4L2DeviceFormat format;
> - format.fourcc = V4L2PixelFormat::fromPixelFormat(cfgPixFmt);
> + format.fourcc = V4L2PixelFormat::fromPixelFormat(cfgPixFmt)[0];
> format.size = cfg.size;
> format.colorSpace = cfg.colorSpace;
>
> @@ -783,7 +783,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> RPi::Stream *stream = i == maxIndex ? &data->isp_[Isp::Output0]
> : &data->isp_[Isp::Output1];
>
> - V4L2PixelFormat fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);
> + V4L2PixelFormat fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat)[0];
> format.size = cfg.size;
> format.fourcc = fourcc;
> format.colorSpace = cfg.colorSpace;
> @@ -829,7 +829,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> maxSize = Size(320, 240);
> format = {};
> format.size = maxSize;
> - format.fourcc = V4L2PixelFormat::fromPixelFormat(formats::YUV420);
> + format.fourcc = V4L2PixelFormat::fromPixelFormat(formats::YUV420)[0];
> /* No one asked for output, so the color space doesn't matter. */
> format.colorSpace = ColorSpace::Jpeg;
> ret = data->isp_[Isp::Output0].dev()->setFormat(&format);
> @@ -862,7 +862,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>
> output1Format.size = (format.size / 2).boundedTo(limit).alignedDownTo(2, 2);
> output1Format.colorSpace = format.colorSpace;
> - output1Format.fourcc = V4L2PixelFormat::fromPixelFormat(formats::YUV420);
> + output1Format.fourcc = V4L2PixelFormat::fromPixelFormat(formats::YUV420)[0];
>
> LOG(RPI, Debug) << "Setting ISP Output1 (internal) to "
> << output1Format;
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> index 6f175758665d..f4e71bab00ed 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> @@ -81,7 +81,7 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)
> cfg->bufferCount = RKISP1_BUFFER_COUNT;
>
> V4L2DeviceFormat format;
> - format.fourcc = V4L2PixelFormat::fromPixelFormat(cfg->pixelFormat);
> + format.fourcc = V4L2PixelFormat::fromPixelFormat(cfg->pixelFormat)[0];
> format.size = cfg->size;
>
> int ret = video_->tryFormat(&format);
> @@ -147,7 +147,7 @@ int RkISP1Path::configure(const StreamConfiguration &config,
>
> const PixelFormatInfo &info = PixelFormatInfo::info(config.pixelFormat);
> V4L2DeviceFormat outputFormat;
> - outputFormat.fourcc = V4L2PixelFormat::fromPixelFormat(config.pixelFormat);
> + outputFormat.fourcc = V4L2PixelFormat::fromPixelFormat(config.pixelFormat)[0];
> outputFormat.size = config.size;
> outputFormat.planesCount = info.numPlanes();
>
> @@ -156,7 +156,7 @@ int RkISP1Path::configure(const StreamConfiguration &config,
> return ret;
>
> if (outputFormat.size != config.size ||
> - outputFormat.fourcc != V4L2PixelFormat::fromPixelFormat(config.pixelFormat)) {
> + outputFormat.fourcc != V4L2PixelFormat::fromPixelFormat(config.pixelFormat)[0]) {
> LOG(RkISP1, Error)
> << "Unable to configure capture in " << config.toString();
> return -EINVAL;
> diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
> index 62d173bb6cd1..47e2150b4332 100644
> --- a/src/libcamera/pipeline/simple/converter.cpp
> +++ b/src/libcamera/pipeline/simple/converter.cpp
> @@ -46,7 +46,7 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg,
> const StreamConfiguration &outputCfg)
> {
> V4L2PixelFormat videoFormat =
> - V4L2PixelFormat::fromPixelFormat(inputCfg.pixelFormat);
> + V4L2PixelFormat::fromPixelFormat(inputCfg.pixelFormat)[0];
>
> V4L2DeviceFormat format;
> format.fourcc = videoFormat;
> @@ -71,7 +71,7 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg,
> }
>
> /* Set the pixel format and size on the output. */
> - videoFormat = V4L2PixelFormat::fromPixelFormat(outputCfg.pixelFormat);
> + videoFormat = V4L2PixelFormat::fromPixelFormat(outputCfg.pixelFormat)[0];
> format = {};
> format.fourcc = videoFormat;
> format.size = outputCfg.size;
> @@ -210,7 +210,7 @@ std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input)
> * enumerate the conversion capabilities on its output (V4L2 capture).
> */
> V4L2DeviceFormat v4l2Format;
> - v4l2Format.fourcc = V4L2PixelFormat::fromPixelFormat(input);
> + v4l2Format.fourcc = V4L2PixelFormat::fromPixelFormat(input)[0];
> v4l2Format.size = { 1, 1 };
>
> int ret = m2m_->output()->setFormat(&v4l2Format);
> @@ -287,7 +287,7 @@ SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat,
> const Size &size)
> {
> V4L2DeviceFormat format;
> - format.fourcc = V4L2PixelFormat::fromPixelFormat(pixelFormat);
> + format.fourcc = V4L2PixelFormat::fromPixelFormat(pixelFormat)[0];
> format.size = size;
>
> int ret = m2m_->capture()->tryFormat(&format);
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index bc0cb1a00c2a..33a7b4e57deb 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -918,7 +918,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
> return Invalid;
> } else {
> V4L2DeviceFormat format;
> - format.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);
> + format.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat)[0];
> format.size = cfg.size;
>
> int ret = data_->video_->tryFormat(&format);
> @@ -1028,7 +1028,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
> return ret;
>
> /* Configure the video node. */
> - V4L2PixelFormat videoFormat = V4L2PixelFormat::fromPixelFormat(pipeConfig->captureFormat);
> + V4L2PixelFormat videoFormat = V4L2PixelFormat::fromPixelFormat(pipeConfig->captureFormat)[0];
>
> V4L2DeviceFormat captureFormat;
> captureFormat.fourcc = videoFormat;
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 53b2f23ab029..338aff02aff0 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -149,7 +149,7 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()
> cfg.bufferCount = 4;
>
> V4L2DeviceFormat format;
> - format.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);
> + format.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat)[0];
> format.size = cfg.size;
>
> int ret = data_->video_->tryFormat(&format);
> @@ -205,7 +205,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
> int ret;
>
> V4L2DeviceFormat format;
> - format.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);
> + format.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat)[0];
> format.size = cfg.size;
>
> ret = data->video_->setFormat(&format);
> @@ -213,7 +213,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
> return ret;
>
> if (format.size != cfg.size ||
> - format.fourcc != V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat))
> + format.fourcc != V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat)[0])
> return -EINVAL;
>
> cfg.setStream(&data->stream_);
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 3379ac5cd47d..b88b30fff108 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -171,7 +171,7 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()
> cfg.bufferCount = 4;
>
> V4L2DeviceFormat format;
> - format.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);
> + format.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat)[0];
> format.size = cfg.size;
>
> int ret = data_->video_->tryFormat(&format);
> @@ -275,7 +275,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
> return ret;
>
> V4L2DeviceFormat format;
> - format.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);
> + format.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat)[0];
> format.size = cfg.size;
>
> ret = data->video_->setFormat(&format);
> @@ -283,7 +283,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
> return ret;
>
> if (format.size != cfg.size ||
> - format.fourcc != V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat))
> + format.fourcc != V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat)[0])
> return -EINVAL;
>
> /*
> @@ -598,7 +598,7 @@ int VimcCameraData::allocateMockIPABuffers()
> constexpr unsigned int kBufCount = 2;
>
> V4L2DeviceFormat format;
> - format.fourcc = V4L2PixelFormat::fromPixelFormat(formats::BGR888);
> + format.fourcc = V4L2PixelFormat::fromPixelFormat(formats::BGR888)[0];
> format.size = Size (160, 120);
>
> int ret = video_->setFormat(&format);
> diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> index cf6c1858bd1a..7ab63f092a28 100644
> --- a/src/libcamera/v4l2_pixelformat.cpp
> +++ b/src/libcamera/v4l2_pixelformat.cpp
> @@ -302,26 +302,26 @@ PixelFormat V4L2PixelFormat::toPixelFormat() const
> }
>
> /**
> - * \brief Convert \a pixelFormat to its corresponding V4L2PixelFormat
> + * \brief Convert \a pixelFormat to a list of corresponding V4L2PixelFormat
> * \param[in] pixelFormat The PixelFormat to convert
> * \param[in] multiplanar V4L2 Multiplanar API support flag
> *
> - * Multiple V4L2 formats may exist for one PixelFormat when the format uses
> - * multiple planes, as V4L2 defines separate 4CCs for contiguous and separate
> - * planes formats. Set the \a multiplanar parameter to false to select a format
> - * with contiguous planes, or to true to select a format with non-contiguous
> - * planes.
> + * Multiple V4L2 formats may exist for one PixelFormat. This function returns
> + * the list of V4L2 formats and distinguish if the format describes contiguous
> + * or non-contiguous planes as V4L2 defines separate 4CCs for the two variants.
> + * This function uses the \a multiplanar argument, defaulted to false,
> + * to select formats with non-contiguous or contiguous planes.
> *
> * \return The V4L2PixelFormat corresponding to \a pixelFormat
> */
> -V4L2PixelFormat V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat,
> - bool multiplanar)
> +std::vector<V4L2PixelFormat> V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat,
> + bool multiplanar)
> {
> const PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);
> if (!info.isValid())
> - return V4L2PixelFormat();
> + return {};
>
> - return multiplanar ? info.v4l2Formats.multi[0] : info.v4l2Formats.single[0];
> + return multiplanar ? info.v4l2Formats.multi : info.v4l2Formats.single;
> }
>
> /**
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 26a227da6db2..55ff62cdb430 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -182,7 +182,7 @@ void V4L2CameraProxy::setFmtFromConfig(const StreamConfiguration &streamConfig)
>
> v4l2PixFormat_.width = size.width;
> v4l2PixFormat_.height = size.height;
> - v4l2PixFormat_.pixelformat = V4L2PixelFormat::fromPixelFormat(streamConfig.pixelFormat);
> + v4l2PixFormat_.pixelformat = V4L2PixelFormat::fromPixelFormat(streamConfig.pixelFormat)[0];
> v4l2PixFormat_.field = V4L2_FIELD_NONE;
> v4l2PixFormat_.bytesperline = streamConfig.stride;
> v4l2PixFormat_.sizeimage = streamConfig.frameSize;
> @@ -290,7 +290,7 @@ int V4L2CameraProxy::vidioc_enum_fmt(V4L2CameraFile *file, struct v4l2_fmtdesc *
> return -EINVAL;
>
> PixelFormat format = streamConfig_.formats().pixelformats()[arg->index];
> - V4L2PixelFormat v4l2Format = V4L2PixelFormat::fromPixelFormat(format);
> + V4L2PixelFormat v4l2Format = V4L2PixelFormat::fromPixelFormat(format)[0];
>
> arg->flags = format == formats::MJPEG ? V4L2_FMT_FLAG_COMPRESSED : 0;
> utils::strlcpy(reinterpret_cast<char *>(arg->description),
> @@ -333,7 +333,7 @@ int V4L2CameraProxy::tryFormat(struct v4l2_format *arg)
>
> arg->fmt.pix.width = config.size.width;
> arg->fmt.pix.height = config.size.height;
> - arg->fmt.pix.pixelformat = V4L2PixelFormat::fromPixelFormat(config.pixelFormat);
> + arg->fmt.pix.pixelformat = V4L2PixelFormat::fromPixelFormat(config.pixelFormat)[0];
> arg->fmt.pix.field = V4L2_FIELD_NONE;
> arg->fmt.pix.bytesperline = config.stride;
> arg->fmt.pix.sizeimage = config.frameSize;
> diff --git a/test/libtest/buffer_source.cpp b/test/libtest/buffer_source.cpp
> index 1b261697279a..84f1169c9f5f 100644
> --- a/test/libtest/buffer_source.cpp
> +++ b/test/libtest/buffer_source.cpp
> @@ -72,7 +72,7 @@ int BufferSource::allocate(const StreamConfiguration &config)
> }
>
> format.size = config.size;
> - format.fourcc = V4L2PixelFormat::fromPixelFormat(config.pixelFormat);
> + format.fourcc = V4L2PixelFormat::fromPixelFormat(config.pixelFormat)[0];
> if (video->setFormat(&format)) {
> std::cout << "Failed to set format on output device" << std::endl;
> return TestFail;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list