From eaf82088b102f45b97d12d22728cd069d6b02573 Mon Sep 17 00:00:00 2001 From: Naushir Patuck Date: Thu, 21 Sep 2023 18:55:49 +0200 Subject: libcamera: rpi: Simplify validate() and configure() for YUV/RGB streams 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 Reviewed-by: Jacopo Mondi Signed-off-by: Jacopo Mondi Signed-off-by: Laurent Pinchart --- .../pipeline/rpi/common/pipeline_base.cpp | 49 ++++------------------ src/libcamera/pipeline/rpi/vc4/vc4.cpp | 15 ++----- 2 files changed, 12 insertions(+), 52 deletions(-) diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp index 11d14d43..fad710a6 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -262,56 +262,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 785f0712..c189abaa 100644 --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp @@ -460,6 +460,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; @@ -552,11 +554,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; @@ -565,13 +563,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); -- cgit v1.2.1