| Message ID | 20210805175848.24188-12-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, I'll go write on the blackboard 50 times: "I should look at the full series before commenting" On Thu, Aug 05, 2021 at 08:58:48PM +0300, Laurent Pinchart wrote: > The PipelineHandler controls() and properties() functions are only used > by the Camera class. Now that the controls and properties are stored in > the Camera::Private class, we can drop those functions and access the > private data directly in Camera::controls() and Camera::properties(). > > Suggested-by: Jacopo Mondi <jacopo@jmondi.org> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/internal/pipeline_handler.h | 3 --- > src/libcamera/camera.cpp | 4 ++-- > src/libcamera/pipeline_handler.cpp | 21 ------------------- > 3 files changed, 2 insertions(+), 26 deletions(-) > > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h > index 2f814753f2ae..79e9839fa0de 100644 > --- a/include/libcamera/internal/pipeline_handler.h > +++ b/include/libcamera/internal/pipeline_handler.h > @@ -47,9 +47,6 @@ public: > bool lock(); > void unlock(); > > - const ControlInfoMap &controls(const Camera *camera) const; > - const ControlList &properties(const Camera *camera) const; > - > virtual CameraConfiguration *generateConfiguration(Camera *camera, > const StreamRoles &roles) = 0; > virtual int configure(Camera *camera, CameraConfiguration *config) = 0; > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index f0893f89a1b3..a6cc4ea624c0 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -800,7 +800,7 @@ int Camera::release() > */ > const ControlInfoMap &Camera::controls() const > { > - return _d()->pipe_->controls(this); > + return _d()->controlInfo_; > } > > /** > @@ -813,7 +813,7 @@ const ControlInfoMap &Camera::controls() const > */ > const ControlList &Camera::properties() const > { > - return _d()->pipe_->properties(this); > + return _d()->properties_; > } > > /** > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index 28e09bc00771..bf238377c67a 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -176,27 +176,6 @@ void PipelineHandler::unlock() > media->unlock(); > } > > -/** > - * \brief Retrieve the list of controls for a camera > - * \param[in] camera The camera > - * \context This function is \threadsafe. > - * \return A ControlInfoMap listing the controls support by \a camera > - */ > -const ControlInfoMap &PipelineHandler::controls(const Camera *camera) const > -{ > - return camera->_d()->controlInfo_; > -} > - > -/** > - * \brief Retrieve the list of properties for a camera > - * \param[in] camera The camera > - * \return A ControlList of properties supported by \a camera > - */ > -const ControlList &PipelineHandler::properties(const Camera *camera) const > -{ > - return camera->_d()->properties_; > -} > - > /** > * \fn PipelineHandler::generateConfiguration() > * \brief Generate a camera configuration for a specified camera > -- > Regards, > > Laurent Pinchart >
Hi Jacopo, On Tue, Aug 10, 2021 at 03:44:56PM +0200, Jacopo Mondi wrote: > Hi Laurent, > > I'll go write on the blackboard 50 times: > > "I should look at the full series before commenting" I've added this patch in v2 because you've commneted on it in v1 :-) > On Thu, Aug 05, 2021 at 08:58:48PM +0300, Laurent Pinchart wrote: > > The PipelineHandler controls() and properties() functions are only used > > by the Camera class. Now that the controls and properties are stored in > > the Camera::Private class, we can drop those functions and access the > > private data directly in Camera::controls() and Camera::properties(). > > > > Suggested-by: Jacopo Mondi <jacopo@jmondi.org> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > include/libcamera/internal/pipeline_handler.h | 3 --- > > src/libcamera/camera.cpp | 4 ++-- > > src/libcamera/pipeline_handler.cpp | 21 ------------------- > > 3 files changed, 2 insertions(+), 26 deletions(-) > > > > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h > > index 2f814753f2ae..79e9839fa0de 100644 > > --- a/include/libcamera/internal/pipeline_handler.h > > +++ b/include/libcamera/internal/pipeline_handler.h > > @@ -47,9 +47,6 @@ public: > > bool lock(); > > void unlock(); > > > > - const ControlInfoMap &controls(const Camera *camera) const; > > - const ControlList &properties(const Camera *camera) const; > > - > > virtual CameraConfiguration *generateConfiguration(Camera *camera, > > const StreamRoles &roles) = 0; > > virtual int configure(Camera *camera, CameraConfiguration *config) = 0; > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > > index f0893f89a1b3..a6cc4ea624c0 100644 > > --- a/src/libcamera/camera.cpp > > +++ b/src/libcamera/camera.cpp > > @@ -800,7 +800,7 @@ int Camera::release() > > */ > > const ControlInfoMap &Camera::controls() const > > { > > - return _d()->pipe_->controls(this); > > + return _d()->controlInfo_; > > } > > > > /** > > @@ -813,7 +813,7 @@ const ControlInfoMap &Camera::controls() const > > */ > > const ControlList &Camera::properties() const > > { > > - return _d()->pipe_->properties(this); > > + return _d()->properties_; > > } > > > > /** > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > > index 28e09bc00771..bf238377c67a 100644 > > --- a/src/libcamera/pipeline_handler.cpp > > +++ b/src/libcamera/pipeline_handler.cpp > > @@ -176,27 +176,6 @@ void PipelineHandler::unlock() > > media->unlock(); > > } > > > > -/** > > - * \brief Retrieve the list of controls for a camera > > - * \param[in] camera The camera > > - * \context This function is \threadsafe. > > - * \return A ControlInfoMap listing the controls support by \a camera > > - */ > > -const ControlInfoMap &PipelineHandler::controls(const Camera *camera) const > > -{ > > - return camera->_d()->controlInfo_; > > -} > > - > > -/** > > - * \brief Retrieve the list of properties for a camera > > - * \param[in] camera The camera > > - * \return A ControlList of properties supported by \a camera > > - */ > > -const ControlList &PipelineHandler::properties(const Camera *camera) const > > -{ > > - return camera->_d()->properties_; > > -} > > - > > /** > > * \fn PipelineHandler::generateConfiguration() > > * \brief Generate a camera configuration for a specified camera
On Tue, Aug 10, 2021 at 03:44:56PM +0200, Jacopo Mondi wrote: > Hi Laurent, > > I'll go write on the blackboard 50 times: > > "I should look at the full series before commenting" Does this mean I can get your Reviewed-by ? > On Thu, Aug 05, 2021 at 08:58:48PM +0300, Laurent Pinchart wrote: > > The PipelineHandler controls() and properties() functions are only used > > by the Camera class. Now that the controls and properties are stored in > > the Camera::Private class, we can drop those functions and access the > > private data directly in Camera::controls() and Camera::properties(). > > > > Suggested-by: Jacopo Mondi <jacopo@jmondi.org> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > include/libcamera/internal/pipeline_handler.h | 3 --- > > src/libcamera/camera.cpp | 4 ++-- > > src/libcamera/pipeline_handler.cpp | 21 ------------------- > > 3 files changed, 2 insertions(+), 26 deletions(-) > > > > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h > > index 2f814753f2ae..79e9839fa0de 100644 > > --- a/include/libcamera/internal/pipeline_handler.h > > +++ b/include/libcamera/internal/pipeline_handler.h > > @@ -47,9 +47,6 @@ public: > > bool lock(); > > void unlock(); > > > > - const ControlInfoMap &controls(const Camera *camera) const; > > - const ControlList &properties(const Camera *camera) const; > > - > > virtual CameraConfiguration *generateConfiguration(Camera *camera, > > const StreamRoles &roles) = 0; > > virtual int configure(Camera *camera, CameraConfiguration *config) = 0; > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > > index f0893f89a1b3..a6cc4ea624c0 100644 > > --- a/src/libcamera/camera.cpp > > +++ b/src/libcamera/camera.cpp > > @@ -800,7 +800,7 @@ int Camera::release() > > */ > > const ControlInfoMap &Camera::controls() const > > { > > - return _d()->pipe_->controls(this); > > + return _d()->controlInfo_; > > } > > > > /** > > @@ -813,7 +813,7 @@ const ControlInfoMap &Camera::controls() const > > */ > > const ControlList &Camera::properties() const > > { > > - return _d()->pipe_->properties(this); > > + return _d()->properties_; > > } > > > > /** > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > > index 28e09bc00771..bf238377c67a 100644 > > --- a/src/libcamera/pipeline_handler.cpp > > +++ b/src/libcamera/pipeline_handler.cpp > > @@ -176,27 +176,6 @@ void PipelineHandler::unlock() > > media->unlock(); > > } > > > > -/** > > - * \brief Retrieve the list of controls for a camera > > - * \param[in] camera The camera > > - * \context This function is \threadsafe. > > - * \return A ControlInfoMap listing the controls support by \a camera > > - */ > > -const ControlInfoMap &PipelineHandler::controls(const Camera *camera) const > > -{ > > - return camera->_d()->controlInfo_; > > -} > > - > > -/** > > - * \brief Retrieve the list of properties for a camera > > - * \param[in] camera The camera > > - * \return A ControlList of properties supported by \a camera > > - */ > > -const ControlList &PipelineHandler::properties(const Camera *camera) const > > -{ > > - return camera->_d()->properties_; > > -} > > - > > /** > > * \fn PipelineHandler::generateConfiguration() > > * \brief Generate a camera configuration for a specified camera
Hi Laurent, On Wed, Aug 11, 2021 at 08:51:56PM +0300, Laurent Pinchart wrote: > On Tue, Aug 10, 2021 at 03:44:56PM +0200, Jacopo Mondi wrote: > > Hi Laurent, > > > > I'll go write on the blackboard 50 times: > > > > "I should look at the full series before commenting" > > Does this mean I can get your Reviewed-by ? Sure, sorry, I thought the existing tag was enough Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > > > On Thu, Aug 05, 2021 at 08:58:48PM +0300, Laurent Pinchart wrote: > > > The PipelineHandler controls() and properties() functions are only used > > > by the Camera class. Now that the controls and properties are stored in > > > the Camera::Private class, we can drop those functions and access the > > > private data directly in Camera::controls() and Camera::properties(). > > > > > > Suggested-by: Jacopo Mondi <jacopo@jmondi.org> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > include/libcamera/internal/pipeline_handler.h | 3 --- > > > src/libcamera/camera.cpp | 4 ++-- > > > src/libcamera/pipeline_handler.cpp | 21 ------------------- > > > 3 files changed, 2 insertions(+), 26 deletions(-) > > > > > > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h > > > index 2f814753f2ae..79e9839fa0de 100644 > > > --- a/include/libcamera/internal/pipeline_handler.h > > > +++ b/include/libcamera/internal/pipeline_handler.h > > > @@ -47,9 +47,6 @@ public: > > > bool lock(); > > > void unlock(); > > > > > > - const ControlInfoMap &controls(const Camera *camera) const; > > > - const ControlList &properties(const Camera *camera) const; > > > - > > > virtual CameraConfiguration *generateConfiguration(Camera *camera, > > > const StreamRoles &roles) = 0; > > > virtual int configure(Camera *camera, CameraConfiguration *config) = 0; > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > > > index f0893f89a1b3..a6cc4ea624c0 100644 > > > --- a/src/libcamera/camera.cpp > > > +++ b/src/libcamera/camera.cpp > > > @@ -800,7 +800,7 @@ int Camera::release() > > > */ > > > const ControlInfoMap &Camera::controls() const > > > { > > > - return _d()->pipe_->controls(this); > > > + return _d()->controlInfo_; > > > } > > > > > > /** > > > @@ -813,7 +813,7 @@ const ControlInfoMap &Camera::controls() const > > > */ > > > const ControlList &Camera::properties() const > > > { > > > - return _d()->pipe_->properties(this); > > > + return _d()->properties_; > > > } > > > > > > /** > > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > > > index 28e09bc00771..bf238377c67a 100644 > > > --- a/src/libcamera/pipeline_handler.cpp > > > +++ b/src/libcamera/pipeline_handler.cpp > > > @@ -176,27 +176,6 @@ void PipelineHandler::unlock() > > > media->unlock(); > > > } > > > > > > -/** > > > - * \brief Retrieve the list of controls for a camera > > > - * \param[in] camera The camera > > > - * \context This function is \threadsafe. > > > - * \return A ControlInfoMap listing the controls support by \a camera > > > - */ > > > -const ControlInfoMap &PipelineHandler::controls(const Camera *camera) const > > > -{ > > > - return camera->_d()->controlInfo_; > > > -} > > > - > > > -/** > > > - * \brief Retrieve the list of properties for a camera > > > - * \param[in] camera The camera > > > - * \return A ControlList of properties supported by \a camera > > > - */ > > > -const ControlList &PipelineHandler::properties(const Camera *camera) const > > > -{ > > > - return camera->_d()->properties_; > > > -} > > > - > > > /** > > > * \fn PipelineHandler::generateConfiguration() > > > * \brief Generate a camera configuration for a specified camera > > -- > Regards, > > Laurent Pinchart
Hi Jacopo, On Mon, Aug 16, 2021 at 03:59:54PM +0200, Jacopo Mondi wrote: > On Wed, Aug 11, 2021 at 08:51:56PM +0300, Laurent Pinchart wrote: > > On Tue, Aug 10, 2021 at 03:44:56PM +0200, Jacopo Mondi wrote: > > > Hi Laurent, > > > > > > I'll go write on the blackboard 50 times: > > > > > > "I should look at the full series before commenting" > > > > Does this mean I can get your Reviewed-by ? > > Sure, sorry, I thought the existing tag was enough No worries. I've added Suggested-by myself, I usually refrain from adding Reviewed-by without getting an actual review ;-) > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > > > On Thu, Aug 05, 2021 at 08:58:48PM +0300, Laurent Pinchart wrote: > > > > The PipelineHandler controls() and properties() functions are only used > > > > by the Camera class. Now that the controls and properties are stored in > > > > the Camera::Private class, we can drop those functions and access the > > > > private data directly in Camera::controls() and Camera::properties(). > > > > > > > > Suggested-by: Jacopo Mondi <jacopo@jmondi.org> > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > --- > > > > include/libcamera/internal/pipeline_handler.h | 3 --- > > > > src/libcamera/camera.cpp | 4 ++-- > > > > src/libcamera/pipeline_handler.cpp | 21 ------------------- > > > > 3 files changed, 2 insertions(+), 26 deletions(-) > > > > > > > > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h > > > > index 2f814753f2ae..79e9839fa0de 100644 > > > > --- a/include/libcamera/internal/pipeline_handler.h > > > > +++ b/include/libcamera/internal/pipeline_handler.h > > > > @@ -47,9 +47,6 @@ public: > > > > bool lock(); > > > > void unlock(); > > > > > > > > - const ControlInfoMap &controls(const Camera *camera) const; > > > > - const ControlList &properties(const Camera *camera) const; > > > > - > > > > virtual CameraConfiguration *generateConfiguration(Camera *camera, > > > > const StreamRoles &roles) = 0; > > > > virtual int configure(Camera *camera, CameraConfiguration *config) = 0; > > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > > > > index f0893f89a1b3..a6cc4ea624c0 100644 > > > > --- a/src/libcamera/camera.cpp > > > > +++ b/src/libcamera/camera.cpp > > > > @@ -800,7 +800,7 @@ int Camera::release() > > > > */ > > > > const ControlInfoMap &Camera::controls() const > > > > { > > > > - return _d()->pipe_->controls(this); > > > > + return _d()->controlInfo_; > > > > } > > > > > > > > /** > > > > @@ -813,7 +813,7 @@ const ControlInfoMap &Camera::controls() const > > > > */ > > > > const ControlList &Camera::properties() const > > > > { > > > > - return _d()->pipe_->properties(this); > > > > + return _d()->properties_; > > > > } > > > > > > > > /** > > > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > > > > index 28e09bc00771..bf238377c67a 100644 > > > > --- a/src/libcamera/pipeline_handler.cpp > > > > +++ b/src/libcamera/pipeline_handler.cpp > > > > @@ -176,27 +176,6 @@ void PipelineHandler::unlock() > > > > media->unlock(); > > > > } > > > > > > > > -/** > > > > - * \brief Retrieve the list of controls for a camera > > > > - * \param[in] camera The camera > > > > - * \context This function is \threadsafe. > > > > - * \return A ControlInfoMap listing the controls support by \a camera > > > > - */ > > > > -const ControlInfoMap &PipelineHandler::controls(const Camera *camera) const > > > > -{ > > > > - return camera->_d()->controlInfo_; > > > > -} > > > > - > > > > -/** > > > > - * \brief Retrieve the list of properties for a camera > > > > - * \param[in] camera The camera > > > > - * \return A ControlList of properties supported by \a camera > > > > - */ > > > > -const ControlList &PipelineHandler::properties(const Camera *camera) const > > > > -{ > > > > - return camera->_d()->properties_; > > > > -} > > > > - > > > > /** > > > > * \fn PipelineHandler::generateConfiguration() > > > > * \brief Generate a camera configuration for a specified camera
diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index 2f814753f2ae..79e9839fa0de 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -47,9 +47,6 @@ public: bool lock(); void unlock(); - const ControlInfoMap &controls(const Camera *camera) const; - const ControlList &properties(const Camera *camera) const; - virtual CameraConfiguration *generateConfiguration(Camera *camera, const StreamRoles &roles) = 0; virtual int configure(Camera *camera, CameraConfiguration *config) = 0; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index f0893f89a1b3..a6cc4ea624c0 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -800,7 +800,7 @@ int Camera::release() */ const ControlInfoMap &Camera::controls() const { - return _d()->pipe_->controls(this); + return _d()->controlInfo_; } /** @@ -813,7 +813,7 @@ const ControlInfoMap &Camera::controls() const */ const ControlList &Camera::properties() const { - return _d()->pipe_->properties(this); + return _d()->properties_; } /** diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 28e09bc00771..bf238377c67a 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -176,27 +176,6 @@ void PipelineHandler::unlock() media->unlock(); } -/** - * \brief Retrieve the list of controls for a camera - * \param[in] camera The camera - * \context This function is \threadsafe. - * \return A ControlInfoMap listing the controls support by \a camera - */ -const ControlInfoMap &PipelineHandler::controls(const Camera *camera) const -{ - return camera->_d()->controlInfo_; -} - -/** - * \brief Retrieve the list of properties for a camera - * \param[in] camera The camera - * \return A ControlList of properties supported by \a camera - */ -const ControlList &PipelineHandler::properties(const Camera *camera) const -{ - return camera->_d()->properties_; -} - /** * \fn PipelineHandler::generateConfiguration() * \brief Generate a camera configuration for a specified camera
The PipelineHandler controls() and properties() functions are only used by the Camera class. Now that the controls and properties are stored in the Camera::Private class, we can drop those functions and access the private data directly in Camera::controls() and Camera::properties(). Suggested-by: Jacopo Mondi <jacopo@jmondi.org> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/libcamera/internal/pipeline_handler.h | 3 --- src/libcamera/camera.cpp | 4 ++-- src/libcamera/pipeline_handler.cpp | 21 ------------------- 3 files changed, 2 insertions(+), 26 deletions(-)