[libcamera-devel,04/10] libcamera: pipeline_handler: Pass a non-const Camera to methods

Message ID 20190228162913.6508-5-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • Rework request completion handling
Related show

Commit Message

Laurent Pinchart Feb. 28, 2019, 4:29 p.m. UTC
The start(), stop() and queueRequest() methods receive a const pointer
to the related Camera object. The stop() request will need to modify the
state of the camera, in order to report completion of pending requests.
Un-constify the Camera pointer to that method, and update the start()
and queueRequest() methods similarly for coherency.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/include/pipeline_handler.h |  6 +++---
 src/libcamera/pipeline/ipu3/ipu3.cpp     | 12 ++++++------
 src/libcamera/pipeline/uvcvideo.cpp      | 12 ++++++------
 src/libcamera/pipeline/vimc.cpp          | 12 ++++++------
 4 files changed, 21 insertions(+), 21 deletions(-)

Comments

Niklas Söderlund Feb. 28, 2019, 5 p.m. UTC | #1
Hi Laurent,

Thanks for your patch.

On 2019-02-28 18:29:07 +0200, Laurent Pinchart wrote:
> The start(), stop() and queueRequest() methods receive a const pointer
> to the related Camera object. The stop() request will need to modify the
> state of the camera, in order to report completion of pending requests.
> Un-constify the Camera pointer to that method, and update the start()
> and queueRequest() methods similarly for coherency.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/libcamera/include/pipeline_handler.h |  6 +++---
>  src/libcamera/pipeline/ipu3/ipu3.cpp     | 12 ++++++------
>  src/libcamera/pipeline/uvcvideo.cpp      | 12 ++++++------
>  src/libcamera/pipeline/vimc.cpp          | 12 ++++++------
>  4 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index 4363dcd8ed8e..45e1f07ffca9 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -52,10 +52,10 @@ public:
>  	virtual int allocateBuffers(Camera *camera, Stream *stream) = 0;
>  	virtual int freeBuffers(Camera *camera, Stream *stream) = 0;
>  
> -	virtual int start(const Camera *camera) = 0;
> -	virtual void stop(const Camera *camera) = 0;
> +	virtual int start(Camera *camera) = 0;
> +	virtual void stop(Camera *camera) = 0;
>  
> -	virtual int queueRequest(const Camera *camera, Request *request) = 0;
> +	virtual int queueRequest(Camera *camera, Request *request) = 0;
>  
>  protected:
>  	void registerCamera(std::shared_ptr<Camera> camera);
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index cf5c28577393..248e921117f4 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -39,10 +39,10 @@ public:
>  	int allocateBuffers(Camera *camera, Stream *stream) override;
>  	int freeBuffers(Camera *camera, Stream *stream) override;
>  
> -	int start(const Camera *camera) override;
> -	void stop(const Camera *camera) override;
> +	int start(Camera *camera) override;
> +	void stop(Camera *camera) override;
>  
> -	int queueRequest(const Camera *camera, Request *request) override;
> +	int queueRequest(Camera *camera, Request *request) override;
>  
>  	bool match(DeviceEnumerator *enumerator);
>  
> @@ -213,7 +213,7 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera, Stream *stream)
>  	return 0;
>  }
>  
> -int PipelineHandlerIPU3::start(const Camera *camera)
> +int PipelineHandlerIPU3::start(Camera *camera)
>  {
>  	IPU3CameraData *data = cameraData(camera);
>  	int ret;
> @@ -227,7 +227,7 @@ int PipelineHandlerIPU3::start(const Camera *camera)
>  	return 0;
>  }
>  
> -void PipelineHandlerIPU3::stop(const Camera *camera)
> +void PipelineHandlerIPU3::stop(Camera *camera)
>  {
>  	IPU3CameraData *data = cameraData(camera);
>  
> @@ -235,7 +235,7 @@ void PipelineHandlerIPU3::stop(const Camera *camera)
>  		LOG(IPU3, Info) << "Failed to stop camera " << camera->name();
>  }
>  
> -int PipelineHandlerIPU3::queueRequest(const Camera *camera, Request *request)
> +int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)
>  {
>  	IPU3CameraData *data = cameraData(camera);
>  	Stream *stream = &data->stream_;
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 4ad311163a95..cc513513cb34 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -35,10 +35,10 @@ public:
>  	int allocateBuffers(Camera *camera, Stream *stream) override;
>  	int freeBuffers(Camera *camera, Stream *stream) override;
>  
> -	int start(const Camera *camera) override;
> -	void stop(const Camera *camera) override;
> +	int start(Camera *camera) override;
> +	void stop(Camera *camera) override;
>  
> -	int queueRequest(const Camera *camera, Request *request) override;
> +	int queueRequest(Camera *camera, Request *request) override;
>  
>  	bool match(DeviceEnumerator *enumerator);
>  
> @@ -142,19 +142,19 @@ int PipelineHandlerUVC::freeBuffers(Camera *camera, Stream *stream)
>  	return data->video_->releaseBuffers();
>  }
>  
> -int PipelineHandlerUVC::start(const Camera *camera)
> +int PipelineHandlerUVC::start(Camera *camera)
>  {
>  	UVCCameraData *data = cameraData(camera);
>  	return data->video_->streamOn();
>  }
>  
> -void PipelineHandlerUVC::stop(const Camera *camera)
> +void PipelineHandlerUVC::stop(Camera *camera)
>  {
>  	UVCCameraData *data = cameraData(camera);
>  	data->video_->streamOff();
>  }
>  
> -int PipelineHandlerUVC::queueRequest(const Camera *camera, Request *request)
> +int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request)
>  {
>  	UVCCameraData *data = cameraData(camera);
>  	Buffer *buffer = request->findBuffer(&data->stream_);
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index ef47e7f96436..1d01fa80f8d5 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -35,10 +35,10 @@ public:
>  	int allocateBuffers(Camera *camera, Stream *stream) override;
>  	int freeBuffers(Camera *camera, Stream *stream) override;
>  
> -	int start(const Camera *camera) override;
> -	void stop(const Camera *camera) override;
> +	int start(Camera *camera) override;
> +	void stop(Camera *camera) override;
>  
> -	int queueRequest(const Camera *camera, Request *request) override;
> +	int queueRequest(Camera *camera, Request *request) override;
>  
>  	bool match(DeviceEnumerator *enumerator);
>  
> @@ -142,19 +142,19 @@ int PipelineHandlerVimc::freeBuffers(Camera *camera, Stream *stream)
>  	return data->video_->releaseBuffers();
>  }
>  
> -int PipelineHandlerVimc::start(const Camera *camera)
> +int PipelineHandlerVimc::start(Camera *camera)
>  {
>  	VimcCameraData *data = cameraData(camera);
>  	return data->video_->streamOn();
>  }
>  
> -void PipelineHandlerVimc::stop(const Camera *camera)
> +void PipelineHandlerVimc::stop(Camera *camera)
>  {
>  	VimcCameraData *data = cameraData(camera);
>  	data->video_->streamOff();
>  }
>  
> -int PipelineHandlerVimc::queueRequest(const Camera *camera, Request *request)
> +int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request)
>  {
>  	VimcCameraData *data = cameraData(camera);
>  	Buffer *buffer = request->findBuffer(&data->stream_);
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Jacopo Mondi March 1, 2019, 1:18 p.m. UTC | #2
Hi Laurent,

On Thu, Feb 28, 2019 at 06:00:47PM +0100, Niklas Söderlund wrote:
> Hi Laurent,
>
> Thanks for your patch.
>
> On 2019-02-28 18:29:07 +0200, Laurent Pinchart wrote:
> > The start(), stop() and queueRequest() methods receive a const pointer
> > to the related Camera object. The stop() request will need to modify the
> > state of the camera, in order to report completion of pending requests.
> > Un-constify the Camera pointer to that method, and update the start()
> > and queueRequest() methods similarly for coherency.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

>
> > ---
> >  src/libcamera/include/pipeline_handler.h |  6 +++---
> >  src/libcamera/pipeline/ipu3/ipu3.cpp     | 12 ++++++------
> >  src/libcamera/pipeline/uvcvideo.cpp      | 12 ++++++------
> >  src/libcamera/pipeline/vimc.cpp          | 12 ++++++------
> >  4 files changed, 21 insertions(+), 21 deletions(-)
> >
> > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> > index 4363dcd8ed8e..45e1f07ffca9 100644
> > --- a/src/libcamera/include/pipeline_handler.h
> > +++ b/src/libcamera/include/pipeline_handler.h
> > @@ -52,10 +52,10 @@ public:
> >  	virtual int allocateBuffers(Camera *camera, Stream *stream) = 0;
> >  	virtual int freeBuffers(Camera *camera, Stream *stream) = 0;
> >
> > -	virtual int start(const Camera *camera) = 0;
> > -	virtual void stop(const Camera *camera) = 0;
> > +	virtual int start(Camera *camera) = 0;
> > +	virtual void stop(Camera *camera) = 0;
> >
> > -	virtual int queueRequest(const Camera *camera, Request *request) = 0;
> > +	virtual int queueRequest(Camera *camera, Request *request) = 0;
> >
> >  protected:
> >  	void registerCamera(std::shared_ptr<Camera> camera);
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index cf5c28577393..248e921117f4 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -39,10 +39,10 @@ public:
> >  	int allocateBuffers(Camera *camera, Stream *stream) override;
> >  	int freeBuffers(Camera *camera, Stream *stream) override;
> >
> > -	int start(const Camera *camera) override;
> > -	void stop(const Camera *camera) override;
> > +	int start(Camera *camera) override;
> > +	void stop(Camera *camera) override;
> >
> > -	int queueRequest(const Camera *camera, Request *request) override;
> > +	int queueRequest(Camera *camera, Request *request) override;
> >
> >  	bool match(DeviceEnumerator *enumerator);
> >
> > @@ -213,7 +213,7 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera, Stream *stream)
> >  	return 0;
> >  }
> >
> > -int PipelineHandlerIPU3::start(const Camera *camera)
> > +int PipelineHandlerIPU3::start(Camera *camera)
> >  {
> >  	IPU3CameraData *data = cameraData(camera);
> >  	int ret;
> > @@ -227,7 +227,7 @@ int PipelineHandlerIPU3::start(const Camera *camera)
> >  	return 0;
> >  }
> >
> > -void PipelineHandlerIPU3::stop(const Camera *camera)
> > +void PipelineHandlerIPU3::stop(Camera *camera)
> >  {
> >  	IPU3CameraData *data = cameraData(camera);
> >
> > @@ -235,7 +235,7 @@ void PipelineHandlerIPU3::stop(const Camera *camera)
> >  		LOG(IPU3, Info) << "Failed to stop camera " << camera->name();
> >  }
> >
> > -int PipelineHandlerIPU3::queueRequest(const Camera *camera, Request *request)
> > +int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)
> >  {
> >  	IPU3CameraData *data = cameraData(camera);
> >  	Stream *stream = &data->stream_;
> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> > index 4ad311163a95..cc513513cb34 100644
> > --- a/src/libcamera/pipeline/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo.cpp
> > @@ -35,10 +35,10 @@ public:
> >  	int allocateBuffers(Camera *camera, Stream *stream) override;
> >  	int freeBuffers(Camera *camera, Stream *stream) override;
> >
> > -	int start(const Camera *camera) override;
> > -	void stop(const Camera *camera) override;
> > +	int start(Camera *camera) override;
> > +	void stop(Camera *camera) override;
> >
> > -	int queueRequest(const Camera *camera, Request *request) override;
> > +	int queueRequest(Camera *camera, Request *request) override;
> >
> >  	bool match(DeviceEnumerator *enumerator);
> >
> > @@ -142,19 +142,19 @@ int PipelineHandlerUVC::freeBuffers(Camera *camera, Stream *stream)
> >  	return data->video_->releaseBuffers();
> >  }
> >
> > -int PipelineHandlerUVC::start(const Camera *camera)
> > +int PipelineHandlerUVC::start(Camera *camera)
> >  {
> >  	UVCCameraData *data = cameraData(camera);
> >  	return data->video_->streamOn();
> >  }
> >
> > -void PipelineHandlerUVC::stop(const Camera *camera)
> > +void PipelineHandlerUVC::stop(Camera *camera)
> >  {
> >  	UVCCameraData *data = cameraData(camera);
> >  	data->video_->streamOff();
> >  }
> >
> > -int PipelineHandlerUVC::queueRequest(const Camera *camera, Request *request)
> > +int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request)
> >  {
> >  	UVCCameraData *data = cameraData(camera);
> >  	Buffer *buffer = request->findBuffer(&data->stream_);
> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> > index ef47e7f96436..1d01fa80f8d5 100644
> > --- a/src/libcamera/pipeline/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc.cpp
> > @@ -35,10 +35,10 @@ public:
> >  	int allocateBuffers(Camera *camera, Stream *stream) override;
> >  	int freeBuffers(Camera *camera, Stream *stream) override;
> >
> > -	int start(const Camera *camera) override;
> > -	void stop(const Camera *camera) override;
> > +	int start(Camera *camera) override;
> > +	void stop(Camera *camera) override;
> >
> > -	int queueRequest(const Camera *camera, Request *request) override;
> > +	int queueRequest(Camera *camera, Request *request) override;
> >
> >  	bool match(DeviceEnumerator *enumerator);
> >
> > @@ -142,19 +142,19 @@ int PipelineHandlerVimc::freeBuffers(Camera *camera, Stream *stream)
> >  	return data->video_->releaseBuffers();
> >  }
> >
> > -int PipelineHandlerVimc::start(const Camera *camera)
> > +int PipelineHandlerVimc::start(Camera *camera)
> >  {
> >  	VimcCameraData *data = cameraData(camera);
> >  	return data->video_->streamOn();
> >  }
> >
> > -void PipelineHandlerVimc::stop(const Camera *camera)
> > +void PipelineHandlerVimc::stop(Camera *camera)
> >  {
> >  	VimcCameraData *data = cameraData(camera);
> >  	data->video_->streamOff();
> >  }
> >
> > -int PipelineHandlerVimc::queueRequest(const Camera *camera, Request *request)
> > +int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request)
> >  {
> >  	VimcCameraData *data = cameraData(camera);
> >  	Buffer *buffer = request->findBuffer(&data->stream_);
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch

diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
index 4363dcd8ed8e..45e1f07ffca9 100644
--- a/src/libcamera/include/pipeline_handler.h
+++ b/src/libcamera/include/pipeline_handler.h
@@ -52,10 +52,10 @@  public:
 	virtual int allocateBuffers(Camera *camera, Stream *stream) = 0;
 	virtual int freeBuffers(Camera *camera, Stream *stream) = 0;
 
-	virtual int start(const Camera *camera) = 0;
-	virtual void stop(const Camera *camera) = 0;
+	virtual int start(Camera *camera) = 0;
+	virtual void stop(Camera *camera) = 0;
 
-	virtual int queueRequest(const Camera *camera, Request *request) = 0;
+	virtual int queueRequest(Camera *camera, Request *request) = 0;
 
 protected:
 	void registerCamera(std::shared_ptr<Camera> camera);
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index cf5c28577393..248e921117f4 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -39,10 +39,10 @@  public:
 	int allocateBuffers(Camera *camera, Stream *stream) override;
 	int freeBuffers(Camera *camera, Stream *stream) override;
 
-	int start(const Camera *camera) override;
-	void stop(const Camera *camera) override;
+	int start(Camera *camera) override;
+	void stop(Camera *camera) override;
 
-	int queueRequest(const Camera *camera, Request *request) override;
+	int queueRequest(Camera *camera, Request *request) override;
 
 	bool match(DeviceEnumerator *enumerator);
 
@@ -213,7 +213,7 @@  int PipelineHandlerIPU3::freeBuffers(Camera *camera, Stream *stream)
 	return 0;
 }
 
-int PipelineHandlerIPU3::start(const Camera *camera)
+int PipelineHandlerIPU3::start(Camera *camera)
 {
 	IPU3CameraData *data = cameraData(camera);
 	int ret;
@@ -227,7 +227,7 @@  int PipelineHandlerIPU3::start(const Camera *camera)
 	return 0;
 }
 
-void PipelineHandlerIPU3::stop(const Camera *camera)
+void PipelineHandlerIPU3::stop(Camera *camera)
 {
 	IPU3CameraData *data = cameraData(camera);
 
@@ -235,7 +235,7 @@  void PipelineHandlerIPU3::stop(const Camera *camera)
 		LOG(IPU3, Info) << "Failed to stop camera " << camera->name();
 }
 
-int PipelineHandlerIPU3::queueRequest(const Camera *camera, Request *request)
+int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)
 {
 	IPU3CameraData *data = cameraData(camera);
 	Stream *stream = &data->stream_;
diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
index 4ad311163a95..cc513513cb34 100644
--- a/src/libcamera/pipeline/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo.cpp
@@ -35,10 +35,10 @@  public:
 	int allocateBuffers(Camera *camera, Stream *stream) override;
 	int freeBuffers(Camera *camera, Stream *stream) override;
 
-	int start(const Camera *camera) override;
-	void stop(const Camera *camera) override;
+	int start(Camera *camera) override;
+	void stop(Camera *camera) override;
 
-	int queueRequest(const Camera *camera, Request *request) override;
+	int queueRequest(Camera *camera, Request *request) override;
 
 	bool match(DeviceEnumerator *enumerator);
 
@@ -142,19 +142,19 @@  int PipelineHandlerUVC::freeBuffers(Camera *camera, Stream *stream)
 	return data->video_->releaseBuffers();
 }
 
-int PipelineHandlerUVC::start(const Camera *camera)
+int PipelineHandlerUVC::start(Camera *camera)
 {
 	UVCCameraData *data = cameraData(camera);
 	return data->video_->streamOn();
 }
 
-void PipelineHandlerUVC::stop(const Camera *camera)
+void PipelineHandlerUVC::stop(Camera *camera)
 {
 	UVCCameraData *data = cameraData(camera);
 	data->video_->streamOff();
 }
 
-int PipelineHandlerUVC::queueRequest(const Camera *camera, Request *request)
+int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request)
 {
 	UVCCameraData *data = cameraData(camera);
 	Buffer *buffer = request->findBuffer(&data->stream_);
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index ef47e7f96436..1d01fa80f8d5 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -35,10 +35,10 @@  public:
 	int allocateBuffers(Camera *camera, Stream *stream) override;
 	int freeBuffers(Camera *camera, Stream *stream) override;
 
-	int start(const Camera *camera) override;
-	void stop(const Camera *camera) override;
+	int start(Camera *camera) override;
+	void stop(Camera *camera) override;
 
-	int queueRequest(const Camera *camera, Request *request) override;
+	int queueRequest(Camera *camera, Request *request) override;
 
 	bool match(DeviceEnumerator *enumerator);
 
@@ -142,19 +142,19 @@  int PipelineHandlerVimc::freeBuffers(Camera *camera, Stream *stream)
 	return data->video_->releaseBuffers();
 }
 
-int PipelineHandlerVimc::start(const Camera *camera)
+int PipelineHandlerVimc::start(Camera *camera)
 {
 	VimcCameraData *data = cameraData(camera);
 	return data->video_->streamOn();
 }
 
-void PipelineHandlerVimc::stop(const Camera *camera)
+void PipelineHandlerVimc::stop(Camera *camera)
 {
 	VimcCameraData *data = cameraData(camera);
 	data->video_->streamOff();
 }
 
-int PipelineHandlerVimc::queueRequest(const Camera *camera, Request *request)
+int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request)
 {
 	VimcCameraData *data = cameraData(camera);
 	Buffer *buffer = request->findBuffer(&data->stream_);