[libcamera-devel,v3,8/8] libcamera: ipu3: Connect viewfinder's BufferReady signal

Message ID 20190403150735.27580-9-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: ipu3: Multiple streams support
Related show

Commit Message

Jacopo Mondi April 3, 2019, 3:07 p.m. UTC
Connect the viewfinder buffer ready signal to the IPU3CameraData slot
that complets the buffer first, and if not waiting for other buffers
completes the request as well.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/pipeline/ipu3/ipu3.cpp | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Niklas Söderlund April 5, 2019, 11:53 a.m. UTC | #1
Hi Jacopo,

Thanks for your efforts!

On 2019-04-03 17:07:35 +0200, Jacopo Mondi wrote:
> Connect the viewfinder buffer ready signal to the IPU3CameraData slot
> that complets the buffer first, and if not waiting for other buffers
> completes the request as well.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Depending on the outcome of the question in 7/8 I think this patch looks 
good. I would add my tag if I felt comfortable with 7/8.

> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 17e3e8677e28..706e4f647ed7 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -167,7 +167,7 @@ private:
>  		{
>  		}
>  
> -		void imguOutputBufferReady(Buffer *buffer);
> +		void imguCaptureBufferReady(Buffer *buffer);
>  		void imguInputBufferReady(Buffer *buffer);
>  		void cio2BufferReady(Buffer *buffer);
>  
> @@ -755,7 +755,9 @@ int PipelineHandlerIPU3::registerCameras()
>  		data->imgu_->input_->bufferReady.connect(data.get(),
>  					&IPU3CameraData::imguInputBufferReady);
>  		data->imgu_->output_.dev->bufferReady.connect(data.get(),
> -					&IPU3CameraData::imguOutputBufferReady);
> +					&IPU3CameraData::imguCaptureBufferReady);
> +		data->imgu_->viewfinder_.dev->bufferReady.connect(data.get(),
> +					&IPU3CameraData::imguCaptureBufferReady);
>  
>  		/* Create and register the Camera instance. */
>  		std::string cameraName = cio2->sensor_->entityName() + " "
> @@ -799,13 +801,13 @@ void PipelineHandlerIPU3::IPU3CameraData::imguInputBufferReady(Buffer *buffer)
>   *
>   * Buffers completed from the ImgU output are directed to the application.
>   */
> -void PipelineHandlerIPU3::IPU3CameraData::imguOutputBufferReady(Buffer *buffer)
> +void PipelineHandlerIPU3::IPU3CameraData::imguCaptureBufferReady(Buffer *buffer)
>  {
>  	Request *request = requestFromBuffer(buffer);
>  	ASSERT(request);
>  
> -	pipe_->completeBuffer(camera_, request, buffer);
> -	pipe_->completeRequest(camera_, request);
> +	if (pipe_->completeBuffer(camera_, request, buffer))
> +		pipe_->completeRequest(camera_, request);
>  }
>  
>  /**
> -- 
> 2.21.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart April 5, 2019, 3:45 p.m. UTC | #2
Hello Jacopo,

Thank you for the patch.

On Fri, Apr 05, 2019 at 01:53:48PM +0200, Niklas Söderlund wrote:
> On 2019-04-03 17:07:35 +0200, Jacopo Mondi wrote:
> > Connect the viewfinder buffer ready signal to the IPU3CameraData slot
> > that complets the buffer first, and if not waiting for other buffers

s/complets/completes/

> > completes the request as well.

I'm not sure to understand this sentence :-)

> > 
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> Depending on the outcome of the question in 7/8 I think this patch looks 
> good. I would add my tag if I felt comfortable with 7/8.

Same here, even though I think changes will be needed in
PipelineHandlerIPU3::IPU3CameraData::imguCaptureBufferReady().

> > ---
> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 17e3e8677e28..706e4f647ed7 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -167,7 +167,7 @@ private:
> >  		{
> >  		}
> >  
> > -		void imguOutputBufferReady(Buffer *buffer);
> > +		void imguCaptureBufferReady(Buffer *buffer);
> >  		void imguInputBufferReady(Buffer *buffer);
> >  		void cio2BufferReady(Buffer *buffer);
> >  
> > @@ -755,7 +755,9 @@ int PipelineHandlerIPU3::registerCameras()
> >  		data->imgu_->input_->bufferReady.connect(data.get(),
> >  					&IPU3CameraData::imguInputBufferReady);
> >  		data->imgu_->output_.dev->bufferReady.connect(data.get(),
> > -					&IPU3CameraData::imguOutputBufferReady);
> > +					&IPU3CameraData::imguCaptureBufferReady);
> > +		data->imgu_->viewfinder_.dev->bufferReady.connect(data.get(),
> > +					&IPU3CameraData::imguCaptureBufferReady);
> >  
> >  		/* Create and register the Camera instance. */
> >  		std::string cameraName = cio2->sensor_->entityName() + " "
> > @@ -799,13 +801,13 @@ void PipelineHandlerIPU3::IPU3CameraData::imguInputBufferReady(Buffer *buffer)
> >   *
> >   * Buffers completed from the ImgU output are directed to the application.
> >   */
> > -void PipelineHandlerIPU3::IPU3CameraData::imguOutputBufferReady(Buffer *buffer)
> > +void PipelineHandlerIPU3::IPU3CameraData::imguCaptureBufferReady(Buffer *buffer)
> >  {
> >  	Request *request = requestFromBuffer(buffer);
> >  	ASSERT(request);
> >  
> > -	pipe_->completeBuffer(camera_, request, buffer);
> > -	pipe_->completeRequest(camera_, request);
> > +	if (pipe_->completeBuffer(camera_, request, buffer))
> > +		pipe_->completeRequest(camera_, request);
> >  }
> >  
> >  /**

Patch

diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 17e3e8677e28..706e4f647ed7 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -167,7 +167,7 @@  private:
 		{
 		}
 
-		void imguOutputBufferReady(Buffer *buffer);
+		void imguCaptureBufferReady(Buffer *buffer);
 		void imguInputBufferReady(Buffer *buffer);
 		void cio2BufferReady(Buffer *buffer);
 
@@ -755,7 +755,9 @@  int PipelineHandlerIPU3::registerCameras()
 		data->imgu_->input_->bufferReady.connect(data.get(),
 					&IPU3CameraData::imguInputBufferReady);
 		data->imgu_->output_.dev->bufferReady.connect(data.get(),
-					&IPU3CameraData::imguOutputBufferReady);
+					&IPU3CameraData::imguCaptureBufferReady);
+		data->imgu_->viewfinder_.dev->bufferReady.connect(data.get(),
+					&IPU3CameraData::imguCaptureBufferReady);
 
 		/* Create and register the Camera instance. */
 		std::string cameraName = cio2->sensor_->entityName() + " "
@@ -799,13 +801,13 @@  void PipelineHandlerIPU3::IPU3CameraData::imguInputBufferReady(Buffer *buffer)
  *
  * Buffers completed from the ImgU output are directed to the application.
  */
-void PipelineHandlerIPU3::IPU3CameraData::imguOutputBufferReady(Buffer *buffer)
+void PipelineHandlerIPU3::IPU3CameraData::imguCaptureBufferReady(Buffer *buffer)
 {
 	Request *request = requestFromBuffer(buffer);
 	ASSERT(request);
 
-	pipe_->completeBuffer(camera_, request, buffer);
-	pipe_->completeRequest(camera_, request);
+	if (pipe_->completeBuffer(camera_, request, buffer))
+		pipe_->completeRequest(camera_, request);
 }
 
 /**