[{"id":39376,"web_url":"https://patchwork.libcamera.org/comment/39376/","msgid":"<1d2c6bcb-978c-427d-963b-5ecbeca34a10@collabora.com>","date":"2026-06-24T11:48:22","subject":"Re: [PATCH 01/10] libcamera: v4l2_videodevice: Output cache hit as a\n\tparameter","submitter":{"id":140,"url":"https://patchwork.libcamera.org/api/people/140/","name":"Robert Mader","email":"robert.mader@collabora.com"},"content":"Hi, thanks for the patch\n\nOn 24.06.26 10:58, Bryan O'Donoghue wrote:\n> The existing V4L2BufferCache::get routine evaluates if a cache hit has\n> occured by way of an internal variable. It is in fact very useful to a user\n> of this API to understand if a hit has occured as using logic may wish to\n> differentiate based on hit or miss.\n>\n> This differentiation is required for GPUISP. Rather than add a routine to\n> interrogate if a cache hit exists - add an output parameter to the get\n> routine.\n>\n> In simple terms if you are trying to cache data about the last thing you\n> want to to is interrogate the cache twice so, a `hit` output parameter adds\nnit: to do\n> a small cost in the stack for a large pivot in using code's ability to\n> understand how much work it needs to do on hit v miss.\n>\n> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>\n\nLooks sensible to me. Not sure if my R-B really counts here, however:\n\nReviewed-by: Robert Mader <robert.mader@collabora.com>\n\n> ---\n>   include/libcamera/internal/v4l2_videodevice.h |  2 +-\n>   src/libcamera/v4l2_videodevice.cpp            |  9 ++++++---\n>   test/v4l2_videodevice/buffer_cache.cpp        | 14 +++++++++-----\n>   3 files changed, 16 insertions(+), 9 deletions(-)\n>\n> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> index 10367e4e1..49165a565 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -129,7 +129,7 @@ public:\n>   \t~V4L2BufferCache();\n>   \n>   \tbool isEmpty() const;\n> -\tint get(const FrameBuffer &buffer);\n> +\tint get(const FrameBuffer &buffer, bool &hit);\n>   \tvoid put(unsigned int index);\n>   \n>   private:\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index ca8759830..6496eb324 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -217,6 +217,7 @@ bool V4L2BufferCache::isEmpty() const\n>   /**\n>    * \\brief Find the best V4L2 buffer for a FrameBuffer\n>    * \\param[in] buffer The FrameBuffer\n> + * \\param[out] hit. Indicates if there was a cache hit\n>    *\n>    * Find the best V4L2 buffer index to be used for the FrameBuffer \\a buffer\n>    * based on previous mappings of frame buffers to V4L2 buffers. If a free V4L2\n> @@ -227,12 +228,13 @@ bool V4L2BufferCache::isEmpty() const\n>    * \\return The index of the best V4L2 buffer, or -ENOENT if no free V4L2 buffer\n>    * is available\n>    */\n> -int V4L2BufferCache::get(const FrameBuffer &buffer)\n> +int V4L2BufferCache::get(const FrameBuffer &buffer, bool &hit)\n>   {\n> -\tbool hit = false;\n>   \tint use = -1;\n>   \tuint64_t oldest = UINT64_MAX;\n>   \n> +\thit = false;\n> +\n>   \tfor (unsigned int index = 0; index < cache_.size(); index++) {\n>   \t\tconst Entry &entry = cache_[index];\n>   \n> @@ -1649,6 +1651,7 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer, const V4L2Request *request\n>   {\n>   \tstruct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {};\n>   \tstruct v4l2_buffer buf = {};\n> +\tbool hit;\n>   \tint ret;\n>   \n>   \tif (state_ == State::Stopping) {\n> @@ -1666,7 +1669,7 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer, const V4L2Request *request\n>   \t\treturn -ENOENT;\n>   \t}\n>   \n> -\tret = cache_->get(*buffer);\n> +\tret = cache_->get(*buffer, hit);\n>   \tif (ret < 0)\n>   \t\treturn ret;\n>   \n> diff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp\n> index 613e0d7ce..e41c30059 100644\n> --- a/test/v4l2_videodevice/buffer_cache.cpp\n> +++ b/test/v4l2_videodevice/buffer_cache.cpp\n> @@ -35,7 +35,8 @@ public:\n>   \t{\n>   \t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n>   \t\t\tint nBuffer = i % buffers.size();\n> -\t\t\tint index = cache->get(*buffers[nBuffer].get());\n> +\t\t\tbool hit;\n> +\t\t\tint index = cache->get(*buffers[nBuffer].get(), hit);\n>   \n>   \t\t\tif (index != nBuffer) {\n>   \t\t\t\tstd::cout << \"Expected index \" << nBuffer\n> @@ -60,7 +61,8 @@ public:\n>   \n>   \t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n>   \t\t\tint nBuffer = dist(generator_);\n> -\t\t\tint index = cache->get(*buffers[nBuffer].get());\n> +\t\t\tbool hit;\n> +\t\t\tint index = cache->get(*buffers[nBuffer].get(), hit);\n>   \n>   \t\t\tif (index < 0) {\n>   \t\t\t\tstd::cout << \"Failed lookup from cache\"\n> @@ -90,7 +92,8 @@ public:\n>   \n>   \t\t/* Pick a hot buffer at random and store its index. */\n>   \t\tint hotBuffer = dist(generator_);\n> -\t\tint hotIndex = cache->get(*buffers[hotBuffer].get());\n> +\t\tbool hit;\n> +\t\tint hotIndex = cache->get(*buffers[hotBuffer].get(), hit);\n>   \t\tcache->put(hotIndex);\n>   \n>   \t\t/*\n> @@ -106,7 +109,7 @@ public:\n>   \t\t\telse\n>   \t\t\t\tnBuffer = dist(generator_);\n>   \n> -\t\t\tindex = cache->get(*buffers[nBuffer].get());\n> +\t\t\tindex = cache->get(*buffers[nBuffer].get(), hit);\n>   \n>   \t\t\tif (index < 0) {\n>   \t\t\t\tstd::cout << \"Failed lookup from cache\"\n> @@ -135,7 +138,8 @@ public:\n>   \n>   \t\tfor (const auto &buffer : buffers) {\n>   \t\t\tFrameBuffer &b = *buffer.get();\n> -\t\t\tcache.get(b);\n> +\t\t\tbool hit;\n> +\t\t\tcache.get(b, hit);\n>   \t\t}\n>   \n>   \t\tif (cache.isEmpty())","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 B4852C3306\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 24 Jun 2026 11:48:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0BF9F6589C;\n\tWed, 24 Jun 2026 13:48:32 +0200 (CEST)","from sender4-op-o12.zoho.com (sender4-op-o12.zoho.com\n\t[136.143.188.12])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6329565718\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 24 Jun 2026 13:48:30 +0200 (CEST)","by mx.zohomail.com with SMTPS id 1782301705351172.09788753208932; \n\tWed, 24 Jun 2026 04:48:25 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=collabora.com\n\theader.i=robert.mader@collabora.com header.b=\"hKHcZ59u\"; \n\tdkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1782301707; cv=none; \n\td=zohomail.com; s=zohoarc; \n\tb=FR/CmsChUbVirZ2o4QNwC4Nz70SpfgeWtpVKGo+drhXJsey+bU7PQC8fADb4k8Ev8WMTGCUfVIDktpYO14HY9K0M6F2lKn+JnBRC4ekhn08hAU5gBrXb3WZj1rfE3rfa4x4JQ8xOXiU9z7Z0TvPAAHx74WkakYbK8QeyyeatyjM=","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; \n\ts=zohoarc; t=1782301707;\n\th=Content-Type:Content-Transfer-Encoding:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To:Cc;\n\tbh=3iKLoPfWGcphK3DDjikBfRKZ3umJW9KV2LnuwgxKcr4=; \n\tb=g1nVdN3Lp4JgrD6gGDR12nKURWEygrxhhuzqvr5fyq//9Tr9Ac0sVzTHqBJ8tpkZcVnG6jvhhg+i4aWSgKlN+HibfDOPb6ko80Anmt2ySCo7jENaHnA1tm6ABpUC6Jo2dPBOorZIVatcb5f5yaNEbNXtxmbfeeu/P95jrOl4dFM=","ARC-Authentication-Results":"i=1; mx.zohomail.com;\n\tdkim=pass  header.i=collabora.com;\n\tspf=pass  smtp.mailfrom=robert.mader@collabora.com;\n\tdmarc=pass header.from=<robert.mader@collabora.com>","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1782301707;\n\ts=zohomail; d=collabora.com; i=robert.mader@collabora.com;\n\th=Message-ID:Date:Date:MIME-Version:Subject:Subject:To:To:References:From:From:In-Reply-To:Content-Type:Content-Transfer-Encoding:Message-Id:Reply-To:Cc;\n\tbh=3iKLoPfWGcphK3DDjikBfRKZ3umJW9KV2LnuwgxKcr4=;\n\tb=hKHcZ59uh3sL/Hu0oWHk2L7DLGa29UFIL3kNAQXfIBDhphqkJT99OKjQov/QaOXx\n\t8GQoSzB3hWszYP1M14QnQA5zySItrfS3PoZJ/I5W3rMcUEOFq+7ASa2gB4tIvbd4XOU\n\t0UJcK1XXJOcYwUfLG4iqqj/0XcV+mw0O+57uDU+Y=","Message-ID":"<1d2c6bcb-978c-427d-963b-5ecbeca34a10@collabora.com>","Date":"Wed, 24 Jun 2026 13:48:22 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 01/10] libcamera: v4l2_videodevice: Output cache hit as a\n\tparameter","To":"libcamera-devel@lists.libcamera.org","References":"<20260624085849.873784-1-bryan.odonoghue@linaro.org>\n\t<20260624085849.873784-2-bryan.odonoghue@linaro.org>","Content-Language":"en-US, de-DE, en-GB","From":"Robert Mader <robert.mader@collabora.com>","In-Reply-To":"<20260624085849.873784-2-bryan.odonoghue@linaro.org>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":39391,"web_url":"https://patchwork.libcamera.org/comment/39391/","msgid":"<20260624143828.GA851255@killaraus.ideasonboard.com>","date":"2026-06-24T14:38:28","subject":"Re: [PATCH 01/10] libcamera: v4l2_videodevice: Output cache hit as a\n\tparameter","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Jun 24, 2026 at 09:58:40AM +0100, Bryan O'Donoghue wrote:\n> The existing V4L2BufferCache::get routine evaluates if a cache hit has\n> occured by way of an internal variable. It is in fact very useful to a user\n> of this API to understand if a hit has occured as using logic may wish to\n> differentiate based on hit or miss.\n> \n> This differentiation is required for GPUISP. Rather than add a routine to\n> interrogate if a cache hit exists - add an output parameter to the get\n> routine.\n> \n> In simple terms if you are trying to cache data about the last thing you\n> want to to is interrogate the cache twice so, a `hit` output parameter adds\n> a small cost in the stack for a large pivot in using code's ability to\n> understand how much work it needs to do on hit v miss.\n> \n> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>\n> ---\n>  include/libcamera/internal/v4l2_videodevice.h |  2 +-\n>  src/libcamera/v4l2_videodevice.cpp            |  9 ++++++---\n>  test/v4l2_videodevice/buffer_cache.cpp        | 14 +++++++++-----\n>  3 files changed, 16 insertions(+), 9 deletions(-)\n> \n> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> index 10367e4e1..49165a565 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -129,7 +129,7 @@ public:\n>  \t~V4L2BufferCache();\n>  \n>  \tbool isEmpty() const;\n> -\tint get(const FrameBuffer &buffer);\n> +\tint get(const FrameBuffer &buffer, bool &hit);\n>  \tvoid put(unsigned int index);\n>  \n>  private:\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index ca8759830..6496eb324 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -217,6 +217,7 @@ bool V4L2BufferCache::isEmpty() const\n>  /**\n>   * \\brief Find the best V4L2 buffer for a FrameBuffer\n>   * \\param[in] buffer The FrameBuffer\n> + * \\param[out] hit. Indicates if there was a cache hit\n\nWrong syntax.\n\n>   *\n>   * Find the best V4L2 buffer index to be used for the FrameBuffer \\a buffer\n>   * based on previous mappings of frame buffers to V4L2 buffers. If a free V4L2\n> @@ -227,12 +228,13 @@ bool V4L2BufferCache::isEmpty() const\n>   * \\return The index of the best V4L2 buffer, or -ENOENT if no free V4L2 buffer\n>   * is available\n>   */\n> -int V4L2BufferCache::get(const FrameBuffer &buffer)\n> +int V4L2BufferCache::get(const FrameBuffer &buffer, bool &hit)\n\nIt's not nice to force all callers to add a new local variable. That\nbeing said, I was surprised that the function isn't called anywhere else\nthan in V4L2VideoDevice::queueBuffer() and in the unit tests.\nInvestigating a bit more, I realized you use V4L2BufferCache in patch\n10/10 only, in a way that is entirely unrelated to V4L2. That seems a\nbit of a hack. I see two better options:\n\n- Don't use V4L2BufferCache in eGL, write your own cache handling logic.\n\n- Rename the V4L2BufferCache class and turn it into a more generic\n  cache.\n\nThe latter will require a bit more work to decide on the scope of that\nclass.\n\n>  {\n> -\tbool hit = false;\n>  \tint use = -1;\n>  \tuint64_t oldest = UINT64_MAX;\n>  \n> +\thit = false;\n> +\n>  \tfor (unsigned int index = 0; index < cache_.size(); index++) {\n>  \t\tconst Entry &entry = cache_[index];\n>  \n> @@ -1649,6 +1651,7 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer, const V4L2Request *request\n>  {\n>  \tstruct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {};\n>  \tstruct v4l2_buffer buf = {};\n> +\tbool hit;\n>  \tint ret;\n>  \n>  \tif (state_ == State::Stopping) {\n> @@ -1666,7 +1669,7 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer, const V4L2Request *request\n>  \t\treturn -ENOENT;\n>  \t}\n>  \n> -\tret = cache_->get(*buffer);\n> +\tret = cache_->get(*buffer, hit);\n>  \tif (ret < 0)\n>  \t\treturn ret;\n>  \n> diff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp\n> index 613e0d7ce..e41c30059 100644\n> --- a/test/v4l2_videodevice/buffer_cache.cpp\n> +++ b/test/v4l2_videodevice/buffer_cache.cpp\n> @@ -35,7 +35,8 @@ public:\n>  \t{\n>  \t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n>  \t\t\tint nBuffer = i % buffers.size();\n> -\t\t\tint index = cache->get(*buffers[nBuffer].get());\n> +\t\t\tbool hit;\n> +\t\t\tint index = cache->get(*buffers[nBuffer].get(), hit);\n>  \n>  \t\t\tif (index != nBuffer) {\n>  \t\t\t\tstd::cout << \"Expected index \" << nBuffer\n> @@ -60,7 +61,8 @@ public:\n>  \n>  \t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n>  \t\t\tint nBuffer = dist(generator_);\n> -\t\t\tint index = cache->get(*buffers[nBuffer].get());\n> +\t\t\tbool hit;\n> +\t\t\tint index = cache->get(*buffers[nBuffer].get(), hit);\n>  \n>  \t\t\tif (index < 0) {\n>  \t\t\t\tstd::cout << \"Failed lookup from cache\"\n> @@ -90,7 +92,8 @@ public:\n>  \n>  \t\t/* Pick a hot buffer at random and store its index. */\n>  \t\tint hotBuffer = dist(generator_);\n> -\t\tint hotIndex = cache->get(*buffers[hotBuffer].get());\n> +\t\tbool hit;\n> +\t\tint hotIndex = cache->get(*buffers[hotBuffer].get(), hit);\n>  \t\tcache->put(hotIndex);\n>  \n>  \t\t/*\n> @@ -106,7 +109,7 @@ public:\n>  \t\t\telse\n>  \t\t\t\tnBuffer = dist(generator_);\n>  \n> -\t\t\tindex = cache->get(*buffers[nBuffer].get());\n> +\t\t\tindex = cache->get(*buffers[nBuffer].get(), hit);\n>  \n>  \t\t\tif (index < 0) {\n>  \t\t\t\tstd::cout << \"Failed lookup from cache\"\n> @@ -135,7 +138,8 @@ public:\n>  \n>  \t\tfor (const auto &buffer : buffers) {\n>  \t\t\tFrameBuffer &b = *buffer.get();\n> -\t\t\tcache.get(b);\n> +\t\t\tbool hit;\n> +\t\t\tcache.get(b, hit);\n>  \t\t}\n>  \n>  \t\tif (cache.isEmpty())","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 9DBC2C330F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 24 Jun 2026 14:38:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BB2BC658A6;\n\tWed, 24 Jun 2026 16:38:32 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C2B3A65718\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 24 Jun 2026 16:38:30 +0200 (CEST)","from killaraus.ideasonboard.com\n\t(2001-14ba-70f3-e800--a06.rev.dnainternet.fi\n\t[IPv6:2001:14ba:70f3:e800::a06])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0BC1E227;\n\tWed, 24 Jun 2026 16:37:50 +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=\"Gr70WZQL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1782311871;\n\tbh=R6ncilvy1b8pUJANfmrMHbNeT5lf/GA1X0U1cL95OFA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Gr70WZQLUs2sAmXAMtLbb1hkKV/dW1tR/0aY7h/0xrn/ThFPGcsaGQnOTrGXkIB3q\n\t4a4P4oBzbmYwYcb+J82NoS+Lwt695HB7F5WvQY+B0tI3GB+FADjyZAVocmSt9pCiM3\n\t3xRSRIkRK2ATXpXeTUg0sMXdgOTnD2zFwX93VBqc=","Date":"Wed, 24 Jun 2026 17:38:28 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Bryan O'Donoghue <bryan.odonoghue@linaro.org>","Cc":"libcamera-devel@lists.libcamera.org, pavel@ucw.cz","Subject":"Re: [PATCH 01/10] libcamera: v4l2_videodevice: Output cache hit as a\n\tparameter","Message-ID":"<20260624143828.GA851255@killaraus.ideasonboard.com>","References":"<20260624085849.873784-1-bryan.odonoghue@linaro.org>\n\t<20260624085849.873784-2-bryan.odonoghue@linaro.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20260624085849.873784-2-bryan.odonoghue@linaro.org>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":39406,"web_url":"https://patchwork.libcamera.org/comment/39406/","msgid":"<d0efd1bb-7426-4049-85a3-e9ee0be5b97a@linaro.org>","date":"2026-06-25T08:31:20","subject":"Re: [PATCH 01/10] libcamera: v4l2_videodevice: Output cache hit as a\n\tparameter","submitter":{"id":175,"url":"https://patchwork.libcamera.org/api/people/175/","name":"Bryan O'Donoghue","email":"bryan.odonoghue@linaro.org"},"content":"On 24/06/2026 15:38, Laurent Pinchart wrote:\n> - Don't use V4L2BufferCache in eGL, write your own cache handling logic.\n\nI'm happy to do this, I acted on Barnabas suggestion of \"something like \nV4L2BufferCache\" perhaps too literally ;)\n\n---\nbod","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 27DBFC3306\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 25 Jun 2026 08:31:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A6B73658C1;\n\tThu, 25 Jun 2026 10:31:24 +0200 (CEST)","from mail-ej1-x636.google.com (mail-ej1-x636.google.com\n\t[IPv6:2a00:1450:4864:20::636])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A20B2656DE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 25 Jun 2026 10:31:22 +0200 (CEST)","by mail-ej1-x636.google.com with SMTP id\n\ta640c23a62f3a-c074ac8adaaso295942066b.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 25 Jun 2026 01:31:22 -0700 (PDT)","from [192.168.0.101] ([109.76.33.201])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-c11fbbe4a3asm129600466b.25.2026.06.25.01.31.21\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tThu, 25 Jun 2026 01:31:21 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"xeDD2mjK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=linaro.org; s=google; t=1782376282; x=1782981082;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:in-reply-to:content-language:from\n\t:references:cc:to:subject:user-agent:mime-version:date:message-id\n\t:from:to:cc:subject:date:message-id:reply-to;\n\tbh=hOzShKW0Jg8G0eqeN7f7wFIA4e1tO1Yv3viHpywA5F0=;\n\tb=xeDD2mjKDNof+yIpTBTK9JUOGenCEnYeMvdAmQaC/nXtZagd3xLQeh7ERXRz4CQkQD\n\twE/2amEq0Ko3kO3VnEpEcgcU3yL6wmKJ5vCxs4EjKGrcZuk+bkHgWHBdRGxbWBVCz5zu\n\tphMpVZa4lAxPbn7ufU//XagzTl0nDNG6TgsDKE9zh61z6E2VuCcRRNtyE2LvcO8W1zg3\n\t14gj8ho+4yhIWqHMEcO2HQHFnGwzCu3+i1GIRW3gXA4zosOAUhQO/mPgIxA1D9WUbzZb\n\tJaL1rVtDdr3zF/2ZBWGpdhvBcHOo7/CCTbKW09wMWpOwtkER6D2TNwV8GnneJiRxUGFr\n\tF41A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20251104; t=1782376282; x=1782981082;\n\th=content-transfer-encoding:in-reply-to:content-language:from\n\t:references:cc:to:subject:user-agent:mime-version:date:message-id\n\t:x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=hOzShKW0Jg8G0eqeN7f7wFIA4e1tO1Yv3viHpywA5F0=;\n\tb=gh9C68jEC3NtAvvi42DHo4dfDrM+3+1s5mTaOej/IgUc5zuIXMtzkef9M+wgXHCvAo\n\trA84xQGtclzZhSCnRj+MguaHouxzZhC3MqpN7x1ZXVdITgBNVmM8Z0VIDCJq6xI7mZAY\n\tSnU810YQYiRvIF8JkZAxTT4armUQOxYGTyTSzqOAKS1mZCYPfhhRKBGHv/M103hs6Lq8\n\tq93CECbaPeCp7gul4Gmi5qHQLf5jarhsTPASgsklO+hT4hJ9nUgXde6h9REpNXGdfZNv\n\tycurHYw/Aba17vIk0cv8irlNXOltIIekKr/sWkQ2k5VAbd7CzepGaEAuvoNMeA7DyhZH\n\tEwZA==","X-Gm-Message-State":"AOJu0YzKdPCdTawkxZNDHAjjnh6ElAp+uzKICKIrfH+XISpoOMXLJFxv\n\tiMyYP/TYOBPlekfqXD5KL7FZQCjKAUBsK1dHLCTaPz/hF6/FFrzH5NGznww4m5Au2Xo=","X-Gm-Gg":"AfdE7ckqbpQ/R05D0PCLrEg29GcKI9m0maDlqdO0i24HA+N7xTO8d6zVMgHYXpDAngF\n\tFQAlOlsJsB1xD1s0miTdi3KfWTnOjRtRze8ZZq0fihBinGzCiZ5NADabJjo4UKz8J3H4EzYrzfV\n\tpG8O5vyQhkphByN1hcgLrjiu9iZ72Mponm0DYrRXAXW0W0eaGIiGWZtNFhG9VzRIZ1wp2ZKKP6Z\n\tzkopb58tgEguT9danYEHMxpkE9TW57RkfZYdbAtam0Hn88oPAW7dFVnOTHd3Zte1IYliu3BfNTZ\n\tvbJlar7S6LS2p4e/+CGQLLUJ751HFwb/Kka17UyiEi2mkEfnpsFZyNy6D3YQcNp4pOTZn1gnOmp\n\tCtSnWBC6BWGqJH/W+G2E7rkMRGGHTf4vT2N0qQ1VVgOanTTEES9ynOFtCR60Aml1ZrxjO07J32u\n\tgFj0wnenY4PPl13ZeYeU+f+KsI","X-Received":"by 2002:a17:907:934b:b0:bee:875d:af12 with SMTP id\n\ta640c23a62f3a-c1205b29185mr84585266b.0.1782376281926; \n\tThu, 25 Jun 2026 01:31:21 -0700 (PDT)","Message-ID":"<d0efd1bb-7426-4049-85a3-e9ee0be5b97a@linaro.org>","Date":"Thu, 25 Jun 2026 09:31:20 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 01/10] libcamera: v4l2_videodevice: Output cache hit as a\n\tparameter","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, pavel@ucw.cz","References":"<20260624085849.873784-1-bryan.odonoghue@linaro.org>\n\t<20260624085849.873784-2-bryan.odonoghue@linaro.org>\n\t<20260624143828.GA851255@killaraus.ideasonboard.com>","From":"Bryan O'Donoghue <bryan.odonoghue@linaro.org>","Content-Language":"en-US","In-Reply-To":"<20260624143828.GA851255@killaraus.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":39410,"web_url":"https://patchwork.libcamera.org/comment/39410/","msgid":"<585ad55a-ac3b-4633-a1f0-63efbb65aa2f@ideasonboard.com>","date":"2026-06-25T10:22:36","subject":"Re: [PATCH 01/10] libcamera: v4l2_videodevice: Output cache hit as a\n\tparameter","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 06. 24. 16:38 keltezéssel, Laurent Pinchart írta:\n> On Wed, Jun 24, 2026 at 09:58:40AM +0100, Bryan O'Donoghue wrote:\n>> The existing V4L2BufferCache::get routine evaluates if a cache hit has\n>> occured by way of an internal variable. It is in fact very useful to a user\n>> of this API to understand if a hit has occured as using logic may wish to\n>> differentiate based on hit or miss.\n>>\n>> This differentiation is required for GPUISP. Rather than add a routine to\n>> interrogate if a cache hit exists - add an output parameter to the get\n>> routine.\n>>\n>> In simple terms if you are trying to cache data about the last thing you\n>> want to to is interrogate the cache twice so, a `hit` output parameter adds\n>> a small cost in the stack for a large pivot in using code's ability to\n>> understand how much work it needs to do on hit v miss.\n>>\n>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>\n>> ---\n>>   include/libcamera/internal/v4l2_videodevice.h |  2 +-\n>>   src/libcamera/v4l2_videodevice.cpp            |  9 ++++++---\n>>   test/v4l2_videodevice/buffer_cache.cpp        | 14 +++++++++-----\n>>   3 files changed, 16 insertions(+), 9 deletions(-)\n>>\n>> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n>> index 10367e4e1..49165a565 100644\n>> --- a/include/libcamera/internal/v4l2_videodevice.h\n>> +++ b/include/libcamera/internal/v4l2_videodevice.h\n>> @@ -129,7 +129,7 @@ public:\n>>   \t~V4L2BufferCache();\n>>\n>>   \tbool isEmpty() const;\n>> -\tint get(const FrameBuffer &buffer);\n>> +\tint get(const FrameBuffer &buffer, bool &hit);\n>>   \tvoid put(unsigned int index);\n>>\n>>   private:\n>> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n>> index ca8759830..6496eb324 100644\n>> --- a/src/libcamera/v4l2_videodevice.cpp\n>> +++ b/src/libcamera/v4l2_videodevice.cpp\n>> @@ -217,6 +217,7 @@ bool V4L2BufferCache::isEmpty() const\n>>   /**\n>>    * \\brief Find the best V4L2 buffer for a FrameBuffer\n>>    * \\param[in] buffer The FrameBuffer\n>> + * \\param[out] hit. Indicates if there was a cache hit\n> \n> Wrong syntax.\n> \n>>    *\n>>    * Find the best V4L2 buffer index to be used for the FrameBuffer \\a buffer\n>>    * based on previous mappings of frame buffers to V4L2 buffers. If a free V4L2\n>> @@ -227,12 +228,13 @@ bool V4L2BufferCache::isEmpty() const\n>>    * \\return The index of the best V4L2 buffer, or -ENOENT if no free V4L2 buffer\n>>    * is available\n>>    */\n>> -int V4L2BufferCache::get(const FrameBuffer &buffer)\n>> +int V4L2BufferCache::get(const FrameBuffer &buffer, bool &hit)\n> \n> It's not nice to force all callers to add a new local variable. That\n\nMaybe it could be be part of the return value, e.g. `std::pair<int, bool>` or similar.\n(Or I suppose `bool *hit = nullptr` as an optional out parameter, but I like the return\nvalue better.)\n\n\n> being said, I was surprised that the function isn't called anywhere else\n> than in V4L2VideoDevice::queueBuffer() and in the unit tests.\n> Investigating a bit more, I realized you use V4L2BufferCache in patch\n> 10/10 only, in a way that is entirely unrelated to V4L2. That seems a\n> bit of a hack. I see two better options:\n> \n> - Don't use V4L2BufferCache in eGL, write your own cache handling logic.\n> \n> - Rename the V4L2BufferCache class and turn it into a more generic\n>    cache.\n> \n> The latter will require a bit more work to decide on the scope of that\n> class.\n\nI would somewhat be in favor of making something \"generic\" that would work for\nboth cases, since the idea is the same in both cases: have a cache of\nframebuffer -> index mappings, and I don't see that it would fundamentally change.\nSo it seems to me that it wouldn't be a mistake to try to avoid duplication here.\n\n\n> \n>>   {\n>> -\tbool hit = false;\n>>   \tint use = -1;\n>>   \tuint64_t oldest = UINT64_MAX;\n>>\n>> +\thit = false;\n>> +\n>>   \tfor (unsigned int index = 0; index < cache_.size(); index++) {\n>>   \t\tconst Entry &entry = cache_[index];\n>>\n>> @@ -1649,6 +1651,7 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer, const V4L2Request *request\n>>   {\n>>   \tstruct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {};\n>>   \tstruct v4l2_buffer buf = {};\n>> +\tbool hit;\n>>   \tint ret;\n>>\n>>   \tif (state_ == State::Stopping) {\n>> @@ -1666,7 +1669,7 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer, const V4L2Request *request\n>>   \t\treturn -ENOENT;\n>>   \t}\n>>\n>> -\tret = cache_->get(*buffer);\n>> +\tret = cache_->get(*buffer, hit);\n>>   \tif (ret < 0)\n>>   \t\treturn ret;\n>>\n>> diff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp\n>> index 613e0d7ce..e41c30059 100644\n>> --- a/test/v4l2_videodevice/buffer_cache.cpp\n>> +++ b/test/v4l2_videodevice/buffer_cache.cpp\n>> @@ -35,7 +35,8 @@ public:\n>>   \t{\n>>   \t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n>>   \t\t\tint nBuffer = i % buffers.size();\n>> -\t\t\tint index = cache->get(*buffers[nBuffer].get());\n>> +\t\t\tbool hit;\n>> +\t\t\tint index = cache->get(*buffers[nBuffer].get(), hit);\n>>\n>>   \t\t\tif (index != nBuffer) {\n>>   \t\t\t\tstd::cout << \"Expected index \" << nBuffer\n>> @@ -60,7 +61,8 @@ public:\n>>\n>>   \t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n>>   \t\t\tint nBuffer = dist(generator_);\n>> -\t\t\tint index = cache->get(*buffers[nBuffer].get());\n>> +\t\t\tbool hit;\n>> +\t\t\tint index = cache->get(*buffers[nBuffer].get(), hit);\n>>\n>>   \t\t\tif (index < 0) {\n>>   \t\t\t\tstd::cout << \"Failed lookup from cache\"\n>> @@ -90,7 +92,8 @@ public:\n>>\n>>   \t\t/* Pick a hot buffer at random and store its index. */\n>>   \t\tint hotBuffer = dist(generator_);\n>> -\t\tint hotIndex = cache->get(*buffers[hotBuffer].get());\n>> +\t\tbool hit;\n>> +\t\tint hotIndex = cache->get(*buffers[hotBuffer].get(), hit);\n>>   \t\tcache->put(hotIndex);\n>>\n>>   \t\t/*\n>> @@ -106,7 +109,7 @@ public:\n>>   \t\t\telse\n>>   \t\t\t\tnBuffer = dist(generator_);\n>>\n>> -\t\t\tindex = cache->get(*buffers[nBuffer].get());\n>> +\t\t\tindex = cache->get(*buffers[nBuffer].get(), hit);\n>>\n>>   \t\t\tif (index < 0) {\n>>   \t\t\t\tstd::cout << \"Failed lookup from cache\"\n>> @@ -135,7 +138,8 @@ public:\n>>\n>>   \t\tfor (const auto &buffer : buffers) {\n>>   \t\t\tFrameBuffer &b = *buffer.get();\n>> -\t\t\tcache.get(b);\n>> +\t\t\tbool hit;\n>> +\t\t\tcache.get(b, hit);\n>>   \t\t}\n>>\n>>   \t\tif (cache.isEmpty())\n> \n> --\n> Regards,\n> \n> Laurent Pinchart","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 8537FC3261\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 25 Jun 2026 10:22:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AD578623CB;\n\tThu, 25 Jun 2026 12:22:43 +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 B8753623CB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 25 Jun 2026 12:22:41 +0200 (CEST)","from [192.168.33.46] (185.221.143.75.nat.pool.zt.hu\n\t[185.221.143.75])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A3B52212;\n\tThu, 25 Jun 2026 12:22:00 +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=\"DU5SWKfP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1782382921;\n\tbh=vmVBHiRFvEuYxhXCT7DQ0go3eN1FIFcQUbF8+N6F7JY=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=DU5SWKfPcYM3elmRgy0HqSq+E2vZfwwAv/Racv3BRKS7zQlFjDQGUrOr7d3Lkm/Fq\n\tBrSip/2fUv2CXds4gEYmC2FKrvgQ8rquuPZ6kahPNQabHrLg+CzpUNPqxNBEwWjMje\n\tHqoKs1xPBLHOiobvT0R2cfCWdCghnhDlzlZtx3pc=","Message-ID":"<585ad55a-ac3b-4633-a1f0-63efbb65aa2f@ideasonboard.com>","Date":"Thu, 25 Jun 2026 12:22:36 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 01/10] libcamera: v4l2_videodevice: Output cache hit as a\n\tparameter","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tBryan O'Donoghue <bryan.odonoghue@linaro.org>","Cc":"libcamera-devel@lists.libcamera.org, pavel@ucw.cz","References":"<20260624085849.873784-1-bryan.odonoghue@linaro.org>\n\t<20260624085849.873784-2-bryan.odonoghue@linaro.org>\n\t<Rnbt-tJf9IfIX7XyzwFUdL3Z0gihoMZKtJNZWdkZOXGE25nCNzfkE10_5u96ya23uonAlR7cSYV5H5_GTUiskQ==@protonmail.internalid>\n\t<20260624143828.GA851255@killaraus.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260624143828.GA851255@killaraus.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":39452,"web_url":"https://patchwork.libcamera.org/comment/39452/","msgid":"<20260626113956.GA2340889@killaraus.ideasonboard.com>","date":"2026-06-26T11:39:56","subject":"Re: [PATCH 01/10] libcamera: v4l2_videodevice: Output cache hit as a\n\tparameter","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Jun 25, 2026 at 12:22:36PM +0200, Barnabás Pőcze wrote:\n> 2026. 06. 24. 16:38 keltezéssel, Laurent Pinchart írta:\n> > On Wed, Jun 24, 2026 at 09:58:40AM +0100, Bryan O'Donoghue wrote:\n> >> The existing V4L2BufferCache::get routine evaluates if a cache hit has\n> >> occured by way of an internal variable. It is in fact very useful to a user\n> >> of this API to understand if a hit has occured as using logic may wish to\n> >> differentiate based on hit or miss.\n> >>\n> >> This differentiation is required for GPUISP. Rather than add a routine to\n> >> interrogate if a cache hit exists - add an output parameter to the get\n> >> routine.\n> >>\n> >> In simple terms if you are trying to cache data about the last thing you\n> >> want to to is interrogate the cache twice so, a `hit` output parameter adds\n> >> a small cost in the stack for a large pivot in using code's ability to\n> >> understand how much work it needs to do on hit v miss.\n> >>\n> >> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>\n> >> ---\n> >>   include/libcamera/internal/v4l2_videodevice.h |  2 +-\n> >>   src/libcamera/v4l2_videodevice.cpp            |  9 ++++++---\n> >>   test/v4l2_videodevice/buffer_cache.cpp        | 14 +++++++++-----\n> >>   3 files changed, 16 insertions(+), 9 deletions(-)\n> >>\n> >> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> >> index 10367e4e1..49165a565 100644\n> >> --- a/include/libcamera/internal/v4l2_videodevice.h\n> >> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> >> @@ -129,7 +129,7 @@ public:\n> >>   \t~V4L2BufferCache();\n> >>\n> >>   \tbool isEmpty() const;\n> >> -\tint get(const FrameBuffer &buffer);\n> >> +\tint get(const FrameBuffer &buffer, bool &hit);\n> >>   \tvoid put(unsigned int index);\n> >>\n> >>   private:\n> >> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> >> index ca8759830..6496eb324 100644\n> >> --- a/src/libcamera/v4l2_videodevice.cpp\n> >> +++ b/src/libcamera/v4l2_videodevice.cpp\n> >> @@ -217,6 +217,7 @@ bool V4L2BufferCache::isEmpty() const\n> >>   /**\n> >>    * \\brief Find the best V4L2 buffer for a FrameBuffer\n> >>    * \\param[in] buffer The FrameBuffer\n> >> + * \\param[out] hit. Indicates if there was a cache hit\n> > \n> > Wrong syntax.\n> > \n> >>    *\n> >>    * Find the best V4L2 buffer index to be used for the FrameBuffer \\a buffer\n> >>    * based on previous mappings of frame buffers to V4L2 buffers. If a free V4L2\n> >> @@ -227,12 +228,13 @@ bool V4L2BufferCache::isEmpty() const\n> >>    * \\return The index of the best V4L2 buffer, or -ENOENT if no free V4L2 buffer\n> >>    * is available\n> >>    */\n> >> -int V4L2BufferCache::get(const FrameBuffer &buffer)\n> >> +int V4L2BufferCache::get(const FrameBuffer &buffer, bool &hit)\n> > \n> > It's not nice to force all callers to add a new local variable. That\n> \n> Maybe it could be be part of the return value, e.g. `std::pair<int, bool>` or similar.\n> (Or I suppose `bool *hit = nullptr` as an optional out parameter, but I like the return\n> value better.)\n\nReturning a pair (or tuple - would there be a meaningful difference ?)\nseems fine to me. I think we're a bit too shy in libcamera when it comes\nto returning tuples and using structured binding.\n\n> > being said, I was surprised that the function isn't called anywhere else\n> > than in V4L2VideoDevice::queueBuffer() and in the unit tests.\n> > Investigating a bit more, I realized you use V4L2BufferCache in patch\n> > 10/10 only, in a way that is entirely unrelated to V4L2. That seems a\n> > bit of a hack. I see two better options:\n> > \n> > - Don't use V4L2BufferCache in eGL, write your own cache handling logic.\n> > \n> > - Rename the V4L2BufferCache class and turn it into a more generic\n> >    cache.\n> > \n> > The latter will require a bit more work to decide on the scope of that\n> > class.\n> \n> I would somewhat be in favor of making something \"generic\" that would work for\n> both cases, since the idea is the same in both cases: have a cache of\n> framebuffer -> index mappings, and I don't see that it would fundamentally change.\n> So it seems to me that it wouldn't be a mistake to try to avoid duplication here.\n\nAck.\n\n> >>   {\n> >> -\tbool hit = false;\n> >>   \tint use = -1;\n> >>   \tuint64_t oldest = UINT64_MAX;\n> >>\n> >> +\thit = false;\n> >> +\n> >>   \tfor (unsigned int index = 0; index < cache_.size(); index++) {\n> >>   \t\tconst Entry &entry = cache_[index];\n> >>\n> >> @@ -1649,6 +1651,7 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer, const V4L2Request *request\n> >>   {\n> >>   \tstruct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {};\n> >>   \tstruct v4l2_buffer buf = {};\n> >> +\tbool hit;\n> >>   \tint ret;\n> >>\n> >>   \tif (state_ == State::Stopping) {\n> >> @@ -1666,7 +1669,7 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer, const V4L2Request *request\n> >>   \t\treturn -ENOENT;\n> >>   \t}\n> >>\n> >> -\tret = cache_->get(*buffer);\n> >> +\tret = cache_->get(*buffer, hit);\n> >>   \tif (ret < 0)\n> >>   \t\treturn ret;\n> >>\n> >> diff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp\n> >> index 613e0d7ce..e41c30059 100644\n> >> --- a/test/v4l2_videodevice/buffer_cache.cpp\n> >> +++ b/test/v4l2_videodevice/buffer_cache.cpp\n> >> @@ -35,7 +35,8 @@ public:\n> >>   \t{\n> >>   \t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n> >>   \t\t\tint nBuffer = i % buffers.size();\n> >> -\t\t\tint index = cache->get(*buffers[nBuffer].get());\n> >> +\t\t\tbool hit;\n> >> +\t\t\tint index = cache->get(*buffers[nBuffer].get(), hit);\n> >>\n> >>   \t\t\tif (index != nBuffer) {\n> >>   \t\t\t\tstd::cout << \"Expected index \" << nBuffer\n> >> @@ -60,7 +61,8 @@ public:\n> >>\n> >>   \t\tfor (unsigned int i = 0; i < buffers.size() * 100; i++) {\n> >>   \t\t\tint nBuffer = dist(generator_);\n> >> -\t\t\tint index = cache->get(*buffers[nBuffer].get());\n> >> +\t\t\tbool hit;\n> >> +\t\t\tint index = cache->get(*buffers[nBuffer].get(), hit);\n> >>\n> >>   \t\t\tif (index < 0) {\n> >>   \t\t\t\tstd::cout << \"Failed lookup from cache\"\n> >> @@ -90,7 +92,8 @@ public:\n> >>\n> >>   \t\t/* Pick a hot buffer at random and store its index. */\n> >>   \t\tint hotBuffer = dist(generator_);\n> >> -\t\tint hotIndex = cache->get(*buffers[hotBuffer].get());\n> >> +\t\tbool hit;\n> >> +\t\tint hotIndex = cache->get(*buffers[hotBuffer].get(), hit);\n> >>   \t\tcache->put(hotIndex);\n> >>\n> >>   \t\t/*\n> >> @@ -106,7 +109,7 @@ public:\n> >>   \t\t\telse\n> >>   \t\t\t\tnBuffer = dist(generator_);\n> >>\n> >> -\t\t\tindex = cache->get(*buffers[nBuffer].get());\n> >> +\t\t\tindex = cache->get(*buffers[nBuffer].get(), hit);\n> >>\n> >>   \t\t\tif (index < 0) {\n> >>   \t\t\t\tstd::cout << \"Failed lookup from cache\"\n> >> @@ -135,7 +138,8 @@ public:\n> >>\n> >>   \t\tfor (const auto &buffer : buffers) {\n> >>   \t\t\tFrameBuffer &b = *buffer.get();\n> >> -\t\t\tcache.get(b);\n> >> +\t\t\tbool hit;\n> >> +\t\t\tcache.get(b, hit);\n> >>   \t\t}\n> >>\n> >>   \t\tif (cache.isEmpty())","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 C5D79C3264\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 26 Jun 2026 11:40:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F0E77658FB;\n\tFri, 26 Jun 2026 13:39:59 +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 E7B36658F2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 26 Jun 2026 13:39:57 +0200 (CEST)","from killaraus.ideasonboard.com\n\t(2001-14ba-70f3-e800--a06.rev.dnainternet.fi\n\t[IPv6:2001:14ba:70f3:e800::a06])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D0F2C1E6;\n\tFri, 26 Jun 2026 13:39:16 +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=\"X2izP0K7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1782473957;\n\tbh=sbGFiP1Lm74JFqfmzrHtExk0DkGc7St8oMJf9HP+6k0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=X2izP0K7WVlITpnpSsY2BWPql/fHPImHVr8cQdPXbhW2/75YzvwJEdzzLv4KCPh43\n\trMape6bzU0KZXoA1XJGfa4SLAtRHQWCTiKdiDrJ+uLU/Mv+AADDUsEva1TdPdFLHJ1\n\t/ZM0T2HU60/ZODRbfKXhfZhRHT2iCY7mxJvf2WJo=","Date":"Fri, 26 Jun 2026 14:39:56 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"Bryan O'Donoghue <bryan.odonoghue@linaro.org>,\n\tlibcamera-devel@lists.libcamera.org, pavel@ucw.cz","Subject":"Re: [PATCH 01/10] libcamera: v4l2_videodevice: Output cache hit as a\n\tparameter","Message-ID":"<20260626113956.GA2340889@killaraus.ideasonboard.com>","References":"<20260624085849.873784-1-bryan.odonoghue@linaro.org>\n\t<20260624085849.873784-2-bryan.odonoghue@linaro.org>\n\t<Rnbt-tJf9IfIX7XyzwFUdL3Z0gihoMZKtJNZWdkZOXGE25nCNzfkE10_5u96ya23uonAlR7cSYV5H5_GTUiskQ==@protonmail.internalid>\n\t<20260624143828.GA851255@killaraus.ideasonboard.com>\n\t<585ad55a-ac3b-4633-a1f0-63efbb65aa2f@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<585ad55a-ac3b-4633-a1f0-63efbb65aa2f@ideasonboard.com>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]