[{"id":26527,"web_url":"https://patchwork.libcamera.org/comment/26527/","msgid":"<20230302100305.hhqxnrssfgyl6kdz@uno.localdomain>","date":"2023-03-02T10:03:05","subject":"Re: [libcamera-devel] [PATCH v1 4/8] libcamera: camera: Validate\n\tMandatoryStream in queueRequest()","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Fri, Feb 03, 2023 at 09:44:20AM +0000, Naushir Patuck via libcamera-devel wrote:\n> Add some validation in queueRequest() to ensure that a frame buffer is\n> provided in a Request if the MandatoryStream flag has been set in the\n> StreamConfiguration for every active stream.\n>\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> ---\n>  src/libcamera/camera.cpp | 12 ++++++++++++\n>  1 file changed, 12 insertions(+)\n>\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 0da167a7bc51..c0a368c1e66c 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -1146,6 +1146,18 @@ int Camera::queueRequest(Request *request)\n>  \t\t}\n>  \t}\n>\n> +\tfor (auto const stream : d->activeStreams_) {\n> +\t\tconst StreamConfiguration &config = stream->configuration();\n> +\t\tFrameBuffer *buffer = request->findBuffer(stream);\n> +\n> +\t\tif (!buffer && config.hints & StreamConfiguration::Hint::MandatoryStream) {\n> +\t\t\tLOG(Camera, Error)\n> +\t\t\t\t<< \"No buffer provided for mandatory stream\";\n> +\t\t\trequest->_d()->reset();\n> +\t\t\treturn -ENOMEM;\n\nENOMEM is documented as\n * \\retval -ENOMEM No buffer memory was available to handle the request\n\nI would rather use EINVAL\n * \\retval -EINVAL The request is invalid\n\nWe don't document the expectations on the Request being reset by the\nCamera if it fails. Why do you need to do it here (not saying it's\nwrong, just wondering if we should document that)\n\n\n> +\t\t}\n> +\t}\n> +\n>  \td->pipe_->invokeMethod(&PipelineHandler::queueRequest,\n>  \t\t\t       ConnectionTypeQueued, request);\n>\n> --\n> 2.25.1\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 9A9C7BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  2 Mar 2023 10:03:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CF321626C6;\n\tThu,  2 Mar 2023 11:03:08 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0574B62668\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  2 Mar 2023 11:03:07 +0100 (CET)","from ideasonboard.com (host-87-18-61-24.retail.telecomitalia.it\n\t[87.18.61.24])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 614A256A;\n\tThu,  2 Mar 2023 11:03:07 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1677751388;\n\tbh=EPJTfEqx07nuyAE3WLS+xinmTyRuNHZbTmUXVe0JYUc=;\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=e7gXtPm/T0VwGgbm16eombpGYI4/0VCiXyOl6OyKt/QaYL5dZlhSK4pB754ziMgfe\n\t2bndXO/JcC0UskPB5pEoz+U/NThFTnixX4hy+CJySWc12t/ZkcRzsPTro/OeuR5nxL\n\t2d3P6tQ0fqbqhjg1wU9Kqy0ZP7pmylYNXLzO+LJdb27p3SoutjSP/rUDRpZXgsAcAF\n\tifOcApr6mguSYCtdmwDaG1PFvA/tKpAHVre8meRqr/00I2wervgttFqiZdqv0EBiM/\n\t+C0Qilwr7xbQcCSO/JaJaP2Ja5GQU/VIoxIC194KeMvTGZFXn3OJLFSQMlEd08g9Hu\n\ty/R47VyPGXr2g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1677751387;\n\tbh=EPJTfEqx07nuyAE3WLS+xinmTyRuNHZbTmUXVe0JYUc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mH8eXeg1RLyM4FcRk9CLZHxa8rBzO2HA3Ynu/Lq8TCH/xUYYV2zlEyLopXzfmZ7RL\n\tK3/dMAJ0lN+fq84BFTLWBExzc89xAFzrFOth6ArTYro4CmqewCcx01TEZtJUgnUtj0\n\tSakWnXMoOdJ7FE6JmJgJ4UIGIFkuKVVH8KYj1cl4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"mH8eXeg1\"; dkim-atps=neutral","Date":"Thu, 2 Mar 2023 11:03:05 +0100","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<20230302100305.hhqxnrssfgyl6kdz@uno.localdomain>","References":"<20230203094424.25243-1-naush@raspberrypi.com>\n\t<20230203094424.25243-5-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230203094424.25243-5-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v1 4/8] libcamera: camera: Validate\n\tMandatoryStream in queueRequest()","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":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo.mondi@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>"}}]