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

[Laurent Pinchart]

On Wed, Nov 06, 2024 at 02:42:08PM +0100, Hans de Goede wrote:
> On 6-Nov-24 12:22 PM, Laurent Pinchart wrote:
> > 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).
> 
> I do plan to keep supporting and improving the atomisp code. We first
> got it working (as in displaying any image at all) in 2021 and since
> then a lot has been improved / changed already.
> 
> But it has been very slow going since this is a spare time side
> project for me.

Thanks for the confirmation, and for the work you're doing there. Slow
is absolutely fine.

> >> 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.
> 
> Right, note atomisp already used the MC API. So this would mean
> having a second /dev/video# MC node with the ISP as the parent /
> source I presume.

Yes that's correct. And eventually there should be a third video node,
to pass parameters to the ISP.

> And then use videobuf2 
> 
> >> 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 :-)
> 
> ATM I'm mostly alone. It seems there is some interest in getting
> things to work but no one has really stepped up to help.
> 
> I hope that getting a minimal POC with Firefox / snapshot
> showing video through pipewire on these devices ready will help
> to gather some attention and hopefully more help.
> 
> So I'm happy to read that you write "As a first step I have no
> issue about that, we don't have to make everything perfect overnight"
> 
> which I hope means that we can merge a swstats based pipeline
> handler as a first step and hopefully that will help with
> bootstrapping some sort of community of atomisp users and
> contributors.

Yes, I'm fine with that. I trust that you will work with the other swisp
developers to make sure you won't step on each other's toes, now that
there will be multiple users of the code.

-- 
Regards,

Laurent Pinchart