[{"id":23210,"web_url":"https://patchwork.libcamera.org/comment/23210/","msgid":"","date":"2022-05-29T10:05:05","subject":"Re: [libcamera-devel] [PATCH v2 2/5] android: camera_stream: Create\n\tallocator unconditionally","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Jacopo/Paul,\n\nOn 5/27/22 11:34, Paul Elder via libcamera-devel wrote:\n> From: Jacopo Mondi \n>\n> With the introduction of PlatformBufferAllocator all CameraStream can\n> be used to allocate buffers on-demand.\n>\n> Create CameraStream::allocator_ and the associated mutex for all types\n> of stream.\n>\n> Signed-off-by: Jacopo Mondi \n\n\nLooks good!\n\n>\n> ---\n> No change in v2\n>\n> If we want to make CameraStream::mutex_ into non-pointer (as Hiro\n> suggested), it should be done on top. Do we want to do that? (I suppose\n> it doesn't affect this patch itself)\n\n\nYes, I guess would be best on top, explaining the benefits/tradeoffs\n\nReviewed-by: Umang Jain \n\n> ---\n> src/android/camera_stream.cpp | 6 ++----\n> 1 file changed, 2 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> index 154e088e..045e6006 100644\n> --- a/src/android/camera_stream.cpp\n> +++ b/src/android/camera_stream.cpp\n> @@ -128,10 +128,8 @@ int CameraStream::configure()\n> \t\tworker_->start();\n> \t}\n> \n> -\tif (type_ == Type::Internal) {\n> -\t\tallocator_ = std::make_unique(cameraDevice_);\n> -\t\tmutex_ = std::make_unique();\n> -\t}\n> +\tallocator_ = std::make_unique(cameraDevice_);\n> +\tmutex_ = std::make_unique();\n> \n> \tcamera3Stream_->max_buffers = configuration().bufferCount;\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 9072BBD161\n\tfor ;\n\tSun, 29 May 2022 10:05:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B068B65633;\n\tSun, 29 May 2022 12:05:10 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F39CF60415\n\tfor ;\n\tSun, 29 May 2022 12:05:08 +0200 (CEST)","from [192.168.1.68] (235.red-81-36-186.dynamicip.rima-tde.net\n\t[81.36.186.235])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 532146DC;\n\tSun, 29 May 2022 12:05:08 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1653818710;\n\tbh=xUVPKV+2+z+M5tLWChYHAcQu8uOFM2XgBpMXieNhdTo=;\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:\n\tFrom;\n\tb=VjEats8YAEX6y4iNm9uRY8zz6wHeVlD95QFS0Myu/CRVPnwm10hCxgQnven/h/7Sf\n\te97zh+V9oeiJrpT1C+AEMSYm/CVxtEB+PWEHWsljBjNyOtO7GarMBOJmTEAJWe7vXj\n\tIfVtWRSjp6zAIb9A9Fen6X/PmzzBreNE0w5aR5G9QkJv7Jv6/UjRuKRLN65swUJMXl\n\tNaWfvNz/xewMDFwImRo7ml4ZKOu8i6c11KeJcxx1gaF+IRuYdfSi/Mo4gxkpuOgldJ\n\tk1RUYGX8agnoG2uwY5MsP5F2X+LUc3O/8mIt/D1l3/5qdu967a0CJ3t2kNtleJaUp0\n\tQHrXPRDBqavRQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1653818708;\n\tbh=xUVPKV+2+z+M5tLWChYHAcQu8uOFM2XgBpMXieNhdTo=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=vbN/2xuWRgXF30GTYyhoFQ77FJQpqFG1WbTZwKC0Qigs89IAqxOMfLUuPJT171CzZ\n\tEpCr7k918+r+7L8a6/SXP/nQvExTB5rG6GKQNdt1leFgZC/0DOWy37u69iZ8qyUqVE\n\tp/IcWjbrGwp+rb9BL/XxZfYv2MyluL1Le5CZV99o="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"vbN/2xuW\"; dkim-atps=neutral","Message-ID":"","Date":"Sun, 29 May 2022 12:05:05 +0200","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220527093440.953377-1-paul.elder@ideasonboard.com>\n\t<20220527093440.953377-3-paul.elder@ideasonboard.com>","In-Reply-To":"<20220527093440.953377-3-paul.elder@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2 2/5] android: camera_stream: Create\n\tallocator unconditionally","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":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23242,"web_url":"https://patchwork.libcamera.org/comment/23242/","msgid":"<20220530095346.kxhy5dulun3v3s2b@uno.localdomain>","date":"2022-05-30T09:53:46","subject":"Re: [libcamera-devel] [PATCH v2 2/5] android: camera_stream: Create\n\tallocator unconditionally","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n\nOn Fri, May 27, 2022 at 06:34:37PM +0900, Paul Elder wrote:\n> From: Jacopo Mondi \n>\n> With the introduction of PlatformBufferAllocator all CameraStream can\n> be used to allocate buffers on-demand.\n>\n> Create CameraStream::allocator_ and the associated mutex for all types\n> of stream.\n>\n> Signed-off-by: Jacopo Mondi \n>\n> ---\n> No change in v2\n>\n> If we want to make CameraStream::mutex_ into non-pointer (as Hiro\n> suggested), it should be done on top. Do we want to do that? (I suppose\n> it doesn't affect this patch itself)\n\nHere or on top seems equivalent to me.\n\n> ---\n> src/android/camera_stream.cpp | 6 ++----\n> 1 file changed, 2 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> index 154e088e..045e6006 100644\n> --- a/src/android/camera_stream.cpp\n> +++ b/src/android/camera_stream.cpp\n> @@ -128,10 +128,8 @@ int CameraStream::configure()\n> \t\tworker_->start();\n> \t}\n>\n> -\tif (type_ == Type::Internal) {\n> -\t\tallocator_ = std::make_unique(cameraDevice_);\n> -\t\tmutex_ = std::make_unique();\n> -\t}\n> +\tallocator_ = std::make_unique(cameraDevice_);\n> +\tmutex_ = std::make_unique();\n>\n> \tcamera3Stream_->max_buffers = configuration().bufferCount;\n>\n> --\n> 2.30.2\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 20D78BD161\n\tfor ;\n\tMon, 30 May 2022 09:53:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3D10B65633;\n\tMon, 30 May 2022 11:53:50 +0200 (CEST)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A1DA46040B\n\tfor ;\n\tMon, 30 May 2022 11:53:48 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 32B1824000C;\n\tMon, 30 May 2022 09:53:47 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1653904430;\n\tbh=I1oEP+4YxtTmAVWEbtnRUtL00nzWApLMVL/hf6cEMug=;\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=GleyJFFwX1tZkaTDbwUrbBH4WQK9BmJILsB779jK//BJXxQLnkDYOImFNpk8xT1uU\n\tM9blaEhNA03q5FlTXHSMRcpFbjBik82sR8zH8K/RL+aclAJ8fuMcURtFqVFWdd//Oc\n\tfQooItl1358E/04MtLLORvti4N84mTlwJHXzEFWaNoN9/hS+RmoTzcmOdr5WytwvDG\n\tAxv1i/SsTe8tmOBw7u/1vp5o/kOwV1vwpnKiRO05erBYCz+MTa3FfVEXz2hABaoInK\n\tLQCMjEavXBV2QiO8p7ITo97HUZKgttvn0mwxkbr/fjjWg/XikgyV+5mZ/UyXY2J7yQ\n\tEgsbL3REWAC7Q==","Date":"Mon, 30 May 2022 11:53:46 +0200","To":"Paul Elder ","Message-ID":"<20220530095346.kxhy5dulun3v3s2b@uno.localdomain>","References":"<20220527093440.953377-1-paul.elder@ideasonboard.com>\n\t<20220527093440.953377-3-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220527093440.953377-3-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 2/5] android: camera_stream: Create\n\tallocator unconditionally","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":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23287,"web_url":"https://patchwork.libcamera.org/comment/23287/","msgid":"","date":"2022-06-02T07:56:36","subject":"Re: [libcamera-devel] [PATCH v2 2/5] android: camera_stream: Create\n\tallocator unconditionally","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul and Jacopo,\n\nThank you for the patch.\n\nOn Fri, May 27, 2022 at 06:34:37PM +0900, Paul Elder via libcamera-devel wrote:\n> From: Jacopo Mondi \n> \n> With the introduction of PlatformBufferAllocator all CameraStream can\n> be used to allocate buffers on-demand.\n\nIt's been a while, so I don't recall how the two are related. It would\nbe nice to extend the commit message to explain this better.\n\n> Create CameraStream::allocator_ and the associated mutex for all types\n> of stream.\n> \n> Signed-off-by: Jacopo Mondi \n> \n> ---\n> No change in v2\n> \n> If we want to make CameraStream::mutex_ into non-pointer (as Hiro\n> suggested), it should be done on top. Do we want to do that? (I suppose\n> it doesn't affect this patch itself)\n\nIt would be nice to do so on top indeed.\n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/android/camera_stream.cpp | 6 ++----\n> 1 file changed, 2 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> index 154e088e..045e6006 100644\n> --- a/src/android/camera_stream.cpp\n> +++ b/src/android/camera_stream.cpp\n> @@ -128,10 +128,8 @@ int CameraStream::configure()\n> \t\tworker_->start();\n> \t}\n> \n> -\tif (type_ == Type::Internal) {\n> -\t\tallocator_ = std::make_unique(cameraDevice_);\n> -\t\tmutex_ = std::make_unique();\n> -\t}\n> +\tallocator_ = std::make_unique(cameraDevice_);\n> +\tmutex_ = std::make_unique();\n> \n> \tcamera3Stream_->max_buffers = configuration().bufferCount;\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 E3CE7BD160\n\tfor ;\n\tThu, 2 Jun 2022 07:56:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3DAAB6563A;\n\tThu, 2 Jun 2022 09:56:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 66DE965633\n\tfor ;\n\tThu, 2 Jun 2022 09:56:41 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(lmontsouris-659-1-41-236.w92-154.abo.wanadoo.fr [92.154.76.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E44186BD;\n\tThu, 2 Jun 2022 09:56:40 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1654156602;\n\tbh=08kyPkVAKZF3YLdat3ZixX9n4laJKnMZE5MzgaRbZac=;\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=w+KVLiyANq8/sf9ZlFZkWqnzTEp7xl9a6yUEInr/yCJN7dlGSs7exxVFM3AUrtuaz\n\t8EqhkaqQGogBp8nB7Z8vZlcyHwjjkByWnQF4iWKHnFUcXd38CxUN5hkYb2EFD5PMO4\n\tHcr4eJOsLk9tbCNAB4BKkGl+XrLFiAlYRJpPfbgc/m+Y1qj9pk2iVirSnJZjxudaJG\n\tCwj1ctFm6pvDt2q9Jc5WSulVtUyTuLAzHg1YebU7ZGM5pUFnhaIPSI7qgNq+QHCb5m\n\twFnS4Gnbmy6j5kmJQZ8apZSuDesE5KEn9TSp7UebF61LY07KDAJTc5UtMH9iAK7f7I\n\tFgIR3CDLMCfzw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1654156601;\n\tbh=08kyPkVAKZF3YLdat3ZixX9n4laJKnMZE5MzgaRbZac=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=a61HIHc2RS50dScUHEGSAHotC2Se1tlsvA/YHOBHTfgJX4FjJBOgCt04+KtIaPuBY\n\t+52AGVSD8ytW5Qur709HASSISLXRlUyqfNkfpIb5eNjezEZInb3qVih24WZDqaznkA\n\tgR8AFY19oe6Bsg0mM/Nqs7OvUU9wjlgrT4S5EESQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"a61HIHc2\"; dkim-atps=neutral","Date":"Thu, 2 Jun 2022 10:56:36 +0300","To":"Paul Elder ","Message-ID":"","References":"<20220527093440.953377-1-paul.elder@ideasonboard.com>\n\t<20220527093440.953377-3-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220527093440.953377-3-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 2/5] android: camera_stream: Create\n\tallocator unconditionally","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@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]