[5/7] libcamera: mali-c55: Simplify bufferReady()

20306 diff mbox series

bufferReady() is needlessly complicated; simplify it.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 src/libcamera/pipeline/mali-c55/mali-c55.cpp | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Quoting Daniel Scally (2024-06-13 16:59:47)
> bufferReady() is needlessly complicated; simplify it.
> 
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
>  src/libcamera/pipeline/mali-c55/mali-c55.cpp | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> index 2b1d766a..910eac5e 100644
> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> @@ -1028,12 +1028,8 @@ void PipelineHandlerMaliC55::bufferReady(FrameBuffer *buffer)
>  {
>         Request *request = buffer->request();
>  
> -       completeBuffer(request, buffer);
> -
> -       if (request->hasPendingBuffers())
> -               return;
> -
> -       completeRequest(request);
> +       if (completeBuffer(request, buffer))
> +               completeRequest(request);

is that just because there's only one stream supported currently?

Or do I recall that completeBuffer tells us if there is still some
buffers on the request maybe?

Aha - right the PipelineHandlerBase does this for us and tells us if all
the buffers in the request are completed.


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

>  }
>  
>  void PipelineHandlerMaliC55::registerMaliCamera(std::unique_ptr<MaliC55CameraData> data,
> -- 
> 2.30.2
>

On Thu, Jun 13, 2024 at 08:36:39PM +0100, Kieran Bingham wrote:
> Quoting Daniel Scally (2024-06-13 16:59:47)
> > bufferReady() is needlessly complicated; simplify it.
> > 
> > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/mali-c55/mali-c55.cpp | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> > index 2b1d766a..910eac5e 100644
> > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> > @@ -1028,12 +1028,8 @@ void PipelineHandlerMaliC55::bufferReady(FrameBuffer *buffer)
> >  {
> >         Request *request = buffer->request();
> >  
> > -       completeBuffer(request, buffer);
> > -
> > -       if (request->hasPendingBuffers())
> > -               return;
> > -
> > -       completeRequest(request);
> > +       if (completeBuffer(request, buffer))
> > +               completeRequest(request);
> 
> is that just because there's only one stream supported currently?
> 
> Or do I recall that completeBuffer tells us if there is still some
> buffers on the request maybe?
> 
> Aha - right the PipelineHandlerBase does this for us and tells us if all
> the buffers in the request are completed.

A better commit message would have avoided researching that.

> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> >  }
> >  
> >  void PipelineHandlerMaliC55::registerMaliCamera(std::unique_ptr<MaliC55CameraData> data,