| Message ID | 20191003174941.1296988-2-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, Thank you for the patch. On Thu, Oct 03, 2019 at 07:49:31PM +0200, Niklas Söderlund wrote: > The IPA acts on a camera and not on a pipeline which can expose more > then one camera. Move the IPA reference to the CameraData. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/include/pipeline_handler.h | 2 ++ > src/libcamera/pipeline/vimc.cpp | 12 +++++------- > src/libcamera/pipeline_handler.cpp | 8 ++++++++ > 3 files changed, 15 insertions(+), 7 deletions(-) > > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h > index 1fdef9cea40f1f0a..42b90a4bf1a6e414 100644 > --- a/src/libcamera/include/pipeline_handler.h > +++ b/src/libcamera/include/pipeline_handler.h > @@ -14,6 +14,7 @@ > #include <string> > #include <vector> > > +#include <ipa/ipa_interface.h> > #include <libcamera/controls.h> > #include <libcamera/stream.h> > > @@ -43,6 +44,7 @@ public: > PipelineHandler *pipe_; > std::list<Request *> queuedRequests_; > ControlInfoMap controlInfo_; > + std::unique_ptr<IPAInterface> ipa_; > > private: > CameraData(const CameraData &) = delete; > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index 26477dccbb8fcd5b..f1cfd0ed35cfd9cd 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -101,8 +101,6 @@ private: > return static_cast<VimcCameraData *>( > PipelineHandler::cameraData(camera)); > } > - > - std::unique_ptr<IPAInterface> ipa_; > }; > > VimcCameraConfiguration::VimcCameraConfiguration() > @@ -353,13 +351,13 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) > if (!media) > return false; > > - ipa_ = IPAManager::instance()->createIPA(this, 0, 0); > - if (ipa_ == nullptr) > + std::unique_ptr<VimcCameraData> data = utils::make_unique<VimcCameraData>(this); > + > + data->ipa_ = IPAManager::instance()->createIPA(this, 0, 0); > + if (data->ipa_ == nullptr) > LOG(VIMC, Warning) << "no matching IPA found"; > else > - ipa_->init(); > - > - std::unique_ptr<VimcCameraData> data = utils::make_unique<VimcCameraData>(this); > + data->ipa_->init(); > > /* Locate and open the capture video node. */ > if (data->init(media)) > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index 3e54aa23d92b9a36..2d69dea843dd0980 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -96,6 +96,14 @@ LOG_DEFINE_CATEGORY(Pipeline) > * creating the camera, and shall not be modified afterwards. > */ > > +/** > + * \var CameraData::ipa_ > + * \brief The IPA module used by the camera > + * > + * Reference to the Image Processing Algorithms (IPA) operating on the camera's > + * stream(s). If no IPAs are in operation this should be set to nullptr. How about "If no IPA exists for the camera, this field is set to nullptr." or something similar ? This is a requirement, and "should" isn't strong enough. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + */ > + > /** > * \class PipelineHandler > * \brief Create and manage cameras based on a set of media devices
diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 1fdef9cea40f1f0a..42b90a4bf1a6e414 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -14,6 +14,7 @@ #include <string> #include <vector> +#include <ipa/ipa_interface.h> #include <libcamera/controls.h> #include <libcamera/stream.h> @@ -43,6 +44,7 @@ public: PipelineHandler *pipe_; std::list<Request *> queuedRequests_; ControlInfoMap controlInfo_; + std::unique_ptr<IPAInterface> ipa_; private: CameraData(const CameraData &) = delete; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 26477dccbb8fcd5b..f1cfd0ed35cfd9cd 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -101,8 +101,6 @@ private: return static_cast<VimcCameraData *>( PipelineHandler::cameraData(camera)); } - - std::unique_ptr<IPAInterface> ipa_; }; VimcCameraConfiguration::VimcCameraConfiguration() @@ -353,13 +351,13 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) if (!media) return false; - ipa_ = IPAManager::instance()->createIPA(this, 0, 0); - if (ipa_ == nullptr) + std::unique_ptr<VimcCameraData> data = utils::make_unique<VimcCameraData>(this); + + data->ipa_ = IPAManager::instance()->createIPA(this, 0, 0); + if (data->ipa_ == nullptr) LOG(VIMC, Warning) << "no matching IPA found"; else - ipa_->init(); - - std::unique_ptr<VimcCameraData> data = utils::make_unique<VimcCameraData>(this); + data->ipa_->init(); /* Locate and open the capture video node. */ if (data->init(media)) diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 3e54aa23d92b9a36..2d69dea843dd0980 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -96,6 +96,14 @@ LOG_DEFINE_CATEGORY(Pipeline) * creating the camera, and shall not be modified afterwards. */ +/** + * \var CameraData::ipa_ + * \brief The IPA module used by the camera + * + * Reference to the Image Processing Algorithms (IPA) operating on the camera's + * stream(s). If no IPAs are in operation this should be set to nullptr. + */ + /** * \class PipelineHandler * \brief Create and manage cameras based on a set of media devices
The IPA acts on a camera and not on a pipeline which can expose more then one camera. Move the IPA reference to the CameraData. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/libcamera/include/pipeline_handler.h | 2 ++ src/libcamera/pipeline/vimc.cpp | 12 +++++------- src/libcamera/pipeline_handler.cpp | 8 ++++++++ 3 files changed, 15 insertions(+), 7 deletions(-)