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

[Hans de Goede]

Hi,

On 8/25/24 2:58 AM, Laurent Pinchart wrote:
> Hi Hans,
> 
> Thank you for the patch.
> 
> On Tue, Aug 20, 2024 at 09:50:15PM +0200, Hans de Goede wrote:
>> Some pipeline handlers may want to open / close /dev/video# nodes
>> from pipe_->acquireDevices() / pipe_->releaseDevices().
> 
> See my reply to 3/5. I'm fine with doing so in the UVC pipeline handler
> now, but not in "some pipeline handlers" before we redesign the
> application-facing API.

Ack, I'll update the commit message for v2.

>> 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.
> 
> This will effectively serialize all calls to acquire() and release(),
> removing the need for the functions to be thread-safe. The locks will
> also not be needed anymore. Documentation should be updated.

Good point about the locks no longer being needed because of
the invokeMethod call serializing things. I'll drop the lock_
and I'll update the \context of acquire[Device]() and
release[Device]() to match.

Regards,

Hans


>> 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);
>>  
>