| Message ID | 20190208230138.8182-1-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | d8f2ed7d0d7f0de7184916da59cd2097529bd1c9 |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your work. On 2019-02-09 01:01:38 +0200, Laurent Pinchart wrote: > Reorder the member declaration order in the PipelineHandler class to > match the control flow order, and to declare variables after methods > according to the coding style. Update the documentation accordingly, > preserving the order within the public, protected and private sections, > but grouping related methods together between the sections. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/include/pipeline_handler.h | 10 +-- > src/libcamera/pipeline_handler.cpp | 101 +++++++++++------------ > 2 files changed, 55 insertions(+), 56 deletions(-) > > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h > index 7f2ec2975db0..4363dcd8ed8e 100644 > --- a/src/libcamera/include/pipeline_handler.h > +++ b/src/libcamera/include/pipeline_handler.h > @@ -42,6 +42,8 @@ public: > PipelineHandler(CameraManager *manager); > virtual ~PipelineHandler(); > > + virtual bool match(DeviceEnumerator *enumerator) = 0; > + > virtual std::map<Stream *, StreamConfiguration> > streamConfiguration(Camera *camera, std::vector<Stream *> &streams) = 0; > virtual int configureStreams(Camera *camera, > @@ -55,20 +57,18 @@ public: > > virtual int queueRequest(const Camera *camera, Request *request) = 0; > > - virtual bool match(DeviceEnumerator *enumerator) = 0; > - > protected: > - CameraManager *manager_; > - > void registerCamera(std::shared_ptr<Camera> camera); > void hotplugMediaDevice(MediaDevice *media); > > CameraData *cameraData(const Camera *camera); > void setCameraData(const Camera *camera, std::unique_ptr<CameraData> data); > > + CameraManager *manager_; > + > private: > - virtual void disconnect(); > void mediaDeviceDisconnected(MediaDevice *media); > + virtual void disconnect(); > > std::vector<std::weak_ptr<Camera>> cameras_; > std::map<const Camera *, std::unique_ptr<CameraData>> cameraData_; > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index cc2d4643ec2f..4e111d6d2f55 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -75,6 +75,36 @@ PipelineHandler::~PipelineHandler() > { > }; > > +/** > + * \fn PipelineHandler::match(DeviceEnumerator *enumerator) > + * \brief Match media devices and create camera instances > + * \param enumerator The enumerator providing all media devices found in the > + * system > + * > + * This function is the main entry point of the pipeline handler. It is called > + * by the camera manager with the \a enumerator passed as an argument. It shall > + * acquire from the \a enumerator all the media devices it needs for a single > + * pipeline, create one or multiple Camera instances and register them with the > + * camera manager. > + * > + * If all media devices needed by the pipeline handler are found, they must all > + * be acquired by a call to MediaDevice::acquire(). This function shall then > + * create the corresponding Camera instances, store them internally, and return > + * true. Otherwise it shall not acquire any media device (or shall release all > + * the media devices is has acquired by calling MediaDevice::release()) and > + * return false. > + * > + * If multiple instances of a pipeline are available in the system, the > + * PipelineHandler class will be instanciated once per instance, and its match() > + * function called for every instance. Each call shall acquire media devices for > + * one pipeline instance, until all compatible media devices are exhausted. > + * > + * If this function returns true, a new instance of the pipeline handler will > + * be created and its match() function called. > + * > + * \return true if media devices have been acquired and camera instances > + * created, or false otherwise > + */ > > /** > * \fn PipelineHandler::streamConfiguration() > @@ -176,46 +206,6 @@ PipelineHandler::~PipelineHandler() > * \return 0 on success or a negative error code on error > */ > > -/** > - * \fn PipelineHandler::match(DeviceEnumerator *enumerator) > - * \brief Match media devices and create camera instances > - * \param enumerator The enumerator providing all media devices found in the > - * system > - * > - * This function is the main entry point of the pipeline handler. It is called > - * by the camera manager with the \a enumerator passed as an argument. It shall > - * acquire from the \a enumerator all the media devices it needs for a single > - * pipeline, create one or multiple Camera instances and register them with the > - * camera manager. > - * > - * If all media devices needed by the pipeline handler are found, they must all > - * be acquired by a call to MediaDevice::acquire(). This function shall then > - * create the corresponding Camera instances, store them internally, and return > - * true. Otherwise it shall not acquire any media device (or shall release all > - * the media devices is has acquired by calling MediaDevice::release()) and > - * return false. > - * > - * If multiple instances of a pipeline are available in the system, the > - * PipelineHandler class will be instanciated once per instance, and its match() > - * function called for every instance. Each call shall acquire media devices for > - * one pipeline instance, until all compatible media devices are exhausted. > - * > - * If this function returns true, a new instance of the pipeline handler will > - * be created and its match() function called. > - * > - * \return true if media devices have been acquired and camera instances > - * created, or false otherwise > - */ > - > -/** > - * \var PipelineHandler::manager_ > - * \brief The Camera manager associated with the pipeline handler > - * > - * The camera manager pointer is stored in the pipeline handler for the > - * convenience of pipeline handler implementations. It remains valid and > - * constant for the whole lifetime of the pipeline handler. > - */ > - > /** > * \brief Register a camera to the camera manager and pipeline handler > * \param[in] camera The camera to be added > @@ -246,6 +236,17 @@ void PipelineHandler::hotplugMediaDevice(MediaDevice *media) > media->disconnected.connect(this, &PipelineHandler::mediaDeviceDisconnected); > } > > +/** > + * \brief Slot for the MediaDevice disconnected signal > + */ > +void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media) > +{ > + if (cameras_.empty()) > + return; > + > + disconnect(); > +} > + > /** > * \brief Device disconnection handler > * > @@ -272,17 +273,6 @@ void PipelineHandler::disconnect() > cameras_.clear(); > } > > -/** > - * \brief Slot for the MediaDevice disconnected signal > - */ > -void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media) > -{ > - if (cameras_.empty()) > - return; > - > - disconnect(); > -} > - > /** > * \brief Retrieve the pipeline-specific data associated with a Camera > * \param camera The camera whose data to retrieve > @@ -331,6 +321,15 @@ void PipelineHandler::setCameraData(const Camera *camera, > cameraData_[camera] = std::move(data); > } > > +/** > + * \var PipelineHandler::manager_ > + * \brief The Camera manager associated with the pipeline handler > + * > + * The camera manager pointer is stored in the pipeline handler for the > + * convenience of pipeline handler implementations. It remains valid and > + * constant for the whole lifetime of the pipeline handler. > + */ > + > /** > * \class PipelineHandlerFactory > * \brief Registration of PipelineHandler classes and creation of instances > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 7f2ec2975db0..4363dcd8ed8e 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -42,6 +42,8 @@ public: PipelineHandler(CameraManager *manager); virtual ~PipelineHandler(); + virtual bool match(DeviceEnumerator *enumerator) = 0; + virtual std::map<Stream *, StreamConfiguration> streamConfiguration(Camera *camera, std::vector<Stream *> &streams) = 0; virtual int configureStreams(Camera *camera, @@ -55,20 +57,18 @@ public: virtual int queueRequest(const Camera *camera, Request *request) = 0; - virtual bool match(DeviceEnumerator *enumerator) = 0; - protected: - CameraManager *manager_; - void registerCamera(std::shared_ptr<Camera> camera); void hotplugMediaDevice(MediaDevice *media); CameraData *cameraData(const Camera *camera); void setCameraData(const Camera *camera, std::unique_ptr<CameraData> data); + CameraManager *manager_; + private: - virtual void disconnect(); void mediaDeviceDisconnected(MediaDevice *media); + virtual void disconnect(); std::vector<std::weak_ptr<Camera>> cameras_; std::map<const Camera *, std::unique_ptr<CameraData>> cameraData_; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index cc2d4643ec2f..4e111d6d2f55 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -75,6 +75,36 @@ PipelineHandler::~PipelineHandler() { }; +/** + * \fn PipelineHandler::match(DeviceEnumerator *enumerator) + * \brief Match media devices and create camera instances + * \param enumerator The enumerator providing all media devices found in the + * system + * + * This function is the main entry point of the pipeline handler. It is called + * by the camera manager with the \a enumerator passed as an argument. It shall + * acquire from the \a enumerator all the media devices it needs for a single + * pipeline, create one or multiple Camera instances and register them with the + * camera manager. + * + * If all media devices needed by the pipeline handler are found, they must all + * be acquired by a call to MediaDevice::acquire(). This function shall then + * create the corresponding Camera instances, store them internally, and return + * true. Otherwise it shall not acquire any media device (or shall release all + * the media devices is has acquired by calling MediaDevice::release()) and + * return false. + * + * If multiple instances of a pipeline are available in the system, the + * PipelineHandler class will be instanciated once per instance, and its match() + * function called for every instance. Each call shall acquire media devices for + * one pipeline instance, until all compatible media devices are exhausted. + * + * If this function returns true, a new instance of the pipeline handler will + * be created and its match() function called. + * + * \return true if media devices have been acquired and camera instances + * created, or false otherwise + */ /** * \fn PipelineHandler::streamConfiguration() @@ -176,46 +206,6 @@ PipelineHandler::~PipelineHandler() * \return 0 on success or a negative error code on error */ -/** - * \fn PipelineHandler::match(DeviceEnumerator *enumerator) - * \brief Match media devices and create camera instances - * \param enumerator The enumerator providing all media devices found in the - * system - * - * This function is the main entry point of the pipeline handler. It is called - * by the camera manager with the \a enumerator passed as an argument. It shall - * acquire from the \a enumerator all the media devices it needs for a single - * pipeline, create one or multiple Camera instances and register them with the - * camera manager. - * - * If all media devices needed by the pipeline handler are found, they must all - * be acquired by a call to MediaDevice::acquire(). This function shall then - * create the corresponding Camera instances, store them internally, and return - * true. Otherwise it shall not acquire any media device (or shall release all - * the media devices is has acquired by calling MediaDevice::release()) and - * return false. - * - * If multiple instances of a pipeline are available in the system, the - * PipelineHandler class will be instanciated once per instance, and its match() - * function called for every instance. Each call shall acquire media devices for - * one pipeline instance, until all compatible media devices are exhausted. - * - * If this function returns true, a new instance of the pipeline handler will - * be created and its match() function called. - * - * \return true if media devices have been acquired and camera instances - * created, or false otherwise - */ - -/** - * \var PipelineHandler::manager_ - * \brief The Camera manager associated with the pipeline handler - * - * The camera manager pointer is stored in the pipeline handler for the - * convenience of pipeline handler implementations. It remains valid and - * constant for the whole lifetime of the pipeline handler. - */ - /** * \brief Register a camera to the camera manager and pipeline handler * \param[in] camera The camera to be added @@ -246,6 +236,17 @@ void PipelineHandler::hotplugMediaDevice(MediaDevice *media) media->disconnected.connect(this, &PipelineHandler::mediaDeviceDisconnected); } +/** + * \brief Slot for the MediaDevice disconnected signal + */ +void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media) +{ + if (cameras_.empty()) + return; + + disconnect(); +} + /** * \brief Device disconnection handler * @@ -272,17 +273,6 @@ void PipelineHandler::disconnect() cameras_.clear(); } -/** - * \brief Slot for the MediaDevice disconnected signal - */ -void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media) -{ - if (cameras_.empty()) - return; - - disconnect(); -} - /** * \brief Retrieve the pipeline-specific data associated with a Camera * \param camera The camera whose data to retrieve @@ -331,6 +321,15 @@ void PipelineHandler::setCameraData(const Camera *camera, cameraData_[camera] = std::move(data); } +/** + * \var PipelineHandler::manager_ + * \brief The Camera manager associated with the pipeline handler + * + * The camera manager pointer is stored in the pipeline handler for the + * convenience of pipeline handler implementations. It remains valid and + * constant for the whole lifetime of the pipeline handler. + */ + /** * \class PipelineHandlerFactory * \brief Registration of PipelineHandler classes and creation of instances
Reorder the member declaration order in the PipelineHandler class to match the control flow order, and to declare variables after methods according to the coding style. Update the documentation accordingly, preserving the order within the public, protected and private sections, but grouping related methods together between the sections. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/include/pipeline_handler.h | 10 +-- src/libcamera/pipeline_handler.cpp | 101 +++++++++++------------ 2 files changed, 55 insertions(+), 56 deletions(-)