[libcamera-devel,v4,00/11] libcamerea: Add support for exclusive access to cameras between processes

1205 mbox series

Hi,

This series aims to solve mutably exclusive access to a  camera between
two or more processes by the use of lockf(). In the process of adding
support for this the MediaDevice implementation is cleaned up a bit and
stricter call patterns are enforced. At the same time a small load is
taken of pipeline handler implementations.

As a camera might share resources with other cameras from the same
pipeline handler instance I have chosen to put the granularity of the
lock at the media device level. When a camera is acquired by an
applications all media devices the pipeline handler it belongs to have
claimed are locked and becomes unavailable to other processes. Other
process can still enumerate locked media devices and list all cameras
they provide but they can't use any of the cameras who are backed by a
locked media device.

Patches 1/11 -- 7/11 deals with tidying up and adding more restrictions
to the MediaDevices and how they handled inside pipeline handlers. While
patches 8/11 -- 11/11 adds the new locking scheme which depends on the
new stricter handling of media devices.

* Changes since v3
- Renamed media_device_basic.cpp to media_device_acquire.cpp.
- Improved documentation.
- Minor code improvements.

* Changes since v2
- Moved cleanup from 2/11 into 1/11 hence dropping all tags from 1/11 as
  it was a big move.
- Iterate of MediaDevice references in 10/11.
- Fixed spelling in commit messages.

* Changes since v1
- Spelling in commit messages.
- Explicitly prohibit nested locking inside the same libcamera instance,
  thanks Kieran for spotting this!

Niklas Söderlund (11):
  libcamera: Always check return value of MediaDevice::acquire()
  libcamera: media_device: Open and close media device inside populate()
  libcamera: media_device: Only read device information in populate()
  libcamera: media_device: Handle media device fd in acquire() and
    release()
  test: media_device: Create a common MediaDeviceTest base class
  test: media_device: Add test for acquire() and release()
  libcamera: media_device: Make open() and close() private
  libcamera: media_device: Add functions to lock device for other
    processes
  libcamera: pipeline_handler: Keep track of MediaDevice
  libcamera: pipeline_handler: Add functions to lock a whole pipeline
  libcamera: camera: Lock the pipeline handler in acquire()

 src/libcamera/camera.cpp                      |  16 +-
 src/libcamera/device_enumerator.cpp           |   8 +-
 src/libcamera/include/media_device.h          |  10 +-
 src/libcamera/include/pipeline_handler.h      |   7 +
 src/libcamera/media_device.cpp                | 243 +++++++++++-------
 src/libcamera/pipeline/ipu3/ipu3.cpp          |  71 +----
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  58 ++---
 src/libcamera/pipeline/uvcvideo.cpp           |  24 +-
 src/libcamera/pipeline/vimc.cpp               |  19 +-
 src/libcamera/pipeline_handler.cpp            |  68 +++++
 test/media_device/media_device_acquire.cpp    |  33 +++
 test/media_device/media_device_link_test.cpp  |  74 ++----
 test/media_device/media_device_print_test.cpp |  17 --
 test/media_device/media_device_test.cpp       |  36 +++
 test/media_device/media_device_test.h         |  34 +++
 test/media_device/meson.build                 |  10 +-
 test/pipeline/ipu3/ipu3_pipeline_test.cpp     |   5 -
 test/v4l2_device/v4l2_device_test.cpp         |   4 -
 test/v4l2_subdevice/v4l2_subdevice_test.cpp   |  17 +-
 19 files changed, 422 insertions(+), 332 deletions(-)
 create mode 100644 test/media_device/media_device_acquire.cpp
 create mode 100644 test/media_device/media_device_test.cpp
 create mode 100644 test/media_device/media_device_test.h

I addressed Laurents comments and pushed this series.

On 2019-05-17 02:54:36 +0200, Niklas Söderlund wrote:
> Hi,
> 
> This series aims to solve mutably exclusive access to a  camera between
> two or more processes by the use of lockf(). In the process of adding
> support for this the MediaDevice implementation is cleaned up a bit and
> stricter call patterns are enforced. At the same time a small load is
> taken of pipeline handler implementations.
> 
> As a camera might share resources with other cameras from the same
> pipeline handler instance I have chosen to put the granularity of the
> lock at the media device level. When a camera is acquired by an
> applications all media devices the pipeline handler it belongs to have
> claimed are locked and becomes unavailable to other processes. Other
> process can still enumerate locked media devices and list all cameras
> they provide but they can't use any of the cameras who are backed by a
> locked media device.
> 
> Patches 1/11 -- 7/11 deals with tidying up and adding more restrictions
> to the MediaDevices and how they handled inside pipeline handlers. While
> patches 8/11 -- 11/11 adds the new locking scheme which depends on the
> new stricter handling of media devices.
> 
> * Changes since v3
> - Renamed media_device_basic.cpp to media_device_acquire.cpp.
> - Improved documentation.
> - Minor code improvements.
> 
> * Changes since v2
> - Moved cleanup from 2/11 into 1/11 hence dropping all tags from 1/11 as
>   it was a big move.
> - Iterate of MediaDevice references in 10/11.
> - Fixed spelling in commit messages.
> 
> * Changes since v1
> - Spelling in commit messages.
> - Explicitly prohibit nested locking inside the same libcamera instance,
>   thanks Kieran for spotting this!
> 
> Niklas Söderlund (11):
>   libcamera: Always check return value of MediaDevice::acquire()
>   libcamera: media_device: Open and close media device inside populate()
>   libcamera: media_device: Only read device information in populate()
>   libcamera: media_device: Handle media device fd in acquire() and
>     release()
>   test: media_device: Create a common MediaDeviceTest base class
>   test: media_device: Add test for acquire() and release()
>   libcamera: media_device: Make open() and close() private
>   libcamera: media_device: Add functions to lock device for other
>     processes
>   libcamera: pipeline_handler: Keep track of MediaDevice
>   libcamera: pipeline_handler: Add functions to lock a whole pipeline
>   libcamera: camera: Lock the pipeline handler in acquire()
> 
>  src/libcamera/camera.cpp                      |  16 +-
>  src/libcamera/device_enumerator.cpp           |   8 +-
>  src/libcamera/include/media_device.h          |  10 +-
>  src/libcamera/include/pipeline_handler.h      |   7 +
>  src/libcamera/media_device.cpp                | 243 +++++++++++-------
>  src/libcamera/pipeline/ipu3/ipu3.cpp          |  71 +----
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  58 ++---
>  src/libcamera/pipeline/uvcvideo.cpp           |  24 +-
>  src/libcamera/pipeline/vimc.cpp               |  19 +-
>  src/libcamera/pipeline_handler.cpp            |  68 +++++
>  test/media_device/media_device_acquire.cpp    |  33 +++
>  test/media_device/media_device_link_test.cpp  |  74 ++----
>  test/media_device/media_device_print_test.cpp |  17 --
>  test/media_device/media_device_test.cpp       |  36 +++
>  test/media_device/media_device_test.h         |  34 +++
>  test/media_device/meson.build                 |  10 +-
>  test/pipeline/ipu3/ipu3_pipeline_test.cpp     |   5 -
>  test/v4l2_device/v4l2_device_test.cpp         |   4 -
>  test/v4l2_subdevice/v4l2_subdevice_test.cpp   |  17 +-
>  19 files changed, 422 insertions(+), 332 deletions(-)
>  create mode 100644 test/media_device/media_device_acquire.cpp
>  create mode 100644 test/media_device/media_device_test.cpp
>  create mode 100644 test/media_device/media_device_test.h
> 
> -- 
> 2.21.0
>