[libcamera-devel] [PATCH 1/2] libcamera: ipu3: Make it easier to read when a request may be completed
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Jun 29 22:40:08 CEST 2020
Hi Niklas,
Thank you for the patch.
On Mon, Jun 29, 2020 at 05:35:17PM +0200, Niklas Söderlund wrote:
> It's confusing to read the code and understand that a request is only
> completed before being processed by the ImgU if it only contains a
> single RAW buffer. Add a boolean variable with a explanatory name to
> make this clearer, no functional change.
Be careful, if you think the existing code lacks clarity here, I'll make
you document every single function of the pipeline handler classes :-)
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> src/libcamera/pipeline/ipu3/ipu3.cpp | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 1bdad209de6e47fb..4e75bd40c66f821a 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -874,10 +874,13 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
> * If the request contains a buffer for the RAW stream only, complete it
> * now as there's no need for ImgU processing.
> */
> - if (request->findBuffer(&rawStream_) &&
> - pipe_->completeBuffer(camera_, request, buffer)) {
> - pipe_->completeRequest(camera_, request);
> - return;
> + if (request->findBuffer(&rawStream_)) {
> + bool requestComplete =
> + pipe_->completeBuffer(camera_, request, buffer);
I'd have named the variable complete or isComplete just to avoid the
line break :-)
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> + if (requestComplete) {
> + pipe_->completeRequest(camera_, request);
> + return;
> + }
> }
>
> imgu_->input_->queueBuffer(buffer);
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list