[libcamera-devel] [RFC 0/2] Add support for pipeline specific data to Cameras

[Laurent Pinchart]

Hi Niklas,

On Wed, Jan 23, 2019 at 01:07:58AM +0100, Niklas Söderlund wrote:
> On 2019-01-22 19:12:23 +0100, Jacopo Mondi wrote:
> > RFC as I'm not sure about the idea of delegating ownership of platform data
> > to Cameras, as that requires the pipeline handler to dynamically allocate the
> > resources (if they go away at pipeline handler destruction time, is pointless
> > to store them in Camera).
> > 
> > The other way around is the idea of borrowing pipeline handler data to Cameras,
> > but as Cameras are shared objects, they might stay around longer that pipeline
> > handlers, and thus I felt it is safer to tie the CameraData lifetime to the one
> > of the Camera instance they're associated to.
> 
> While working with the Camera API and it's interaction with the pipeline 
> handler I felt a need for something similar you have here. My initial 
> idea to avoid lifetime management issues which you deal with in this 
> series was for the PipelineHandler to be able to set a unsigned int as a 
> cookie on the Camera at creation time, which it could retrieve with it's 
> later interaction with a Camera object.
> 
> In the end I went a different way, as it became clear we would need an 
> two way association between the Camera and the PipelineHandler anyhow.  
> That is the PipelineHandler would need to keep a reference to the 
> Camera, so I ended up using the pointer to the Camera object as the 
> cookie as this like a numerical cookie avoids all lifetime issues. That 
> way whenever a PipelineHandler is handed a Camera object it can use the 
> pointer to
> 
> 1. Validate that the Camera object in question really is its 
>    responsibility to handle.

Given that all calls to the pipeline handler will go through the Camera
object, I think any such validation should be done there to avoid
duplicating code in pipeline handlers. Furthermore, as the Camera object
will need to store a pointer to the pipeline handler in order to
firmward the calls, I don't see how we could get it wrong :-)

> 2. Use the pointer as a cookie to look-up any camera specific data in 
>    its private data structures.

This would require all pipeline handlers to implement the equivalent of
a std::map<Camera *, PrivateData *>. Isn't Jacopo's approach simpler as
it moves this to a pointer in the Camera object itself, avoiding lookup
code duplication in the pipeline handlers.

> I think that is quiet neat as it uses infrastructure we need anyhow to 
> properly manage the interactions between the two objects.
> 
> > Jacopo Mondi (2):
> >   libcamera: camera: Add CameraData
> >   libcamera: ipu3: Create CIO2 V4L2 devices
> > 
> >  include/libcamera/camera.h           | 13 ++++++++
> >  src/libcamera/camera.cpp             | 50 ++++++++++++++++++++++++++++
> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 42 +++++++++++++++++++++++
> >  3 files changed, 105 insertions(+)
> > 

-- 
Regards,

Laurent Pinchart