[libcamera-devel] [PATCH v2] libcamera: pipeline: simple: Support camera sensors that contain an ISP

[Dafna Hirschfeld]

Hi,

On 03.03.21 00:53, Laurent Pinchart wrote:
> Camera sensors can include an ISP. For instance, the AP1302 external ISP
> can be connected to up to two raw camera sensors, and the combination of
> the sensors and ISP is considered as a (smart) camera sensor from
> libcamera's point of view.
> 
> The CameraSensor class has limited support for this already. Extend the
> simple pipeline handler to support such sensors, by using the media
> entity corresponding to the ISP instead of the raw camera sensor's
> entity.
> 
> We don't need to handle the case where an entity in the SoC would expose
> the MEDIA_ENT_F_PROC_VIDEO_ISP function, as pipeline containing an ISP
> would have a dedicated pipeline handler.
> 
> The implementation is limited as it won't support other multi-entity
> camera sensors (such as CCS). While this would be worth supporting, we
> don't have a test platform with a CCS-compatible sensor at this point,
> so let's not over-engineer the solution. Extending support to CCS (and
> possibly other sensor topologies) will likely involve helpers that can
> be used by other pipeline handlers (such as generic graph walk helpers
> for instance) and extensions to the CameraSensor class.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> Changes since v1:
> 
> - Fix link handling
> ---
>   src/libcamera/pipeline/simple/simple.cpp | 88 ++++++++++++++++++++----
>   1 file changed, 75 insertions(+), 13 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 36f55e8709f1..aa6155f0596f 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -245,6 +245,8 @@ private:
>   			PipelineHandler::cameraData(camera));
>   	}
>   
> +	std::vector<MediaEntity *> locateSensors();
> +
>   	void bufferReady(FrameBuffer *buffer);
>   	void converterInputDone(FrameBuffer *buffer);
>   	void converterOutputDone(FrameBuffer *buffer);
> @@ -869,6 +871,78 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)
>    * Match and Setup
>    */
>   
> +std::vector<MediaEntity *> SimplePipelineHandler::locateSensors()
> +{
> +	std::vector<MediaEntity *> entities;
> +
> +	/*
> +	 * Gather all the camera sensor entities based on the function they
> +	 * expose.
> +	 */
> +	for (MediaEntity *entity : media_->entities()) {
> +		if (entity->function() == MEDIA_ENT_F_CAM_SENSOR)
> +			entities.push_back(entity);
> +	}
> +
> +	if (entities.empty())
> +		return {};
> +
> +	/*
> +	 * Sensors can be made of multiple entities. For instance, a raw sensor
> +	 * can be connected to an ISP, and the combination of both should be
> +	 * treated as one sensor. To support this, as a crude heuristic, check
> +	 * the downstream entity from the camera sensor, and if it is an ISP,
> +	 * use it instead of the sensor.
> +	 */
> +	std::vector<MediaEntity *> sensors;
> +
> +	for (MediaEntity *entity : entities) {
> +		/*
> +		 * Locate the downstream entity by following the first enabled
> +		 * link from a source pad.
> +		 */
> +		const MediaPad *pad = nullptr;
> +		const MediaLink *link = nullptr;
> +
> +		for (const MediaPad *p : entity->pads()) {
> +			if (p->flags() & MEDIA_PAD_FL_SOURCE) {
> +				pad = p;
> +				break;
> +			}
> +		}
> +
> +		if (!pad)
> +			continue;
> +
> +		for (const MediaLink *l : pad->links()) {
> +			if (l->flags() & MEDIA_LNK_FL_ENABLED ||
> +			    !(l->flags() & MEDIA_LNK_FL_IMMUTABLE)) {

Same issue as discussed in the irc, this condition is always true.

Thanks,
Dafna

> +				link = l;
> +				break;
> +			}
> +		}
> +
> +		if (!link)
> +			continue;
> +
> +		MediaEntity *remote = link->sink()->entity();
> +		if (remote->function() == MEDIA_ENT_F_PROC_VIDEO_ISP)
> +			sensors.push_back(remote);
> +		else
> +			sensors.push_back(entity);
> +	}
> +
> +	/*
> +	 * Remove duplicates, in case multiple sensors are connected to the
> +	 * same ISP.
> +	 */
> +	std::sort(sensors.begin(), sensors.end());
> +	auto last = std::unique(sensors.begin(), sensors.end());
> +	sensors.erase(last, sensors.end());
> +
> +	return sensors;
> +}
> +
>   bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
>   {
>   	const SimplePipelineInfo *info = nullptr;
> @@ -892,19 +966,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
>   	}
>   
>   	/* Locate the sensors. */
> -	std::vector<MediaEntity *> sensors;
> -
> -	for (MediaEntity *entity : media_->entities()) {
> -		switch (entity->function()) {
> -		case MEDIA_ENT_F_CAM_SENSOR:
> -			sensors.push_back(entity);
> -			break;
> -
> -		default:
> -			break;
> -		}
> -	}
> -
> +	std::vector<MediaEntity *> sensors = locateSensors();
>   	if (sensors.empty()) {
>   		LOG(SimplePipeline, Error) << "No sensor found";
>   		return false;
>