[{"id":30884,"web_url":"https://patchwork.libcamera.org/comment/30884/","msgid":"","date":"2024-08-21T17:32:09","subject":"Re: [PATCH 4/5] camera: Use invokeMethod() for pipe_->acquire() and\n\tpipe_->release()","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Hans,\n\nAh I left the comments on the 1st/5 patch too early. Should've checked all\npatches in this series first.\n\nIIUC, the users of libcamera should only get access to `libcamera::Camera`'s\nAPIs, which are the only callers of `libcamera::PipelineHandler`'s APIs.\nIf this is correct, I agree that we don't need to make\n`PipelineHandler::acquire`\nand `PipelineHandler::release` thread-safe.\n\nIf we go with this patch, we can remove `PipelineHandler::lock_`, and the\ndescriptions of the two functions should be updated.\n\nBR,\nHarvey\n\n\nOn Tue, Aug 20, 2024 at 9:50 PM Hans de Goede wrote:\n\n> Some pipeline handlers may want to open / close /dev/video# nodes\n> from pipe_->acquireDevices() / pipe_->releaseDevices().\n>\n> V4L2VideoDevice::open() creates an EventNotifier, this notifier needs\n> to be created from the CameraManager thread.\n>\n> Use invokeMethod() for pipe_->acquire() and pipe_->release() so that\n> any EventNotifiers created are created from the CameraManager thread\n> context.\n>\n> Signed-off-by: Hans de Goede \n> ---\n> src/libcamera/camera.cpp | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 4e393f89..61925e83 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -995,7 +995,8 @@ int Camera::acquire()\n> if (ret < 0)\n> return ret == -EACCES ? -EBUSY : ret;\n>\n> - if (!d->pipe_->acquire(this)) {\n> + if (!d->pipe_->invokeMethod(&PipelineHandler::acquire,\n> + ConnectionTypeBlocking, this)) {\n> LOG(Camera, Info)\n> << \"Pipeline handler in use by another process\";\n> return -EBUSY;\n> @@ -1030,7 +1031,8 @@ int Camera::release()\n> return ret == -EACCES ? -EBUSY : ret;\n>\n> if (d->isAcquired())\n> - d->pipe_->release(this);\n> + d->pipe_->invokeMethod(&PipelineHandler::release,\n> + ConnectionTypeBlocking, this);\n>\n> d->setState(Private::CameraAvailable);\n>\n> --\n> 2.46.0\n>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 34F1DC323E\n\tfor ;\n\tWed, 21 Aug 2024 17:32:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E66EE633CF;\n\tWed, 21 Aug 2024 19:32:22 +0200 (CEST)","from mail-lj1-x22a.google.com (mail-lj1-x22a.google.com\n\t[IPv6:2a00:1450:4864:20::22a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D395D63394\n\tfor ;\n\tWed, 21 Aug 2024 19:32:20 +0200 (CEST)","by mail-lj1-x22a.google.com with SMTP id\n\t38308e7fff4ca-2ef2fbf1d14so10028541fa.1\n\tfor ;\n\tWed, 21 Aug 2024 10:32:20 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"ZYph1Urx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1724261540; x=1724866340;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=vizF3raUbVpgWsjujAa+fZ/VrcOhdLgwKAC3SJKL50c=;\n\tb=ZYph1UrxLb92p3Vi+timEVG47r6CW0qMXyXETeXn1aSD2HlYzFkorjOhjN7d9E+uyG\n\tX26GfHNid5js/ZxeeB1K96YiK6JtJkKRlqjA8BIj9MCRdo3SrpHJaQcZoaZ24+6+Zzob\n\tdXGZiG4p44JwXC16vYG45W+hsAcwg8QN1Ha/w=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1724261540; x=1724866340;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=vizF3raUbVpgWsjujAa+fZ/VrcOhdLgwKAC3SJKL50c=;\n\tb=XoXwg1X3imv+BP1amOCtJuveouomBFod9wzTt3NinVOv/FsXLAoaELeKCRV0Js8O6g\n\t/qvoSyZt244NUFBdXF0afcPbhUC/wN+DwWZk3LnhNb00s1Nfp6A1WEM58r3l8GJWuWSH\n\tDtBjQ534nqicM3vaOllYilh5l5Q9vo08ziqnOAMtdQ+lfXqvnwB6rwRe83WCaMvm/E2p\n\trMMVuZ8tfw7uoNWx3NIhAINuf4HnnATeSVki7TtO1HhxeFr/alA5dDeBfN/NjDBdiIIO\n\t7kIkDwrc4E4HVhRBhPvgGRmemm3X1i7UPZ60Ugs1ksXuIoalOn1qjSVCVJHzKM+kTcrU\n\tSgWA==","X-Gm-Message-State":"AOJu0YyuJ9uKTCVijxvz2rcziIXP/PSJGS/C7Y8FDlpiA+DyBwll8Z+d\n\tFNrXj+M6+TaUPPSzegocGLOgLTm5wc4yqPUBxnVqlLD5fXKWyZ4iC3wdvYAbv+8fHMRUxOIM6iN\n\thOiaCNJAaKZi3Nh2jINSsumRgpUHewkfJJRNR","X-Google-Smtp-Source":"AGHT+IGBogImaODw/f7ZUV+C38uxQH7/yzevOcbv7i0n0qHESRi1+2qF2347HNRIQblWFNl5z9Cv2Ik+R0EukRVrYFQ=","X-Received":"by 2002:a2e:a9a1:0:b0:2f3:e2f0:b09a with SMTP id\n\t38308e7fff4ca-2f401e45c63mr1495721fa.4.1724261539789; Wed, 21 Aug 2024\n\t10:32:19 -0700 (PDT)","MIME-Version":"1.0","References":"<20240820195016.16028-1-hdegoede@redhat.com>\n\t<20240820195016.16028-5-hdegoede@redhat.com>","In-Reply-To":"<20240820195016.16028-5-hdegoede@redhat.com>","From":"Cheng-Hao Yang ","Date":"Wed, 21 Aug 2024 19:32:09 +0200","Message-ID":"","Subject":"Re: [PATCH 4/5] camera: Use invokeMethod() for pipe_->acquire() and\n\tpipe_->release()","To":"Hans de Goede ","Cc":"libcamera-devel@lists.libcamera.org, Maxime Ripard , \n\tMilan Zamazal ","Content-Type":"multipart/alternative; boundary=\"000000000000945986062034eca9\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30894,"web_url":"https://patchwork.libcamera.org/comment/30894/","msgid":"<20240825005823.GC17238@pendragon.ideasonboard.com>","date":"2024-08-25T00:58:23","subject":"Re: [PATCH 4/5] camera: Use invokeMethod() for pipe_->acquire() and\n\tpipe_->release()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hans,\n\nThank you for the patch.\n\nOn Tue, Aug 20, 2024 at 09:50:15PM +0200, Hans de Goede wrote:\n> Some pipeline handlers may want to open / close /dev/video# nodes\n> from pipe_->acquireDevices() / pipe_->releaseDevices().\n\nSee my reply to 3/5. I'm fine with doing so in the UVC pipeline handler\nnow, but not in \"some pipeline handlers\" before we redesign the\napplication-facing API.\n\n> V4L2VideoDevice::open() creates an EventNotifier, this notifier needs\n> to be created from the CameraManager thread.\n> \n> Use invokeMethod() for pipe_->acquire() and pipe_->release() so that\n> any EventNotifiers created are created from the CameraManager thread\n> context.\n\nThis will effectively serialize all calls to acquire() and release(),\nremoving the need for the functions to be thread-safe. The locks will\nalso not be needed anymore. Documentation should be updated.\n\n> Signed-off-by: Hans de Goede \n> ---\n> src/libcamera/camera.cpp | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 4e393f89..61925e83 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -995,7 +995,8 @@ int Camera::acquire()\n> \tif (ret < 0)\n> \t\treturn ret == -EACCES ? -EBUSY : ret;\n> \n> -\tif (!d->pipe_->acquire(this)) {\n> +\tif (!d->pipe_->invokeMethod(&PipelineHandler::acquire,\n> +\t\t\t\t ConnectionTypeBlocking, this)) {\n> \t\tLOG(Camera, Info)\n> \t\t\t<< \"Pipeline handler in use by another process\";\n> \t\treturn -EBUSY;\n> @@ -1030,7 +1031,8 @@ int Camera::release()\n> \t\treturn ret == -EACCES ? -EBUSY : ret;\n> \n> \tif (d->isAcquired())\n> -\t\td->pipe_->release(this);\n> +\t\td->pipe_->invokeMethod(&PipelineHandler::release,\n> +\t\t\t\t ConnectionTypeBlocking, this);\n> \n> \td->setState(Private::CameraAvailable);\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id ABAB0C323E\n\tfor ;\n\tSun, 25 Aug 2024 00:58:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B35CA633CF;\n\tSun, 25 Aug 2024 02:58:28 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1424261902\n\tfor ;\n\tSun, 25 Aug 2024 02:58:27 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B483A480;\n\tSun, 25 Aug 2024 02:57:21 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"atESYCXF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1724547441;\n\tbh=XpcQdCtPa5tRSsZTCC6WXMnrRNXu5MlDfylu+bimxoo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=atESYCXFqaycf9mJldSa3HBo48A1Ao6o6XDJ4SeJbAYmp2cafIpQDGOX7Jaka3HzP\n\tFvmToU4IIy92MjLNflTXV3xo/mTyegjsJdxYAofOfwaNzFz+iB8fj7a/g8Duvag8Jv\n\tsPq7WZFCSb1Sws9nGhhIkXdwRvZ1tZ73piRRthVM=","Date":"Sun, 25 Aug 2024 03:58:23 +0300","From":"Laurent Pinchart ","To":"Hans de Goede ","Cc":"libcamera-devel@lists.libcamera.org, Maxime Ripard , \n\tMilan Zamazal ","Subject":"Re: [PATCH 4/5] camera: Use invokeMethod() for pipe_->acquire() and\n\tpipe_->release()","Message-ID":"<20240825005823.GC17238@pendragon.ideasonboard.com>","References":"<20240820195016.16028-1-hdegoede@redhat.com>\n\t<20240820195016.16028-5-hdegoede@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240820195016.16028-5-hdegoede@redhat.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30925,"web_url":"https://patchwork.libcamera.org/comment/30925/","msgid":"","date":"2024-08-27T14:28:31","subject":"Re: [PATCH 4/5] camera: Use invokeMethod() for pipe_->acquire() and\n\tpipe_->release()","submitter":{"id":102,"url":"https://patchwork.libcamera.org/api/people/102/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi Harvey,\n\nOn 8/21/24 7:32 PM, Cheng-Hao Yang wrote:\n> Hi Hans,\n> \n> Ah I left the comments on the 1st/5 patch too early. Should've checked all\n> patches in this series first.\n> \n> IIUC, the users of libcamera should only get access to `libcamera::Camera`'s\n> APIs, which are the only callers of `libcamera::PipelineHandler`'s APIs.\n> If this is correct, I agree that we don't need to make `PipelineHandler::acquire`\n> and `PipelineHandler::release` thread-safe.\n> \n> If we go with this patch, we can remove `PipelineHandler::lock_`, and the\n> descriptions of the two functions should be updated.\n\n`PipelineHandler::lock_` is still useful when PipelineHandler::acquire() /\nPipelineHandler::release() are called at the same time from different\nthreads for 2 different cameras.\n\nWhen this happens we still need the lock to make sure the useCount_ variable\nupdating works properly.\n\nRegards,\n\nHans\n\n\n\n> On Tue, Aug 20, 2024 at 9:50 PM Hans de Goede > wrote:\n> \n> Some pipeline handlers may want to open / close /dev/video# nodes\n> from pipe_->acquireDevices() / pipe_->releaseDevices().\n> \n> V4L2VideoDevice::open() creates an EventNotifier, this notifier needs\n> to be created from the CameraManager thread.\n> \n> Use invokeMethod() for pipe_->acquire() and pipe_->release() so that\n> any EventNotifiers created are created from the CameraManager thread\n> context.\n> \n> Signed-off-by: Hans de Goede >\n> ---\n>  src/libcamera/camera.cpp | 6 ++++--\n>  1 file changed, 4 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 4e393f89..61925e83 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -995,7 +995,8 @@ int Camera::acquire()\n>         if (ret < 0)\n>                 return ret == -EACCES ? -EBUSY : ret;\n> \n> -       if (!d->pipe_->acquire(this)) {\n> +       if (!d->pipe_->invokeMethod(&PipelineHandler::acquire,\n> +                                   ConnectionTypeBlocking, this)) {\n>                 LOG(Camera, Info)\n>                         << \"Pipeline handler in use by another process\";\n>                 return -EBUSY;\n> @@ -1030,7 +1031,8 @@ int Camera::release()\n>                 return ret == -EACCES ? -EBUSY : ret;\n> \n>         if (d->isAcquired())\n> -               d->pipe_->release(this);\n> +               d->pipe_->invokeMethod(&PipelineHandler::release,\n> +                                      ConnectionTypeBlocking, this);\n> \n>         d->setState(Private::CameraAvailable);\n> \n> -- \n> 2.46.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 74519C323E\n\tfor ;\n\tTue, 27 Aug 2024 14:28:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3D84F63460;\n\tTue, 27 Aug 2024 16:28:40 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4617E61901\n\tfor ;\n\tTue, 27 Aug 2024 16:28:38 +0200 (CEST)","from mail-lf1-f72.google.com (mail-lf1-f72.google.com\n\t[209.85.167.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-47-L72w6QZMOW240IDZFIFcdA-1; Tue, 27 Aug 2024 10:28:35 -0400","by mail-lf1-f72.google.com with SMTP id\n\t2adb3069b0e04-53341717c62so4856977e87.2\n\tfor ;\n\tTue, 27 Aug 2024 07:28:35 -0700 (PDT)","from ?IPV6:2001:1c00:c32:7800:5bfa:a036:83f0:f9ec?\n\t(2001-1c00-0c32-7800-5bfa-a036-83f0-f9ec.cable.dynamic.v6.ziggo.nl.\n\t[2001:1c00:c32:7800:5bfa:a036:83f0:f9ec])\n\tby smtp.gmail.com with ESMTPSA id\n\t4fb4d7f45d1cf-5c0bb1c4977sm1065432a12.10.2024.08.27.07.28.32\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tTue, 27 Aug 2024 07:28:32 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"MowLIVFX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1724768917;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=L/ti8qB91Tq17XCXXX6vW4kD9OKxtGIAmAaOFSEZg78=;\n\tb=MowLIVFX3563XKaz+FPf6yeVrKtMku7whY2WJMHzQ5cmph10anDViwjmUmsd3VjNGMdUv+\n\tIge+K4Yhx6q/n1o83w2t61hvbsy3GAP1/V5rrAxHTRBcX16N+m2lDZiW36AMtn1vixEBcf\n\tEgJFKueIP4t1U9p1g4X7vxIuYxfFx/M=","X-MC-Unique":"L72w6QZMOW240IDZFIFcdA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1724768914; x=1725373714;\n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:cc:to:subject:user-agent:mime-version:date:message-id\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=L/ti8qB91Tq17XCXXX6vW4kD9OKxtGIAmAaOFSEZg78=;\n\tb=qpnTFVC9VFTRY1MasWEuGy7CuhZHC9C3RwAT6NNPaVxR2Vx+kN4H/qFornDT/Lzat0\n\t+cBZay3TucSVB0n5DBBlLOSPFpBhP4BRn+4oBvzW/mQ1e+OP/4DS4M+gnaLlsCzupDzK\n\trH1oP6J8pIbI0NZnud72ii0u7wuNH7IwGl9+fyLPkiPJnrdq+TGiNkKaXNvH3edeeJR4\n\tvxn1CNY7hbrgjQMIs501Tmhj/TFff5COFi6g27vRJrEWIPVBAZIowOIOBjQlnHIycNwy\n\taAH/uhKWRZuIbL25SoeVWF74RS38ElrkxZXvE4k3IYVuv1BBtbHw0AszJneseYgC2Qw7\n\tdZig==","X-Gm-Message-State":"AOJu0YxhR2gnh5D8/alXcQWuDGvCGh1wG6i3f+HWWhSz87EuQSb2F02w\n\tzbHIKM+5eKrb9hSwcsNGlbZdoDQDn6nMlLJbYD4rk4ZYDQcurfpeqK+d4CII7sUzfcTFhAYTbrR\n\tBF6Yh1Rw43ZVZsEQiaEcsmQPhlc5mExUoieFASsr6YQ3L2lB2CyZoWIhYEjc+FzaWd2E6Wgk=","X-Received":["by 2002:a05:6512:318a:b0:52c:e393:6634 with SMTP id\n\t2adb3069b0e04-5343883ba64mr9539425e87.33.1724768913885; \n\tTue, 27 Aug 2024 07:28:33 -0700 (PDT)","by 2002:a05:6512:318a:b0:52c:e393:6634 with SMTP id\n\t2adb3069b0e04-5343883ba64mr9539400e87.33.1724768913223; \n\tTue, 27 Aug 2024 07:28:33 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IHlVGuc2p+r7LH1sW4dmGGg6pQZYInyzzNWMHmgaiJVpqIbSWm5Da0bLR6RBpsOaK2gLn1/qw==","Message-ID":"","Date":"Tue, 27 Aug 2024 16:28:31 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 4/5] camera: Use invokeMethod() for pipe_->acquire() and\n\tpipe_->release()","To":"Cheng-Hao Yang ","Cc":"libcamera-devel@lists.libcamera.org, Maxime Ripard , \n\tMilan Zamazal ","References":"<20240820195016.16028-1-hdegoede@redhat.com>\n\t<20240820195016.16028-5-hdegoede@redhat.com>\n\t","From":"Hans de Goede ","In-Reply-To":"","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Language":"en-US, nl","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30926,"web_url":"https://patchwork.libcamera.org/comment/30926/","msgid":"","date":"2024-08-27T14:34:50","subject":"Re: [PATCH 4/5] camera: Use invokeMethod() for pipe_->acquire() and\n\tpipe_->release()","submitter":{"id":102,"url":"https://patchwork.libcamera.org/api/people/102/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 8/25/24 2:58 AM, Laurent Pinchart wrote:\n> Hi Hans,\n> \n> Thank you for the patch.\n> \n> On Tue, Aug 20, 2024 at 09:50:15PM +0200, Hans de Goede wrote:\n>> Some pipeline handlers may want to open / close /dev/video# nodes\n>> from pipe_->acquireDevices() / pipe_->releaseDevices().\n> \n> See my reply to 3/5. I'm fine with doing so in the UVC pipeline handler\n> now, but not in \"some pipeline handlers\" before we redesign the\n> application-facing API.\n\nAck, I'll update the commit message for v2.\n\n>> V4L2VideoDevice::open() creates an EventNotifier, this notifier needs\n>> to be created from the CameraManager thread.\n>>\n>> Use invokeMethod() for pipe_->acquire() and pipe_->release() so that\n>> any EventNotifiers created are created from the CameraManager thread\n>> context.\n> \n> This will effectively serialize all calls to acquire() and release(),\n> removing the need for the functions to be thread-safe. The locks will\n> also not be needed anymore. Documentation should be updated.\n\nGood point about the locks no longer being needed because of\nthe invokeMethod call serializing things. I'll drop the lock_\nand I'll update the \\context of acquire[Device]() and\nrelease[Device]() to match.\n\nRegards,\n\nHans\n\n\n>> Signed-off-by: Hans de Goede \n>> ---\n>> src/libcamera/camera.cpp | 6 ++++--\n>> 1 file changed, 4 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n>> index 4e393f89..61925e83 100644\n>> --- a/src/libcamera/camera.cpp\n>> +++ b/src/libcamera/camera.cpp\n>> @@ -995,7 +995,8 @@ int Camera::acquire()\n>> \tif (ret < 0)\n>> \t\treturn ret == -EACCES ? -EBUSY : ret;\n>> \n>> -\tif (!d->pipe_->acquire(this)) {\n>> +\tif (!d->pipe_->invokeMethod(&PipelineHandler::acquire,\n>> +\t\t\t\t ConnectionTypeBlocking, this)) {\n>> \t\tLOG(Camera, Info)\n>> \t\t\t<< \"Pipeline handler in use by another process\";\n>> \t\treturn -EBUSY;\n>> @@ -1030,7 +1031,8 @@ int Camera::release()\n>> \t\treturn ret == -EACCES ? -EBUSY : ret;\n>> \n>> \tif (d->isAcquired())\n>> -\t\td->pipe_->release(this);\n>> +\t\td->pipe_->invokeMethod(&PipelineHandler::release,\n>> +\t\t\t\t ConnectionTypeBlocking, this);\n>> \n>> \td->setState(Private::CameraAvailable);\n>> \n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 83362C324C\n\tfor ;\n\tTue, 27 Aug 2024 14:35:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7CA0263466;\n\tTue, 27 Aug 2024 16:34:59 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8965861901\n\tfor ;\n\tTue, 27 Aug 2024 16:34:58 +0200 (CEST)","from mail-ej1-f72.google.com (mail-ej1-f72.google.com\n\t[209.85.218.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-295-EXpKMsCtNFWi1oMMyH3fSA-1; Tue, 27 Aug 2024 10:34:56 -0400","by mail-ej1-f72.google.com with SMTP id\n\ta640c23a62f3a-a7a83fad218so464722866b.3\n\tfor ;\n\tTue, 27 Aug 2024 07:34:53 -0700 (PDT)","from ?IPV6:2001:1c00:c32:7800:5bfa:a036:83f0:f9ec?\n\t(2001-1c00-0c32-7800-5bfa-a036-83f0-f9ec.cable.dynamic.v6.ziggo.nl.\n\t[2001:1c00:c32:7800:5bfa:a036:83f0:f9ec])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-a86e549df50sm117572866b.53.2024.08.27.07.34.50\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tTue, 27 Aug 2024 07:34:50 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"fJcrGYdu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1724769297;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=tn6ZEQqDgLlHpHnsBLVxg6QiWN7U3MhPAAOeHBDSJmY=;\n\tb=fJcrGYdudRH735N7I7lnQ1k+7k+YaMYm8smFJhNAbc1EQ2aSSjzt14urkpokUbD/wI6qOW\n\tqEoIiR+H/lal2vkntYIElZyEAmNea7h6ADUqoy0CqkMV6xc7OV7QX5xnACIVyALPPO98lv\n\t+rLcFk3fgeSE738W0tfAKGllq+bIIAM=","X-MC-Unique":"EXpKMsCtNFWi1oMMyH3fSA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1724769292; x=1725374092;\n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:cc:to:subject:user-agent:mime-version:date:message-id\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=tn6ZEQqDgLlHpHnsBLVxg6QiWN7U3MhPAAOeHBDSJmY=;\n\tb=piIlbmBXHGX8nRkcxqyHQKTal/0YxeXndiuXnJ1EAx0EBoi9y3JDTDw1jYO23TlZ9W\n\tUlRdASphhbBkfx6/TwUsJZNgy7ViHzCwDZb4pusfk0gCf6V/rpLCU0NsOSsJtrRjCfHF\n\tIqcDwYjusxG4NfbJMIhgH3CSHW7tpG72/QQkA/K9py8B01Ve1+D989wocanD25UYzOsO\n\tUVo32a2sdVGQV6E+s13dTht5I0JS0bVOyDfXZNYnO9TKWmeeJ+ezcKHRcAZ++lgSI4Lo\n\tsSZygnuZc8PlxHA4j95/12fJp8YKGYl1jzKeLaMtuhx/u8keXT2Bil+Y2cCylAQG35bK\n\tl9sA==","X-Gm-Message-State":"AOJu0Ywv/EBpPWL9NJlVlRNkYnYGXRTVLjBO5YGbgpq/uSdjtrJ+Suqo\n\to0EoESNUOUZGTh0cjJOkrNmi88oG7/oyOXo6BSVfqrdHayvXcsrYOiFsR7ul1/U3HGsu9O4eyv5\n\tz8fxTCmTR4d35BgZmM1a/ouvlk/vdzZOPmbeOQb3alu8mZ23E2eMDsz9ciiPZLOOZ4K8dJ7eHGK\n\tU4AcU=","X-Received":["by 2002:a17:907:2cc6:b0:a86:7e5b:469e with SMTP id\n\ta640c23a62f3a-a86a54d1388mr952950566b.68.1724769291732; \n\tTue, 27 Aug 2024 07:34:51 -0700 (PDT)","by 2002:a17:907:2cc6:b0:a86:7e5b:469e with SMTP id\n\ta640c23a62f3a-a86a54d1388mr952948666b.68.1724769291195; \n\tTue, 27 Aug 2024 07:34:51 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IE57lLo15Hz2qsnZi1fjAxNmWkZ2qYAXHUUvpBdnzD9XfOka7ox8bM9HPbhq/4ULRu6lPAvqw==","Message-ID":"","Date":"Tue, 27 Aug 2024 16:34:50 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 4/5] camera: Use invokeMethod() for pipe_->acquire() and\n\tpipe_->release()","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, Maxime Ripard , \n\tMilan Zamazal ","References":"<20240820195016.16028-1-hdegoede@redhat.com>\n\t<20240820195016.16028-5-hdegoede@redhat.com>\n\t<20240825005823.GC17238@pendragon.ideasonboard.com>","From":"Hans de Goede ","In-Reply-To":"<20240825005823.GC17238@pendragon.ideasonboard.com>","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Language":"en-US, nl","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]