[libcamera-devel] [PATCH 02/10] libcamera: pipeline: simple: Add sink and source pads to entity data

[Kieran Bingham]

On 17/08/2021 01:02, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Mon, Aug 16, 2021 at 03:41:21PM +0100, Kieran Bingham wrote:
>> On 05/08/2021 23:24, Laurent Pinchart wrote:
>>> Record the sink and source pads through which an entity is traversed in
>>> the list of entities stored in the camera data. This prepares for
>>> implementing mutually exclusive access to entities between cameras.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>> ---
>>>  src/libcamera/pipeline/simple/simple.cpp | 38 ++++++++++++++++++++----
>>>  1 file changed, 32 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
>>> index b29fff9820e5..e0695d052629 100644
>>> --- a/src/libcamera/pipeline/simple/simple.cpp
>>> +++ b/src/libcamera/pipeline/simple/simple.cpp
>>> @@ -166,7 +166,22 @@ public:
>>>  	}
>>>  
>>>  	struct Entity {
>>> +		/* The media entity, always valid. */
>>>  		MediaEntity *entity;
>>> +		/*
>>> +		 * The local sink pad connected to the upstream entity, null for
>>> +		 * the camera sensor at the beginning of the pipeline.
>>> +		 */
>>> +		const MediaPad *sink;
>>> +		/*
>>> +		 * The local source pad connected to the downstream entity, null
>>> +		 * for the video node at the end of the pipeline.
>>> +		 */
>>> +		const MediaPad *source;
>>> +		/*
>>> +		 * The link to the downstream entity, null for the video node at
>>> +		 * the end of the pipeline.
>>> +		 */
>>>  		MediaLink *link;
>>>  	};
>>>  
>>> @@ -288,16 +303,18 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
>>>  	 * encoders and image converters, and will end in a CSI capture device.
>>>  	 */
>>>  	std::unordered_set<MediaEntity *> visited;
>>> -	std::queue<MediaEntity *> queue;
>>> +	std::queue<std::tuple<MediaEntity *, MediaPad *>> queue;
>>>  
>>>  	/* Remember at each entity where we came from. */
>>>  	std::unordered_map<MediaEntity *, Entity> parents;
>>>  	MediaEntity *entity = nullptr;
>>>  
>>> -	queue.push(sensor);
>>> +	queue.push({ sensor, nullptr });
>>>  
>>>  	while (!queue.empty()) {
>>> -		entity = queue.front();
>>> +		MediaPad *sinkPad;
>>> +
>>> +		std::tie(entity, sinkPad) = queue.front();
>>>  		queue.pop();
>>>  
>>>  		/* Found the capture device. */
>>> @@ -317,8 +334,8 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
>>>  			for (MediaLink *link : pad->links()) {
>>>  				MediaEntity *next = link->sink()->entity();
>>>  				if (visited.find(next) == visited.end()) {
>>> -					queue.push(next);
>>> -					parents.insert({ next, { entity, link } });
>>> +					queue.push({ next, link->sink() });
>>> +					parents.insert({ next, { entity, sinkPad, pad, link } });
>>>  				}
>>>  			}
>>>  		}
>>> @@ -349,7 +366,16 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
>>>  	LOG(SimplePipeline, Debug)
>>>  		<< "Found pipeline: "
>>>  		<< utils::join(entities_, " -> ",
>>> -			       [](const Entity &e) { return e.entity->name(); });
>>> +			       [](const Entity &e) {
>>> +				       std::string s = "[";
>>> +				       if (e.sink)
>>> +					       s += std::to_string(e.sink->index()) + "|";
>>> +				       s += e.entity->name();
>>> +				       if (e.source)
>>> +					       s += "|" + std::to_string(e.source->index());
>>> +				       s += "]";
>>> +				       return s;
>>> +			       });
>>
>> An example of the changes to string formatting would be nice in the
>> commit message, but not essential.
> 
> Note that this is just a debug message, not the main point of the patch
> :-) I'll add

Of course, that's why I said not essential

> 
> The debug message that displays detected pipelines now prints pads to
> describe the pipeline more precisely:
> 
> [0:00:35.901275750] [260] DEBUG SimplePipeline simple.cpp:404 Found pipeline: [imx290 2-001a|0] -> [0|csis-32e40000.csi|1] -> [0|mxc_isi.0|1] -> [0|mxc_isi.0.capture]
> 

Thanks, I just think it's a good idea to show what the expected change
is when something changes something that is user (or developer, in this
instance) visible, to show the intention of the update.


>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>
>>>  }
>>>  
>>>  int SimpleCameraData::init()
>