[{"id":34736,"web_url":"https://patchwork.libcamera.org/comment/34736/","msgid":"<175128033580.2538045.10290009961564519689@ping.linuxembedded.co.uk>","date":"2025-06-30T10:45:35","subject":"Re: [PATCH v1 0/6] rkisp1: Allow usage of more than 4 buffers","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stefan Klug (2025-06-30 09:11:15)\n> Hi all,\n> \n> In the rkisp1 pipeline the value of StreamConfiuration.bufferCount is\n> reset to 4 on validate(). This effectively limits the number of buffers\n> that are in the loop to 4. As soon as the consumers\n> (gstream/pipewire...) of the buffers need more than 1 buffer at a time,\n> we start to see frame drops. Sven resurrected an old series [1] that\n> solves the issue at the expense of an additional control and breaking\n> changes in the API.\n> \n> In the IoB code camp two weeks ago there were some discussions regarding\n> improvements in the rkisp1 regulation and per-frame-control (PFC). PFC\n> has been an ongoing topic for quite some time now and it seems to slowly\n> converge. Still we were hesitant in breaking the API or introducing new\n> controls before the overall concepts have stabilized enough.\n> \n> To mitigate the pain mentioned above we came up with a minimal changeset\n> that allows to request more than 4 buffers. Only 4 (or the pipeline\n> depth reported by the pipeline) buffers are queued into the device.\n> This has the same benefits as [1] but doesn't require an API break.\n> \n> Updates in Version 1 of the series (after the RFC):\n> - Included the feedback from reviews of the RFC (For etails see the chaneglog in\n> the individual patches)\n> - Included two patches with fixes from Sven an Kieran\n> - I modified patch (6/6) from Kieran a bit after the review comment from\n> Laurent. @Kieran could you have a look at that as I don't have a easy way to\n> replicate the issue? Ideally I would like to merge that patch into patch 2/6\n> as it fixes an issue introduced there. For now I left it separate for easier\n> review.\n\nI haven't understood yet how they are tied for 6/6 and 2/6 - so I do'nt\nmind them being merged or kept separate. If the request queue was\nalready there before this series then a separate patch is fine I think.\nIt seems like 2/6 is just moving the queue into the camera data ?\n\nWhichever way you prefer.\n\nHowever there are some intereseting CI failures on this series:\n\nhttps://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1460735\n\n\n> \n> Best regards,\n> Stefan\n> \n> Kieran Bingham (1):\n> libcamera: pipeline_handler: cancel waiting requests first\n> \n> Nícolas F. R. A. Prado (1):\n> libcamera: pipeline: rkisp1: Don't rely on bufferCount\n> \n> Stefan Klug (4):\n> libcamera: pipeline_handler: Move waitingRequests_ into camera class\n> libcamera: pipeline_handler: Allow to limit the number of queued\n> requests\n> pipeline: rkisp1: Limit the maximum number of buffers queued in\n> pipeline: rkisp1: Properly handle the bufferCount set in the stream\n> configuration\n> \n> include/libcamera/internal/camera.h | 2 +\n> include/libcamera/internal/pipeline_handler.h | 9 ++--\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 28 ++++++----\n> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 7 +--\n> src/libcamera/pipeline/rkisp1/rkisp1_path.h | 4 +-\n> src/libcamera/pipeline_handler.cpp | 54 +++++++++++++------\n> 6 files changed, 65 insertions(+), 39 deletions(-)\n> \n> -- \n> 2.48.1\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 867FDC3237\n\tfor ;\n\tMon, 30 Jun 2025 10:45:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 94D6F68E04;\n\tMon, 30 Jun 2025 12:45:39 +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 ADDAE61527\n\tfor ;\n\tMon, 30 Jun 2025 12:45:38 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0040AC75;\n\tMon, 30 Jun 2025 12:45: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=\"RpE+e8dA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751280317;\n\tbh=3elEu5f+435BoSkf9I4m5HImn0NQpqI/sa1F027KpvA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=RpE+e8dAzDoEqL4v8v65WrO/gxEspt4pRSVcZj3TQQ9IjpE7g5y/4ZtNAji4zAOxb\n\txg+RJN5+z8DubUJsSVZlUX4eCpbbT66V2KkEM1nmECJfLVHHTUYK6uc8QV57LQyAqO\n\te4w3hEoKRWgjx++le+dT5tJD0T3NGR6I9I1AYXBc=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250630081126.2384387-1-stefan.klug@ideasonboard.com>","References":"<20250630081126.2384387-1-stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH v1 0/6] rkisp1: Allow usage of more than 4 buffers","From":"Kieran Bingham ","Cc":"Stefan Klug ","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 30 Jun 2025 11:45:35 +0100","Message-ID":"<175128033580.2538045.10290009961564519689@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]