[libcamera-devel] [PATCH] libcamera: pipeline: vimc: Use appropriate media bus format

[Laurent Pinchart]

Pick the correct media bus format based on the video pixel format on the
capture node.

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
 src/libcamera/pipeline/vimc.cpp | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

This patch, while being correct (I think :-)), shouldn't be integrated,
as it breaks VIMC camera support with Qt < 5.14.0. On 5.14.0, the
viewfinder reports the following supported native formats (in that
order):

- DRM_FORMAT_ABGR8888
- DRM_FORMAT_ARGB8888
- DRM_FORMAT_BGR888
- DRM_FORMAT_RGB888

The first two formats are not supported by the VIMC pipeline handler,
the last two are. The third format, DRM_FORMAT_BGR888, is thus picked,
which corresponds to MEDIA_BUS_FMT_RGB888_1X24, the default today.

On Qt < 5.14.0, the third format isn't supported, so the fourth format,
DRM_FORMAT_RGB888, will be picked. This results in a different pipeline
configuration, with the debayering subdev source pad being configured
with MEDIA_BUS_FMT_BGR888_1X24. The kernel driver is meant to support
that, but in drivers/media/platform/vimc/vimc-debayer.c the
vimc_deb_set_fmt() function handles the source pad format with

	/*
	 * Do not change the format of the source pad,
	 * it is propagated from the sink
	 */
	if (VIMC_IS_SRC(fmt->pad)) {
		fmt->format = *sink_fmt;
		/* TODO: Add support for other formats */
		fmt->format.code = vdeb->src_code;
	}

This needs to be fixed on the kernel side.

diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index b04a9726efa5..ace58381fe92 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -6,8 +6,8 @@
  */
 
 #include <algorithm>
-#include <array>
 #include <iomanip>
+#include <map>
 #include <tuple>
 
 #include <linux/media-bus-format.h>
@@ -103,10 +103,10 @@ private:
 
 namespace {
 
-static const std::array<PixelFormat, 3> pixelformats{
-	PixelFormat(DRM_FORMAT_RGB888),
-	PixelFormat(DRM_FORMAT_BGR888),
-	PixelFormat(DRM_FORMAT_BGRA8888),
+static const std::map<PixelFormat, uint32_t> pixelformats{
+	{ PixelFormat(DRM_FORMAT_RGB888), MEDIA_BUS_FMT_BGR888_1X24 },
+	{ PixelFormat(DRM_FORMAT_BGR888), MEDIA_BUS_FMT_RGB888_1X24 },
+	{ PixelFormat(DRM_FORMAT_BGRA8888), MEDIA_BUS_FMT_ARGB8888_1X32 },
 };
 
 } /* namespace */
@@ -132,8 +132,7 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()
 	StreamConfiguration &cfg = config_[0];
 
 	/* Adjust the pixel format. */
-	if (std::find(pixelformats.begin(), pixelformats.end(), cfg.pixelFormat) ==
-	    pixelformats.end()) {
+	if (pixelformats.find(cfg.pixelFormat) == pixelformats.end()) {
 		LOG(VIMC, Debug) << "Adjusting format to RGB24";
 		cfg.pixelFormat = PixelFormat(DRM_FORMAT_BGR888);
 		status = Adjusted;
@@ -174,12 +173,12 @@ CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,
 
 	std::map<PixelFormat, std::vector<SizeRange>> formats;
 
-	for (PixelFormat pixelformat : pixelformats) {
+	for (const auto &pixelformat : pixelformats) {
 		/* The scaler hardcodes a x3 scale-up ratio. */
 		std::vector<SizeRange> sizes{
 			SizeRange{ { 48, 48 }, { 4096, 2160 } }
 		};
-		formats[pixelformat] = sizes;
+		formats[pixelformat.first] = sizes;
 	}
 
 	StreamConfiguration cfg(formats);
@@ -214,7 +213,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
 	if (ret)
 		return ret;
 
-	subformat.mbus_code = MEDIA_BUS_FMT_RGB888_1X24;
+	subformat.mbus_code = pixelformats.find(cfg.pixelFormat)->second;
 	ret = data->debayer_->setFormat(1, &subformat);
 	if (ret)
 		return ret;
-- 
Regards,

Laurent Pinchart