| Message ID | 20190228021626.15062-5-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, Thank you for the patch. On Thu, Feb 28, 2019 at 03:16:26AM +0100, Niklas Söderlund wrote: > Before calling into the pipeline handler make sure the streams provided > by the application actually belongs to the camera. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/camera.cpp | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index 4f0833300b9b7ffc..52992e577b6ab011 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -337,6 +337,11 @@ Camera::streamConfiguration(std::set<Stream *> &streams) > if (disconnected_ || !streams.size()) > return std::map<Stream *, StreamConfiguration>{}; > > + for (Stream *stream : streams) { > + if (streams_.find(stream) == streams_.end()) > + return std::map<Stream *, StreamConfiguration>{}; > + } > + > return pipe_->streamConfiguration(this, streams); > } > > @@ -378,6 +383,11 @@ int Camera::configureStreams(std::map<Stream *, StreamConfiguration> &config) > return -EINVAL; > } > > + for (auto const &iter : config) { > + if (streams_.find(iter.first) == streams_.end()) > + return -EINVAL; > + } > + > ret = pipe_->configureStreams(this, config); > if (ret) > return ret; The patch itself is fine, so Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> but before you apply it, is it really worth it ?
Hi Laurent, Thanks for your feedback. On 2019-02-28 19:17:09 +0200, Laurent Pinchart wrote: > Hi Niklas, > > Thank you for the patch. > > On Thu, Feb 28, 2019 at 03:16:26AM +0100, Niklas Söderlund wrote: > > Before calling into the pipeline handler make sure the streams provided > > by the application actually belongs to the camera. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > src/libcamera/camera.cpp | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > > index 4f0833300b9b7ffc..52992e577b6ab011 100644 > > --- a/src/libcamera/camera.cpp > > +++ b/src/libcamera/camera.cpp > > @@ -337,6 +337,11 @@ Camera::streamConfiguration(std::set<Stream *> &streams) > > if (disconnected_ || !streams.size()) > > return std::map<Stream *, StreamConfiguration>{}; > > > > + for (Stream *stream : streams) { > > + if (streams_.find(stream) == streams_.end()) > > + return std::map<Stream *, StreamConfiguration>{}; > > + } > > + > > return pipe_->streamConfiguration(this, streams); > > } > > > > @@ -378,6 +383,11 @@ int Camera::configureStreams(std::map<Stream *, StreamConfiguration> &config) > > return -EINVAL; > > } > > > > + for (auto const &iter : config) { > > + if (streams_.find(iter.first) == streams_.end()) > > + return -EINVAL; > > + } > > + > > ret = pipe_->configureStreams(this, config); > > if (ret) > > return ret; > > The patch itself is fine, so > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > but before you apply it, is it really worth it ? I think it is, but we can discuss this further as it's not critical to add the state machine. I will drop this patch from next version and post it separately if we decide it's worth it. > > -- > Regards, > > Laurent Pinchart
Hi, On 2019-02-28 19:43:14 +0100, Niklas Söderlund wrote: > On 2019-02-28 19:17:09 +0200, Laurent Pinchart wrote: [snip] > > > > but before you apply it, is it really worth it ? > > I think it is, but we can discuss this further as it's not critical to > add the state machine. I will drop this patch from next version and post > it separately if we decide it's worth it. As discussed on IRC we see value in this change and I have pushed it with Jacopo's and Laurent's tag, thanks!
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 4f0833300b9b7ffc..52992e577b6ab011 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -337,6 +337,11 @@ Camera::streamConfiguration(std::set<Stream *> &streams) if (disconnected_ || !streams.size()) return std::map<Stream *, StreamConfiguration>{}; + for (Stream *stream : streams) { + if (streams_.find(stream) == streams_.end()) + return std::map<Stream *, StreamConfiguration>{}; + } + return pipe_->streamConfiguration(this, streams); } @@ -378,6 +383,11 @@ int Camera::configureStreams(std::map<Stream *, StreamConfiguration> &config) return -EINVAL; } + for (auto const &iter : config) { + if (streams_.find(iter.first) == streams_.end()) + return -EINVAL; + } + ret = pipe_->configureStreams(this, config); if (ret) return ret;
Before calling into the pipeline handler make sure the streams provided by the application actually belongs to the camera. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/libcamera/camera.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+)