[PATCH 4/5] camera: Use invokeMethod() for pipe_->acquire() and pipe_->release()

[Cheng-Hao Yang]

Hi Hans,

Ah I left the comments on the 1st/5 patch too early. Should've checked all
patches in this series first.

IIUC, the users of libcamera should only get access to `libcamera::Camera`'s
APIs, which are the only callers of `libcamera::PipelineHandler`'s APIs.
If this is correct, I agree that we don't need to make
`PipelineHandler::acquire`
and `PipelineHandler::release` thread-safe.

If we go with this patch, we can remove `PipelineHandler::lock_`, and the
descriptions of the two functions should be updated.

BR,
Harvey


On Tue, Aug 20, 2024 at 9:50 PM Hans de Goede <hdegoede at redhat.com> wrote:

> Some pipeline handlers may want to open / close /dev/video# nodes
> from pipe_->acquireDevices() / pipe_->releaseDevices().
>
> V4L2VideoDevice::open() creates an EventNotifier, this notifier needs
> to be created from the CameraManager thread.
>
> Use invokeMethod() for pipe_->acquire() and pipe_->release() so that
> any EventNotifiers created are created from the CameraManager thread
> context.
>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  src/libcamera/camera.cpp | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 4e393f89..61925e83 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -995,7 +995,8 @@ int Camera::acquire()
>         if (ret < 0)
>                 return ret == -EACCES ? -EBUSY : ret;
>
> -       if (!d->pipe_->acquire(this)) {
> +       if (!d->pipe_->invokeMethod(&PipelineHandler::acquire,
> +                                   ConnectionTypeBlocking, this)) {
>                 LOG(Camera, Info)
>                         << "Pipeline handler in use by another process";
>                 return -EBUSY;
> @@ -1030,7 +1031,8 @@ int Camera::release()
>                 return ret == -EACCES ? -EBUSY : ret;
>
>         if (d->isAcquired())
> -               d->pipe_->release(this);
> +               d->pipe_->invokeMethod(&PipelineHandler::release,
> +                                      ConnectionTypeBlocking, this);
>
>         d->setState(Private::CameraAvailable);
>
> --
> 2.46.0
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20240821/ede1d31b/attachment.htm>