[libcamera-devel] libcamera: pipeline: simple: Check if converter_ is a nullptr

[Kieran Bingham]

Quoting Laurent Pinchart via libcamera-devel (2023-02-26 15:41:36)
> On Fri, Feb 24, 2023 at 11:52:35AM +0530, Umang Jain via libcamera-devel wrote:
> > Hi Suhrid,
> > 
> > Thank you for the patch.
> > 
> > On 2/24/23 2:17 AM, Suhrid Subramaniam via libcamera-devel wrote:
> > > - If no converter is found, converter_ becomes a nullptr and
> > > !converter_->isValid() causes a segmentation fault.
> > > - Avoid this by checking if converter_ is a nullptr.
> > >
> > > Signed-off-by: Suhrid Subramaniam <suhrid.subramaniam at mediatek.com>
> > > ---
> > >   src/libcamera/pipeline/simple/simple.cpp | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > > index 24ded4db..2423ec10 100644
> > > --- a/src/libcamera/pipeline/simple/simple.cpp
> > > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > > @@ -493,7 +493,7 @@ int SimpleCameraData::init()
> > >     MediaDevice *converter = pipe->converter();
> > >     if (converter) {
> > >             converter_ = ConverterFactoryBase::create(converter);
> > > -           if (!converter_->isValid()) {
> > > +           if (!converter_) {
> > 
> > it seems this change will drop the validity check so maybe :
> >          if (!converter_ && converter_->isValid()) {
> > 
> > The error message below might be need to get tweaked a bit.
> > 
> > On the other hand, I wonder if ConverterFactoryBase::create() should be 
> > allowed? to return a non-valid converter pointer or not.
> 
> I don't see a use case for that, so I'd rather check for !valid in
> ::create() and return nullptr in that case.

Yup - that answers my question. If we don't use isValid() it should be
dropped.

But if it's removed here, but should be kept, please add it into
::create(). Probably as a patch preceeding this one.

--
Kieran


> 
> > >                     LOG(SimplePipeline, Warning)
> > >                             << "Failed to create converter, disabling format conversion";
> > >                     converter_.reset();
> 
> -- 
> Regards,
> 
> Laurent Pinchart