[libcamera-devel] [PATCH 3/3] libcamera: ipu3: Adjust comment on ImgU configuration

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2020-09-24 16:51:43 +0200, Jacopo Mondi wrote:
> Be more precise in commenting why the ImgU shall not be configured
> if only the RAW stream is requested.
> 
> As an example, if the ImgU gets unecessary configured:
> cam -srole=viewfinder -c2 -C -> WORKS
> cam -srole=stillraw -c2 -C -> WORKS
> cam -srole=viewfinder -c2 -C -> Failed to queue buffer 0: Invalid argument
> 
> Since commit 1bd2e981a209 ("libcamera: ipu3: Fix RAW+YUV capture")

Unless you are _very_ lucky the commit hash will not be the same when 
this patch is merged :-)

> the ImgU configuration procedure also correctly implements the
> assumption that at least one of the YUV output is being operated. If
> that's not the case, as in the RAW-only capture use case, the code
> tries to access a non-existing configuration. One more reason to
> exit early if no YUV stream is requested.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>

With Laurent's comment addressed and the commit hash removed (or proof 
of guaranteed hash collision ;-P)

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

> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 477f85a45755..2f90ad2e2780 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -459,9 +459,9 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>  		return ret;
>  
>  	/*
> -	 * If the ImgU gets configured with proper IF, BDS and GDC sizes, it
> -	 * is then expected that frames are dequeued from its main output
> -	 * otherwise the system stalls.
> +	 * If the ImgU gets configured it is then expected that buffers are
> +	 * queued to its outputs, otherwise the next capture session
> +	 * that uses the ImgU fails at queueing buffers at its input.
>  	 *
>  	 * If no ImgU configuration has been computed, it means only a RAW
>  	 * stream has been requested: return here to skip the ImgU configuration
> -- 
> 2.28.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund