[{"id":25805,"web_url":"https://patchwork.libcamera.org/comment/25805/","msgid":"","date":"2022-11-16T07:42:12","subject":"Re: [libcamera-devel] [PATCH 0/2] Support resource acquisition at\n\t'acquire()'","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Wed, Nov 16, 2022 at 12:17:22AM +0000, Kieran Bingham via libcamera-devel wrote:\n> Pipeline handlers may benefit from being able to postpone resource\n> acquisition until the acquire() operation, or even release any temporary\n> resources used during initialisation and only use them when acquired.\n> \n> This series hopes to balance the updates to the\n> PipelineHandler->release(Camera) call, with a corresponding\n> PipelineHandler->acquire(camera) and facilitate pipeline handlers\n> managing resources only when the camera is in use.\n> \n> While the pipeline patch could make some progress, the uvcvideo patch in\n> this series is not working. I fear this may be a symptom of a bug in\n> either our thread or fdNotifier / event mechanisms, but haven't been\n> able to track it down yet.\n\nDid you read my reply to the patch that introduces releaseDevice() ? One\nof my concerns is that it ties resource lifetime to ownership lifetime,\nwhich makes it impossible to keep a device acquired for exclusive access\nwithout also holding onto resources.\n\nCould you please expand on the design rationale for this series ? The\nrationale should also be reflected in high-level documentation.\n\n> Kieran Bingham (2):\n> libcamera: pipeline: Add an acquireDevice method\n> [DNI/RFC] pipeline: uvcvideo: Only open devices upon acquire\n> \n> include/libcamera/internal/pipeline_handler.h | 3 +-\n> src/libcamera/camera.cpp | 2 +-\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 55 +++++++++++++++----\n> src/libcamera/pipeline_handler.cpp | 23 +++++++-\n\nMissing update to the pipeline handler writer's guide (releaseDevice()\nalso needs to be documented).\n\n> 4 files changed, 69 insertions(+), 14 deletions(-)","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 1750FBE08B\n\tfor ;\n\tWed, 16 Nov 2022 07:42:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6955463087;\n\tWed, 16 Nov 2022 08:42:30 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2F7E461F31\n\tfor ;\n\tWed, 16 Nov 2022 08:42:28 +0100 (CET)","from pendragon.ideasonboard.com\n\t(host-62-245-140-144.customer.m-online.net [62.245.140.144])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9ADA8891;\n\tWed, 16 Nov 2022 08:42:27 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1668584550;\n\tbh=Ju4HC0Am//vA1ofKgWb5KgmuRxGB47U0QWdtt0GGFnk=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=YvJSPT1RmxWYex4y3byt9D72NWfUiQGgxThy3lN9jcITFqAIIGXhqp59KaV+SAiO6\n\trydveZZEkAjWbLh9fG4dPnaSB4/HuALOt647JRJaDjKwfumwt/qb3qN7SS5RRRKXzl\n\tV8vqOzsYVfP7ftQpt/gMxDQDMYaJ2qN2/R49E7XJ64+mHQ3EFgE6mrw6mouWJKxKy9\n\tMO5L1TFL++6FL2qfFylqQDQYndpbfYSCDy/8ZHrgCDo65y7C0B3Rl6nLg1axHigaRB\n\thux00bquO2XxAcawSshbb68Mv/3CC0X0BdPDbTbxHOCueD2xkjeMhwVUAZWQWKf8QM\n\t214deN1I2bIBQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1668584547;\n\tbh=Ju4HC0Am//vA1ofKgWb5KgmuRxGB47U0QWdtt0GGFnk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=p0gen/QQBNpkIlnTjmg6kyAr0MeE6VlElE8GIOLNOVEMZRYiQjNyuKmMJjP41DAAq\n\tVei/eeqKBhXkNLY9+Gnew5UfunNknpAEVFKTb3cW+hlZfPinj352nwSmYvjK8/rc1y\n\tzgHP2naIOQyX5dqT0z54v/3OTrH96qiJTnE1sxoQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"p0gen/QQ\"; dkim-atps=neutral","Date":"Wed, 16 Nov 2022 09:42:12 +0200","To":"Kieran Bingham ","Message-ID":"","References":"<20221116001724.3938045-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221116001724.3938045-1-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 0/2] Support resource acquisition at\n\t'acquire()'","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25807,"web_url":"https://patchwork.libcamera.org/comment/25807/","msgid":"<166860441956.50677.11615289944039464951@Monstersaurus>","date":"2022-11-16T13:13:39","subject":"Re: [libcamera-devel] [PATCH 0/2] Support resource acquisition at\n\t'acquire()'","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2022-11-16 07:42:12)\n> Hi Kieran,\n> \n> On Wed, Nov 16, 2022 at 12:17:22AM +0000, Kieran Bingham via libcamera-devel wrote:\n> > Pipeline handlers may benefit from being able to postpone resource\n> > acquisition until the acquire() operation, or even release any temporary\n> > resources used during initialisation and only use them when acquired.\n> > \n> > This series hopes to balance the updates to the\n> > PipelineHandler->release(Camera) call, with a corresponding\n> > PipelineHandler->acquire(camera) and facilitate pipeline handlers\n> > managing resources only when the camera is in use.\n> > \n> > While the pipeline patch could make some progress, the uvcvideo patch in\n> > this series is not working. I fear this may be a symptom of a bug in\n> > either our thread or fdNotifier / event mechanisms, but haven't been\n> > able to track it down yet.\n> \n> Did you read my reply to the patch that introduces releaseDevice() ? One\n> of my concerns is that it ties resource lifetime to ownership lifetime,\n> which makes it impossible to keep a device acquired for exclusive access\n> without also holding onto resources.\n\nIsn't it the opposite right now though? We currently have\nimplementations (uvcvideo) where the resources are held at construction\nof the Camera class, and *kept open*.\n\nThis proposal is to reduce that resource acquisition until an\napplication actually states it indeeds to acquire the camera.\n\nSee Bug #168: https://bugs.libcamera.org/show_bug.cgi?id=168\n\nWhat do you see is different between an application acquiring a camera\nexclusively by keeping it acquired, with resources obtained at that\npoint, and the current situation where the camera resources are\nconsumed, even though the camera is not acquired?\n\nPerhaps would you rather see a clear resource acquisition only at ...\nconfigure time? We would then have the unbalance that we don't have an\nunconfigure() call to explicitly release those resources, which is why\nthis is expected to match and be symetrical with the patch provided by\nDavid.\n\n--\nKieran\n\n\n> Could you please expand on the design rationale for this series ? The\n> rationale should also be reflected in high-level documentation.\n\nSure - As I said in the replies to Davids series, this initial posting\nwas really to ask for help on the DNI/RFC patch ... which I suspect /\nfear highlights a potential threading/event bug (or me just doing\nsomething wrong).\n\n\n\n> \n> > Kieran Bingham (2):\n> > libcamera: pipeline: Add an acquireDevice method\n> > [DNI/RFC] pipeline: uvcvideo: Only open devices upon acquire\n> > \n> > include/libcamera/internal/pipeline_handler.h | 3 +-\n> > src/libcamera/camera.cpp | 2 +-\n> > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 55 +++++++++++++++----\n> > src/libcamera/pipeline_handler.cpp | 23 +++++++-\n> \n> Missing update to the pipeline handler writer's guide (releaseDevice()\n> also needs to be documented).\n> \n> > 4 files changed, 69 insertions(+), 14 deletions(-)\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 8C7A0BE08B\n\tfor ;\n\tWed, 16 Nov 2022 13:13:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ED2ED61F3B;\n\tWed, 16 Nov 2022 14:13:43 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9005861F32\n\tfor ;\n\tWed, 16 Nov 2022 14:13:42 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E3FE149C;\n\tWed, 16 Nov 2022 14:13:41 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1668604424;\n\tbh=nHDpS6XKHKM6sdBgN0VwrSH95cCSsKUrvHmZJ2/k7+Q=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=lG1AxkFp+R4ZRMpZl3UcgOkjYOKGHl3Ngm49e5YY7KeMO4IvalpVg9kHR1X9u/30w\n\t1OH+TONA6oYzygjVj9af6uhReSl5zDYPO/30BVQCevM3bOcZCpQzwwUtFNJPf3zkde\n\t2mIaFGqmeUOypPEjF+aYVpDe4+V6NQaWlfbD8Lq35U8KoBv9QR+vLHZHk1SOZcDIfQ\n\t6mH4Wr3cX1ahLoJe5mUEw701RpmS/pXprkxwDyeYI2wQfvlKVXF6CgY+AjZcewV7De\n\tLh50EQ1dc+smauRZ2Cf/BcSPK5Yk50T2yQ6Ai2ynVtyrSiuL/ZYquSCQ5JaNW8a6DA\n\tjVJaBv5dCI57Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1668604422;\n\tbh=nHDpS6XKHKM6sdBgN0VwrSH95cCSsKUrvHmZJ2/k7+Q=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=Zo+uzO6bDwj6Hnhw7xwaW9guBeKRfWJLx2bFiFNpKOUM2squ8Q0w2mU5iKN8nFLdJ\n\txYv3xLRusc0Em3zXnYL0eb5/K2scga7SZWlYmO6iSCZi91iyDUqN9eFv6sTI0KKxKd\n\tHZ2uc1HEy/DchNK0aFyNpbMinP5G6EfVnB9ZIl6s="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Zo+uzO6b\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20221116001724.3938045-1-kieran.bingham@ideasonboard.com>\n\t","To":"Laurent Pinchart ","Date":"Wed, 16 Nov 2022 13:13:39 +0000","Message-ID":"<166860441956.50677.11615289944039464951@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH 0/2] Support resource acquisition at\n\t'acquire()'","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]