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

[Jacopo Mondi]

Hi Niklas, Laurent,

On Tue, Sep 29, 2020 at 07:39:28AM +0200, Niklas Söderlund wrote:
> 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 :-)
>

You're very right!

> > 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>
>

Thanks, with this fixed I'll now push the series

Thanks
  j

> > ---
> >  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