| Message ID | 20240818154324.71536-1-hdegoede@redhat.com |
|---|---|
| State | Accepted |
| Commit | a3fb0b3d0aec6477c205e7300dd19e54fee20d54 |
| Headers | show |
| Series |
|
| Related | show |
Hi Hans, On Sun, Aug 18, 2024 at 5:43 PM Hans de Goede <hdegoede@redhat.com> wrote: > PipelineHandler::acquire() only locks the media devices when the first > camera is acquired. If a second camera of a pipeline is acquired only > useCount_ is increased and nothing else is done. > > When releasing cameras PipelineHandler::release() should only unlock > the media devices when the last camera is released. But the old code > unlocked on every release(). > > Fix PipelineHandler::release() to only release the media devices when > the last camera is released. > > Yes, it makes sense to me that we should avoid other processes accessing the media devices until the current process releases all acquires/usages. Thanks for the fix! Reviewed-by: Harvey Yang <chenghaoyang@chromium.org> BR, Harvey Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > src/libcamera/pipeline_handler.cpp | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline_handler.cpp > b/src/libcamera/pipeline_handler.cpp > index 5a6de685..a20cd27d 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -205,7 +205,8 @@ void PipelineHandler::release(Camera *camera) > > ASSERT(useCount_); > > - unlockMediaDevices(); > + if (useCount_ == 1) > + unlockMediaDevices(); > > releaseDevice(camera); > > -- > 2.46.0 > >
Hi Hans, Thank you for the patch. On Sun, Aug 18, 2024 at 05:43:24PM +0200, Hans de Goede wrote: > PipelineHandler::acquire() only locks the media devices when the first > camera is acquired. If a second camera of a pipeline is acquired only > useCount_ is increased and nothing else is done. > > When releasing cameras PipelineHandler::release() should only unlock > the media devices when the last camera is released. But the old code > unlocked on every release(). > > Fix PipelineHandler::release() to only release the media devices when > the last camera is released. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/pipeline_handler.cpp | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index 5a6de685..a20cd27d 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -205,7 +205,8 @@ void PipelineHandler::release(Camera *camera) > > ASSERT(useCount_); > > - unlockMediaDevices(); > + if (useCount_ == 1) > + unlockMediaDevices(); > > releaseDevice(camera); >
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 5a6de685..a20cd27d 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -205,7 +205,8 @@ void PipelineHandler::release(Camera *camera) ASSERT(useCount_); - unlockMediaDevices(); + if (useCount_ == 1) + unlockMediaDevices(); releaseDevice(camera);
PipelineHandler::acquire() only locks the media devices when the first camera is acquired. If a second camera of a pipeline is acquired only useCount_ is increased and nothing else is done. When releasing cameras PipelineHandler::release() should only unlock the media devices when the last camera is released. But the old code unlocked on every release(). Fix PipelineHandler::release() to only release the media devices when the last camera is released. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- src/libcamera/pipeline_handler.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)