[libcamera-devel] [PATCH v2 5/6] pipeline: raspberrypi: Account for a missing Unicam embedded data node

[Laurent Pinchart]

Hi Naush,

Thank you for the patch.

On Fri, Oct 22, 2021 at 03:39:06PM +0100, Naushir Patuck wrote:
> The unicam driver no longer regesters an embedded data node if the sensor does

s/regesters/registers/

> not provide this stream. Account for this in the pipeline handler match routine
> by not assuming it is always present.
> 
> Add a warning if Unicam and the CamHelper do not agree on the presense of sensor
> embedded data, and disable its useage in these cases.

s/useage/usage/

> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Review-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  .../pipeline/raspberrypi/raspberrypi.cpp         | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 2b70b877e70a..c5e9607c7d95 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -984,7 +984,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
>  	DeviceMatch unicam("unicam");
>  	DeviceMatch isp("bcm2835-isp");
>  
> -	unicam.add("unicam-embedded");
>  	unicam.add("unicam-image");
>  
>  	isp.add("bcm2835-isp0-output0"); /* Input */
> @@ -1005,9 +1004,16 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
>  		return false;
>  
>  	/* Locate and open the unicam video streams. */
> -	data->unicam_[Unicam::Embedded] = RPi::Stream("Unicam Embedded", unicam_->getEntityByName("unicam-embedded"));
>  	data->unicam_[Unicam::Image] = RPi::Stream("Unicam Image", unicam_->getEntityByName("unicam-image"));
>  
> +	/* An embedded data node will not be present if the sensor does not support it. */
> +	MediaEntity *embeddedEntity = unicam_->getEntityByName("unicam-embedded");
> +	if (embeddedEntity) {
> +		data->unicam_[Unicam::Embedded] = RPi::Stream("Unicam Embedded", embeddedEntity);
> +		data->unicam_[Unicam::Embedded].dev()->bufferReady.connect(data.get(),
> +									   &RPiCameraData::unicamBufferDequeue);

It would be nice to keep signal connection uniform, but that's no big
deal.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +	}
> +
>  	/* Tag the ISP input stream as an import stream. */
>  	data->isp_[Isp::Input] = RPi::Stream("ISP Input", isp_->getEntityByName("bcm2835-isp0-output0"), true);
>  	data->isp_[Isp::Output0] = RPi::Stream("ISP Output0", isp_->getEntityByName("bcm2835-isp0-capture1"));
> @@ -1017,7 +1023,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
>  	/* Wire up all the buffer connections. */
>  	data->unicam_[Unicam::Image].dev()->frameStart.connect(data.get(), &RPiCameraData::frameStarted);
>  	data->unicam_[Unicam::Image].dev()->bufferReady.connect(data.get(), &RPiCameraData::unicamBufferDequeue);
> -	data->unicam_[Unicam::Embedded].dev()->bufferReady.connect(data.get(), &RPiCameraData::unicamBufferDequeue);
>  	data->isp_[Isp::Input].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispInputDequeue);
>  	data->isp_[Isp::Output0].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispOutputDequeue);
>  	data->isp_[Isp::Output1].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispOutputDequeue);
> @@ -1045,6 +1050,11 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
>  		return false;
>  	}
>  
> +	if (sensorConfig.sensorMetadata ^ !!embeddedEntity) {
> +		LOG(RPI, Warning) << "Mismatch between Unicam and CamHelper for embedded data usage!";
> +		sensorConfig.sensorMetadata = false;
> +	}
> +
>  	/*
>  	 * Open all Unicam and ISP streams. The exception is the embedded data
>  	 * stream, which only gets opened below if the IPA reports that the sensor

-- 
Regards,

Laurent Pinchart