[PATCH 3/5] pipeline_handler: Add acquireDevice() method to mirror existing releaseDevice()

[Hans de Goede]

Hi,

On 8/25/24 2:49 AM, Laurent Pinchart wrote:

<snip>

>>> @@ -213,12 +215,35 @@ void PipelineHandler::release(Camera *camera)
>>>         --useCount_;
>>>  }
>>>
>>> +/**
>>> + * \brief Acquire resources associated with this camera
>>> + * \param[in] camera The camera for which to acquire resources
>>> + *
>>> + * Pipeline handlers may override this in order to get resources
>>> + * such as open /dev nodes are allocate buffers when a camera is
> 
> s at open /dev nodes at opening devices/
> s/are allocate/and allocating/

Ack, fixed for v2.

> As discussed previously, I think we need to improve the
> application-facing API to make resource and power management more
> explicit. Camera::acquire() was never meant for this purpose. I
> understand we need a short term solution to the power consumption issue
> for UVC cameras without an enormous amount of yak shaving

Ack, I have posted a RFC with the application-facing API changes we discussed
for this:

https://lists.libcamera.org/pipermail/libcamera-devel/2024-August/044384.html

> and I'm fine
> with this patch series as an interim measure, but I don't want to see
> acquireDevice() being used in random ways by pipeline handlers other
> than UVC before we address the more general problem. This should be
> documented here.

Ok I have added the following block to the docbook comment to address
this for v2:

 * This is used by the uvcvideo pipeline handler to delay opening /dev/video#
 * until the camera is acquired to avoid excess power consumption. The delayed
 * opening of /dev/video# is a special case because the kernel uvcvideo driver
 * powers on the USB device as soon as /dev/video# is opened. This behavior
 * must *not* be copied by other drivers.

>>> + * acquired.
> 
> Please reflow these paragraphs to 80 columns.

Ack, done for v2.

> 
>>> + *
>>> + * Note this is called once for every camera which is acquired,
> 
> s/which/that/
> 
>>> + * so if there are shared resources the pipeline handler must take
> 
> s/shared resources/resources shared by multiple cameras/
> 
>>> + * care to not release them until releaseDevice() has been called for
>>> + * all previously acquired cameras.
> 
> Doesn't this paragraph belong to releaseDevice() ?

that is a better place yes, I have moved it for v2.

Regards,

Hans