[libcamera-devel] [PATCH] libcamera: camera: Constify controls argument to start()
Niklas Söderlund
niklas.soderlund at ragnatech.se
Thu Feb 18 15:13:23 CET 2021
On 2021-02-18 14:37:55 +0100, Jacopo Mondi wrote:
> Hi Niklas, Laurent,
>
> On Thu, Feb 18, 2021 at 01:46:08PM +0100, Niklas Söderlund wrote:
> > Hi Laurent,
> >
> > Thanks for your patch.
> >
> > On 2021-02-18 08:53:29 +0200, Laurent Pinchart wrote:
> > > The ControlList passed to the Camera::start() function isn't modified.
> > > Make it const.
> >
> > To align with convection elsewhere should we not make it a const
> > reference ?
> >
>
> I think the idea is to be able to call Camera::start() without any
> argument. A cost & would force the caller to pass in an empty control
> list and would require all users to update as it's a mjor API change.
That is a good point, but can't that be solved with a default argument?
>
> The patch looks good
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> Thanks
> j
>
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > > include/libcamera/camera.h | 2 +-
> > > include/libcamera/internal/pipeline_handler.h | 2 +-
> > > src/libcamera/camera.cpp | 2 +-
> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--
> > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 ++--
> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++--
> > > src/libcamera/pipeline/simple/simple.cpp | 4 ++--
> > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 ++--
> > > src/libcamera/pipeline/vimc/vimc.cpp | 4 ++--
> > > 9 files changed, 15 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> > > index bd81fb54502e..326b14d0ca01 100644
> > > --- a/include/libcamera/camera.h
> > > +++ b/include/libcamera/camera.h
> > > @@ -100,7 +100,7 @@ public:
> > > std::unique_ptr<Request> createRequest(uint64_t cookie = 0);
> > > int queueRequest(Request *request);
> > >
> > > - int start(ControlList *controls = nullptr);
> > > + int start(const ControlList *controls = nullptr);
> > > int stop();
> > >
> > > private:
> > > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> > > index d455d3c9dc4f..6aca0b46f43d 100644
> > > --- a/include/libcamera/internal/pipeline_handler.h
> > > +++ b/include/libcamera/internal/pipeline_handler.h
> > > @@ -76,7 +76,7 @@ public:
> > > virtual int exportFrameBuffers(Camera *camera, Stream *stream,
> > > std::vector<std::unique_ptr<FrameBuffer>> *buffers) = 0;
> > >
> > > - virtual int start(Camera *camera, ControlList *controls) = 0;
> > > + virtual int start(Camera *camera, const ControlList *controls) = 0;
> > > virtual void stop(Camera *camera) = 0;
> > >
> > > int queueRequest(Request *request);
> > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > > index c9c79b91e3ae..84edbb8f494d 100644
> > > --- a/src/libcamera/camera.cpp
> > > +++ b/src/libcamera/camera.cpp
> > > @@ -1018,7 +1018,7 @@ int Camera::queueRequest(Request *request)
> > > * \retval -ENODEV The camera has been disconnected from the system
> > > * \retval -EACCES The camera is not in a state where it can be started
> > > */
> > > -int Camera::start(ControlList *controls)
> > > +int Camera::start(const ControlList *controls)
> > > {
> > > Private *const d = LIBCAMERA_D_PTR();
> > >
> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > index 3e6b88af4188..76fc7efd408b 100644
> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > @@ -126,7 +126,7 @@ public:
> > > int exportFrameBuffers(Camera *camera, Stream *stream,
> > > std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > >
> > > - int start(Camera *camera, ControlList *controls) override;
> > > + int start(Camera *camera, const ControlList *controls) override;
> > > void stop(Camera *camera) override;
> > >
> > > int queueRequestDevice(Camera *camera, Request *request) override;
> > > @@ -646,7 +646,7 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)
> > > return 0;
> > > }
> > >
> > > -int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *controls)
> > > +int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlList *controls)
> > > {
> > > std::map<uint32_t, ControlInfoMap> entityControls;
> > > IPU3CameraData *data = cameraData(camera);
> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > index 15aa600ed581..46361bf3fa51 100644
> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > @@ -252,7 +252,7 @@ public:
> > > int exportFrameBuffers(Camera *camera, Stream *stream,
> > > std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > >
> > > - int start(Camera *camera, ControlList *controls) override;
> > > + int start(Camera *camera, const ControlList *controls) override;
> > > void stop(Camera *camera) override;
> > >
> > > int queueRequestDevice(Camera *camera, Request *request) override;
> > > @@ -760,7 +760,7 @@ int PipelineHandlerRPi::exportFrameBuffers([[maybe_unused]] Camera *camera, Stre
> > > return ret;
> > > }
> > >
> > > -int PipelineHandlerRPi::start(Camera *camera, ControlList *controls)
> > > +int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)
> > > {
> > > RPiCameraData *data = cameraData(camera);
> > > int ret;
> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > index a794501a9c8d..538c01392293 100644
> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > @@ -142,7 +142,7 @@ public:
> > > int exportFrameBuffers(Camera *camera, Stream *stream,
> > > std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > >
> > > - int start(Camera *camera, ControlList *controls) override;
> > > + int start(Camera *camera, const ControlList *controls) override;
> > > void stop(Camera *camera) override;
> > >
> > > int queueRequestDevice(Camera *camera, Request *request) override;
> > > @@ -745,7 +745,7 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)
> > > return 0;
> > > }
> > >
> > > -int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *controls)
> > > +int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlList *controls)
> > > {
> > > RkISP1CameraData *data = cameraData(camera);
> > > int ret;
> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > > index 23320d2687e1..35cd34ddbf54 100644
> > > --- a/src/libcamera/pipeline/simple/simple.cpp
> > > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > > @@ -125,7 +125,7 @@ public:
> > > int exportFrameBuffers(Camera *camera, Stream *stream,
> > > std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > >
> > > - int start(Camera *camera, ControlList *controls) override;
> > > + int start(Camera *camera, const ControlList *controls) override;
> > > void stop(Camera *camera) override;
> > >
> > > bool match(DeviceEnumerator *enumerator) override;
> > > @@ -641,7 +641,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,
> > > return data->video_->exportBuffers(count, buffers);
> > > }
> > >
> > > -int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] ControlList *controls)
> > > +int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlList *controls)
> > > {
> > > SimpleCameraData *data = cameraData(camera);
> > > V4L2VideoDevice *video = data->video_;
> > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > index 08a594175091..031f96e28449 100644
> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > @@ -72,7 +72,7 @@ public:
> > > int exportFrameBuffers(Camera *camera, Stream *stream,
> > > std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > >
> > > - int start(Camera *camera, ControlList *controls) override;
> > > + int start(Camera *camera, const ControlList *controls) override;
> > > void stop(Camera *camera) override;
> > >
> > > int queueRequestDevice(Camera *camera, Request *request) override;
> > > @@ -232,7 +232,7 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,
> > > return data->video_->exportBuffers(count, buffers);
> > > }
> > >
> > > -int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] ControlList *controls)
> > > +int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList *controls)
> > > {
> > > UVCCameraData *data = cameraData(camera);
> > > unsigned int count = data->stream_.configuration().bufferCount;
> > > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> > > index 78b47916b4db..57c65b021c46 100644
> > > --- a/src/libcamera/pipeline/vimc/vimc.cpp
> > > +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> > > @@ -86,7 +86,7 @@ public:
> > > int exportFrameBuffers(Camera *camera, Stream *stream,
> > > std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > >
> > > - int start(Camera *camera, ControlList *controls) override;
> > > + int start(Camera *camera, const ControlList *controls) override;
> > > void stop(Camera *camera) override;
> > >
> > > int queueRequestDevice(Camera *camera, Request *request) override;
> > > @@ -307,7 +307,7 @@ int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream,
> > > return data->video_->exportBuffers(count, buffers);
> > > }
> > >
> > > -int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] ControlList *controls)
> > > +int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] const ControlList *controls)
> > > {
> > > VimcCameraData *data = cameraData(camera);
> > > unsigned int count = data->stream_.configuration().bufferCount;
> > > --
> > > Regards,
> > >
> > > Laurent Pinchart
> > >
> > > _______________________________________________
> > > libcamera-devel mailing list
> > > libcamera-devel at lists.libcamera.org
> > > https://lists.libcamera.org/listinfo/libcamera-devel
> >
> > --
> > Regards,
> > Niklas Söderlund
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list