[libcamera-devel] [PATCH 0/4] Raspberry Pi: Embedded data usage

[Laurent Pinchart]

Hi Naush,

On Wed, Feb 17, 2021 at 06:24:01AM +0000, Naushir Patuck wrote:
> On Tue, 16 Feb 2021 at 23:47, Laurent Pinchart wrote:
> > On Tue, Feb 16, 2021 at 10:31:36AM +0000, Naushir Patuck wrote:
> > > Hi,
> > >
> > > This series of patches addresses the wasteful usage of embedded data
> > > streams on sensors that do not supply embedded data. We switch to
> > > using control lists to pass exposure and gain values from DelayedCtrls
> > > into the IPA in these cases.
> > >
> > > The breakdown of patches is as follows:
> > >
> > > Patch 1/4
> > > Stores the DelayedCtrls::get() provided control list with the bayer
> > > framebuffer in a queue, and pass this control list to the IPA on a
> > > RPi::IPA_EVENT_SIGNAL_ISP_PREPARE event.  If there is no embedded
> > > data, the IPA will simply pull the exposure and gain values from this
> > > control list.
> >
> > A rework of the IPA API has been merged, which causes extensive
> > conflicts in this patch. I'm sorry for the inconvenience, but could you
> > rebase the series ?
> 
> Sure no problem.  I'll prepare an updated version shortly.
> 
> > Not something that needs to be addressed now, but do you think it would
> > make sense to take an extra step and move parsing of embedded data to
> > the pipeline handler, always passing controls to the IPA ?
> 
> This does seem like a clean approach.  However, I do see a couple of
> problems:
> 
> - Currently, the CamHelper and parser objects live in the IPA domain, so
> the pipeline handler does not have access to them.

The idea would be to move the parser objects to the pipeline handler
side :-)

> - There may be cases when the sensor embedded data has vendor/sensor
> specific information stored within it that might be useful to the IPA.
> For example, phase detect pixel information is something that could be
> useful for AF algorithms. In these cases, a control list might not be
> appropriate to pass this information across.

We would need to create data types to store the information, which may
or may not rely on ControlList, I have no strong preference there. Even
if we decide to keep the embedded data parser on the IPA side, they
should move to libipa as they shouldn't be specific to one particular
IPA, so we'll have to think about how to represent the output of the
parser.

> Ofcourse, things will be different once libcamera has a full
> implementation of the sensor database, and provisions for these could
> be made.

Yes, that's the context, it would be part of the work done on the snesor
database.

> > > Patch 2/4
> > > Remove the MdParserRPi object, it is not used anymore with the above
> > > change.
> > >
> > > Patch 3/4
> > > This change selectively turns on the Unicam embedded data node on
> > > sensors that support embedded data.
> > >
> > > Patch 4/4
> > > With the IPA now able to use control lists to extract exposure and
> > > gain values, we add a flag in the pipeline handler to relax the strict
> > > bayer <-> embedded data buffer matching routine.  If this flag is set,
> > > and no match is found, the ipa reverts to the control list values.
> > > This avoids a few possible frame drops on heavily loaded systems.
> > >
> > > Regards,
> > > Naush
> > >
> > > Naushir Patuck (4):
> > >   pipeline: ipa: raspberrypi: Pass exposure/gain values to IPA though
> > >     controls
> > >   ipa: raspberrypi: Remove MdParserRPi
> > >   pipeline: raspberrypi: Only enabled embedded stream when available
> > >   pipeline: raspberrypi: Allow either strict or non-strict buffer
> > >     matching
> > >
> > >  src/ipa/raspberrypi/cam_helper.cpp            |   9 +-
> > >  src/ipa/raspberrypi/cam_helper_imx219.cpp     |   4 +-
> > >  src/ipa/raspberrypi/cam_helper_ov5647.cpp     |   3 +-
> > >  src/ipa/raspberrypi/md_parser_rpi.cpp         |  37 ----
> > >  src/ipa/raspberrypi/md_parser_rpi.hpp         |  32 ---
> > >  src/ipa/raspberrypi/meson.build               |   1 -
> > >  src/ipa/raspberrypi/raspberrypi.cpp           | 142 ++++++++-----
> > >  .../pipeline/raspberrypi/raspberrypi.cpp      | 194 +++++++++++-------
> > >  8 files changed, 211 insertions(+), 211 deletions(-)
> > >  delete mode 100644 src/ipa/raspberrypi/md_parser_rpi.cpp
> > >  delete mode 100644 src/ipa/raspberrypi/md_parser_rpi.hpp

-- 
Regards,

Laurent Pinchart