| Message ID | 20220723095330.43542-5-jacopo@jmondi.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Sat, Jul 23, 2022 at 11:53:26AM +0200, Jacopo Mondi via libcamera-devel wrote: > This is a partial revert of commit 395d43d6d75b ("libcamera: > v4l2_videodevice: Drop toV4L2PixelFormat()") > > The function was removed because it incorrectly maps non-contigous V4L2 > format variants (ie V4L2_PIX_FMT_YUV420M) to the API version supported > by the video device (singleplanar API and multiplanar API). It was > decided at the time to remove the function and let its users call > directly V4L2PixelFormat::fromPixelFormat() which accepts a > 'multiplanar' flags. > > As we now aim to associate multiple V4L2PixelFormat to a single > libcamera format, it is necessary to verify which of them is actually > supported by the video device. > > Hence re-introduce V4L2VideoDevice::toV4L2PixelFormat() and convert all > the V4L2PixelFormat::fromPixelFormat() users to use it. > > The V4L2 compatibility layer is the only outliner as it doesn't have a > video device to poke, hence it still uses > V4L2PixelFormat::fromPixelFormat() and defaults to the first format. > > Next patches will implement the device format matching logic and handle > the non-contiguous plane issue in V4L2VideoDevice::toV4L2PixelFormat(). > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > Documentation/guides/pipeline-handler.rst | 7 ++-- > include/libcamera/internal/v4l2_videodevice.h | 2 ++ > src/libcamera/pipeline/ipu3/cio2.cpp | 2 +- > src/libcamera/pipeline/ipu3/imgu.cpp | 2 +- > .../pipeline/raspberrypi/raspberrypi.cpp | 34 +++++++++++-------- > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 6 ++-- > src/libcamera/pipeline/simple/converter.cpp | 10 +++--- > src/libcamera/pipeline/simple/simple.cpp | 2 +- > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 +-- > src/libcamera/pipeline/vimc/vimc.cpp | 8 ++--- > src/libcamera/v4l2_videodevice.cpp | 17 ++++++++++ > test/libtest/buffer_source.cpp | 2 +- > 12 files changed, 61 insertions(+), 35 deletions(-) > > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst > index 2d55666d094f..dcb6971e46f0 100644 > --- a/Documentation/guides/pipeline-handler.rst > +++ b/Documentation/guides/pipeline-handler.rst > @@ -971,7 +971,8 @@ with the fourcc and size attributes to apply directly to the capture device > node. The fourcc attribute is a `V4L2PixelFormat`_ and differs from the > ``libcamera::PixelFormat``. Converting the format requires knowledge of the > plane configuration for multiplanar formats, so you must explicitly convert it > -using the helper ``V4L2PixelFormat::fromPixelFormat()``. > +using the helper ``V4L2VideoDevice::toV4L2PixelFormat()`` provided by the > +V4L2VideoDevice instance of which the format will be applied on. > > .. _V4L2DeviceFormat: https://libcamera.org/api-html/classlibcamera_1_1V4L2DeviceFormat.html > .. _V4L2PixelFormat: https://libcamera.org/api-html/classlibcamera_1_1V4L2PixelFormat.html > @@ -981,7 +982,7 @@ Add the following code beneath the code from above: > .. code-block:: cpp > > V4L2DeviceFormat format = {}; > - format.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat); > + format.fourcc = data->video_->toV4L2PixelFormat(cfg.pixelFormat); > format.size = cfg.size; > > Set the video device format defined above using the > @@ -1001,7 +1002,7 @@ Continue the implementation with the following code: > return ret; > > if (format.size != cfg.size || > - format.fourcc != V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat)) > + format.fourcc != data->video_->toV4L2PixelFormat(cfg.pixelFormat)) > return -EINVAL; > > Finally, store and set stream-specific data reflecting the state of the stream. > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h > index 8525acbc558d..29fa0bbaf670 100644 > --- a/include/libcamera/internal/v4l2_videodevice.h > +++ b/include/libcamera/internal/v4l2_videodevice.h > @@ -228,6 +228,8 @@ public: > static std::unique_ptr<V4L2VideoDevice> > fromEntityName(const MediaDevice *media, const std::string &entity); > > + V4L2PixelFormat toV4L2PixelFormat(const PixelFormat &pixelFormat) const; > + > protected: > std::string logPrefix() const override; > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp > index 4a13d16973e9..d4e523af24b4 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)[0]; > + outputFormat->fourcc = output_->toV4L2PixelFormat(itInfo->second); > outputFormat->size = sensorFormat.size; > outputFormat->planesCount = 1; > > diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp > index 251164001ffd..531879f18183 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)[0]; > + outputFormat->fourcc = dev->toV4L2PixelFormat(formats::NV12); > outputFormat->size = cfg.size; > outputFormat->planesCount = 2; > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 6e813bcb094a..4f60224729e3 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -90,13 +90,14 @@ PixelFormat mbusCodeToPixelFormat(unsigned int mbus_code, > return pix; > } > > -V4L2DeviceFormat toV4L2DeviceFormat(const V4L2SubdeviceFormat &format, > +V4L2DeviceFormat toV4L2DeviceFormat(const V4L2VideoDevice *dev, > + const V4L2SubdeviceFormat &format, > BayerFormat::Packing packingReq) > { > const PixelFormat pix = mbusCodeToPixelFormat(format.mbus_code, packingReq); > V4L2DeviceFormat deviceFormat; > > - deviceFormat.fourcc = V4L2PixelFormat::fromPixelFormat(pix)[0]; > + deviceFormat.fourcc = dev->toV4L2PixelFormat(pix); > deviceFormat.size = format.size; > deviceFormat.colorSpace = format.colorSpace; > return deviceFormat; > @@ -422,15 +423,15 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() > * Calculate the best sensor mode we can use based on > * the user request. > */ > + V4L2VideoDevice *unicam = data_->unicam_[Unicam::Image].dev(); > const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat); > unsigned int bitDepth = info.isValid() ? info.bitsPerPixel : defaultRawBitDepth; > V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size, bitDepth); > BayerFormat::Packing packing = BayerFormat::Packing::CSI2; > if (info.isValid() && !info.packed) > packing = BayerFormat::Packing::None; > - V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat, > - packing); > - int ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat); > + V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(unicam, sensorFormat, packing); > + int ret = unicam->tryFormat(&unicamFormat); > if (ret) > return Invalid; > > @@ -516,14 +517,14 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() > > V4L2VideoDevice::Formats fmts = dev->formats(); > > - if (fmts.find(V4L2PixelFormat::fromPixelFormat(cfgPixFmt)[0]) == fmts.end()) { > + if (fmts.find(dev->toV4L2PixelFormat(cfgPixFmt)) == 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)[0]; > + format.fourcc = dev->toV4L2PixelFormat(cfgPixFmt); > format.size = cfg.size; > format.colorSpace = cfg.colorSpace; > > @@ -751,8 +752,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) > if (ret) > return ret; > > - V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat, packing); > - ret = data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat); > + V4L2VideoDevice *unicam = data->unicam_[Unicam::Image].dev(); > + V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(unicam, sensorFormat, packing); > + ret = unicam->setFormat(&unicamFormat); > if (ret) > return ret; > > @@ -783,7 +785,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)[0]; > + V4L2PixelFormat fourcc = stream->dev()->toV4L2PixelFormat(cfg.pixelFormat); > format.size = cfg.size; > format.fourcc = fourcc; > format.colorSpace = cfg.colorSpace; > @@ -826,13 +828,15 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) > * statistics coming from the hardware. > */ > if (!output0Set) { > + V4L2VideoDevice *dev = data->isp_[Isp::Output0].dev(); > + > maxSize = Size(320, 240); > format = {}; > format.size = maxSize; > - format.fourcc = V4L2PixelFormat::fromPixelFormat(formats::YUV420)[0]; > + format.fourcc = dev->toV4L2PixelFormat(formats::YUV420); > /* No one asked for output, so the color space doesn't matter. */ > format.colorSpace = ColorSpace::Jpeg; > - ret = data->isp_[Isp::Output0].dev()->setFormat(&format); > + ret = dev->setFormat(&format); > if (ret) { > LOG(RPI, Error) > << "Failed to set default format on ISP Output0: " > @@ -856,18 +860,20 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) > * colour denoise will not run. > */ > if (!output1Set) { > + V4L2VideoDevice *dev = data->isp_[Isp::Output1].dev(); > + > V4L2DeviceFormat output1Format; > constexpr Size maxDimensions(1200, 1200); > const Size limit = maxDimensions.boundedToAspectRatio(format.size); > > output1Format.size = (format.size / 2).boundedTo(limit).alignedDownTo(2, 2); > output1Format.colorSpace = format.colorSpace; > - output1Format.fourcc = V4L2PixelFormat::fromPixelFormat(formats::YUV420)[0]; > + output1Format.fourcc = dev->toV4L2PixelFormat(formats::YUV420); > > LOG(RPI, Debug) << "Setting ISP Output1 (internal) to " > << output1Format; > > - ret = data->isp_[Isp::Output1].dev()->setFormat(&output1Format); > + ret = dev->setFormat(&output1Format); > if (ret) { > LOG(RPI, Error) << "Failed to set format on ISP Output1: " > << ret; > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > index f4e71bab00ed..2d38f0fb37ab 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)[0]; > + format.fourcc = video_->toV4L2PixelFormat(cfg->pixelFormat); > 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)[0]; > + outputFormat.fourcc = video_->toV4L2PixelFormat(config.pixelFormat); > 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)[0]) { > + outputFormat.fourcc != video_->toV4L2PixelFormat(config.pixelFormat)) { > 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 47e2150b4332..acaaa64c949a 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)[0]; > + m2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat); > > 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)[0]; > + videoFormat = m2m_->capture()->toV4L2PixelFormat(outputCfg.pixelFormat); > 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)[0]; > + v4l2Format.fourcc = m2m_->output()->toV4L2PixelFormat(input); > v4l2Format.size = { 1, 1 }; > > int ret = m2m_->output()->setFormat(&v4l2Format); > @@ -220,7 +220,7 @@ std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input) > return {}; > } > > - if (v4l2Format.fourcc != V4L2PixelFormat::fromPixelFormat(input)) { > + if (v4l2Format.fourcc != m2m_->output()->toV4L2PixelFormat(input)) { > LOG(SimplePipeline, Debug) > << "Input format " << input << " not supported."; > return {}; > @@ -287,7 +287,7 @@ SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat, > const Size &size) > { > V4L2DeviceFormat format; > - format.fourcc = V4L2PixelFormat::fromPixelFormat(pixelFormat)[0]; > + format.fourcc = m2m_->capture()->toV4L2PixelFormat(pixelFormat); > 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 33a7b4e57deb..65a483c14379 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)[0]; > + format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); > format.size = cfg.size; > > int ret = data_->video_->tryFormat(&format); > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > index 338aff02aff0..cf870d26e3c3 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)[0]; > + format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); > 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)[0]; > + format.fourcc = data->video_->toV4L2PixelFormat(cfg.pixelFormat); > format.size = cfg.size; > > ret = data->video_->setFormat(&format); > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp > index b88b30fff108..153cf849bb18 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)[0]; > + format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); > 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)[0]; > + format.fourcc = data->video_->toV4L2PixelFormat(cfg.pixelFormat); > 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)[0]) > + format.fourcc != data->video_->toV4L2PixelFormat(cfg.pixelFormat)) > return -EINVAL; > > /* > @@ -598,7 +598,7 @@ int VimcCameraData::allocateMockIPABuffers() > constexpr unsigned int kBufCount = 2; > > V4L2DeviceFormat format; > - format.fourcc = V4L2PixelFormat::fromPixelFormat(formats::BGR888)[0]; > + format.fourcc = video_->toV4L2PixelFormat(formats::BGR888); > format.size = Size (160, 120); > > int ret = video_->setFormat(&format); > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index 63911339f96e..43c3d0f69266 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -1989,6 +1989,23 @@ V4L2VideoDevice::fromEntityName(const MediaDevice *media, > return std::make_unique<V4L2VideoDevice>(mediaEntity); > } > > +/** > + * \brief Convert \a PixelFormat to its corresponding V4L2 FourCC > + * \param[in] pixelFormat The PixelFormat to convert > + * > + * For multiplanar formats, the V4L2 format variant (contiguous or > + * non-contiguous planes) is selected automatically based on the capabilities > + * of the video device. If the video device supports the V4L2 multiplanar API, > + * non-contiguous formats are preferred. > + * > + * \return The V4L2_PIX_FMT_* pixel format code corresponding to \a pixelFormat > + */ > +V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelFormat) const > +{ > + return V4L2PixelFormat::fromPixelFormat(pixelFormat, > + caps_.isMultiplanar())[0]; To avoid modifying the existing behaviour, I'd drop the caps_.isMultiplanar(), as all callers of V4L2PixelFormat::fromPixelFormat() currently pass false (implicitly) as the second argument. The documentation should be updated to state that for multiplanar formats the function returns the V4L2 contiguous planes variant unconditionally (it will be updated later in the series). I would also like to see this patch moving to the beginning of the series. > +} > + > /** > * \class V4L2M2MDevice > * \brief Memory-to-Memory video device > diff --git a/test/libtest/buffer_source.cpp b/test/libtest/buffer_source.cpp > index 84f1169c9f5f..dde11f365e43 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)[0]; > + format.fourcc = video->toV4L2PixelFormat(config.pixelFormat); > if (video->setFormat(&format)) { > std::cout << "Failed to set format on output device" << std::endl; > return TestFail;
diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index 2d55666d094f..dcb6971e46f0 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -971,7 +971,8 @@ with the fourcc and size attributes to apply directly to the capture device node. The fourcc attribute is a `V4L2PixelFormat`_ and differs from the ``libcamera::PixelFormat``. Converting the format requires knowledge of the plane configuration for multiplanar formats, so you must explicitly convert it -using the helper ``V4L2PixelFormat::fromPixelFormat()``. +using the helper ``V4L2VideoDevice::toV4L2PixelFormat()`` provided by the +V4L2VideoDevice instance of which the format will be applied on. .. _V4L2DeviceFormat: https://libcamera.org/api-html/classlibcamera_1_1V4L2DeviceFormat.html .. _V4L2PixelFormat: https://libcamera.org/api-html/classlibcamera_1_1V4L2PixelFormat.html @@ -981,7 +982,7 @@ Add the following code beneath the code from above: .. code-block:: cpp V4L2DeviceFormat format = {}; - format.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat); + format.fourcc = data->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; Set the video device format defined above using the @@ -1001,7 +1002,7 @@ Continue the implementation with the following code: return ret; if (format.size != cfg.size || - format.fourcc != V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat)) + format.fourcc != data->video_->toV4L2PixelFormat(cfg.pixelFormat)) return -EINVAL; Finally, store and set stream-specific data reflecting the state of the stream. diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h index 8525acbc558d..29fa0bbaf670 100644 --- a/include/libcamera/internal/v4l2_videodevice.h +++ b/include/libcamera/internal/v4l2_videodevice.h @@ -228,6 +228,8 @@ public: static std::unique_ptr<V4L2VideoDevice> fromEntityName(const MediaDevice *media, const std::string &entity); + V4L2PixelFormat toV4L2PixelFormat(const PixelFormat &pixelFormat) const; + protected: std::string logPrefix() const override; diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index 4a13d16973e9..d4e523af24b4 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)[0]; + outputFormat->fourcc = output_->toV4L2PixelFormat(itInfo->second); outputFormat->size = sensorFormat.size; outputFormat->planesCount = 1; diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp index 251164001ffd..531879f18183 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)[0]; + outputFormat->fourcc = dev->toV4L2PixelFormat(formats::NV12); outputFormat->size = cfg.size; outputFormat->planesCount = 2; diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 6e813bcb094a..4f60224729e3 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -90,13 +90,14 @@ PixelFormat mbusCodeToPixelFormat(unsigned int mbus_code, return pix; } -V4L2DeviceFormat toV4L2DeviceFormat(const V4L2SubdeviceFormat &format, +V4L2DeviceFormat toV4L2DeviceFormat(const V4L2VideoDevice *dev, + const V4L2SubdeviceFormat &format, BayerFormat::Packing packingReq) { const PixelFormat pix = mbusCodeToPixelFormat(format.mbus_code, packingReq); V4L2DeviceFormat deviceFormat; - deviceFormat.fourcc = V4L2PixelFormat::fromPixelFormat(pix)[0]; + deviceFormat.fourcc = dev->toV4L2PixelFormat(pix); deviceFormat.size = format.size; deviceFormat.colorSpace = format.colorSpace; return deviceFormat; @@ -422,15 +423,15 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() * Calculate the best sensor mode we can use based on * the user request. */ + V4L2VideoDevice *unicam = data_->unicam_[Unicam::Image].dev(); const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat); unsigned int bitDepth = info.isValid() ? info.bitsPerPixel : defaultRawBitDepth; V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size, bitDepth); BayerFormat::Packing packing = BayerFormat::Packing::CSI2; if (info.isValid() && !info.packed) packing = BayerFormat::Packing::None; - V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat, - packing); - int ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat); + V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(unicam, sensorFormat, packing); + int ret = unicam->tryFormat(&unicamFormat); if (ret) return Invalid; @@ -516,14 +517,14 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() V4L2VideoDevice::Formats fmts = dev->formats(); - if (fmts.find(V4L2PixelFormat::fromPixelFormat(cfgPixFmt)[0]) == fmts.end()) { + if (fmts.find(dev->toV4L2PixelFormat(cfgPixFmt)) == 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)[0]; + format.fourcc = dev->toV4L2PixelFormat(cfgPixFmt); format.size = cfg.size; format.colorSpace = cfg.colorSpace; @@ -751,8 +752,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) if (ret) return ret; - V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat, packing); - ret = data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat); + V4L2VideoDevice *unicam = data->unicam_[Unicam::Image].dev(); + V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(unicam, sensorFormat, packing); + ret = unicam->setFormat(&unicamFormat); if (ret) return ret; @@ -783,7 +785,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)[0]; + V4L2PixelFormat fourcc = stream->dev()->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; format.fourcc = fourcc; format.colorSpace = cfg.colorSpace; @@ -826,13 +828,15 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) * statistics coming from the hardware. */ if (!output0Set) { + V4L2VideoDevice *dev = data->isp_[Isp::Output0].dev(); + maxSize = Size(320, 240); format = {}; format.size = maxSize; - format.fourcc = V4L2PixelFormat::fromPixelFormat(formats::YUV420)[0]; + format.fourcc = dev->toV4L2PixelFormat(formats::YUV420); /* No one asked for output, so the color space doesn't matter. */ format.colorSpace = ColorSpace::Jpeg; - ret = data->isp_[Isp::Output0].dev()->setFormat(&format); + ret = dev->setFormat(&format); if (ret) { LOG(RPI, Error) << "Failed to set default format on ISP Output0: " @@ -856,18 +860,20 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) * colour denoise will not run. */ if (!output1Set) { + V4L2VideoDevice *dev = data->isp_[Isp::Output1].dev(); + V4L2DeviceFormat output1Format; constexpr Size maxDimensions(1200, 1200); const Size limit = maxDimensions.boundedToAspectRatio(format.size); output1Format.size = (format.size / 2).boundedTo(limit).alignedDownTo(2, 2); output1Format.colorSpace = format.colorSpace; - output1Format.fourcc = V4L2PixelFormat::fromPixelFormat(formats::YUV420)[0]; + output1Format.fourcc = dev->toV4L2PixelFormat(formats::YUV420); LOG(RPI, Debug) << "Setting ISP Output1 (internal) to " << output1Format; - ret = data->isp_[Isp::Output1].dev()->setFormat(&output1Format); + ret = dev->setFormat(&output1Format); if (ret) { LOG(RPI, Error) << "Failed to set format on ISP Output1: " << ret; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp index f4e71bab00ed..2d38f0fb37ab 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)[0]; + format.fourcc = video_->toV4L2PixelFormat(cfg->pixelFormat); 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)[0]; + outputFormat.fourcc = video_->toV4L2PixelFormat(config.pixelFormat); 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)[0]) { + outputFormat.fourcc != video_->toV4L2PixelFormat(config.pixelFormat)) { 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 47e2150b4332..acaaa64c949a 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)[0]; + m2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat); 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)[0]; + videoFormat = m2m_->capture()->toV4L2PixelFormat(outputCfg.pixelFormat); 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)[0]; + v4l2Format.fourcc = m2m_->output()->toV4L2PixelFormat(input); v4l2Format.size = { 1, 1 }; int ret = m2m_->output()->setFormat(&v4l2Format); @@ -220,7 +220,7 @@ std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input) return {}; } - if (v4l2Format.fourcc != V4L2PixelFormat::fromPixelFormat(input)) { + if (v4l2Format.fourcc != m2m_->output()->toV4L2PixelFormat(input)) { LOG(SimplePipeline, Debug) << "Input format " << input << " not supported."; return {}; @@ -287,7 +287,7 @@ SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat, const Size &size) { V4L2DeviceFormat format; - format.fourcc = V4L2PixelFormat::fromPixelFormat(pixelFormat)[0]; + format.fourcc = m2m_->capture()->toV4L2PixelFormat(pixelFormat); 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 33a7b4e57deb..65a483c14379 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)[0]; + format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; int ret = data_->video_->tryFormat(&format); diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 338aff02aff0..cf870d26e3c3 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)[0]; + format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); 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)[0]; + format.fourcc = data->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; ret = data->video_->setFormat(&format); diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index b88b30fff108..153cf849bb18 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)[0]; + format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); 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)[0]; + format.fourcc = data->video_->toV4L2PixelFormat(cfg.pixelFormat); 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)[0]) + format.fourcc != data->video_->toV4L2PixelFormat(cfg.pixelFormat)) return -EINVAL; /* @@ -598,7 +598,7 @@ int VimcCameraData::allocateMockIPABuffers() constexpr unsigned int kBufCount = 2; V4L2DeviceFormat format; - format.fourcc = V4L2PixelFormat::fromPixelFormat(formats::BGR888)[0]; + format.fourcc = video_->toV4L2PixelFormat(formats::BGR888); format.size = Size (160, 120); int ret = video_->setFormat(&format); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 63911339f96e..43c3d0f69266 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1989,6 +1989,23 @@ V4L2VideoDevice::fromEntityName(const MediaDevice *media, return std::make_unique<V4L2VideoDevice>(mediaEntity); } +/** + * \brief Convert \a PixelFormat to its corresponding V4L2 FourCC + * \param[in] pixelFormat The PixelFormat to convert + * + * For multiplanar formats, the V4L2 format variant (contiguous or + * non-contiguous planes) is selected automatically based on the capabilities + * of the video device. If the video device supports the V4L2 multiplanar API, + * non-contiguous formats are preferred. + * + * \return The V4L2_PIX_FMT_* pixel format code corresponding to \a pixelFormat + */ +V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelFormat) const +{ + return V4L2PixelFormat::fromPixelFormat(pixelFormat, + caps_.isMultiplanar())[0]; +} + /** * \class V4L2M2MDevice * \brief Memory-to-Memory video device diff --git a/test/libtest/buffer_source.cpp b/test/libtest/buffer_source.cpp index 84f1169c9f5f..dde11f365e43 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)[0]; + format.fourcc = video->toV4L2PixelFormat(config.pixelFormat); if (video->setFormat(&format)) { std::cout << "Failed to set format on output device" << std::endl; return TestFail;
This is a partial revert of commit 395d43d6d75b ("libcamera: v4l2_videodevice: Drop toV4L2PixelFormat()") The function was removed because it incorrectly maps non-contigous V4L2 format variants (ie V4L2_PIX_FMT_YUV420M) to the API version supported by the video device (singleplanar API and multiplanar API). It was decided at the time to remove the function and let its users call directly V4L2PixelFormat::fromPixelFormat() which accepts a 'multiplanar' flags. As we now aim to associate multiple V4L2PixelFormat to a single libcamera format, it is necessary to verify which of them is actually supported by the video device. Hence re-introduce V4L2VideoDevice::toV4L2PixelFormat() and convert all the V4L2PixelFormat::fromPixelFormat() users to use it. The V4L2 compatibility layer is the only outliner as it doesn't have a video device to poke, hence it still uses V4L2PixelFormat::fromPixelFormat() and defaults to the first format. Next patches will implement the device format matching logic and handle the non-contiguous plane issue in V4L2VideoDevice::toV4L2PixelFormat(). Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- Documentation/guides/pipeline-handler.rst | 7 ++-- include/libcamera/internal/v4l2_videodevice.h | 2 ++ src/libcamera/pipeline/ipu3/cio2.cpp | 2 +- src/libcamera/pipeline/ipu3/imgu.cpp | 2 +- .../pipeline/raspberrypi/raspberrypi.cpp | 34 +++++++++++-------- src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 6 ++-- src/libcamera/pipeline/simple/converter.cpp | 10 +++--- src/libcamera/pipeline/simple/simple.cpp | 2 +- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 +-- src/libcamera/pipeline/vimc/vimc.cpp | 8 ++--- src/libcamera/v4l2_videodevice.cpp | 17 ++++++++++ test/libtest/buffer_source.cpp | 2 +- 12 files changed, 61 insertions(+), 35 deletions(-)