[libcamera-devel,v4,12/12] libcamera: rpi: Simplify validate() and configure() for YUV/RGB streams
diff mbox series

Message ID 20230916121930.29490-13-jacopo.mondi@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: Introduce SensorConfiguration
Related show

Commit Message

Jacopo Mondi Sept. 16, 2023, 12:19 p.m. UTC
From: Naushir Patuck <naush@raspberrypi.com>

This commit simplifies the validate() and configure() calls in the
pipeline handler in a number of ways:

- Determine the V4L2DeviceFormat structure fields for all streams
  in validate(), cache them and reuse in configure() instead of
  re-generating this structure multiple times.

- Remove setting a default pixel format in validate(), this code patch
  will not be used.

- Use the recently added updateStreamConfig() and toV4L2DeviceFormat()
  helpers to populate fields in the V4L2DeviceFormat and StreamConfiguration
  structures to reduce code duplication.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 .../pipeline/rpi/common/pipeline_base.cpp     | 49 ++++---------------
 src/libcamera/pipeline/rpi/vc4/vc4.cpp        | 15 ++----
 2 files changed, 12 insertions(+), 52 deletions(-)

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
index c69e076aac71..653faff12353 100644
--- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
@@ -258,56 +258,25 @@  CameraConfiguration::Status RPiCameraConfiguration::validate()
 
 	/* Further fixups on the ISP output streams. */
 	for (auto &out : outStreams_) {
-		StreamConfiguration &cfg = config_.at(out.index);
-		PixelFormat &cfgPixFmt = cfg.pixelFormat;
-		V4L2VideoDevice::Formats fmts = out.dev->formats();
-
-		if (fmts.find(out.dev->toV4L2PixelFormat(cfgPixFmt)) == fmts.end()) {
-			/* If we cannot find a native format, use a default one. */
-			cfgPixFmt = formats::YUV420;
-			status = Adjusted;
-		}
-
-		V4L2DeviceFormat format;
-		format.fourcc = out.dev->toV4L2PixelFormat(cfg.pixelFormat);
-		format.size = cfg.size;
 
 		/*
-		 * platformValidate may have worked out the correct stride so we
-		 * must pass it in. This also needs the planesCount to be set
-		 * correctly or the stride will be ignored.
+		 * We want to send the associated YCbCr info through to the driver.
+		 *
+		 * But for RGB streams, the YCbCr info gets overwritten on the way back
+		 * so we must check against what the stream cfg says, not what we actually
+		 * requested (which carefully included the YCbCr info)!
 		 */
-		const PixelFormat &pixFormat = format.fourcc.toPixelFormat();
-		const PixelFormatInfo &info = PixelFormatInfo::info(pixFormat);
-		format.planesCount = info.numPlanes();
-		format.planes[0].bpl = cfg.stride;
-
-		/* We want to send the associated YCbCr info through to the driver. */
-		format.colorSpace = yuvColorSpace_;
+		out.format.colorSpace = yuvColorSpace_;
 
 		LOG(RPI, Debug)
-			<< "Try color space " << ColorSpace::toString(cfg.colorSpace);
+			<< "Try color space " << ColorSpace::toString(out.cfg->colorSpace);
 
-		int ret = out.dev->tryFormat(&format);
+		int ret = out.dev->tryFormat(&out.format);
 		if (ret)
 			return Invalid;
 
-		/*
-		 * But for RGB streams, the YCbCr info gets overwritten on the way back
-		 * so we must check against what the stream cfg says, not what we actually
-		 * requested (which carefully included the YCbCr info)!
-		 */
-		if (cfg.colorSpace != format.colorSpace) {
+		if (RPi::PipelineHandlerBase::updateStreamConfig(out.cfg, out.format))
 			status = Adjusted;
-			LOG(RPI, Debug)
-				<< "Color space changed from "
-				<< ColorSpace::toString(cfg.colorSpace) << " to "
-				<< ColorSpace::toString(format.colorSpace);
-		}
-
-		cfg.colorSpace = format.colorSpace;
-		cfg.stride = format.planes[0].bpl;
-		cfg.frameSize = format.planes[0].size;
 	}
 
 	return status;
diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
index 1a03c88d7ee3..a1813ba65725 100644
--- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
+++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
@@ -457,6 +457,8 @@  CameraConfiguration::Status Vc4CameraData::platformValidate(RPi::RPiCameraConfig
 		 * have that fixed up in the code above.
 		 */
 		outStreams[i].dev = isp_[i == 0 ? Isp::Output0 : Isp::Output1].dev();
+
+		outStreams[i].format = RPi::PipelineHandlerBase::toV4L2DeviceFormat(outStreams[i].dev, outStreams[i].cfg);
 	}
 
 	return status;
@@ -549,11 +551,7 @@  int Vc4CameraData::platformConfigure(const RPi::RPiCameraConfiguration *rpiConfi
 
 		/* The largest resolution gets routed to the ISP Output 0 node. */
 		RPi::Stream *stream = i == 0 ? &isp_[Isp::Output0] : &isp_[Isp::Output1];
-
-		V4L2PixelFormat fourcc = stream->dev()->toV4L2PixelFormat(cfg->pixelFormat);
-		format.size = cfg->size;
-		format.fourcc = fourcc;
-		format.colorSpace = cfg->colorSpace;
+		format = outStreams[i].format;
 
 		LOG(RPI, Debug) << "Setting " << stream->name() << " to "
 				<< format;
@@ -562,13 +560,6 @@  int Vc4CameraData::platformConfigure(const RPi::RPiCameraConfiguration *rpiConfi
 		if (ret)
 			return -EINVAL;
 
-		if (format.size != cfg->size || format.fourcc != fourcc) {
-			LOG(RPI, Error)
-				<< "Failed to set requested format on " << stream->name()
-				<< ", returned " << format;
-			return -EINVAL;
-		}
-
 		LOG(RPI, Debug)
 			<< "Stream " << stream->name() << " has color space "
 			<< ColorSpace::toString(cfg->colorSpace);