[libcamera-devel] [PATCH v1 4/6] pipeline: raspberrypi: Add a reconfigured flag

[Naushir Patuck]

Hi Laurent,

Thank you for your feedback!

On Sun, 13 Mar 2022 at 16:04, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:

> Hi Naush,
>
> Thank you for the patch.
>
> On Mon, Mar 07, 2022 at 12:46:31PM +0000, Naushir Patuck via
> libcamera-devel wrote:
> > Add a flag to indicate a call to PipelineHandlerRPi::configure() has
> taken
> > place. This flag signals that buffer allocations need to be done on the
> next
> > call to PipelineHandlerRPi::start().
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 21 ++++++++++++-------
> >  1 file changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index 8bb9fc429912..b17ffa235ac7 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -185,7 +185,7 @@ public:
> >       RPiCameraData(PipelineHandler *pipe)
> >               : Camera::Private(pipe), state_(State::Stopped),
> >                 supportsFlips_(false), flipsAlterBayerOrder_(false),
> > -               dropFrameCount_(0), ispOutputCount_(0)
> > +               dropFrameCount_(0), reconfigured_(true),
> ispOutputCount_(0)
>
> Given that you can't start a camera without configuring it first, this
> could be initialized to false.
>

I think the value of the flag here is correct, but perhaps my naming of the
variable
needs to change.  This flags tells the pipeline handler that we need to
allocate
buffers before starting.  So perhaps a rename to something like
allocateBuffers_
might be more appropriate.

I'll change it in the next version.

Regards,
Naush



>
> >       {
> >       }
> >
> > @@ -284,6 +284,9 @@ public:
> >        */
> >       std::optional<int32_t> notifyGainsUnity_;
> >
> > +     /* Has this camera been reconfigured? */
> > +     bool reconfigured_;
> > +
> >  private:
> >       void checkRequestCompleted();
> >       void fillRequestMetadata(const ControlList &bufferControls,
> > @@ -961,6 +964,7 @@ int PipelineHandlerRPi::configure(Camera *camera,
> CameraConfiguration *config)
> >                               << " on pad " << sinkPad->index();
> >       }
> >
> > +     data->reconfigured_ = true;
> >       return ret;
> >  }
> >
> > @@ -981,12 +985,15 @@ int PipelineHandlerRPi::start(Camera *camera,
> const ControlList *controls)
> >       RPiCameraData *data = cameraData(camera);
> >       int ret;
> >
> > -     /* Allocate buffers for internal pipeline usage. */
> > -     ret = prepareBuffers(camera);
> > -     if (ret) {
> > -             LOG(RPI, Error) << "Failed to allocate buffers";
> > -             stop(camera);
> > -             return ret;
> > +     if (data->reconfigured_) {
> > +             /* Allocate buffers for internal pipeline usage. */
> > +             ret = prepareBuffers(camera);
> > +             if (ret) {
> > +                     LOG(RPI, Error) << "Failed to allocate buffers";
> > +                     stop(camera);
> > +                     return ret;
> > +             }
> > +             data->reconfigured_ = false;
> >       }
> >
> >       /* Check if a ScalerCrop control was specified. */
>
> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220314/3c20eff0/attachment-0001.htm>