[{"id":30860,"web_url":"https://patchwork.libcamera.org/comment/30860/","msgid":"","date":"2024-08-19T13:51:10","subject":"Re: [PATCH] pipeline_handler: Fix unlocking media devices too early\n\twhen there are multiple cameras","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Hans,\n\nOn Sun, Aug 18, 2024 at 5:43 PM Hans de Goede wrote:\n\n> PipelineHandler::acquire() only locks the media devices when the first\n> camera is acquired. If a second camera of a pipeline is acquired only\n> useCount_ is increased and nothing else is done.\n>\n> When releasing cameras PipelineHandler::release() should only unlock\n> the media devices when the last camera is released. But the old code\n> unlocked on every release().\n>\n> Fix PipelineHandler::release() to only release the media devices when\n> the last camera is released.\n>\n>\nYes, it makes sense to me that we should avoid other processes accessing\nthe media devices until the current process releases all acquires/usages.\n\nThanks for the fix!\n\nReviewed-by: Harvey Yang \n\nBR,\nHarvey\n\nSigned-off-by: Hans de Goede \n> ---\n> src/libcamera/pipeline_handler.cpp | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/libcamera/pipeline_handler.cpp\n> b/src/libcamera/pipeline_handler.cpp\n> index 5a6de685..a20cd27d 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -205,7 +205,8 @@ void PipelineHandler::release(Camera *camera)\n>\n> ASSERT(useCount_);\n>\n> - unlockMediaDevices();\n> + if (useCount_ == 1)\n> + unlockMediaDevices();\n>\n> releaseDevice(camera);\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 A1DC6C323E\n\tfor ;\n\tMon, 19 Aug 2024 13:51:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 80366633BE;\n\tMon, 19 Aug 2024 15:51:24 +0200 (CEST)","from mail-lj1-x232.google.com (mail-lj1-x232.google.com\n\t[IPv6:2a00:1450:4864:20::232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5DA62633B3\n\tfor ;\n\tMon, 19 Aug 2024 15:51:22 +0200 (CEST)","by mail-lj1-x232.google.com with SMTP id\n\t38308e7fff4ca-2ef27bfd15bso48494981fa.2\n\tfor ;\n\tMon, 19 Aug 2024 06:51:22 -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=\"WU5OWL65\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1724075481; x=1724680281;\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=id0BvUXBvLtHpAr3m7MlButgFoMzv+x1kvV9PJcQxA0=;\n\tb=WU5OWL65i0VGUdOifzd/56o6ojwe8lTnJcovp/wTcFeNPikX5EB2jQZfjU0bSLQPur\n\tUYjQlO6Z32rK/0SM7hKxkGHncToo4CThYhFGq6ByuuYwLDP9EonY7Ujrgo5TzQq6I0nB\n\tue/Z8NtAio/xqP02S07lcE7IE33da2uHbC3nk=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1724075481; x=1724680281;\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=id0BvUXBvLtHpAr3m7MlButgFoMzv+x1kvV9PJcQxA0=;\n\tb=r1/SXXD+/XNF4yEqNVdwNGTENQIOC0BMdCGV6bwNjbMLV/IY37YoNeEO+PId6oJd09\n\tTakvZ/GyjBQdoBGgZPsNk6GEpo35SORL9A2UMCWGoqHBlzFXhCnZ0J0QbwiKjgi/eNRi\n\td7GPLvcM4DBZVVmnPoiKHzxK/Z3Ohdm5yYP7RyFjcKfhYb9VWs30CumdL3BB1+IRYNh2\n\tQISVZpMB1cMJwQMY1fSu8NIojBUGQ9E++p5Po79537xZepjJBf84rkJfSaKKhr5UWuVZ\n\tr6qObFV26TPTPnvZHUZeFrG3AM8nXz0bwXx2NYJ1OV2QdCa9FlC8S1nk+jRkldsQKqED\n\tTMIw==","X-Gm-Message-State":"AOJu0Ywa9yxlv03U5pXzxwXXChilwfrhcE21oMjJVjIDQTATEaIVfbZ5\n\tfzLdexVq8cJi2F78L6lOHp/VIZLYSqDU8knGPwaz9AxkUr8DFpeKoTIDLmlP8x5b1sgk01CbGgJ\n\tj9moeYaTaqgmhi6IwJhXIZUYiXsuzeXvvPXnO","X-Google-Smtp-Source":"AGHT+IHMJzBaY8JpW7MCu/5C8UUNe0/Nr0/5xRGOvjYDaBQea1qG3pTJiIc1TXAs+f1Yb6dmo9/fH8KSLzsh8bRRujA=","X-Received":"by 2002:a2e:a544:0:b0:2ef:2b44:9977 with SMTP id\n\t38308e7fff4ca-2f3be585d2fmr76190521fa.18.1724075481351;\n\tMon, 19 Aug 2024 06:51:21 -0700 (PDT)","MIME-Version":"1.0","References":"<20240818154324.71536-1-hdegoede@redhat.com>","In-Reply-To":"<20240818154324.71536-1-hdegoede@redhat.com>","From":"Cheng-Hao Yang ","Date":"Mon, 19 Aug 2024 15:51:10 +0200","Message-ID":"","Subject":"Re: [PATCH] pipeline_handler: Fix unlocking media devices too early\n\twhen there are multiple cameras","To":"Hans de Goede ","Cc":"libcamera-devel@lists.libcamera.org, Milan Zamazal ,\n\tMaxime Ripard ","Content-Type":"multipart/alternative; boundary=\"000000000000a1dc6c0620099a4a\"","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":30891,"web_url":"https://patchwork.libcamera.org/comment/30891/","msgid":"<20240825000555.GD24650@pendragon.ideasonboard.com>","date":"2024-08-25T00:05:55","subject":"Re: [PATCH] pipeline_handler: Fix unlocking media devices too early\n\twhen there are multiple cameras","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 Sun, Aug 18, 2024 at 05:43:24PM +0200, Hans de Goede wrote:\n> PipelineHandler::acquire() only locks the media devices when the first\n> camera is acquired. If a second camera of a pipeline is acquired only\n> useCount_ is increased and nothing else is done.\n> \n> When releasing cameras PipelineHandler::release() should only unlock\n> the media devices when the last camera is released. But the old code\n> unlocked on every release().\n> \n> Fix PipelineHandler::release() to only release the media devices when\n> the last camera is released.\n> \n> Signed-off-by: Hans de Goede \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/libcamera/pipeline_handler.cpp | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 5a6de685..a20cd27d 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -205,7 +205,8 @@ void PipelineHandler::release(Camera *camera)\n> \n> \tASSERT(useCount_);\n> \n> -\tunlockMediaDevices();\n> +\tif (useCount_ == 1)\n> +\t\tunlockMediaDevices();\n> \n> \treleaseDevice(camera);\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 D3168BDE17\n\tfor ;\n\tSun, 25 Aug 2024 00:06:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3D5CB633CF;\n\tSun, 25 Aug 2024 02:06:01 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E95B763393\n\tfor ;\n\tSun, 25 Aug 2024 02:05:58 +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 7B783480;\n\tSun, 25 Aug 2024 02:04:53 +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=\"DMWOSnXt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1724544293;\n\tbh=99d0qYiNbIjeOHN0zQ2MUmNlJVvfbNaZjc4Wa9dKZ1A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=DMWOSnXtdFKZDzSPOrvF48cxJ0tIiEepppH5IEaoHBOf6/H3VCIVAYyDtQzRunpu3\n\tIjIwbVL7bRJ1ALtHEr5CdGXERO5Oib9xBY/78VOBYLMYpRhnXSdYzzQn2aDnBL8w6W\n\tQza2jhh8FcaIPtjyRcpv3mAPrvnVEqJQQhk3WV1s=","Date":"Sun, 25 Aug 2024 03:05:55 +0300","From":"Laurent Pinchart ","To":"Hans de Goede ","Cc":"libcamera-devel@lists.libcamera.org, Milan Zamazal ,\n\tMaxime Ripard ","Subject":"Re: [PATCH] pipeline_handler: Fix unlocking media devices too early\n\twhen there are multiple cameras","Message-ID":"<20240825000555.GD24650@pendragon.ideasonboard.com>","References":"<20240818154324.71536-1-hdegoede@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240818154324.71536-1-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\" "}}]