| Message ID | 20240613155949.1041061-6-dan.scally@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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,
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); } void PipelineHandlerMaliC55::registerMaliCamera(std::unique_ptr<MaliC55CameraData> data,
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(-)