| Message ID | 20190125170400.24821-6-jacopo@jmondi.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Fri, Jan 25, 2019 at 06:04:00PM +0100, Jacopo Mondi wrote: > Once a pipeline-specific data has been associated with a camera, refuse > to update it in order to avoid invalidating the previously set > reference, which might still be owned by the caller. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/include/pipeline_handler.h | 2 +- > src/libcamera/pipeline_handler.cpp | 23 ++++++++++++++--------- > 2 files changed, 15 insertions(+), 10 deletions(-) > > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h > index ca77a40..01a0f0b 100644 > --- a/src/libcamera/include/pipeline_handler.h > +++ b/src/libcamera/include/pipeline_handler.h > @@ -47,7 +47,7 @@ protected: > void hotplugMediaDevice(MediaDevice *media); > > CameraData *cameraData(const Camera *camera); > - void setCameraData(const Camera *camera, std::unique_ptr<CameraData> data); > + int setCameraData(const Camera *camera, std::unique_ptr<CameraData> data); > > private: > virtual void disconnect(); > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index dc55f8f..7e927a7 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -212,21 +212,26 @@ CameraData *PipelineHandler::cameraData(const Camera *camera) > * information with \a camera. Ownership of \a data is transferred to > * the PipelineHandler. > * > - * If pipeline-specific data has already been associated with the camera by a > - * previous call to this method, is it replaced by \a data and the previous data > - * are deleted, rendering all references to them invalid. > + * Once pipeline-specific data has already been associated with the camera > + * by a previous call to this method it cannot be changed later on. Maybe "Pipeline-specific data can only be set once. Any attempt to call this method after the first one with the same camera will return an error and won't affect the pipeline-specific data." ? Up to you. > * > * The data can be retrieved by pipeline handlers using the cameraData() method. > + * > + * \return 0 on success, or a negative error code if camera specific data have > + * already been set > */ > -void PipelineHandler::setCameraData(const Camera *camera, > - std::unique_ptr<CameraData> data) > +int PipelineHandler::setCameraData(const Camera *camera, > + std::unique_ptr<CameraData> data) > { > - if (cameraData_.count(camera)) > - LOG(Pipeline, Debug) > - << "Replacing data associated with " > - << camera->name(); > + if (cameraData_.find(camera) != cameraData_.end()) { > + LOG(Pipeline, Error) > + << "Replacing data associated with a camera is forbidden "; s/ ";/";/ Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + return -EINVAL; > + } > > cameraData_[camera] = std::move(data); > + > + return 0; > } > > /**
diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index ca77a40..01a0f0b 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -47,7 +47,7 @@ protected: void hotplugMediaDevice(MediaDevice *media); CameraData *cameraData(const Camera *camera); - void setCameraData(const Camera *camera, std::unique_ptr<CameraData> data); + int setCameraData(const Camera *camera, std::unique_ptr<CameraData> data); private: virtual void disconnect(); diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index dc55f8f..7e927a7 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -212,21 +212,26 @@ CameraData *PipelineHandler::cameraData(const Camera *camera) * information with \a camera. Ownership of \a data is transferred to * the PipelineHandler. * - * If pipeline-specific data has already been associated with the camera by a - * previous call to this method, is it replaced by \a data and the previous data - * are deleted, rendering all references to them invalid. + * Once pipeline-specific data has already been associated with the camera + * by a previous call to this method it cannot be changed later on. * * The data can be retrieved by pipeline handlers using the cameraData() method. + * + * \return 0 on success, or a negative error code if camera specific data have + * already been set */ -void PipelineHandler::setCameraData(const Camera *camera, - std::unique_ptr<CameraData> data) +int PipelineHandler::setCameraData(const Camera *camera, + std::unique_ptr<CameraData> data) { - if (cameraData_.count(camera)) - LOG(Pipeline, Debug) - << "Replacing data associated with " - << camera->name(); + if (cameraData_.find(camera) != cameraData_.end()) { + LOG(Pipeline, Error) + << "Replacing data associated with a camera is forbidden "; + return -EINVAL; + } cameraData_[camera] = std::move(data); + + return 0; } /**
Once a pipeline-specific data has been associated with a camera, refuse to update it in order to avoid invalidating the previously set reference, which might still be owned by the caller. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/include/pipeline_handler.h | 2 +- src/libcamera/pipeline_handler.cpp | 23 ++++++++++++++--------- 2 files changed, 15 insertions(+), 10 deletions(-)