| Message ID | 20210131224702.8838-11-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Delegated to: | Laurent Pinchart |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, 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@ideasonboard.com> I presume it's so they can be indexed by ... an index ? > Signed-off-by: Laurent Pinchart <laurent.pinchart@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; > } >
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@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@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; > > }
Hi Laurent, On Mon, Feb 01, 2021 at 12:46:52AM +0200, Laurent Pinchart wrote: > To prepare for multiple streams support, store the streams in a vector > in the SimpleCameraData class. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@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 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
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; }
To prepare for multiple streams support, store the streams in a vector in the SimpleCameraData class. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/pipeline/simple/simple.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)