[libcamera-devel] Docs about buffers

[Kieran Bingham]

Quoting Dorota Czaplejewicz (2021-11-18 17:06:23)
> On Thu, 18 Nov 2021 15:32:23 +0000
> Kieran Bingham <kieran.bingham at ideasonboard.com> wrote:
> 
> > I would suspect that the move to make FrameBuffer 'extensible' hasn't
> > moved all possible things to the private implementation yet. Cancelling
> > a buffer shouldn't be exposed from the public API.
> > 
> > If you'd like to fix this as an exercise to help you learn the code
> > base, I'll let you fix this. If you don't want to - I'll fix it and
> > submit a patch. But this might be an 'easy' task that might help you dig
> > deeper around the codebase, so I don't want to take the fix away from
> > you if you want it.
> > 
> > FrameBuffer::cancel() should likely be moved to
> > FrameBuffer::Private::cancel().
> > 
> > You'll see that there is an 'include/libcamera/internal/framebuffer.h'
> > with the 'private' implementation, while
> > 'include/libcamera/framebuffer.h' has the public API version.
> 
> I'll take it for the learning experience.
> 
> From what I'm seeing, the intention is to have the public class final, and the private class extensible, regardless of member visibility.

Yes, the objects in the Public API should more or less all become
'Extensible' so that we can change the internal implementation without
breaking the public ABI/interface.

This is sometimes known as the Pimpl ... or 'private implementation'

  https://en.cppreference.com/w/cpp/language/pimpl

 
> I'm not familiar with this pattern, so is the plan to move all public API members of private visibility to the private class? I don't see a need for them after all, as they will never be accessible from interfacing code.

Not all public API members no. Only 'private' ones.
You've found one that is in public but as you've noted, shouldn't be
exposed to applications as a public API - it's an internal detail.

Pipeline handlers will 'know' when to cancel a buffer, because they are
responsible for handling the flow of buffers along the pipeline. So you
will see that if a pipeline handler detects it is shutting down, or
streamOff or such - it's likely got to handle 'cancelling' any currently
queued buffers.

(The same will happen with Requests, which can only be cancelled by
Pipeline handlers, but that is not yet Extensible... it's coming very
soon though).



> 
> I'm asking because ::cancel references MetaData, which is currently part of the public API too, and the ::Private class holds no reference to it.

The private class can obtain the public version through the O_PTR

The only code example using this so far is the CameraManager...

src/libcamera/camera_manager.cpp:
void CameraManager::Private::createPipelineHandlers()
{
	CameraManager *const o = LIBCAMERA_O_PTR();
	...
}


If needed, the metadata could also get moved into the private
allocation, and the 
  const FrameMetadata &metadata() const;

method would then have to be implemented in the public class as an
internal call to retrieve it from the private class.

And now I'm fearing if this becomes more of a rabbit hole, than an easy
quick 'get started' task. Feel free to ping me on any questions on
IRC/Matrix if you need anything.

--
Kieran