[libcamera-devel] [PATCH] pipeline: rpi: ipa_base: Parse config files after platformRegister()

[Jacopo Mondi]

On Wed, May 24, 2023 at 02:00:37PM +0100, David Plowman via libcamera-devel wrote:
> Hi Naush
>
> Thanks for the patch!
>
> On Tue, 23 May 2023 at 13:03, Naushir Patuck via libcamera-devel
> <libcamera-devel at lists.libcamera.org> wrote:
> >
> > If a user uses a pipeline handler config file where a custom timeout
> > value is specified, it would lead to a segmentation fault when
> > loadPipelineConfiguration() tried to access the as yet uninitialised
> > V4L2VideoDevice object.
> >
> > To fix this, parse the pipeline handler config file only after fully
> > initialising and registering the pipeline handler and V4L2VideoDevice
> > objects.
> >
> > Fixes: 6c71ee1f15305 ("pipeline: raspberrypi: Introduce PipelineHandlerBase class")
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
>
> Looks good to me. (Actually, I think "the as yet" is probably OK...)

Ah sorry, thought it was a typo.. I didn't mean to correct a native
speaker :)

>
> Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
>
> Thanks
> David
>
> > ---
> >  src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > index ba1797bcfef0..3bb5ec531e4f 100644
> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > @@ -888,16 +888,16 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr<RPi::CameraData> &camera
> >         }
> >         data->nativeBayerOrder_ = bayerFormat.order;
> >
> > +       ret = platformRegister(cameraData, frontend, backend);
> > +       if (ret)
> > +               return ret;
> > +
> >         ret = data->loadPipelineConfiguration();
> >         if (ret) {
> >                 LOG(RPI, Error) << "Unable to load pipeline configuration";
> >                 return ret;
> >         }
> >
> > -       ret = platformRegister(cameraData, frontend, backend);
> > -       if (ret)
> > -               return ret;
> > -
> >         /* Setup the general IPA signal handlers. */
> >         data->frontendDevice()->dequeueTimeout.connect(data, &RPi::CameraData::cameraTimeout);
> >         data->frontendDevice()->frameStart.connect(data, &RPi::CameraData::frameStarted);
> > --
> > 2.34.1
> >