[libcamera-devel,RFC,0/2] android: Introduce CameraWorker

9983 mbox series

This two small patches, sent as RFC mostly for comments on the design,
implement a worker in the libcamera HAL that is used to handle synchronization
fences before queueing a Request to the libcamera::Camera.

The camera framework provides, for each buffer part of a capture request
an acquisition fence the camera HAL is supposed to wait on before using
the buffer. The wait procedure cannot be performed in the Camera HAL thread, as
the HAL runs in the global CameraManager thread.

As fences are handled through a simple poll, it would be possible to install
a Notifier for each fence and handle their completion through a Signal in the
CameraDevice. Although this would make the class over-complicated.

Introducing a worker that synchronously waits on fences using its own thread
is a cleaner design a provides a cleaner interface for the CameraDevice.

The waitFence() function implementes the same mechanism as the libdrm provided
sync_wait() function without introducing an explicit dependency in libcamera.


Jacopo Mondi (2):
  android: camera_worker: Introduce CameraWorker
  android: camera_device: Queue request to Worker

 src/android/camera_device.cpp | 21 ++++------
 src/android/camera_device.h   |  3 ++
 src/android/camera_worker.cpp | 67 +++++++++++++++++++++++++++++++
 src/android/camera_worker.h   | 75 +++++++++++++++++++++++++++++++++++
 src/android/meson.build       |  1 +
 5 files changed, 154 insertions(+), 13 deletions(-)
 create mode 100644 src/android/camera_worker.cpp
 create mode 100644 src/android/camera_worker.h

--
2.28.0

Hi Jacopo,

On Tue, Oct 06, 2020 at 06:06:35PM +0200, Jacopo Mondi wrote:
> This two small patches, sent as RFC mostly for comments on the design,
> implement a worker in the libcamera HAL that is used to handle synchronization
> fences before queueing a Request to the libcamera::Camera.
> 
> The camera framework provides, for each buffer part of a capture request
> an acquisition fence the camera HAL is supposed to wait on before using
> the buffer. The wait procedure cannot be performed in the Camera HAL thread, as
> the HAL runs in the global CameraManager thread.

All the operations exposed by the HAL to the camera service are run in
the camera service thread, not the libcamera CameraManager.

> As fences are handled through a simple poll, it would be possible to install
> a Notifier for each fence and handle their completion through a Signal in the
> CameraDevice. Although this would make the class over-complicated.
> 
> Introducing a worker that synchronously waits on fences using its own thread
> is a cleaner design a provides a cleaner interface for the CameraDevice.
> 
> The waitFence() function implementes the same mechanism as the libdrm provided
> sync_wait() function without introducing an explicit dependency in libcamera.
> 
> 
> Jacopo Mondi (2):
>   android: camera_worker: Introduce CameraWorker
>   android: camera_device: Queue request to Worker
> 
>  src/android/camera_device.cpp | 21 ++++------
>  src/android/camera_device.h   |  3 ++
>  src/android/camera_worker.cpp | 67 +++++++++++++++++++++++++++++++
>  src/android/camera_worker.h   | 75 +++++++++++++++++++++++++++++++++++
>  src/android/meson.build       |  1 +
>  5 files changed, 154 insertions(+), 13 deletions(-)
>  create mode 100644 src/android/camera_worker.cpp
>  create mode 100644 src/android/camera_worker.h

Hi Laurent,

On Thu, Oct 08, 2020 at 06:17:46AM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Tue, Oct 06, 2020 at 06:06:35PM +0200, Jacopo Mondi wrote:
> > This two small patches, sent as RFC mostly for comments on the design,
> > implement a worker in the libcamera HAL that is used to handle synchronization
> > fences before queueing a Request to the libcamera::Camera.
> >
> > The camera framework provides, for each buffer part of a capture request
> > an acquisition fence the camera HAL is supposed to wait on before using
> > the buffer. The wait procedure cannot be performed in the Camera HAL thread, as
> > the HAL runs in the global CameraManager thread.
>
> All the operations exposed by the HAL to the camera service are run in
> the camera service thread, not the libcamera CameraManager.
>

Correct, I mixed up the two (in a reply to Niklas too)

> > As fences are handled through a simple poll, it would be possible to install
> > a Notifier for each fence and handle their completion through a Signal in the
> > CameraDevice. Although this would make the class over-complicated.
> >
> > Introducing a worker that synchronously waits on fences using its own thread
> > is a cleaner design a provides a cleaner interface for the CameraDevice.
> >
> > The waitFence() function implementes the same mechanism as the libdrm provided
> > sync_wait() function without introducing an explicit dependency in libcamera.
> >
> >
> > Jacopo Mondi (2):
> >   android: camera_worker: Introduce CameraWorker
> >   android: camera_device: Queue request to Worker
> >
> >  src/android/camera_device.cpp | 21 ++++------
> >  src/android/camera_device.h   |  3 ++
> >  src/android/camera_worker.cpp | 67 +++++++++++++++++++++++++++++++
> >  src/android/camera_worker.h   | 75 +++++++++++++++++++++++++++++++++++
> >  src/android/meson.build       |  1 +
> >  5 files changed, 154 insertions(+), 13 deletions(-)
> >  create mode 100644 src/android/camera_worker.cpp
> >  create mode 100644 src/android/camera_worker.h
>
> --
> Regards,
>
> Laurent Pinchart