[{"id":28417,"web_url":"https://patchwork.libcamera.org/comment/28417/","msgid":"<170480587928.3044059.3273889949177592860@ping.linuxembedded.co.uk>","date":"2024-01-09T13:11:19","subject":"Re: [libcamera-devel] [PATCH v1] libcamera: framebuffer_allocator:\n\tRemove unnecessary `clear()`","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze via libcamera-devel (2023-12-09 02:22:28)\n> The vector in question is destroyed when the item in the `buffers_` map\n> is destroyed as a result of the `erase()` call. A vector's destructor\n> already does all the things that `clear()` does,\n> so calling it earlier is not needed.\n\nThis looks correct to me. I guess it's a micro-optimisation, but it\nseems valid.\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n> ---\n>  src/libcamera/framebuffer_allocator.cpp | 2 --\n>  1 file changed, 2 deletions(-)\n> \n> diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp\n> index dabd9219..94389735 100644\n> --- a/src/libcamera/framebuffer_allocator.cpp\n> +++ b/src/libcamera/framebuffer_allocator.cpp\n> @@ -121,8 +121,6 @@ int FrameBufferAllocator::free(Stream *stream)\n>         if (iter == buffers_.end())\n>                 return -EINVAL;\n>  \n> -       std::vector<std::unique_ptr<FrameBuffer>> &buffers = iter->second;\n> -       buffers.clear();\n>         buffers_.erase(iter);\n>  \n>         return 0;\n> -- \n> 2.43.0\n> \n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 C0B56BDB1D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  9 Jan 2024 13:11:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1FB7B62B32;\n\tTue,  9 Jan 2024 14:11:24 +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 2ED7B61D7B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  9 Jan 2024 14:11:22 +0100 (CET)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 05139922;\n\tTue,  9 Jan 2024 14:10:17 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1704805884;\n\tbh=jgNNIJtrDkbHBfmG6LwGb8dFo20FypLnUiDy0eeBeII=;\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:\n\tFrom;\n\tb=W0haotRG/N0XnfDpQtQ9q6Z8wEm7osTWUsf/qr5bh7mlAt5jI6H0t+9VHFJLJGM0G\n\trlNHCUMxCP9yKfsFDaew1QpFWYWg9b34TUQpKQgFPI1ZJGn+qgqVhYP91RZ+53RF/b\n\tlSmqd3gGsyJMUtuWA2i63iyCj5PHSyuN3BfQjAF4jdG76bC8j2zsfQ7wPdaV1V8geA\n\tvwnOI6PYxsTIG6hRjxNzs2rcu5/MZ3cnT570ZRpPbk9w/meX7YqRegCi1/By4cadxF\n\t/CQfOll7j/8xWLCSThacPJn2Ut6rvaG6FUonpnLUnx/GKz+76WBIc8DTrjNrWSDMcr\n\taNEXrlzoS/qjQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1704805818;\n\tbh=jgNNIJtrDkbHBfmG6LwGb8dFo20FypLnUiDy0eeBeII=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=t/MBYFC4w5aLTF6D+0f6LmuNIVshJ9wZF1AFTvqETWCGSKjvsp06F3RT/nsWSbRg7\n\t6CHAtffSl4i5nihGUEzDdoGGhCJS0UgdNrOpOcHk8F7FwCq3aHNV7N3nXyL6ZMsM0j\n\t1/HUQBQgCcE4RjvTjWHBMh/l04chPgd6VJJ7aw+4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"t/MBYFC4\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20231209022227.595197-1-pobrn@protonmail.com>","References":"<20231209022227.595197-1-pobrn@protonmail.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 09 Jan 2024 13:11:19 +0000","Message-ID":"<170480587928.3044059.3273889949177592860@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v1] libcamera: framebuffer_allocator:\n\tRemove unnecessary `clear()`","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28418,"web_url":"https://patchwork.libcamera.org/comment/28418/","msgid":"<20240109132050.GB17461@pendragon.ideasonboard.com>","date":"2024-01-09T13:20:50","subject":"Re: [libcamera-devel] [PATCH v1] libcamera: framebuffer_allocator:\n\tRemove unnecessary `clear()`","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Jan 09, 2024 at 01:11:19PM +0000, Kieran Bingham via libcamera-devel wrote:\n> Quoting Barnabás Pőcze via libcamera-devel (2023-12-09 02:22:28)\n> > The vector in question is destroyed when the item in the `buffers_` map\n> > is destroyed as a result of the `erase()` call. A vector's destructor\n> > already does all the things that `clear()` does,\n> > so calling it earlier is not needed.\n> \n> This looks correct to me. I guess it's a micro-optimisation, but it\n> seems valid.\n\nIt may have been there before\n\ncommit a4be7bb5ff4d4dce1fdc942a103f6360dad91f11\nAuthor: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nDate:   Sun Jan 19 21:42:09 2020 +0200\n\n    libcamera: camera: Move private data members to private implementation\n\nto make sure the FrameBuffer instances get destroyed before freeing the\nunderlying buffers. That commit changed the order of the operations and\nfreed the underlying buffers first, apparently without causing issues.\nIn any case, there's no need for a separate .clear() now.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n> > ---\n> >  src/libcamera/framebuffer_allocator.cpp | 2 --\n> >  1 file changed, 2 deletions(-)\n> > \n> > diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp\n> > index dabd9219..94389735 100644\n> > --- a/src/libcamera/framebuffer_allocator.cpp\n> > +++ b/src/libcamera/framebuffer_allocator.cpp\n> > @@ -121,8 +121,6 @@ int FrameBufferAllocator::free(Stream *stream)\n> >         if (iter == buffers_.end())\n> >                 return -EINVAL;\n> >  \n> > -       std::vector<std::unique_ptr<FrameBuffer>> &buffers = iter->second;\n> > -       buffers.clear();\n> >         buffers_.erase(iter);\n> >  \n> >         return 0;","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 24870C323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  9 Jan 2024 13:20:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3D45762B41;\n\tTue,  9 Jan 2024 14:20: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 34D8461D7B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  9 Jan 2024 14:20:42 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C978C922;\n\tTue,  9 Jan 2024 14:19:37 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1704806443;\n\tbh=Y2bxSwEpiehRgmuN3RbcSRyQVNrQSt9fE6QN6h98rnQ=;\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=ZYHbXLGdjBHP6gPA2PVq+y60d5O5BIAAxOkKYWVzrXhvFgtvcqCNw9zQfTIY7DhHq\n\tA4eNnuryLPOUW0UdrJaPy3FWb2nabyaUkzGec8PlRrW2Y8fI6s+/9+vSEoIEYh+ImG\n\td7gS4KdDUdsZmFK4wN683hxPw9yR8qugk8KnyoFrv7w8i6fdw03q0Xrqqh/vgNCgsp\n\tbsoqGggHVz7L6UC8zTB8A7vDu+bmgHxX4snL11gLnyg9P41bR3t15pYtYC7mcYdQFF\n\txru+yH4A14zF2QCkQZacReWeTfVo25AeIvN8zbEZlK/QbGglaqc9indU9gtjlRJLWy\n\t3Tt89lE2YvFQg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1704806378;\n\tbh=Y2bxSwEpiehRgmuN3RbcSRyQVNrQSt9fE6QN6h98rnQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EB3K5cN/uYTQs9aXpOb476NPDqWVTmv6rpfZkaQW1Pnj9bwgyTTJexvKDHre+68z3\n\tHpULEVZTvVM97RXyTuSJMXUkl9ohMtxUajSwhSoTOy6yEOF3NMb4ixXbgjEuo6iM0Y\n\tZvb7qLIoS0sYIAOn5+aAfZpCRxP4G1qbRqmP/vwQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"EB3K5cN/\"; dkim-atps=neutral","Date":"Tue, 9 Jan 2024 15:20:50 +0200","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20240109132050.GB17461@pendragon.ideasonboard.com>","References":"<20231209022227.595197-1-pobrn@protonmail.com>\n\t<170480587928.3044059.3273889949177592860@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<170480587928.3044059.3273889949177592860@ping.linuxembedded.co.uk>","Subject":"Re: [libcamera-devel] [PATCH v1] libcamera: framebuffer_allocator:\n\tRemove unnecessary `clear()`","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]