<div dir="ltr"><div dir="ltr">Hi Hans,<div><br></div><div>Ah I left the comments on the 1st/5 patch too early. Should've checked all</div><div>patches in this series first.</div><div><br></div><div>IIUC, the users of libcamera should only get access to `libcamera::Camera`'s</div><div>APIs, which are the only callers of `libcamera::PipelineHandler`'s APIs.</div><div>If this is correct, I agree that we don't need to make `PipelineHandler::acquire`</div><div>and `PipelineHandler::release` thread-safe.</div><div><br></div><div>If we go with this patch, we can remove `PipelineHandler::lock_`, and the</div><div>descriptions of the two functions should be updated.</div><div><br></div><div>BR,<br>Harvey</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Aug 20, 2024 at 9:50 PM Hans de Goede <<a href="mailto:hdegoede@redhat.com">hdegoede@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Some pipeline handlers may want to open / close /dev/video# nodes<br>
from pipe_->acquireDevices() / pipe_->releaseDevices().<br>
<br>
V4L2VideoDevice::open() creates an EventNotifier, this notifier needs<br>
to be created from the CameraManager thread.<br>
<br>
Use invokeMethod() for pipe_->acquire() and pipe_->release() so that<br>
any EventNotifiers created are created from the CameraManager thread<br>
context.<br>
<br>
Signed-off-by: Hans de Goede <<a href="mailto:hdegoede@redhat.com" target="_blank">hdegoede@redhat.com</a>><br>
---<br>
 src/libcamera/camera.cpp | 6 ++++--<br>
 1 file changed, 4 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp<br>
index 4e393f89..61925e83 100644<br>
--- a/src/libcamera/camera.cpp<br>
+++ b/src/libcamera/camera.cpp<br>
@@ -995,7 +995,8 @@ int Camera::acquire()<br>
        if (ret < 0)<br>
                return ret == -EACCES ? -EBUSY : ret;<br>
<br>
-       if (!d->pipe_->acquire(this)) {<br>
+       if (!d->pipe_->invokeMethod(&PipelineHandler::acquire,<br>
+                                   ConnectionTypeBlocking, this)) {<br>
                LOG(Camera, Info)<br>
                        << "Pipeline handler in use by another process";<br>
                return -EBUSY;<br>
@@ -1030,7 +1031,8 @@ int Camera::release()<br>
                return ret == -EACCES ? -EBUSY : ret;<br>
<br>
        if (d->isAcquired())<br>
-               d->pipe_->release(this);<br>
+               d->pipe_->invokeMethod(&PipelineHandler::release,<br>
+                                      ConnectionTypeBlocking, this);<br>
<br>
        d->setState(Private::CameraAvailable);<br>
<br>
-- <br>
2.46.0<br>
<br>
</blockquote></div></div>