[libcamera-devel] [PATCH] libcamera: camera: Constify controls argument to start()
Jacopo Mondi
jacopo at jmondi.org
Thu Feb 18 14:37:55 CET 2021
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.
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
More information about the libcamera-devel
mailing list