[libcamera-devel] Debayering

[Jacopo Mondi]

Hi Dorota,

On Tue, Nov 16, 2021 at 12:41:50PM +0100, Dorota Czaplejewicz wrote:
> On Mon, 15 Nov 2021 12:16:39 +0200
> Laurent Pinchart <laurent.pinchart at ideasonboard.com> wrote:
>
> > Hi Dorota,
> >
> > On Sat, Nov 13, 2021 at 11:56:04AM +0100, Dorota Czaplejewicz wrote:
> > >
> > > "Quick hacks" is not what I intend to merge. Rather, I found the
> > > libcamera codebase difficult to learn, and I don't see the point in
> >
> > Is there anything in particular that would have made it easier ?
>
> After digging in some more, I can point out some things already.
>
> Initially I thought there was nearly zero documentation on the internals. I used three ways to access the code: stepping through executing code, reading header files, and looking through the documentation web site.

even if we strive for 'zero-Doxygen warnings' (aka all the public
fields in the external and internal API should have a documentation
block associated) I understand putting all the pieces together might
not be easy. Can I suggest you to have a look at the pipeline handler
development guide in $builddir/Documentation/html/guides/pipeline-handler.html
which might be a bit stale in some parts but gives an overview about
the internal interaction between the Camera and the pipeline
handlers. We do also have an IPA implementer guide in
$builddir//Documentation/html/guides/ipa.html which should instead
provide an overview of the pipeline handler - IPA interaction model.

When it comes to the single classes, as said, we aim to zero doxygen
warnings and all publicly visible fields in the code base are
documented. For sure the documentation has to be expanded and
enriched but if there's one thing we tried to be inflexible about was
having a piece of documentation associated with each (internally and
externally) visible component of the library.

>
> Then I discovered that there's documentation in the beginning of the .cpp file, as well as some strings interspersed through some files (notably, simple.cpp, and camera.cpp). But it took me a long time to notice them, and I only found out where the documentation for base classes is hours after I started looking at the code.

Well, each core library .cpp has Doxygen documentation for the file,
the classes it contains and all public visible fields in the class. The
/** block are pretty visible. As you mention camera.cpp, it starts
with a 80 lines documentation block, it's hard to miss it :)

The single pipeline handler implementations are not Doxygen documented
as they all implement the same interface as defined by the
PipelineHandler base class. We could reconsider that if we have a way
to formally document each implementation without demanding all public
fields to have a (redundant) documentation block associated.

>
> The main problem with learning libcamera is that it's complex. I don't yet know if it's incidental or not, having discovered the docs literally 5 minutes ago.
>
> The other problem is that documentation is far separated from the things it documents. Part of that is caused by the choice of C++, which splits headers and source files, which necessarily creates a choice between:
> 1. prototypes being separate from documentation,
> 2. bodies being separate from documentation,
> 3. documentation being duplicated, or
> 4. documentation being entirely separate from code, like needing a separate browser.
>
> In the case of class fields and abstract methods, there's only one place where they are mentioned, which is the declaration. In libcamera, they are not documented there, but instead the documentation is inserted in the corresponding cpp file, which… doesn't have either the declaration of the body of the documented item, making it honestly confusing. I think moving those to header files would have saved me a lot of wandering about the sources.
>
We initially discusses if it was more appropriate to document in the
header files or in the .cpp files, and as reported by the codying
style document we decided to go for the .cpp to keep documentation
closer to implementation. Looking back, the documentation is mostly
about the API, so we could have gone with documentation in the header
as well, but honestly once the documentation is compiled it doesn't
make much difference.

Do you find the Doxygen documentation unhelpful because not enough
complete ? Or what is missing is a general overview of the internal
architecture ?

> Following this precedent, the header files could also contain the docs for the rest of the items, to be able to create a complete picture of a module in one place (although I'm much less confident about this).
>
> I'll report back once I have some knowledge from reading the comments.
>
> Cheers,
> Dorota


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20211116/43b5e2aa/attachment.sig>