[{"id":12670,"web_url":"https://patchwork.libcamera.org/comment/12670/","msgid":"<72ee78e7-fba7-b2a6-29c3-a270897ae979@ideasonboard.com>","date":"2020-09-23T12:09:29","subject":"Re: [libcamera-devel] [PATCH v3 4/8] android: camera_device: Clear\n\tallocator at configureStream","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 22/09/2020 10:47, Jacopo Mondi wrote:\n> The configureStream operation might be called by the Android framework\n\ns/might/could/\nNot sure why, it just sounds better in my internal grammar parser.\n\n> in two successive capture session without going through a close().\n\ns/session/sessions/\n\n> \n> Clear all the allocated buffers before configuring the camera streams.\n> \n> Reviewed-by: Niklas Söderlund \n> Reviewed-by: Hirokazu Honda \n\nReviewed-by: Kieran Bingham \n\n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_device.cpp | 7 ++++---\n> 1 file changed, 4 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 98b8159ebd8c..42fb9ea4e113 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1182,12 +1182,13 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t}\n> \n> \t/*\n> -\t * Clear and remove any existing configuration from previous calls, and\n> -\t * ensure the required entries are available without further\n> -\t * reallocation.\n> +\t * Clear and remove any existing configuration and memory allocated from\n> +\t * previous calls, and ensure the required entries are available without\n> +\t * further reallocation.\n> \t */\n> \tstreams_.clear();\n> \tstreams_.reserve(stream_list->num_streams);\n> +\tallocator_.clear();\n\n> \n> \t/* First handle all non-MJPEG streams. */\n> \tcamera3_stream_t *jpegStream = nullptr;\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 BD96FC3B5C\n\tfor ;\n\tWed, 23 Sep 2020 12:09:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4E5B062FDE;\n\tWed, 23 Sep 2020 14:09:36 +0200 (CEST)","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 99D0860576\n\tfor ;\n\tWed, 23 Sep 2020 14:09:34 +0200 (CEST)","from [192.168.0.20]\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 3B6302FD;\n\tWed, 23 Sep 2020 14:09:32 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"EHS2BZyK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1600862972;\n\tbh=tKbtvlPN3/wcEr/ruPVVlfACCvFQ3WkO3Fz+zGH5/Cg=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=EHS2BZyKm8u/mbakvdObA7EIkwXifjHYO8hDT7jQMuZmCNgO3iC+BEWEaBq6Pg81G\n\toHJJ4XT9UPq1qENTruUieuuT95v0T0/chZ9HRcMTxQy6WWYIav56Nr/R3Id+JxlsHk\n\tIVB/PyY9zUb4wzM6hgkz8SYMLqMRYg/O1rou3fSo=","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","References":"<20200922094738.5327-1-jacopo@jmondi.org>\n\t<20200922094738.5327-5-jacopo@jmondi.org>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<72ee78e7-fba7-b2a6-29c3-a270897ae979@ideasonboard.com>","Date":"Wed, 23 Sep 2020 13:09:29 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200922094738.5327-5-jacopo@jmondi.org>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v3 4/8] android: camera_device: Clear\n\tallocator at configureStream","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"hanlinchen@chromium.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12850,"web_url":"https://patchwork.libcamera.org/comment/12850/","msgid":"<20200929015131.GN14614@pendragon.ideasonboard.com>","date":"2020-09-29T01:51:31","subject":"Re: [libcamera-devel] [PATCH v3 4/8] android: camera_device: Clear\n\tallocator at configureStream","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Tue, Sep 22, 2020 at 11:47:34AM +0200, Jacopo Mondi wrote:\n> The configureStream operation might be called by the Android framework\n> in two successive capture session without going through a close().\n\nWith s/session/sessions/,\n\nReviewed-by: Laurent Pinchart \n\n> Clear all the allocated buffers before configuring the camera streams.\n> \n> Reviewed-by: Niklas Söderlund \n> Reviewed-by: Hirokazu Honda \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_device.cpp | 7 ++++---\n> 1 file changed, 4 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 98b8159ebd8c..42fb9ea4e113 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1182,12 +1182,13 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t}\n> \n> \t/*\n> -\t * Clear and remove any existing configuration from previous calls, and\n> -\t * ensure the required entries are available without further\n> -\t * reallocation.\n> +\t * Clear and remove any existing configuration and memory allocated from\n> +\t * previous calls, and ensure the required entries are available without\n> +\t * further reallocation.\n> \t */\n> \tstreams_.clear();\n> \tstreams_.reserve(stream_list->num_streams);\n> +\tallocator_.clear();\n> \n> \t/* First handle all non-MJPEG streams. */\n> \tcamera3_stream_t *jpegStream = nullptr;","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 560AFC3B5C\n\tfor ;\n\tTue, 29 Sep 2020 01:52:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E8A0561FE5;\n\tTue, 29 Sep 2020 03:52:07 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8FE5C60365\n\tfor ;\n\tTue, 29 Sep 2020 03:52:06 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EC190540;\n\tTue, 29 Sep 2020 03:52:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"L7lKKrHZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1601344326;\n\tbh=Jxds6KY2Tgf1NpXtgrMQpAjySLTS87yh+UokJKUN6kY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=L7lKKrHZrrjbRhEpHnw8k8wKkSmI7JcQZd+0GPYrjYkMbS3iAME8ecEms7oakuOHU\n\tvBl8+sQqCXUAGpoqStVONNF42Ume0BZeVqXBEdKxnrACp0XBQojlUt5y6Pvsyek/MK\n\tmrnv5tkWyn9mdpDgCdMKYPNwFMlfYa36lZszJzZo=","Date":"Tue, 29 Sep 2020 04:51:31 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200929015131.GN14614@pendragon.ideasonboard.com>","References":"<20200922094738.5327-1-jacopo@jmondi.org>\n\t<20200922094738.5327-5-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200922094738.5327-5-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v3 4/8] android: camera_device: Clear\n\tallocator at configureStream","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","Cc":"hanlinchen@chromium.org, libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12851,"web_url":"https://patchwork.libcamera.org/comment/12851/","msgid":"<20200929015331.GO14614@pendragon.ideasonboard.com>","date":"2020-09-29T01:53:31","subject":"Re: [libcamera-devel] [PATCH v3 4/8] android: camera_device: Clear\n\tallocator at configureStream","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Sep 29, 2020 at 04:51:32AM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n> \n> Thank you for the patch.\n> \n> On Tue, Sep 22, 2020 at 11:47:34AM +0200, Jacopo Mondi wrote:\n> > The configureStream operation might be called by the Android framework\n> > in two successive capture session without going through a close().\n> \n> With s/session/sessions/,\n> \n> Reviewed-by: Laurent Pinchart \n\nNot necessarily for this patch, but shouldn't we also free buffers (and\nprobably clear streams_ too) in CameraDevice::close() ?\n\n> > Clear all the allocated buffers before configuring the camera streams.\n> > \n> > Reviewed-by: Niklas Söderlund \n> > Reviewed-by: Hirokazu Honda \n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/android/camera_device.cpp | 7 ++++---\n> > 1 file changed, 4 insertions(+), 3 deletions(-)\n> > \n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 98b8159ebd8c..42fb9ea4e113 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -1182,12 +1182,13 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > \t}\n> > \n> > \t/*\n> > -\t * Clear and remove any existing configuration from previous calls, and\n> > -\t * ensure the required entries are available without further\n> > -\t * reallocation.\n> > +\t * Clear and remove any existing configuration and memory allocated from\n> > +\t * previous calls, and ensure the required entries are available without\n> > +\t * further reallocation.\n> > \t */\n> > \tstreams_.clear();\n> > \tstreams_.reserve(stream_list->num_streams);\n> > +\tallocator_.clear();\n> > \n> > \t/* First handle all non-MJPEG streams. */\n> > \tcamera3_stream_t *jpegStream = nullptr;","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 0E576C3B5B\n\tfor ;\n\tTue, 29 Sep 2020 01:54:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8711B62048;\n\tTue, 29 Sep 2020 03:54:07 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 884FF60365\n\tfor ;\n\tTue, 29 Sep 2020 03:54:06 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 13497540;\n\tTue, 29 Sep 2020 03:54:06 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"bNsuK/EZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1601344446;\n\tbh=A6Vayo3paeQRhVb89pc/vpvIXzLRTm90v8tSkMjPsjY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bNsuK/EZ8GPICgvVrQ5xaJltTOPKJMLxuDOoyz78+I8xynACmrFDnULs6Ap1nAxqO\n\twhj6kSy2uMmfhzM08HVPFVlDhjwC4rKNBe6mseq2z7rko4xQnO2JvmCOUTchh05n/v\n\tvnbj3FGKjJksbD+w5LQcpQ1vmhQd06Ng65Kdh5LA=","Date":"Tue, 29 Sep 2020 04:53:31 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200929015331.GO14614@pendragon.ideasonboard.com>","References":"<20200922094738.5327-1-jacopo@jmondi.org>\n\t<20200922094738.5327-5-jacopo@jmondi.org>\n\t<20200929015131.GN14614@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200929015131.GN14614@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 4/8] android: camera_device: Clear\n\tallocator at configureStream","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","Cc":"hanlinchen@chromium.org, libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12908,"web_url":"https://patchwork.libcamera.org/comment/12908/","msgid":"<20200930110453.ahq4nzss6vdk7yfa@uno.localdomain>","date":"2020-09-30T11:04:53","subject":"Re: [libcamera-devel] [PATCH v3 4/8] android: camera_device: Clear\n\tallocator at configureStream","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Tue, Sep 29, 2020 at 04:53:31AM +0300, Laurent Pinchart wrote:\n> On Tue, Sep 29, 2020 at 04:51:32AM +0300, Laurent Pinchart wrote:\n> > Hi Jacopo,\n> >\n> > Thank you for the patch.\n> >\n> > On Tue, Sep 22, 2020 at 11:47:34AM +0200, Jacopo Mondi wrote:\n> > > The configureStream operation might be called by the Android framework\n> > > in two successive capture session without going through a close().\n> >\n> > With s/session/sessions/,\n> >\n> > Reviewed-by: Laurent Pinchart \n>\n> Not necessarily for this patch, but shouldn't we also free buffers (and\n> probably clear streams_ too) in CameraDevice::close() ?\n\nI guess it doesn't hurt, indeed. When the camera is closed it's\nindeed relevant to release all the allocated buffers, even if any new\nallocation happens at configureStream() time, where we already call\nfreeAll().\n\nRegarding streams_, being that internal memory released at\nconfigureStream() time before any other CameraStream is created, I\nthink it's less relevant... I can make a separate patch to do so on\ntop of this one.\n\n\n>\n> > > Clear all the allocated buffers before configuring the camera streams.\n> > >\n> > > Reviewed-by: Niklas Söderlund \n> > > Reviewed-by: Hirokazu Honda \n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > src/android/camera_device.cpp | 7 ++++---\n> > > 1 file changed, 4 insertions(+), 3 deletions(-)\n> > >\n> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > index 98b8159ebd8c..42fb9ea4e113 100644\n> > > --- a/src/android/camera_device.cpp\n> > > +++ b/src/android/camera_device.cpp\n> > > @@ -1182,12 +1182,13 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > > \t}\n> > >\n> > > \t/*\n> > > -\t * Clear and remove any existing configuration from previous calls, and\n> > > -\t * ensure the required entries are available without further\n> > > -\t * reallocation.\n> > > +\t * Clear and remove any existing configuration and memory allocated from\n> > > +\t * previous calls, and ensure the required entries are available without\n> > > +\t * further reallocation.\n> > > \t */\n> > > \tstreams_.clear();\n> > > \tstreams_.reserve(stream_list->num_streams);\n> > > +\tallocator_.clear();\n> > >\n> > > \t/* First handle all non-MJPEG streams. */\n> > > \tcamera3_stream_t *jpegStream = nullptr;\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 58AE6C3B5C\n\tfor ;\n\tWed, 30 Sep 2020 11:00:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BABAF62211;\n\tWed, 30 Sep 2020 13:00:58 +0200 (CEST)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 59A2060BD4\n\tfor ;\n\tWed, 30 Sep 2020 13:00:57 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id B1EE7240011;\n\tWed, 30 Sep 2020 11:00:56 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 30 Sep 2020 13:04:53 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20200930110453.ahq4nzss6vdk7yfa@uno.localdomain>","References":"<20200922094738.5327-1-jacopo@jmondi.org>\n\t<20200922094738.5327-5-jacopo@jmondi.org>\n\t<20200929015131.GN14614@pendragon.ideasonboard.com>\n\t<20200929015331.GO14614@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200929015331.GO14614@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 4/8] android: camera_device: Clear\n\tallocator at configureStream","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","Cc":"hanlinchen@chromium.org, libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12926,"web_url":"https://patchwork.libcamera.org/comment/12926/","msgid":"<20201002020040.GT3722@pendragon.ideasonboard.com>","date":"2020-10-02T02:00:40","subject":"Re: [libcamera-devel] [PATCH v3 4/8] android: camera_device: Clear\n\tallocator at configureStream","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Sep 30, 2020 at 01:04:53PM +0200, Jacopo Mondi wrote:\n> On Tue, Sep 29, 2020 at 04:53:31AM +0300, Laurent Pinchart wrote:\n> > On Tue, Sep 29, 2020 at 04:51:32AM +0300, Laurent Pinchart wrote:\n> > > On Tue, Sep 22, 2020 at 11:47:34AM +0200, Jacopo Mondi wrote:\n> > > > The configureStream operation might be called by the Android framework\n> > > > in two successive capture session without going through a close().\n> > >\n> > > With s/session/sessions/,\n> > >\n> > > Reviewed-by: Laurent Pinchart \n> >\n> > Not necessarily for this patch, but shouldn't we also free buffers (and\n> > probably clear streams_ too) in CameraDevice::close() ?\n> \n> I guess it doesn't hurt, indeed. When the camera is closed it's\n> indeed relevant to release all the allocated buffers, even if any new\n> allocation happens at configureStream() time, where we already call\n> freeAll().\n\nIf the camera is closed the buffers will remain there until the camera\nis used again. It's not a memory leak as such as they will eventually be\nfreed, but we will hold on large pieces of memory that are completely\nunused. Not very nice :-)\n\n> Regarding streams_, being that internal memory released at\n> configureStream() time before any other CameraStream is created, I\n> think it's less relevant... I can make a separate patch to do so on\n> top of this one.\n\nstreams_ is less of an issue as it's a fairly small amount of memory,\nbut I think cleaning up properly at close() time would be a good idea.\n\nA separate patch is totally fine.\n\n> > > > Clear all the allocated buffers before configuring the camera streams.\n> > > >\n> > > > Reviewed-by: Niklas Söderlund \n> > > > Reviewed-by: Hirokazu Honda \n> > > > Signed-off-by: Jacopo Mondi \n> > > > ---\n> > > > src/android/camera_device.cpp | 7 ++++---\n> > > > 1 file changed, 4 insertions(+), 3 deletions(-)\n> > > >\n> > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > > index 98b8159ebd8c..42fb9ea4e113 100644\n> > > > --- a/src/android/camera_device.cpp\n> > > > +++ b/src/android/camera_device.cpp\n> > > > @@ -1182,12 +1182,13 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > > > \t}\n> > > >\n> > > > \t/*\n> > > > -\t * Clear and remove any existing configuration from previous calls, and\n> > > > -\t * ensure the required entries are available without further\n> > > > -\t * reallocation.\n> > > > +\t * Clear and remove any existing configuration and memory allocated from\n> > > > +\t * previous calls, and ensure the required entries are available without\n> > > > +\t * further reallocation.\n> > > > \t */\n> > > > \tstreams_.clear();\n> > > > \tstreams_.reserve(stream_list->num_streams);\n> > > > +\tallocator_.clear();\n> > > >\n> > > > \t/* First handle all non-MJPEG streams. */\n> > > > \tcamera3_stream_t *jpegStream = nullptr;","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 61BA8C3B5B\n\tfor ;\n\tFri, 2 Oct 2020 02:01:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F1DCC6229F;\n\tFri, 2 Oct 2020 04:01:19 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7060760BCD\n\tfor ;\n\tFri, 2 Oct 2020 04:01:18 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 95C863B;\n\tFri, 2 Oct 2020 04:01:17 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PNclv+hF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1601604077;\n\tbh=Ez2dEpIgQeFO4Nt8XQcoXdcxE+0jFUAV2FRb370Qnb0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PNclv+hFt6VvQQ9KJByl8cvQKPP7DfuSR0xEXZjagrXru8Zu+o4SawqXEMazOaAN+\n\tn66oU1lnePHzvhT8Y7TLrj5ZKXZMWkHJwWqG+BXMPnLFBUL6Nl3yuMH2+FmwQU9g65\n\tqcTNPA2WaoUOQBsLk6SKZPKV7ppAJysm/u+IcY8U=","Date":"Fri, 2 Oct 2020 05:00:40 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20201002020040.GT3722@pendragon.ideasonboard.com>","References":"<20200922094738.5327-1-jacopo@jmondi.org>\n\t<20200922094738.5327-5-jacopo@jmondi.org>\n\t<20200929015131.GN14614@pendragon.ideasonboard.com>\n\t<20200929015331.GO14614@pendragon.ideasonboard.com>\n\t<20200930110453.ahq4nzss6vdk7yfa@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200930110453.ahq4nzss6vdk7yfa@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v3 4/8] android: camera_device: Clear\n\tallocator at configureStream","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","Cc":"hanlinchen@chromium.org, libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]