| Message ID | 20190122232955.31783-2-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, thanks for this, very welcome On Wed, Jan 23, 2019 at 12:29:53AM +0100, Niklas Söderlund wrote: > The PipelineHandler which creates a Camera is responsible for serving > any operation requested by the user. In order to do so the Camera needs > to store a reference to its creator. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > include/libcamera/camera.h | 8 ++++++-- > src/libcamera/camera.cpp | 15 +++++++++------ > src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++- > src/libcamera/pipeline/uvcvideo.cpp | 2 +- > src/libcamera/pipeline/vimc.cpp | 10 ++-------- > 5 files changed, 20 insertions(+), 18 deletions(-) > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > index 2ea1a6883311cf9f..d3bae4cbee1e0cea 100644 > --- a/include/libcamera/camera.h > +++ b/include/libcamera/camera.h > @@ -12,10 +12,13 @@ > > namespace libcamera { > > +class PipelineHandler; > + > class Camera final > { > public: > - static std::shared_ptr<Camera> create(const std::string &name); > + static std::shared_ptr<Camera> create(const std::string &name, > + class PipelineHandler *pipe); > > Camera(const Camera &) = delete; > void operator=(const Camera &) = delete; > @@ -23,10 +26,11 @@ public: > const std::string &name() const; > > private: > - explicit Camera(const std::string &name); > + explicit Camera(const std::string &name, class PipelineHandler *pipe); You can drop the 'explicit' here, as it avoid copy-construction and conversion-contruction, which can't happen with 2 parameters (if I got this right :) I'm fine with rest, with the minor thing that I feel more natural to have 'this' passed as first parameter of the Camera constructor. Thanks j > ~Camera(); > > std::string name_; > + class PipelineHandler *pipe_; > }; > > } /* namespace libcamera */ > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index acf912bee95cbec4..f198eb4978b12239 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -52,17 +52,20 @@ namespace libcamera { > /** > * \brief Create a camera instance > * \param[in] name The name of the camera device > + * \param[in] pipe The pipeline handler responsible for the camera device > * > * The caller is responsible for guaranteeing unicity of the camera name. > * > * \return A shared pointer to the newly created camera object > */ > -std::shared_ptr<Camera> Camera::create(const std::string &name) > +std::shared_ptr<Camera> Camera::create(const std::string &name, > + class PipelineHandler *pipe) > { > struct Allocator : std::allocator<Camera> { > - void construct(void *p, const std::string &name) > + void construct(void *p, const std::string &name, > + class PipelineHandler *pipe) > { > - ::new(p) Camera(name); > + ::new(p) Camera(name, pipe); > } > void destroy(Camera *p) > { > @@ -70,7 +73,7 @@ std::shared_ptr<Camera> Camera::create(const std::string &name) > } > }; > > - return std::allocate_shared<Camera>(Allocator(), name); > + return std::allocate_shared<Camera>(Allocator(), name, pipe); > } > > /** > @@ -83,8 +86,8 @@ const std::string &Camera::name() const > return name_; > } > > -Camera::Camera(const std::string &name) > - : name_(name) > +Camera::Camera(const std::string &name, class PipelineHandler *pipe) > + : name_(name), pipe_(pipe) > { > } > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 8cbc10acfbb571fd..48d028f7e6cd9b4d 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -171,7 +171,8 @@ void PipelineHandlerIPU3::registerCameras(CameraManager *manager) > continue; > > std::string cameraName = sensor->name() + " " + std::to_string(id); > - std::shared_ptr<Camera> camera = Camera::create(cameraName); > + std::shared_ptr<Camera> camera = Camera::create(cameraName, > + this); > manager->addCamera(std::move(camera)); > > LOG(IPU3, Info) > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > index 3ba69da8b77586e3..3651250b683e7810 100644 > --- a/src/libcamera/pipeline/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo.cpp > @@ -48,7 +48,7 @@ bool PipelineHandlerUVC::match(CameraManager *manager, DeviceEnumerator *enumera > > dev_->acquire(); > > - std::shared_ptr<Camera> camera = Camera::create(dev_->model()); > + std::shared_ptr<Camera> camera = Camera::create(dev_->model(), this); > manager->addCamera(std::move(camera)); > > return true; > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index 82b9237a3d7d93e5..81d8319eb88e06d2 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -57,14 +57,8 @@ bool PipeHandlerVimc::match(CameraManager *manager, DeviceEnumerator *enumerator > > dev_->acquire(); > > - /* > - * NOTE: A more complete Camera implementation could > - * be passed the MediaDevice(s) it controls here or > - * a reference to the PipelineHandler. Which method > - * will be chosen depends on how the Camera > - * object is modeled. > - */ > - std::shared_ptr<Camera> camera = Camera::create("Dummy VIMC Camera"); > + std::shared_ptr<Camera> camera = Camera::create("Dummy VIMC Camera", > + this); > manager->addCamera(std::move(camera)); > > return true; > -- > 2.20.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, Thanks for your feedback. On 2019-01-23 09:56:35 +0100, Jacopo Mondi wrote: > Hi Niklas, > thanks for this, very welcome > > On Wed, Jan 23, 2019 at 12:29:53AM +0100, Niklas Söderlund wrote: > > The PipelineHandler which creates a Camera is responsible for serving > > any operation requested by the user. In order to do so the Camera needs > > to store a reference to its creator. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > include/libcamera/camera.h | 8 ++++++-- > > src/libcamera/camera.cpp | 15 +++++++++------ > > src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++- > > src/libcamera/pipeline/uvcvideo.cpp | 2 +- > > src/libcamera/pipeline/vimc.cpp | 10 ++-------- > > 5 files changed, 20 insertions(+), 18 deletions(-) > > > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > > index 2ea1a6883311cf9f..d3bae4cbee1e0cea 100644 > > --- a/include/libcamera/camera.h > > +++ b/include/libcamera/camera.h > > @@ -12,10 +12,13 @@ > > > > namespace libcamera { > > > > +class PipelineHandler; > > + > > class Camera final > > { > > public: > > - static std::shared_ptr<Camera> create(const std::string &name); > > + static std::shared_ptr<Camera> create(const std::string &name, > > + class PipelineHandler *pipe); > > > > Camera(const Camera &) = delete; > > void operator=(const Camera &) = delete; > > @@ -23,10 +26,11 @@ public: > > const std::string &name() const; > > > > private: > > - explicit Camera(const std::string &name); > > + explicit Camera(const std::string &name, class PipelineHandler *pipe); > > You can drop the 'explicit' here, as it avoid copy-construction and > conversion-contruction, which can't happen with 2 parameters (if I got > this right :) Thanks, did not know that will fix. > > I'm fine with rest, with the minor thing that I feel more natural to > have 'this' passed as first parameter of the Camera constructor. I agree, I will swap the arguments around! > > Thanks > j > > > ~Camera(); > > > > std::string name_; > > + class PipelineHandler *pipe_; > > }; > > > > } /* namespace libcamera */ > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > > index acf912bee95cbec4..f198eb4978b12239 100644 > > --- a/src/libcamera/camera.cpp > > +++ b/src/libcamera/camera.cpp > > @@ -52,17 +52,20 @@ namespace libcamera { > > /** > > * \brief Create a camera instance > > * \param[in] name The name of the camera device > > + * \param[in] pipe The pipeline handler responsible for the camera device > > * > > * The caller is responsible for guaranteeing unicity of the camera name. > > * > > * \return A shared pointer to the newly created camera object > > */ > > -std::shared_ptr<Camera> Camera::create(const std::string &name) > > +std::shared_ptr<Camera> Camera::create(const std::string &name, > > + class PipelineHandler *pipe) > > { > > struct Allocator : std::allocator<Camera> { > > - void construct(void *p, const std::string &name) > > + void construct(void *p, const std::string &name, > > + class PipelineHandler *pipe) > > { > > - ::new(p) Camera(name); > > + ::new(p) Camera(name, pipe); > > } > > void destroy(Camera *p) > > { > > @@ -70,7 +73,7 @@ std::shared_ptr<Camera> Camera::create(const std::string &name) > > } > > }; > > > > - return std::allocate_shared<Camera>(Allocator(), name); > > + return std::allocate_shared<Camera>(Allocator(), name, pipe); > > } > > > > /** > > @@ -83,8 +86,8 @@ const std::string &Camera::name() const > > return name_; > > } > > > > -Camera::Camera(const std::string &name) > > - : name_(name) > > +Camera::Camera(const std::string &name, class PipelineHandler *pipe) > > + : name_(name), pipe_(pipe) > > { > > } > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > index 8cbc10acfbb571fd..48d028f7e6cd9b4d 100644 > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > @@ -171,7 +171,8 @@ void PipelineHandlerIPU3::registerCameras(CameraManager *manager) > > continue; > > > > std::string cameraName = sensor->name() + " " + std::to_string(id); > > - std::shared_ptr<Camera> camera = Camera::create(cameraName); > > + std::shared_ptr<Camera> camera = Camera::create(cameraName, > > + this); > > manager->addCamera(std::move(camera)); > > > > LOG(IPU3, Info) > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > > index 3ba69da8b77586e3..3651250b683e7810 100644 > > --- a/src/libcamera/pipeline/uvcvideo.cpp > > +++ b/src/libcamera/pipeline/uvcvideo.cpp > > @@ -48,7 +48,7 @@ bool PipelineHandlerUVC::match(CameraManager *manager, DeviceEnumerator *enumera > > > > dev_->acquire(); > > > > - std::shared_ptr<Camera> camera = Camera::create(dev_->model()); > > + std::shared_ptr<Camera> camera = Camera::create(dev_->model(), this); > > manager->addCamera(std::move(camera)); > > > > return true; > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > > index 82b9237a3d7d93e5..81d8319eb88e06d2 100644 > > --- a/src/libcamera/pipeline/vimc.cpp > > +++ b/src/libcamera/pipeline/vimc.cpp > > @@ -57,14 +57,8 @@ bool PipeHandlerVimc::match(CameraManager *manager, DeviceEnumerator *enumerator > > > > dev_->acquire(); > > > > - /* > > - * NOTE: A more complete Camera implementation could > > - * be passed the MediaDevice(s) it controls here or > > - * a reference to the PipelineHandler. Which method > > - * will be chosen depends on how the Camera > > - * object is modeled. > > - */ > > - std::shared_ptr<Camera> camera = Camera::create("Dummy VIMC Camera"); > > + std::shared_ptr<Camera> camera = Camera::create("Dummy VIMC Camera", > > + this); > > manager->addCamera(std::move(camera)); > > > > return true; > > -- > > 2.20.1 > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Niklas, Thank you for the patch. On Wed, Jan 23, 2019 at 10:10:44AM +0100, Niklas Söderlund wrote: > On 2019-01-23 09:56:35 +0100, Jacopo Mondi wrote: > > On Wed, Jan 23, 2019 at 12:29:53AM +0100, Niklas Söderlund wrote: > >> The PipelineHandler which creates a Camera is responsible for serving > >> any operation requested by the user. In order to do so the Camera needs > >> to store a reference to its creator. > >> > >> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > >> --- > >> include/libcamera/camera.h | 8 ++++++-- > >> src/libcamera/camera.cpp | 15 +++++++++------ > >> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++- > >> src/libcamera/pipeline/uvcvideo.cpp | 2 +- > >> src/libcamera/pipeline/vimc.cpp | 10 ++-------- > >> 5 files changed, 20 insertions(+), 18 deletions(-) > >> > >> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > >> index 2ea1a6883311cf9f..d3bae4cbee1e0cea 100644 > >> --- a/include/libcamera/camera.h > >> +++ b/include/libcamera/camera.h > >> @@ -12,10 +12,13 @@ > >> > >> namespace libcamera { > >> > >> +class PipelineHandler; > >> + > >> class Camera final > >> { > >> public: > >> - static std::shared_ptr<Camera> create(const std::string &name); > >> + static std::shared_ptr<Camera> create(const std::string &name, > >> + class PipelineHandler *pipe); > >> > >> Camera(const Camera &) = delete; > >> void operator=(const Camera &) = delete; > >> @@ -23,10 +26,11 @@ public: > >> const std::string &name() const; > >> > >> private: > >> - explicit Camera(const std::string &name); > >> + explicit Camera(const std::string &name, class PipelineHandler *pipe); > > > > You can drop the 'explicit' here, as it avoid copy-construction and > > conversion-contruction, which can't happen with 2 parameters (if I got > > this right :) > > Thanks, did not know that will fix. > > > > > I'm fine with rest, with the minor thing that I feel more natural to > > have 'this' passed as first parameter of the Camera constructor. > > I agree, I will swap the arguments around! I would have said the other way around, I think that the camera is first qualified by its name, then by its pipeline handler, but it's not a big deal :-) > >> ~Camera(); > >> > >> std::string name_; > >> + class PipelineHandler *pipe_; This screams of lifetime management issues. How about storing it as a std::shared_ptr<>, given that it will potentially be shared by multiple cameras, and should not be destroyed as long as the camera object keeps a reference to the pipeline handler (otherwise the pipe->*() call fowarding will bring painful surprises) ? > >> }; > >> > >> } /* namespace libcamera */ > >> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > >> index acf912bee95cbec4..f198eb4978b12239 100644 > >> --- a/src/libcamera/camera.cpp > >> +++ b/src/libcamera/camera.cpp > >> @@ -52,17 +52,20 @@ namespace libcamera { > >> /** > >> * \brief Create a camera instance > >> * \param[in] name The name of the camera device > >> + * \param[in] pipe The pipeline handler responsible for the camera device > >> * > >> * The caller is responsible for guaranteeing unicity of the camera name. > >> * > >> * \return A shared pointer to the newly created camera object > >> */ > >> -std::shared_ptr<Camera> Camera::create(const std::string &name) > >> +std::shared_ptr<Camera> Camera::create(const std::string &name, > >> + class PipelineHandler *pipe) > >> { > >> struct Allocator : std::allocator<Camera> { > >> - void construct(void *p, const std::string &name) > >> + void construct(void *p, const std::string &name, > >> + class PipelineHandler *pipe) > >> { > >> - ::new(p) Camera(name); > >> + ::new(p) Camera(name, pipe); > >> } > >> void destroy(Camera *p) > >> { > >> @@ -70,7 +73,7 @@ std::shared_ptr<Camera> Camera::create(const std::string &name) > >> } > >> }; > >> > >> - return std::allocate_shared<Camera>(Allocator(), name); > >> + return std::allocate_shared<Camera>(Allocator(), name, pipe); > >> } > >> > >> /** > >> @@ -83,8 +86,8 @@ const std::string &Camera::name() const > >> return name_; > >> } > >> > >> -Camera::Camera(const std::string &name) > >> - : name_(name) > >> +Camera::Camera(const std::string &name, class PipelineHandler *pipe) > >> + : name_(name), pipe_(pipe) > >> { > >> } > >> > >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > >> index 8cbc10acfbb571fd..48d028f7e6cd9b4d 100644 > >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > >> @@ -171,7 +171,8 @@ void PipelineHandlerIPU3::registerCameras(CameraManager *manager) > >> continue; > >> > >> std::string cameraName = sensor->name() + " " + std::to_string(id); > >> - std::shared_ptr<Camera> camera = Camera::create(cameraName); > >> + std::shared_ptr<Camera> camera = Camera::create(cameraName, > >> + this); > >> manager->addCamera(std::move(camera)); > >> > >> LOG(IPU3, Info) > >> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > >> index 3ba69da8b77586e3..3651250b683e7810 100644 > >> --- a/src/libcamera/pipeline/uvcvideo.cpp > >> +++ b/src/libcamera/pipeline/uvcvideo.cpp > >> @@ -48,7 +48,7 @@ bool PipelineHandlerUVC::match(CameraManager *manager, DeviceEnumerator *enumera > >> > >> dev_->acquire(); > >> > >> - std::shared_ptr<Camera> camera = Camera::create(dev_->model()); > >> + std::shared_ptr<Camera> camera = Camera::create(dev_->model(), this); > >> manager->addCamera(std::move(camera)); > >> > >> return true; > >> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > >> index 82b9237a3d7d93e5..81d8319eb88e06d2 100644 > >> --- a/src/libcamera/pipeline/vimc.cpp > >> +++ b/src/libcamera/pipeline/vimc.cpp > >> @@ -57,14 +57,8 @@ bool PipeHandlerVimc::match(CameraManager *manager, DeviceEnumerator *enumerator > >> > >> dev_->acquire(); > >> > >> - /* > >> - * NOTE: A more complete Camera implementation could > >> - * be passed the MediaDevice(s) it controls here or > >> - * a reference to the PipelineHandler. Which method > >> - * will be chosen depends on how the Camera > >> - * object is modeled. > >> - */ > >> - std::shared_ptr<Camera> camera = Camera::create("Dummy VIMC Camera"); > >> + std::shared_ptr<Camera> camera = Camera::create("Dummy VIMC Camera", > >> + this); > >> manager->addCamera(std::move(camera)); > >> > >> return true;
Hi Laurent, On 2019-01-23 20:03:59 +0200, Laurent Pinchart wrote: > Hi Niklas, > > Thank you for the patch. > > On Wed, Jan 23, 2019 at 10:10:44AM +0100, Niklas Söderlund wrote: > > On 2019-01-23 09:56:35 +0100, Jacopo Mondi wrote: > > > On Wed, Jan 23, 2019 at 12:29:53AM +0100, Niklas Söderlund wrote: > > >> The PipelineHandler which creates a Camera is responsible for serving > > >> any operation requested by the user. In order to do so the Camera needs > > >> to store a reference to its creator. > > >> > > >> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > >> --- > > >> include/libcamera/camera.h | 8 ++++++-- > > >> src/libcamera/camera.cpp | 15 +++++++++------ > > >> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++- > > >> src/libcamera/pipeline/uvcvideo.cpp | 2 +- > > >> src/libcamera/pipeline/vimc.cpp | 10 ++-------- > > >> 5 files changed, 20 insertions(+), 18 deletions(-) > > >> > > >> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > > >> index 2ea1a6883311cf9f..d3bae4cbee1e0cea 100644 > > >> --- a/include/libcamera/camera.h > > >> +++ b/include/libcamera/camera.h > > >> @@ -12,10 +12,13 @@ > > >> > > >> namespace libcamera { > > >> > > >> +class PipelineHandler; > > >> + > > >> class Camera final > > >> { > > >> public: > > >> - static std::shared_ptr<Camera> create(const std::string &name); > > >> + static std::shared_ptr<Camera> create(const std::string &name, > > >> + class PipelineHandler *pipe); > > >> > > >> Camera(const Camera &) = delete; > > >> void operator=(const Camera &) = delete; > > >> @@ -23,10 +26,11 @@ public: > > >> const std::string &name() const; > > >> > > >> private: > > >> - explicit Camera(const std::string &name); > > >> + explicit Camera(const std::string &name, class PipelineHandler *pipe); > > > > > > You can drop the 'explicit' here, as it avoid copy-construction and > > > conversion-contruction, which can't happen with 2 parameters (if I got > > > this right :) > > > > Thanks, did not know that will fix. > > > > > > > > I'm fine with rest, with the minor thing that I feel more natural to > > > have 'this' passed as first parameter of the Camera constructor. > > > > I agree, I will swap the arguments around! > > I would have said the other way around, I think that the camera is first > qualified by its name, then by its pipeline handler, but it's not a big > deal :-) > > > >> ~Camera(); > > >> > > >> std::string name_; > > >> + class PipelineHandler *pipe_; > > This screams of lifetime management issues. How about storing it as a > std::shared_ptr<>, given that it will potentially be shared by multiple > cameras, and should not be destroyed as long as the camera object keeps > a reference to the pipeline handler (otherwise the pipe->*() call > fowarding will bring painful surprises) ? In v2 I moved the lifetime management to the base class PipelineHandler so the specific implementations wont need to care about the Camera objects at all. And as we with that can guarantee that Camera::disconnect() is called before the PipelineHandler is deleted this wont happen. At lest not until we go multithreaded, but that will add a new fauna of issues to solve. So for now I would like to keep the approach I will post later tonight in v2, we can always make it a shared pointer later and use the same design. > > > >> }; > > >> > > >> } /* namespace libcamera */ > > >> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > > >> index acf912bee95cbec4..f198eb4978b12239 100644 > > >> --- a/src/libcamera/camera.cpp > > >> +++ b/src/libcamera/camera.cpp > > >> @@ -52,17 +52,20 @@ namespace libcamera { > > >> /** > > >> * \brief Create a camera instance > > >> * \param[in] name The name of the camera device > > >> + * \param[in] pipe The pipeline handler responsible for the camera device > > >> * > > >> * The caller is responsible for guaranteeing unicity of the camera name. > > >> * > > >> * \return A shared pointer to the newly created camera object > > >> */ > > >> -std::shared_ptr<Camera> Camera::create(const std::string &name) > > >> +std::shared_ptr<Camera> Camera::create(const std::string &name, > > >> + class PipelineHandler *pipe) > > >> { > > >> struct Allocator : std::allocator<Camera> { > > >> - void construct(void *p, const std::string &name) > > >> + void construct(void *p, const std::string &name, > > >> + class PipelineHandler *pipe) > > >> { > > >> - ::new(p) Camera(name); > > >> + ::new(p) Camera(name, pipe); > > >> } > > >> void destroy(Camera *p) > > >> { > > >> @@ -70,7 +73,7 @@ std::shared_ptr<Camera> Camera::create(const std::string &name) > > >> } > > >> }; > > >> > > >> - return std::allocate_shared<Camera>(Allocator(), name); > > >> + return std::allocate_shared<Camera>(Allocator(), name, pipe); > > >> } > > >> > > >> /** > > >> @@ -83,8 +86,8 @@ const std::string &Camera::name() const > > >> return name_; > > >> } > > >> > > >> -Camera::Camera(const std::string &name) > > >> - : name_(name) > > >> +Camera::Camera(const std::string &name, class PipelineHandler *pipe) > > >> + : name_(name), pipe_(pipe) > > >> { > > >> } > > >> > > >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > >> index 8cbc10acfbb571fd..48d028f7e6cd9b4d 100644 > > >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > >> @@ -171,7 +171,8 @@ void PipelineHandlerIPU3::registerCameras(CameraManager *manager) > > >> continue; > > >> > > >> std::string cameraName = sensor->name() + " " + std::to_string(id); > > >> - std::shared_ptr<Camera> camera = Camera::create(cameraName); > > >> + std::shared_ptr<Camera> camera = Camera::create(cameraName, > > >> + this); > > >> manager->addCamera(std::move(camera)); > > >> > > >> LOG(IPU3, Info) > > >> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > > >> index 3ba69da8b77586e3..3651250b683e7810 100644 > > >> --- a/src/libcamera/pipeline/uvcvideo.cpp > > >> +++ b/src/libcamera/pipeline/uvcvideo.cpp > > >> @@ -48,7 +48,7 @@ bool PipelineHandlerUVC::match(CameraManager *manager, DeviceEnumerator *enumera > > >> > > >> dev_->acquire(); > > >> > > >> - std::shared_ptr<Camera> camera = Camera::create(dev_->model()); > > >> + std::shared_ptr<Camera> camera = Camera::create(dev_->model(), this); > > >> manager->addCamera(std::move(camera)); > > >> > > >> return true; > > >> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > > >> index 82b9237a3d7d93e5..81d8319eb88e06d2 100644 > > >> --- a/src/libcamera/pipeline/vimc.cpp > > >> +++ b/src/libcamera/pipeline/vimc.cpp > > >> @@ -57,14 +57,8 @@ bool PipeHandlerVimc::match(CameraManager *manager, DeviceEnumerator *enumerator > > >> > > >> dev_->acquire(); > > >> > > >> - /* > > >> - * NOTE: A more complete Camera implementation could > > >> - * be passed the MediaDevice(s) it controls here or > > >> - * a reference to the PipelineHandler. Which method > > >> - * will be chosen depends on how the Camera > > >> - * object is modeled. > > >> - */ > > >> - std::shared_ptr<Camera> camera = Camera::create("Dummy VIMC Camera"); > > >> + std::shared_ptr<Camera> camera = Camera::create("Dummy VIMC Camera", > > >> + this); > > >> manager->addCamera(std::move(camera)); > > >> > > >> return true; > > -- > Regards, > > Laurent Pinchart
diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 2ea1a6883311cf9f..d3bae4cbee1e0cea 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -12,10 +12,13 @@ namespace libcamera { +class PipelineHandler; + class Camera final { public: - static std::shared_ptr<Camera> create(const std::string &name); + static std::shared_ptr<Camera> create(const std::string &name, + class PipelineHandler *pipe); Camera(const Camera &) = delete; void operator=(const Camera &) = delete; @@ -23,10 +26,11 @@ public: const std::string &name() const; private: - explicit Camera(const std::string &name); + explicit Camera(const std::string &name, class PipelineHandler *pipe); ~Camera(); std::string name_; + class PipelineHandler *pipe_; }; } /* namespace libcamera */ diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index acf912bee95cbec4..f198eb4978b12239 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -52,17 +52,20 @@ namespace libcamera { /** * \brief Create a camera instance * \param[in] name The name of the camera device + * \param[in] pipe The pipeline handler responsible for the camera device * * The caller is responsible for guaranteeing unicity of the camera name. * * \return A shared pointer to the newly created camera object */ -std::shared_ptr<Camera> Camera::create(const std::string &name) +std::shared_ptr<Camera> Camera::create(const std::string &name, + class PipelineHandler *pipe) { struct Allocator : std::allocator<Camera> { - void construct(void *p, const std::string &name) + void construct(void *p, const std::string &name, + class PipelineHandler *pipe) { - ::new(p) Camera(name); + ::new(p) Camera(name, pipe); } void destroy(Camera *p) { @@ -70,7 +73,7 @@ std::shared_ptr<Camera> Camera::create(const std::string &name) } }; - return std::allocate_shared<Camera>(Allocator(), name); + return std::allocate_shared<Camera>(Allocator(), name, pipe); } /** @@ -83,8 +86,8 @@ const std::string &Camera::name() const return name_; } -Camera::Camera(const std::string &name) - : name_(name) +Camera::Camera(const std::string &name, class PipelineHandler *pipe) + : name_(name), pipe_(pipe) { } diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 8cbc10acfbb571fd..48d028f7e6cd9b4d 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -171,7 +171,8 @@ void PipelineHandlerIPU3::registerCameras(CameraManager *manager) continue; std::string cameraName = sensor->name() + " " + std::to_string(id); - std::shared_ptr<Camera> camera = Camera::create(cameraName); + std::shared_ptr<Camera> camera = Camera::create(cameraName, + this); manager->addCamera(std::move(camera)); LOG(IPU3, Info) diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 3ba69da8b77586e3..3651250b683e7810 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -48,7 +48,7 @@ bool PipelineHandlerUVC::match(CameraManager *manager, DeviceEnumerator *enumera dev_->acquire(); - std::shared_ptr<Camera> camera = Camera::create(dev_->model()); + std::shared_ptr<Camera> camera = Camera::create(dev_->model(), this); manager->addCamera(std::move(camera)); return true; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 82b9237a3d7d93e5..81d8319eb88e06d2 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -57,14 +57,8 @@ bool PipeHandlerVimc::match(CameraManager *manager, DeviceEnumerator *enumerator dev_->acquire(); - /* - * NOTE: A more complete Camera implementation could - * be passed the MediaDevice(s) it controls here or - * a reference to the PipelineHandler. Which method - * will be chosen depends on how the Camera - * object is modeled. - */ - std::shared_ptr<Camera> camera = Camera::create("Dummy VIMC Camera"); + std::shared_ptr<Camera> camera = Camera::create("Dummy VIMC Camera", + this); manager->addCamera(std::move(camera)); return true;
The PipelineHandler which creates a Camera is responsible for serving any operation requested by the user. In order to do so the Camera needs to store a reference to its creator. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- include/libcamera/camera.h | 8 ++++++-- src/libcamera/camera.cpp | 15 +++++++++------ src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++- src/libcamera/pipeline/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc.cpp | 10 ++-------- 5 files changed, 20 insertions(+), 18 deletions(-)