[RFC 0/4] libcamera: swstats_cpu: Add processFrame() method

[Laurent Pinchart]

On Sun, Nov 03, 2024 at 01:05:29PM +0100, Hans de Goede wrote:
> On 10-Oct-24 9:45 PM, Laurent Pinchart wrote:
> > On Wed, Oct 09, 2024 at 10:01:06PM +0200, Hans de Goede wrote:
> >> Hi All,
> >>
> >> Here is a patch series adding a processFrame() method to gather
> >> statistics on an entire frame in one go, rather then using the
> >> line by line approach which is used by the somewhat tightly coupled
> >> DebayerCpu class.
> >>
> >> ATM there are no users for processFrame() yet, so the code is
> >> compile tested only which is why it is marked as RFC.
> >>
> >> I see at least 3 possible use-cases for this:
> >>
> >> 1. Gathering sw statistics for 3A on raw bayer buffers before
> >> passing them along for a to-be-added software CPU ISP raw mode.
> >>
> >> 2. Gathering sw statistics for 3A on raw bayer buffers before
> >> doing further processing on the GPU for the software GPU ISP
> >> (possibly as an intermediate step until the stats are gathered
> >> on the GPU too and/or as a way to compare CPU vs GPU stats
> >> for verification purposes).
> >>
> >> 3. I'm working on a pipeline handler for the atomISP2, where
> >> there is a working hw ISP but no usable statistics.
> > 
> > Why are the stats not usable ? Is it a hardware issue, a firmware issue,
> > or "just" that we don't have information about the statistics format ?
> 
> There are 2 problems:
> 
> 1. As you say '"just" that we don't have information about
> the statistics format ?'
> 
> I'll email Sakari to see if he can help with at least a .h files
> with a C struct definition for the buffers.
> 
> One thing to keep in mind here is that the hw using the atomisp
> is getting pretty old. So while I'm very interested in getting
> this going at the same time I also want to not spend too much
> time on this.

I understand that, but let's not forget that the driver is in staging,
and what it implies. There should be continuous effort to clean up the
driver to get it out of staging, and I believe that should include
things like stats support. It doesn't have to be done right away, but we
don't want drivers to bitrot in staging, we have too many of those
already (I'm thinking about the IPU3 ImgU driver in particular that will
need work, or should be removed from the kernel if it becomes clear
nobody will fix its issues).

> Using the swstats_cpu code + reusing the ipa_soft_simple.so IPA
> is a relatively easy / low-risk way of doing this.

As a first step I have no issue about that, we don't have to make
everything perfect overnight :-)

> I actually am about to post a first version of an atomisp
> pipeline handler using this approach to do aec/agc.
> 
> AWB will follow later. First I need to find out where
> the atomisp kernel driver sets the ISP R/G/B gains and export
> those as standard V4L2 controls, rather then whatever custom
> ioctl it is currently using ...
> 
> 2. The kernel driver does receive what it calls "3a stat" buffers
> but atm it simply discards these. I think there is or used to be
> a custom ioctl to retrieve these, but I may have ripped that out
> already. Regardless we need to design a proper userspace API for
> this. Maybe a separate /dev/video# node using videobuf2 to pass
> the buffers?

Yes, that's what ISP drivers do. It also implies using the MC API.

> Anyways this is something to look at if we can
> get help with figuring out the statistics format. So for now I
> have not really looked at this yet.

Being able to get the stats buffers out would also mean someone could
give a go at reverse engineering the format, if we need to go that
route. I don't know how big the atomisp community is though, and if
there could be a volunteer for this task, or if you're alone :-)

> >> The plan is to gather sw statistics on the output buffers
> >> (so on e.g. YUV images) and then re-use the 3A code from
> >> the software ISP (I realize gathering statistics on the
> >> pipeline output buffers is not ideal, but the atomISP driver
> >> has no way to access the raw bayer data).
> > 
> > It's more than non-ideal, it will make algorithms development much more
> > painful :-(
> 
> Yes this is less then ideal. But at least for basic agc + awb
> I think we can make this work.
> 
> Actually for aec/awb it works pretty well since the ISP outputs
> YUV data getting a Y histogram is easy and even in
> the postprocessed data it is easy to see if the image is too
> light / dark.

-- 
Regards,

Laurent Pinchart