[libcamera-devel] [PATCH 10/20] libcamera: pipeline: simple: Store streams in a vector

[Laurent Pinchart]

Hi Kieran,

On Mon, Mar 01, 2021 at 08:24:34PM +0000, Kieran Bingham wrote:
> On 31/01/2021 22:46, Laurent Pinchart wrote:
> > To prepare for multiple streams support, store the streams in a vector
> > in the SimpleCameraData class.
> 
> It's not clear from this patch or description /why/ a vector is better
> than a set yet ... So I assume that will be evident ... soon ? <looks
> onwards>
> 
> Still, given that this patch changes from a set to a vector, and
> assuming we're about to find out the actual reason why it's required:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> I presume it's so they can be indexed by ... an index ?

Note that this patch doesn't switch from storing streams from a set to a
vector. There's a single stream instance stored in
SimpleCameraData::stream_, and the patch turns that into a vector. The
SimpleCameraData::streams() function, which was meant to provide the
std::set<Stream *> required to construct the camera, is removed, but
that's just a consequence of multi-stream support.

> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/simple/simple.cpp | 19 ++++++++++++-------
> >  1 file changed, 12 insertions(+), 7 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index b7a890ab772e..390c87ba74d8 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -61,7 +61,6 @@ public:
> >  	SimpleCameraData(SimplePipelineHandler *pipe, MediaEntity *sensor);
> >  
> >  	bool isValid() const { return sensor_ != nullptr; }
> > -	std::set<Stream *> streams() { return { &stream_ }; }
> >  
> >  	int init();
> >  	int setupLinks();
> > @@ -80,7 +79,7 @@ public:
> >  		SizeRange outputSizes;
> >  	};
> >  
> > -	Stream stream_;
> > +	std::vector<Stream> streams_;
> >  	std::unique_ptr<CameraSensor> sensor_;
> >  	std::list<Entity> entities_;
> >  	V4L2VideoDevice *video_;
> > @@ -169,6 +168,8 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
> >  {
> >  	int ret;
> >  
> > +	streams_.resize(1);
> > +
> >  	/*
> >  	 * Walk the pipeline towards the video node and store all entities
> >  	 * along the way.
> > @@ -620,7 +621,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
> >  		LOG(SimplePipeline, Debug) << "Using format converter";
> >  	}
> >  
> > -	cfg.setStream(&data->stream_);
> > +	cfg.setStream(&data->streams_[0]);
> >  
> >  	return 0;
> >  }
> > @@ -645,7 +646,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] ControlList *c
> >  {
> >  	SimpleCameraData *data = cameraData(camera);
> >  	V4L2VideoDevice *video = data->video_;
> > -	unsigned int count = data->stream_.configuration().bufferCount;
> > +	unsigned int count = data->streams_[0].configuration().bufferCount;
> >  	int ret;
> >  
> >  	if (useConverter_)
> > @@ -696,7 +697,7 @@ void SimplePipelineHandler::stop(Camera *camera)
> >  int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)
> >  {
> >  	SimpleCameraData *data = cameraData(camera);
> > -	Stream *stream = &data->stream_;
> > +	Stream *stream = &data->streams_[0];
> >  
> >  	FrameBuffer *buffer = request->findBuffer(stream);
> >  	if (!buffer) {
> > @@ -825,9 +826,13 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
> >  		if (ret < 0)
> >  			continue;
> >  
> > +		std::set<Stream *> streams;
> > +		std::transform(data->streams_.begin(), data->streams_.end(),
> > +			       std::inserter(streams, streams.end()),
> > +			       [](Stream &stream) { return &stream; });
> > +
> >  		std::shared_ptr<Camera> camera =
> > -			Camera::create(this, data->sensor_->id(),
> > -				       data->streams());
> > +			Camera::create(this, data->sensor_->id(), streams);
> >  		registerCamera(std::move(camera), std::move(data));
> >  		registered = true;
> >  	}

-- 
Regards,

Laurent Pinchart