[libcamera-devel] [PATCH v2 15/18] libcamera: pipeline: simple: enable use of Soft ISP and Soft IPA

[Kieran Bingham]

Quoting Andrei Konovalov via libcamera-devel (2024-01-15 12:30:24)
> Hi Pavel,
> 
> On 15.01.2024 11:15, Pavel Machek wrote:
> > Hi!
> > 
> >>>> +  /*
> >>>> +   * Create SoftwareIsp unconditionally if no converter is used
> >>>> +   * - to be revisited
> >>>> +   */
> >>>> +  if (!converter_) {
> >>>> +          swIsp_ = SoftwareIspFactoryBase::create(pipe, sensor_->controls());
> >>>> +          if (!swIsp_) {
> >>>> +                  LOG(SimplePipeline, Warning)
> >>>> +                          << "Failed to create software ISP, disabling software debayering";
> >>>> +                  swIsp_.reset();
> >>>> +          } else {
> >>>> +                  swIsp_->inputBufferReady.connect(this, &SimpleCameraData::conversionInputDone);
> >>>> +                  swIsp_->outputBufferReady.connect(this, &SimpleCameraData::conversionOutputDone);
> >>>> +                  swIsp_->ispStatsReady.connect(this, &SimpleCameraData::ispStatsReady);
> >>>> +
> >>>> +                  swIsp_->getSignalSetSensorControls().connect(this, &SimpleCameraData::setSensorControls);
> >>>> +          }
> >>>> +  }
> >>>> +
> >>>
> >>> I guess this needs to be revisited before the merge?
> >>
> >> Currently there is a build-time choice of instantiating:
> >>    -Dpipelines=simple/simple -Dipas=simple/simple
> >> or not instantiating:
> >>    -Dpipelines=simple
> >> the Soft ISP and the Soft IPA.
> >>
> >> Does it need to be a run-time option? How should this work from the user perspective then?
> >>
> >> For me the only obvious disadvantage of always creating the Soft ISP/IPA if it
> >> is enabled in build configuration is that in this case the frames can be captured
> >> only in RGB888 or BGR888 formats. Capturing raw bayer data isn't possible, as
> >> Soft ISP/IPA always debayers all the raw bayer formats it supports.
> > 
> > Well, compile-time option is really bad for distros, right? I'm pretty
> > sure someone out there uses bayer capture (millipixels, for example),
> > and most apps will want RGB888 (cam sdl, for example; likely others).
> 
> OK, so the only concern is the ability to capture the raw frames.
> This is probably fairly easy to fix.
> 
> > So long-term we'll really want to support both. Not sure it needs to
> > be solved before merge, but since we had a TODO in the comment, I
> > wanted to point it out.
> 
> I see. Thanks for the remainder! Let me see if I can fix that before the
> next version of the patch set.

Ideally - the SoftISP should always be compiled, and then can be enabled
on a platform basis through the table at
https://gitlab.freedesktop.org/camera/libcamera-softisp/-/blob/SoftwareISP-v05/src/libcamera/pipeline/simple/simple.cpp#L193

It could also be that in the future we might expect a RAW stream output
as well as an RGB888 so that the 3a can run to handle the
autoexposure/gains while still outputing RAW images. But that can be
later. Just moving to the SoftIPA being compiled with the simple
pipeline handler, and enabled on a platform basis is enough to start
with I believe.

--
Kieran


> 
> Thanks,
> Andrei
> 
> > Best regards,
> >                                                               Pavel