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

[Laurent Pinchart]

Hi Hans,

On Tue, Aug 27, 2024 at 04:25:00PM +0200, Hans de Goede wrote:
> 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

Thank you for that. I really appreciate your efforts. I'll reply to the
RFC after reviewing v2 of this series.

> > 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,

Laurent Pinchart