[libcamera-devel] [PATCH v1 3/6] ipa: ipu3: Introduce IPAConfigInfo in IPC

[Laurent Pinchart]

Hi Paul,

On Tue, May 18, 2021 at 07:06:40PM +0900, paul.elder at ideasonboard.com wrote:
> On Tue, May 18, 2021 at 12:47:24PM +0300, Laurent Pinchart wrote:
> > On Tue, May 18, 2021 at 06:43:32PM +0900, paul.elder at ideasonboard.com wrote:
> > > On Sat, May 15, 2021 at 07:30:13PM +0300, Laurent Pinchart wrote:
> > > > On Fri, May 14, 2021 at 10:57:24AM +0100, Kieran Bingham wrote:
> > > > > On 14/05/2021 08:58, Umang Jain wrote:
> > > > > > IPAConfigInfo is a consolidated data structure passed from IPU3
> > > > > > pipeline-handler to IPU3 IPA. The structure can be extended with
> > > > > > additional parameiters to accommodate the requirements of multiple
> > > > > 
> > > > > s/parameiters/parameters/
> > > > > 
> > > > > > IPU3 IPA modules for e.g., an open-source IPA (in-tree) and a closed
> > > > > > source IPA (for ChromeOS).
> > > > > 
> > > > > No need to have the expression after the e.g. It's simply to accommodate
> > > > > the parameters for the IPU3 IPAs.
> > > > > 
> > > > > In fact, this isn't related to supporting multiple IPAs at all. This
> > > > > simply defines the interface. The fact that we have multiple
> > > > > implementations is just a fact on top of that.
> > > > > 
> > > > > > As usual, the documentation for .mojom files are kept in a separate
> > > > > > .cpp file. Hence, create and document IPAConfigInfo in
> > > > > > ipu3_ipa_interface.cpp.
> > > > > > 
> > > > > > Finally, adapt the in-tree IPU3 IPA to use IPAConfigInfo as well.
> > > > > > 
> > > > > > Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> > > > > > ---
> > > > > >  include/libcamera/ipa/ipu3.mojom             | 10 ++++-
> > > > > >  include/libcamera/ipa/ipu3_ipa_interface.cpp | 39 ++++++++++++++++++++
> > > > > 
> > > > > Ok, now I really think we need to have the ipa directory under
> > > > > src/libcamera/ipa as a prerequisite to this series.
> > > > > 
> > > > > Let me know if you'll do that or if you prefer me to do it.
> > > > 
> > > > I'm beginning to wonder if we should write a simple python script that
> > > > would copy comments from a mojom file to a cpp file, and keep the
> > > > documentation in the .mojom file. What do you think ?
> > > 
> > > tbh that's what I wanted to do, but I thought that was over-engineering
> > > it. Maybe it's worth it to stop the clutter from docs-only cpp files?
> > 
> > Can the mojo parser help there, could it be leveraged to extract
> > comments, or does it ignore them completely ?
> 
> That's why I didn't pursue that route; the mojo parser ignores comments
> completely :/
> 
> Actually it's before that, they're dropped by the lexer.

Let's not go down that rabbit hole then. Should you however file a bug
report against mojom to ask for this feature ? If you think it's a good
idea, could you do so ?

-- 
Regards,

Laurent Pinchart