[{"id":24313,"web_url":"https://patchwork.libcamera.org/comment/24313/","msgid":"","date":"2022-08-02T16:35:39","subject":"Re: [libcamera-devel] [PATCH v4 3/6] libcamera: v4l2_videodevice:\n\tReintroduce toV4L2PixelFormat()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Aug 02, 2022 at 06:01:33PM +0200, Jacopo Mondi wrote:\n> This is a partial revert of commit 395d43d6d75b (\"libcamera:\n> v4l2_videodevice: Drop toV4L2PixelFormat()\")\n> \n> The function was removed because it incorrectly maps non-contigous V4L2\n\ns/contigous/contiguous/\n\n> format variants (ie V4L2_PIX_FMT_YUV420M) to the API version supported\n> by the video device (singleplanar API and multiplanar API). It was\n> decided at the time to remove the function and let its users call\n> directly V4L2PixelFormat::fromPixelFormat() which accepts a\n> 'multiplanar' flags.\n> \n> As we aim to associate multiple V4L2PixelFormat to a single libcamera\n> format, the next patches will verify which of them is actually supported\n> by the video device. For now, return the contiguous version\n> unconditionally.\n> \n> Re-introduce V4L2VideoDevice::toV4L2PixelFormat() and convert all\n> the V4L2PixelFormat::fromPixelFormat() users to use it.\n> \n> The V4L2 compatibility layer is the only outlier as it doesn't have a\n> video device to poke, hence it still uses\n> V4L2PixelFormat::fromPixelFormat().\n> \n> Next patches will implement the device format matching logic and handle\n> the non-contiguous plane issue in V4L2VideoDevice::toV4L2PixelFormat().\n> \n> Signed-off-by: Jacopo Mondi \n> Reviewed-by: Laurent Pinchart \n> ---\n> Documentation/guides/pipeline-handler.rst | 9 ++---\n> include/libcamera/internal/v4l2_videodevice.h | 2 ++\n> src/libcamera/pipeline/ipu3/cio2.cpp | 2 +-\n> src/libcamera/pipeline/ipu3/imgu.cpp | 2 +-\n> .../pipeline/raspberrypi/raspberrypi.cpp | 34 +++++++++++--------\n> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 6 ++--\n> src/libcamera/pipeline/simple/converter.cpp | 10 +++---\n> src/libcamera/pipeline/simple/simple.cpp | 4 +--\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 6 ++--\n> src/libcamera/pipeline/vimc/vimc.cpp | 8 ++---\n> src/libcamera/v4l2_videodevice.cpp | 14 ++++++++\n> test/libtest/buffer_source.cpp | 2 +-\n> 12 files changed, 61 insertions(+), 38 deletions(-)\n> \n> diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n> index 2d55666d094f..e1930fdf1630 100644\n> --- a/Documentation/guides/pipeline-handler.rst\n> +++ b/Documentation/guides/pipeline-handler.rst\n> @@ -289,7 +289,7 @@ features:\n> .. code-block:: cpp\n> \n> #include \n> - \n> +\n\nThis seems unrelated.\n\n> #include \"libcamera/internal/pipeline_handler.h\"\n> \n> Run the following commands:\n> @@ -971,7 +971,8 @@ with the fourcc and size attributes to apply directly to the capture device\n> node. The fourcc attribute is a `V4L2PixelFormat`_ and differs from the\n> ``libcamera::PixelFormat``. Converting the format requires knowledge of the\n> plane configuration for multiplanar formats, so you must explicitly convert it\n> -using the helper ``V4L2PixelFormat::fromPixelFormat()``.\n> +using the helper ``V4L2VideoDevice::toV4L2PixelFormat()`` provided by the\n> +V4L2VideoDevice instance that the format will be applied on.\n> \n> .. _V4L2DeviceFormat: https://libcamera.org/api-html/classlibcamera_1_1V4L2DeviceFormat.html\n> .. _V4L2PixelFormat: https://libcamera.org/api-html/classlibcamera_1_1V4L2PixelFormat.html\n> @@ -981,7 +982,7 @@ Add the following code beneath the code from above:\n> .. code-block:: cpp\n> \n> V4L2DeviceFormat format = {};\n> - format.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> + format.fourcc = data->video_->toV4L2PixelFormat(cfg.pixelFormat);\n> format.size = cfg.size;\n> \n> Set the video device format defined above using the\n> @@ -1001,7 +1002,7 @@ Continue the implementation with the following code:\n> return ret;\n> \n> if (format.size != cfg.size ||\n> - format.fourcc != V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat))\n> + format.fourcc != data->video_->toV4L2PixelFormat(cfg.pixelFormat))\n> return -EINVAL;\n> \n> Finally, store and set stream-specific data reflecting the state of the stream.\n> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> index 8525acbc558d..29fa0bbaf670 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -228,6 +228,8 @@ public:\n> \tstatic std::unique_ptr\n> \tfromEntityName(const MediaDevice *media, const std::string &entity);\n> \n> +\tV4L2PixelFormat toV4L2PixelFormat(const PixelFormat &pixelFormat) const;\n> +\n> protected:\n> \tstd::string logPrefix() const override;\n> \n> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> index 08e254f75eee..d4e523af24b4 100644\n> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> @@ -203,7 +203,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n> \tif (itInfo == mbusCodesToPixelFormat.end())\n> \t\treturn -EINVAL;\n> \n> -\toutputFormat->fourcc = V4L2PixelFormat::fromPixelFormat(itInfo->second);\n> +\toutputFormat->fourcc = output_->toV4L2PixelFormat(itInfo->second);\n> \toutputFormat->size = sensorFormat.size;\n> \toutputFormat->planesCount = 1;\n> \n> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\n> index 59305f85073c..531879f18183 100644\n> --- a/src/libcamera/pipeline/ipu3/imgu.cpp\n> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> @@ -558,7 +558,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad,\n> \t\treturn 0;\n> \n> \t*outputFormat = {};\n> -\toutputFormat->fourcc = V4L2PixelFormat::fromPixelFormat(formats::NV12);\n> +\toutputFormat->fourcc = dev->toV4L2PixelFormat(formats::NV12);\n> \toutputFormat->size = cfg.size;\n> \toutputFormat->planesCount = 2;\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 3da17ea90984..e895584d4fbc 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -90,13 +90,14 @@ PixelFormat mbusCodeToPixelFormat(unsigned int mbus_code,\n> \treturn pix;\n> }\n> \n> -V4L2DeviceFormat toV4L2DeviceFormat(const V4L2SubdeviceFormat &format,\n> +V4L2DeviceFormat toV4L2DeviceFormat(const V4L2VideoDevice *dev,\n> +\t\t\t\t const V4L2SubdeviceFormat &format,\n> \t\t\t\t BayerFormat::Packing packingReq)\n> {\n> \tconst PixelFormat pix = mbusCodeToPixelFormat(format.mbus_code, packingReq);\n> \tV4L2DeviceFormat deviceFormat;\n> \n> -\tdeviceFormat.fourcc = V4L2PixelFormat::fromPixelFormat(pix);\n> +\tdeviceFormat.fourcc = dev->toV4L2PixelFormat(pix);\n> \tdeviceFormat.size = format.size;\n> \tdeviceFormat.colorSpace = format.colorSpace;\n> \treturn deviceFormat;\n> @@ -422,15 +423,15 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> \t\t\t * Calculate the best sensor mode we can use based on\n> \t\t\t * the user request.\n> \t\t\t */\n> +\t\t\tV4L2VideoDevice *unicam = data_->unicam_[Unicam::Image].dev();\n> \t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> \t\t\tunsigned int bitDepth = info.isValid() ? info.bitsPerPixel : defaultRawBitDepth;\n> \t\t\tV4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size, bitDepth);\n> \t\t\tBayerFormat::Packing packing = BayerFormat::Packing::CSI2;\n> \t\t\tif (info.isValid() && !info.packed)\n> \t\t\t\tpacking = BayerFormat::Packing::None;\n> -\t\t\tV4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,\n> -\t\t\t\t\t\t\t\t\t packing);\n> -\t\t\tint ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);\n> +\t\t\tV4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(unicam, sensorFormat, packing);\n> +\t\t\tint ret = unicam->tryFormat(&unicamFormat);\n> \t\t\tif (ret)\n> \t\t\t\treturn Invalid;\n> \n> @@ -516,14 +517,14 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> \n> \t\tV4L2VideoDevice::Formats fmts = dev->formats();\n> \n> -\t\tif (fmts.find(V4L2PixelFormat::fromPixelFormat(cfgPixFmt)) == fmts.end()) {\n> +\t\tif (fmts.find(dev->toV4L2PixelFormat(cfgPixFmt)) == fmts.end()) {\n> \t\t\t/* If we cannot find a native format, use a default one. */\n> \t\t\tcfgPixFmt = formats::NV12;\n> \t\t\tstatus = Adjusted;\n> \t\t}\n> \n> \t\tV4L2DeviceFormat format;\n> -\t\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(cfgPixFmt);\n> +\t\tformat.fourcc = dev->toV4L2PixelFormat(cfg.pixelFormat);\n> \t\tformat.size = cfg.size;\n> \t\tformat.colorSpace = cfg.colorSpace;\n> \n> @@ -751,8 +752,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tV4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat, packing);\n> -\tret = data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat);\n> +\tV4L2VideoDevice *unicam = data->unicam_[Unicam::Image].dev();\n> +\tV4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(unicam, sensorFormat, packing);\n> +\tret = unicam->setFormat(&unicamFormat);\n> \tif (ret)\n> \t\treturn ret;\n> \n> @@ -783,7 +785,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> \t\tRPi::Stream *stream = i == maxIndex ? &data->isp_[Isp::Output0]\n> \t\t\t\t\t\t : &data->isp_[Isp::Output1];\n> \n> -\t\tV4L2PixelFormat fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> +\t\tV4L2PixelFormat fourcc = stream->dev()->toV4L2PixelFormat(cfg.pixelFormat);\n> \t\tformat.size = cfg.size;\n> \t\tformat.fourcc = fourcc;\n> \t\tformat.colorSpace = cfg.colorSpace;\n> @@ -826,13 +828,15 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> \t * statistics coming from the hardware.\n> \t */\n> \tif (!output0Set) {\n> +\t\tV4L2VideoDevice *dev = data->isp_[Isp::Output0].dev();\n> +\n> \t\tmaxSize = Size(320, 240);\n> \t\tformat = {};\n> \t\tformat.size = maxSize;\n> -\t\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(formats::YUV420);\n> +\t\tformat.fourcc = dev->toV4L2PixelFormat(formats::YUV420);\n> \t\t/* No one asked for output, so the color space doesn't matter. */\n> \t\tformat.colorSpace = ColorSpace::Jpeg;\n> -\t\tret = data->isp_[Isp::Output0].dev()->setFormat(&format);\n> +\t\tret = dev->setFormat(&format);\n> \t\tif (ret) {\n> \t\t\tLOG(RPI, Error)\n> \t\t\t\t<< \"Failed to set default format on ISP Output0: \"\n> @@ -856,18 +860,20 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> \t * colour denoise will not run.\n> \t */\n> \tif (!output1Set) {\n> +\t\tV4L2VideoDevice *dev = data->isp_[Isp::Output1].dev();\n> +\n> \t\tV4L2DeviceFormat output1Format;\n> \t\tconstexpr Size maxDimensions(1200, 1200);\n> \t\tconst Size limit = maxDimensions.boundedToAspectRatio(format.size);\n> \n> \t\toutput1Format.size = (format.size / 2).boundedTo(limit).alignedDownTo(2, 2);\n> \t\toutput1Format.colorSpace = format.colorSpace;\n> -\t\toutput1Format.fourcc = V4L2PixelFormat::fromPixelFormat(formats::YUV420);\n> +\t\toutput1Format.fourcc = dev->toV4L2PixelFormat(formats::YUV420);\n> \n> \t\tLOG(RPI, Debug) << \"Setting ISP Output1 (internal) to \"\n> \t\t\t\t<< output1Format;\n> \n> -\t\tret = data->isp_[Isp::Output1].dev()->setFormat(&output1Format);\n> +\t\tret = dev->setFormat(&output1Format);\n> \t\tif (ret) {\n> \t\t\tLOG(RPI, Error) << \"Failed to set format on ISP Output1: \"\n> \t\t\t\t\t<< ret;\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> index 6f175758665d..2d38f0fb37ab 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> @@ -81,7 +81,7 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)\n> \tcfg->bufferCount = RKISP1_BUFFER_COUNT;\n> \n> \tV4L2DeviceFormat format;\n> -\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(cfg->pixelFormat);\n> +\tformat.fourcc = video_->toV4L2PixelFormat(cfg->pixelFormat);\n> \tformat.size = cfg->size;\n> \n> \tint ret = video_->tryFormat(&format);\n> @@ -147,7 +147,7 @@ int RkISP1Path::configure(const StreamConfiguration &config,\n> \n> \tconst PixelFormatInfo &info = PixelFormatInfo::info(config.pixelFormat);\n> \tV4L2DeviceFormat outputFormat;\n> -\toutputFormat.fourcc = V4L2PixelFormat::fromPixelFormat(config.pixelFormat);\n> +\toutputFormat.fourcc = video_->toV4L2PixelFormat(config.pixelFormat);\n> \toutputFormat.size = config.size;\n> \toutputFormat.planesCount = info.numPlanes();\n> \n> @@ -156,7 +156,7 @@ int RkISP1Path::configure(const StreamConfiguration &config,\n> \t\treturn ret;\n> \n> \tif (outputFormat.size != config.size ||\n> -\t outputFormat.fourcc != V4L2PixelFormat::fromPixelFormat(config.pixelFormat)) {\n> +\t outputFormat.fourcc != video_->toV4L2PixelFormat(config.pixelFormat)) {\n> \t\tLOG(RkISP1, Error)\n> \t\t\t<< \"Unable to configure capture in \" << config.toString();\n> \t\treturn -EINVAL;\n> diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp\n> index 62d173bb6cd1..acaaa64c949a 100644\n> --- a/src/libcamera/pipeline/simple/converter.cpp\n> +++ b/src/libcamera/pipeline/simple/converter.cpp\n> @@ -46,7 +46,7 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg,\n> \t\t\t\t const StreamConfiguration &outputCfg)\n> {\n> \tV4L2PixelFormat videoFormat =\n> -\t\tV4L2PixelFormat::fromPixelFormat(inputCfg.pixelFormat);\n> +\t\tm2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat);\n> \n> \tV4L2DeviceFormat format;\n> \tformat.fourcc = videoFormat;\n> @@ -71,7 +71,7 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg,\n> \t}\n> \n> \t/* Set the pixel format and size on the output. */\n> -\tvideoFormat = V4L2PixelFormat::fromPixelFormat(outputCfg.pixelFormat);\n> +\tvideoFormat = m2m_->capture()->toV4L2PixelFormat(outputCfg.pixelFormat);\n> \tformat = {};\n> \tformat.fourcc = videoFormat;\n> \tformat.size = outputCfg.size;\n> @@ -210,7 +210,7 @@ std::vector SimpleConverter::formats(PixelFormat input)\n> \t * enumerate the conversion capabilities on its output (V4L2 capture).\n> \t */\n> \tV4L2DeviceFormat v4l2Format;\n> -\tv4l2Format.fourcc = V4L2PixelFormat::fromPixelFormat(input);\n> +\tv4l2Format.fourcc = m2m_->output()->toV4L2PixelFormat(input);\n> \tv4l2Format.size = { 1, 1 };\n> \n> \tint ret = m2m_->output()->setFormat(&v4l2Format);\n> @@ -220,7 +220,7 @@ std::vector SimpleConverter::formats(PixelFormat input)\n> \t\treturn {};\n> \t}\n> \n> -\tif (v4l2Format.fourcc != V4L2PixelFormat::fromPixelFormat(input)) {\n> +\tif (v4l2Format.fourcc != m2m_->output()->toV4L2PixelFormat(input)) {\n> \t\tLOG(SimplePipeline, Debug)\n> \t\t\t<< \"Input format \" << input << \" not supported.\";\n> \t\treturn {};\n> @@ -287,7 +287,7 @@ SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat,\n> \t\t\t\t const Size &size)\n> {\n> \tV4L2DeviceFormat format;\n> -\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(pixelFormat);\n> +\tformat.fourcc = m2m_->capture()->toV4L2PixelFormat(pixelFormat);\n> \tformat.size = size;\n> \n> \tint ret = m2m_->capture()->tryFormat(&format);\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index bc0cb1a00c2a..05ae7d392603 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -918,7 +918,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> \t\t\t\treturn Invalid;\n> \t\t} else {\n> \t\t\tV4L2DeviceFormat format;\n> -\t\t\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> +\t\t\tformat.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);\n> \t\t\tformat.size = cfg.size;\n> \n> \t\t\tint ret = data_->video_->tryFormat(&format);\n> @@ -1028,7 +1028,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> \t\treturn ret;\n> \n> \t/* Configure the video node. */\n> -\tV4L2PixelFormat videoFormat = V4L2PixelFormat::fromPixelFormat(pipeConfig->captureFormat);\n> +\tV4L2PixelFormat videoFormat = video->toV4L2PixelFormat(pipeConfig->captureFormat);\n> \n> \tV4L2DeviceFormat captureFormat;\n> \tcaptureFormat.fourcc = videoFormat;\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 53b2f23ab029..fbe02cdcd520 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -149,7 +149,7 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()\n> \tcfg.bufferCount = 4;\n> \n> \tV4L2DeviceFormat format;\n> -\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> +\tformat.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);\n> \tformat.size = cfg.size;\n> \n> \tint ret = data_->video_->tryFormat(&format);\n> @@ -205,7 +205,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)\n> \tint ret;\n> \n> \tV4L2DeviceFormat format;\n> -\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> +\tformat.fourcc = data->video_->toV4L2PixelFormat(cfg.pixelFormat);\n> \tformat.size = cfg.size;\n> \n> \tret = data->video_->setFormat(&format);\n> @@ -213,7 +213,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)\n> \t\treturn ret;\n> \n> \tif (format.size != cfg.size ||\n> -\t format.fourcc != V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat))\n> +\t format.fourcc != data->video_->toV4L2PixelFormat(cfg.pixelFormat))\n> \t\treturn -EINVAL;\n> \n> \tcfg.setStream(&data->stream_);\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 3379ac5cd47d..153cf849bb18 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -171,7 +171,7 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()\n> \tcfg.bufferCount = 4;\n> \n> \tV4L2DeviceFormat format;\n> -\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> +\tformat.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);\n> \tformat.size = cfg.size;\n> \n> \tint ret = data_->video_->tryFormat(&format);\n> @@ -275,7 +275,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> \t\treturn ret;\n> \n> \tV4L2DeviceFormat format;\n> -\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> +\tformat.fourcc = data->video_->toV4L2PixelFormat(cfg.pixelFormat);\n> \tformat.size = cfg.size;\n> \n> \tret = data->video_->setFormat(&format);\n> @@ -283,7 +283,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> \t\treturn ret;\n> \n> \tif (format.size != cfg.size ||\n> -\t format.fourcc != V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat))\n> +\t format.fourcc != data->video_->toV4L2PixelFormat(cfg.pixelFormat))\n> \t\treturn -EINVAL;\n> \n> \t/*\n> @@ -598,7 +598,7 @@ int VimcCameraData::allocateMockIPABuffers()\n> \tconstexpr unsigned int kBufCount = 2;\n> \n> \tV4L2DeviceFormat format;\n> -\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(formats::BGR888);\n> +\tformat.fourcc = video_->toV4L2PixelFormat(formats::BGR888);\n> \tformat.size = Size (160, 120);\n> \n> \tint ret = video_->setFormat(&format);\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index 63911339f96e..f41afa06f460 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -1989,6 +1989,20 @@ V4L2VideoDevice::fromEntityName(const MediaDevice *media,\n> \treturn std::make_unique(mediaEntity);\n> }\n> \n> +/**\n> + * \\brief Convert \\a PixelFormat to its corresponding V4L2 FourCC\n> + * \\param[in] pixelFormat The PixelFormat to convert\n> + *\n> + * The V4L2 format variant the function returns the contiguous version\n> + * unconditionally.\n> + *\n> + * \\return The V4L2_PIX_FMT_* pixel format code corresponding to \\a pixelFormat\n> + */\n> +V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelFormat) const\n> +{\n> +\treturn V4L2PixelFormat::fromPixelFormat(pixelFormat);\n> +}\n> +\n> /**\n> * \\class V4L2M2MDevice\n> * \\brief Memory-to-Memory video device\n> diff --git a/test/libtest/buffer_source.cpp b/test/libtest/buffer_source.cpp\n> index 1b261697279a..dde11f365e43 100644\n> --- a/test/libtest/buffer_source.cpp\n> +++ b/test/libtest/buffer_source.cpp\n> @@ -72,7 +72,7 @@ int BufferSource::allocate(const StreamConfiguration &config)\n> \t}\n> \n> \tformat.size = config.size;\n> -\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(config.pixelFormat);\n> +\tformat.fourcc = video->toV4L2PixelFormat(config.pixelFormat);\n> \tif (video->setFormat(&format)) {\n> \t\tstd::cout << \"Failed to set format on output device\" << std::endl;\n> \t\treturn TestFail;","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 10AA6C3275\n\tfor ;\n\tTue, 2 Aug 2022 16:35:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8E70463313;\n\tTue, 2 Aug 2022 18:35:45 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A169D603E7\n\tfor ;\n\tTue, 2 Aug 2022 18:35:44 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 131F225B;\n\tTue, 2 Aug 2022 18:35:44 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659458145;\n\tbh=gPhZldpyO9a85tGOvlHO3zuhDJq6hslGIK0D/kF55Ls=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=MLUARMmmoThjfcxWVX0r2NVkE+GgF+zQokJ0mcq/ClwOHZ1Z0bI889KPmPgm4Gb/X\n\trbYvzb7fc2Pu8fm+sDS6QdTp4pc0XTlZWTrHy1UfpV/dvX5eHU9WIwzMlBnFykJadi\n\tJ0i9nHeVZ0RZ5+bDeHbTsxKB7SVxQlu0Az+rAufy/8SON20A2OI2kbynF1nbX0L0wJ\n\tjzs7gj0Ou6EvAdKySUkXRaaqPDWpdmVsKepzEZKoaG5vKaCMdJIRFSiWSgbgOIy14F\n\tLmYQ9iigUdbKn9rJqE8hLcNGnyDV5zXrOuKLuYiqD16m372VrmbIlzTWdEuFmfK1pD\n\tSZ5k3ByrjxlUA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659458144;\n\tbh=gPhZldpyO9a85tGOvlHO3zuhDJq6hslGIK0D/kF55Ls=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QPkqlJ3HuSxYw84Ti5OHoVNmQgUaOyJi0yHKd0XozB20aTnf5gJfnMOrrhyRokb4U\n\tu0JxU9xBIe0oaJ738AxutUD5arNK35+yMp7lJjJYBPJQwWAt8nJjI1CSFwU5KDiyYi\n\tqR7DrDV1sw3IF0zBagoefa8+JulWq5+tEPfLo/sY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"QPkqlJ3H\"; dkim-atps=neutral","Date":"Tue, 2 Aug 2022 19:35:39 +0300","To":"Jacopo Mondi ","Message-ID":"","References":"<20220802160136.63075-1-jacopo@jmondi.org>\n\t<20220802160136.63075-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220802160136.63075-4-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v4 3/6] libcamera: v4l2_videodevice:\n\tReintroduce toV4L2PixelFormat()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, jozef@mlich.cz,\n\tPavel Machek ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24327,"web_url":"https://patchwork.libcamera.org/comment/24327/","msgid":"<20220803101632.fldjj2sn3snuz4ej@uno.localdomain>","date":"2022-08-03T10:16:32","subject":"Re: [libcamera-devel] [PATCH v4 3/6] libcamera: v4l2_videodevice:\n\tReintroduce toV4L2PixelFormat()","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Tue, Aug 02, 2022 at 07:35:39PM +0300, Laurent Pinchart wrote:\n> On Tue, Aug 02, 2022 at 06:01:33PM +0200, Jacopo Mondi wrote:\n> > This is a partial revert of commit 395d43d6d75b (\"libcamera:\n> > v4l2_videodevice: Drop toV4L2PixelFormat()\")\n> >\n> > The function was removed because it incorrectly maps non-contigous V4L2\n>\n> s/contigous/contiguous/\n>\n> > format variants (ie V4L2_PIX_FMT_YUV420M) to the API version supported\n> > by the video device (singleplanar API and multiplanar API). It was\n> > decided at the time to remove the function and let its users call\n> > directly V4L2PixelFormat::fromPixelFormat() which accepts a\n> > 'multiplanar' flags.\n> >\n> > As we aim to associate multiple V4L2PixelFormat to a single libcamera\n> > format, the next patches will verify which of them is actually supported\n> > by the video device. For now, return the contiguous version\n> > unconditionally.\n> >\n> > Re-introduce V4L2VideoDevice::toV4L2PixelFormat() and convert all\n> > the V4L2PixelFormat::fromPixelFormat() users to use it.\n> >\n> > The V4L2 compatibility layer is the only outlier as it doesn't have a\n> > video device to poke, hence it still uses\n> > V4L2PixelFormat::fromPixelFormat().\n> >\n> > Next patches will implement the device format matching logic and handle\n> > the non-contiguous plane issue in V4L2VideoDevice::toV4L2PixelFormat().\n> >\n> > Signed-off-by: Jacopo Mondi \n> > Reviewed-by: Laurent Pinchart \n> > ---\n> > Documentation/guides/pipeline-handler.rst | 9 ++---\n> > include/libcamera/internal/v4l2_videodevice.h | 2 ++\n> > src/libcamera/pipeline/ipu3/cio2.cpp | 2 +-\n> > src/libcamera/pipeline/ipu3/imgu.cpp | 2 +-\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 34 +++++++++++--------\n> > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 6 ++--\n> > src/libcamera/pipeline/simple/converter.cpp | 10 +++---\n> > src/libcamera/pipeline/simple/simple.cpp | 4 +--\n> > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 6 ++--\n> > src/libcamera/pipeline/vimc/vimc.cpp | 8 ++---\n> > src/libcamera/v4l2_videodevice.cpp | 14 ++++++++\n> > test/libtest/buffer_source.cpp | 2 +-\n> > 12 files changed, 61 insertions(+), 38 deletions(-)\n> >\n> > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n> > index 2d55666d094f..e1930fdf1630 100644\n> > --- a/Documentation/guides/pipeline-handler.rst\n> > +++ b/Documentation/guides/pipeline-handler.rst\n> > @@ -289,7 +289,7 @@ features:\n> > .. code-block:: cpp\n> >\n> > #include \n> > -\n> > +\n>\n> This seems unrelated.\n>\n\nAh ups, there were 4 rogue spaces there and my editor removes them\nautomatically. It makes sense to drop them imho, but in a separate\ntiny patch.\n\n> > #include \"libcamera/internal/pipeline_handler.h\"\n> >\n> > Run the following commands:\n> > @@ -971,7 +971,8 @@ with the fourcc and size attributes to apply directly to the capture device\n> > node. The fourcc attribute is a `V4L2PixelFormat`_ and differs from the\n> > ``libcamera::PixelFormat``. Converting the format requires knowledge of the\n> > plane configuration for multiplanar formats, so you must explicitly convert it\n> > -using the helper ``V4L2PixelFormat::fromPixelFormat()``.\n> > +using the helper ``V4L2VideoDevice::toV4L2PixelFormat()`` provided by the\n> > +V4L2VideoDevice instance that the format will be applied on.\n> >\n> > .. _V4L2DeviceFormat: https://libcamera.org/api-html/classlibcamera_1_1V4L2DeviceFormat.html\n> > .. _V4L2PixelFormat: https://libcamera.org/api-html/classlibcamera_1_1V4L2PixelFormat.html\n> > @@ -981,7 +982,7 @@ Add the following code beneath the code from above:\n> > .. code-block:: cpp\n> >\n> > V4L2DeviceFormat format = {};\n> > - format.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> > + format.fourcc = data->video_->toV4L2PixelFormat(cfg.pixelFormat);\n> > format.size = cfg.size;\n> >\n> > Set the video device format defined above using the\n> > @@ -1001,7 +1002,7 @@ Continue the implementation with the following code:\n> > return ret;\n> >\n> > if (format.size != cfg.size ||\n> > - format.fourcc != V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat))\n> > + format.fourcc != data->video_->toV4L2PixelFormat(cfg.pixelFormat))\n> > return -EINVAL;\n> >\n> > Finally, store and set stream-specific data reflecting the state of the stream.\n> > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> > index 8525acbc558d..29fa0bbaf670 100644\n> > --- a/include/libcamera/internal/v4l2_videodevice.h\n> > +++ b/include/libcamera/internal/v4l2_videodevice.h\n> > @@ -228,6 +228,8 @@ public:\n> > \tstatic std::unique_ptr\n> > \tfromEntityName(const MediaDevice *media, const std::string &entity);\n> >\n> > +\tV4L2PixelFormat toV4L2PixelFormat(const PixelFormat &pixelFormat) const;\n> > +\n> > protected:\n> > \tstd::string logPrefix() const override;\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > index 08e254f75eee..d4e523af24b4 100644\n> > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > @@ -203,7 +203,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n> > \tif (itInfo == mbusCodesToPixelFormat.end())\n> > \t\treturn -EINVAL;\n> >\n> > -\toutputFormat->fourcc = V4L2PixelFormat::fromPixelFormat(itInfo->second);\n> > +\toutputFormat->fourcc = output_->toV4L2PixelFormat(itInfo->second);\n> > \toutputFormat->size = sensorFormat.size;\n> > \toutputFormat->planesCount = 1;\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\n> > index 59305f85073c..531879f18183 100644\n> > --- a/src/libcamera/pipeline/ipu3/imgu.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> > @@ -558,7 +558,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad,\n> > \t\treturn 0;\n> >\n> > \t*outputFormat = {};\n> > -\toutputFormat->fourcc = V4L2PixelFormat::fromPixelFormat(formats::NV12);\n> > +\toutputFormat->fourcc = dev->toV4L2PixelFormat(formats::NV12);\n> > \toutputFormat->size = cfg.size;\n> > \toutputFormat->planesCount = 2;\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 3da17ea90984..e895584d4fbc 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -90,13 +90,14 @@ PixelFormat mbusCodeToPixelFormat(unsigned int mbus_code,\n> > \treturn pix;\n> > }\n> >\n> > -V4L2DeviceFormat toV4L2DeviceFormat(const V4L2SubdeviceFormat &format,\n> > +V4L2DeviceFormat toV4L2DeviceFormat(const V4L2VideoDevice *dev,\n> > +\t\t\t\t const V4L2SubdeviceFormat &format,\n> > \t\t\t\t BayerFormat::Packing packingReq)\n> > {\n> > \tconst PixelFormat pix = mbusCodeToPixelFormat(format.mbus_code, packingReq);\n> > \tV4L2DeviceFormat deviceFormat;\n> >\n> > -\tdeviceFormat.fourcc = V4L2PixelFormat::fromPixelFormat(pix);\n> > +\tdeviceFormat.fourcc = dev->toV4L2PixelFormat(pix);\n> > \tdeviceFormat.size = format.size;\n> > \tdeviceFormat.colorSpace = format.colorSpace;\n> > \treturn deviceFormat;\n> > @@ -422,15 +423,15 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > \t\t\t * Calculate the best sensor mode we can use based on\n> > \t\t\t * the user request.\n> > \t\t\t */\n> > +\t\t\tV4L2VideoDevice *unicam = data_->unicam_[Unicam::Image].dev();\n> > \t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> > \t\t\tunsigned int bitDepth = info.isValid() ? info.bitsPerPixel : defaultRawBitDepth;\n> > \t\t\tV4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size, bitDepth);\n> > \t\t\tBayerFormat::Packing packing = BayerFormat::Packing::CSI2;\n> > \t\t\tif (info.isValid() && !info.packed)\n> > \t\t\t\tpacking = BayerFormat::Packing::None;\n> > -\t\t\tV4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,\n> > -\t\t\t\t\t\t\t\t\t packing);\n> > -\t\t\tint ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);\n> > +\t\t\tV4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(unicam, sensorFormat, packing);\n> > +\t\t\tint ret = unicam->tryFormat(&unicamFormat);\n> > \t\t\tif (ret)\n> > \t\t\t\treturn Invalid;\n> >\n> > @@ -516,14 +517,14 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> >\n> > \t\tV4L2VideoDevice::Formats fmts = dev->formats();\n> >\n> > -\t\tif (fmts.find(V4L2PixelFormat::fromPixelFormat(cfgPixFmt)) == fmts.end()) {\n> > +\t\tif (fmts.find(dev->toV4L2PixelFormat(cfgPixFmt)) == fmts.end()) {\n> > \t\t\t/* If we cannot find a native format, use a default one. */\n> > \t\t\tcfgPixFmt = formats::NV12;\n> > \t\t\tstatus = Adjusted;\n> > \t\t}\n> >\n> > \t\tV4L2DeviceFormat format;\n> > -\t\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(cfgPixFmt);\n> > +\t\tformat.fourcc = dev->toV4L2PixelFormat(cfg.pixelFormat);\n> > \t\tformat.size = cfg.size;\n> > \t\tformat.colorSpace = cfg.colorSpace;\n> >\n> > @@ -751,8 +752,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > \tif (ret)\n> > \t\treturn ret;\n> >\n> > -\tV4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat, packing);\n> > -\tret = data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat);\n> > +\tV4L2VideoDevice *unicam = data->unicam_[Unicam::Image].dev();\n> > +\tV4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(unicam, sensorFormat, packing);\n> > +\tret = unicam->setFormat(&unicamFormat);\n> > \tif (ret)\n> > \t\treturn ret;\n> >\n> > @@ -783,7 +785,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > \t\tRPi::Stream *stream = i == maxIndex ? &data->isp_[Isp::Output0]\n> > \t\t\t\t\t\t : &data->isp_[Isp::Output1];\n> >\n> > -\t\tV4L2PixelFormat fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> > +\t\tV4L2PixelFormat fourcc = stream->dev()->toV4L2PixelFormat(cfg.pixelFormat);\n> > \t\tformat.size = cfg.size;\n> > \t\tformat.fourcc = fourcc;\n> > \t\tformat.colorSpace = cfg.colorSpace;\n> > @@ -826,13 +828,15 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > \t * statistics coming from the hardware.\n> > \t */\n> > \tif (!output0Set) {\n> > +\t\tV4L2VideoDevice *dev = data->isp_[Isp::Output0].dev();\n> > +\n> > \t\tmaxSize = Size(320, 240);\n> > \t\tformat = {};\n> > \t\tformat.size = maxSize;\n> > -\t\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(formats::YUV420);\n> > +\t\tformat.fourcc = dev->toV4L2PixelFormat(formats::YUV420);\n> > \t\t/* No one asked for output, so the color space doesn't matter. */\n> > \t\tformat.colorSpace = ColorSpace::Jpeg;\n> > -\t\tret = data->isp_[Isp::Output0].dev()->setFormat(&format);\n> > +\t\tret = dev->setFormat(&format);\n> > \t\tif (ret) {\n> > \t\t\tLOG(RPI, Error)\n> > \t\t\t\t<< \"Failed to set default format on ISP Output0: \"\n> > @@ -856,18 +860,20 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > \t * colour denoise will not run.\n> > \t */\n> > \tif (!output1Set) {\n> > +\t\tV4L2VideoDevice *dev = data->isp_[Isp::Output1].dev();\n> > +\n> > \t\tV4L2DeviceFormat output1Format;\n> > \t\tconstexpr Size maxDimensions(1200, 1200);\n> > \t\tconst Size limit = maxDimensions.boundedToAspectRatio(format.size);\n> >\n> > \t\toutput1Format.size = (format.size / 2).boundedTo(limit).alignedDownTo(2, 2);\n> > \t\toutput1Format.colorSpace = format.colorSpace;\n> > -\t\toutput1Format.fourcc = V4L2PixelFormat::fromPixelFormat(formats::YUV420);\n> > +\t\toutput1Format.fourcc = dev->toV4L2PixelFormat(formats::YUV420);\n> >\n> > \t\tLOG(RPI, Debug) << \"Setting ISP Output1 (internal) to \"\n> > \t\t\t\t<< output1Format;\n> >\n> > -\t\tret = data->isp_[Isp::Output1].dev()->setFormat(&output1Format);\n> > +\t\tret = dev->setFormat(&output1Format);\n> > \t\tif (ret) {\n> > \t\t\tLOG(RPI, Error) << \"Failed to set format on ISP Output1: \"\n> > \t\t\t\t\t<< ret;\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> > index 6f175758665d..2d38f0fb37ab 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> > @@ -81,7 +81,7 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)\n> > \tcfg->bufferCount = RKISP1_BUFFER_COUNT;\n> >\n> > \tV4L2DeviceFormat format;\n> > -\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(cfg->pixelFormat);\n> > +\tformat.fourcc = video_->toV4L2PixelFormat(cfg->pixelFormat);\n> > \tformat.size = cfg->size;\n> >\n> > \tint ret = video_->tryFormat(&format);\n> > @@ -147,7 +147,7 @@ int RkISP1Path::configure(const StreamConfiguration &config,\n> >\n> > \tconst PixelFormatInfo &info = PixelFormatInfo::info(config.pixelFormat);\n> > \tV4L2DeviceFormat outputFormat;\n> > -\toutputFormat.fourcc = V4L2PixelFormat::fromPixelFormat(config.pixelFormat);\n> > +\toutputFormat.fourcc = video_->toV4L2PixelFormat(config.pixelFormat);\n> > \toutputFormat.size = config.size;\n> > \toutputFormat.planesCount = info.numPlanes();\n> >\n> > @@ -156,7 +156,7 @@ int RkISP1Path::configure(const StreamConfiguration &config,\n> > \t\treturn ret;\n> >\n> > \tif (outputFormat.size != config.size ||\n> > -\t outputFormat.fourcc != V4L2PixelFormat::fromPixelFormat(config.pixelFormat)) {\n> > +\t outputFormat.fourcc != video_->toV4L2PixelFormat(config.pixelFormat)) {\n> > \t\tLOG(RkISP1, Error)\n> > \t\t\t<< \"Unable to configure capture in \" << config.toString();\n> > \t\treturn -EINVAL;\n> > diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp\n> > index 62d173bb6cd1..acaaa64c949a 100644\n> > --- a/src/libcamera/pipeline/simple/converter.cpp\n> > +++ b/src/libcamera/pipeline/simple/converter.cpp\n> > @@ -46,7 +46,7 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg,\n> > \t\t\t\t const StreamConfiguration &outputCfg)\n> > {\n> > \tV4L2PixelFormat videoFormat =\n> > -\t\tV4L2PixelFormat::fromPixelFormat(inputCfg.pixelFormat);\n> > +\t\tm2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat);\n> >\n> > \tV4L2DeviceFormat format;\n> > \tformat.fourcc = videoFormat;\n> > @@ -71,7 +71,7 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg,\n> > \t}\n> >\n> > \t/* Set the pixel format and size on the output. */\n> > -\tvideoFormat = V4L2PixelFormat::fromPixelFormat(outputCfg.pixelFormat);\n> > +\tvideoFormat = m2m_->capture()->toV4L2PixelFormat(outputCfg.pixelFormat);\n> > \tformat = {};\n> > \tformat.fourcc = videoFormat;\n> > \tformat.size = outputCfg.size;\n> > @@ -210,7 +210,7 @@ std::vector SimpleConverter::formats(PixelFormat input)\n> > \t * enumerate the conversion capabilities on its output (V4L2 capture).\n> > \t */\n> > \tV4L2DeviceFormat v4l2Format;\n> > -\tv4l2Format.fourcc = V4L2PixelFormat::fromPixelFormat(input);\n> > +\tv4l2Format.fourcc = m2m_->output()->toV4L2PixelFormat(input);\n> > \tv4l2Format.size = { 1, 1 };\n> >\n> > \tint ret = m2m_->output()->setFormat(&v4l2Format);\n> > @@ -220,7 +220,7 @@ std::vector SimpleConverter::formats(PixelFormat input)\n> > \t\treturn {};\n> > \t}\n> >\n> > -\tif (v4l2Format.fourcc != V4L2PixelFormat::fromPixelFormat(input)) {\n> > +\tif (v4l2Format.fourcc != m2m_->output()->toV4L2PixelFormat(input)) {\n> > \t\tLOG(SimplePipeline, Debug)\n> > \t\t\t<< \"Input format \" << input << \" not supported.\";\n> > \t\treturn {};\n> > @@ -287,7 +287,7 @@ SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat,\n> > \t\t\t\t const Size &size)\n> > {\n> > \tV4L2DeviceFormat format;\n> > -\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(pixelFormat);\n> > +\tformat.fourcc = m2m_->capture()->toV4L2PixelFormat(pixelFormat);\n> > \tformat.size = size;\n> >\n> > \tint ret = m2m_->capture()->tryFormat(&format);\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index bc0cb1a00c2a..05ae7d392603 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -918,7 +918,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> > \t\t\t\treturn Invalid;\n> > \t\t} else {\n> > \t\t\tV4L2DeviceFormat format;\n> > -\t\t\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> > +\t\t\tformat.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);\n> > \t\t\tformat.size = cfg.size;\n> >\n> > \t\t\tint ret = data_->video_->tryFormat(&format);\n> > @@ -1028,7 +1028,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> > \t\treturn ret;\n> >\n> > \t/* Configure the video node. */\n> > -\tV4L2PixelFormat videoFormat = V4L2PixelFormat::fromPixelFormat(pipeConfig->captureFormat);\n> > +\tV4L2PixelFormat videoFormat = video->toV4L2PixelFormat(pipeConfig->captureFormat);\n> >\n> > \tV4L2DeviceFormat captureFormat;\n> > \tcaptureFormat.fourcc = videoFormat;\n> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > index 53b2f23ab029..fbe02cdcd520 100644\n> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > @@ -149,7 +149,7 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()\n> > \tcfg.bufferCount = 4;\n> >\n> > \tV4L2DeviceFormat format;\n> > -\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> > +\tformat.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);\n> > \tformat.size = cfg.size;\n> >\n> > \tint ret = data_->video_->tryFormat(&format);\n> > @@ -205,7 +205,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)\n> > \tint ret;\n> >\n> > \tV4L2DeviceFormat format;\n> > -\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> > +\tformat.fourcc = data->video_->toV4L2PixelFormat(cfg.pixelFormat);\n> > \tformat.size = cfg.size;\n> >\n> > \tret = data->video_->setFormat(&format);\n> > @@ -213,7 +213,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)\n> > \t\treturn ret;\n> >\n> > \tif (format.size != cfg.size ||\n> > -\t format.fourcc != V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat))\n> > +\t format.fourcc != data->video_->toV4L2PixelFormat(cfg.pixelFormat))\n> > \t\treturn -EINVAL;\n> >\n> > \tcfg.setStream(&data->stream_);\n> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > index 3379ac5cd47d..153cf849bb18 100644\n> > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > @@ -171,7 +171,7 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()\n> > \tcfg.bufferCount = 4;\n> >\n> > \tV4L2DeviceFormat format;\n> > -\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> > +\tformat.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);\n> > \tformat.size = cfg.size;\n> >\n> > \tint ret = data_->video_->tryFormat(&format);\n> > @@ -275,7 +275,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> > \t\treturn ret;\n> >\n> > \tV4L2DeviceFormat format;\n> > -\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> > +\tformat.fourcc = data->video_->toV4L2PixelFormat(cfg.pixelFormat);\n> > \tformat.size = cfg.size;\n> >\n> > \tret = data->video_->setFormat(&format);\n> > @@ -283,7 +283,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> > \t\treturn ret;\n> >\n> > \tif (format.size != cfg.size ||\n> > -\t format.fourcc != V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat))\n> > +\t format.fourcc != data->video_->toV4L2PixelFormat(cfg.pixelFormat))\n> > \t\treturn -EINVAL;\n> >\n> > \t/*\n> > @@ -598,7 +598,7 @@ int VimcCameraData::allocateMockIPABuffers()\n> > \tconstexpr unsigned int kBufCount = 2;\n> >\n> > \tV4L2DeviceFormat format;\n> > -\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(formats::BGR888);\n> > +\tformat.fourcc = video_->toV4L2PixelFormat(formats::BGR888);\n> > \tformat.size = Size (160, 120);\n> >\n> > \tint ret = video_->setFormat(&format);\n> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> > index 63911339f96e..f41afa06f460 100644\n> > --- a/src/libcamera/v4l2_videodevice.cpp\n> > +++ b/src/libcamera/v4l2_videodevice.cpp\n> > @@ -1989,6 +1989,20 @@ V4L2VideoDevice::fromEntityName(const MediaDevice *media,\n> > \treturn std::make_unique(mediaEntity);\n> > }\n> >\n> > +/**\n> > + * \\brief Convert \\a PixelFormat to its corresponding V4L2 FourCC\n> > + * \\param[in] pixelFormat The PixelFormat to convert\n> > + *\n> > + * The V4L2 format variant the function returns the contiguous version\n> > + * unconditionally.\n> > + *\n> > + * \\return The V4L2_PIX_FMT_* pixel format code corresponding to \\a pixelFormat\n> > + */\n> > +V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelFormat) const\n> > +{\n> > +\treturn V4L2PixelFormat::fromPixelFormat(pixelFormat);\n> > +}\n> > +\n> > /**\n> > * \\class V4L2M2MDevice\n> > * \\brief Memory-to-Memory video device\n> > diff --git a/test/libtest/buffer_source.cpp b/test/libtest/buffer_source.cpp\n> > index 1b261697279a..dde11f365e43 100644\n> > --- a/test/libtest/buffer_source.cpp\n> > +++ b/test/libtest/buffer_source.cpp\n> > @@ -72,7 +72,7 @@ int BufferSource::allocate(const StreamConfiguration &config)\n> > \t}\n> >\n> > \tformat.size = config.size;\n> > -\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(config.pixelFormat);\n> > +\tformat.fourcc = video->toV4L2PixelFormat(config.pixelFormat);\n> > \tif (video->setFormat(&format)) {\n> > \t\tstd::cout << \"Failed to set format on output device\" << std::endl;\n> > \t\treturn TestFail;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7CC26BE173\n\tfor ;\n\tWed, 3 Aug 2022 10:16:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B615363310;\n\tWed, 3 Aug 2022 12:16:38 +0200 (CEST)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3C4B4603E6\n\tfor ;\n\tWed, 3 Aug 2022 12:16:37 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 6323820005;\n\tWed, 3 Aug 2022 10:16:34 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659521798;\n\tbh=wiXh4S7PVbzmD60GBsRaTaGX9a4KijapbazwZsaDRbc=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=gi5GFGNSYevYixLB5Cm9l8NIXmxbMHvkzzL17+YLII51yKTiMfng78qfbYOmI1cy8\n\tyRGbI6xXZ5XAc4enrfjACrNy5AKoNarxt3cnEiZeAKqhC3usVKBdjcYaWSD41wT4U6\n\tcYO3fX3J2Ca+/1b2962vP6q5BjZJhIZyNqT67lpsaTdLDVKtCF+c5qMmCJiFLyvP10\n\t968VgzGQbCRCT/unJWePEmZkXiezEtiNLJr1ET51nuFctMlc7FMxtXB6qNHoT+NDdw\n\tAooLds2j78VtKdA20xYasiM4BizjlSBzOWX8vsn0P1UySgtJbFjyKchASS12mvnIFq\n\tK95P+DbIB0q9w==","Date":"Wed, 3 Aug 2022 12:16:32 +0200","To":"Laurent Pinchart ","Message-ID":"<20220803101632.fldjj2sn3snuz4ej@uno.localdomain>","References":"<20220802160136.63075-1-jacopo@jmondi.org>\n\t<20220802160136.63075-4-jacopo@jmondi.org>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v4 3/6] libcamera: v4l2_videodevice:\n\tReintroduce toV4L2PixelFormat()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org, jozef@mlich.cz,\n\tPavel Machek ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]