[{"id":26365,"web_url":"https://patchwork.libcamera.org/comment/26365/","msgid":"<167485486104.3753862.14112194092521843644@Monstersaurus>","date":"2023-01-27T21:27:41","subject":"Re: [libcamera-devel] [PATCH v6 09/15] libcamera: camera: Validate\n\tMandatoryStream in queueRequest()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck via libcamera-devel (2023-01-27 15:43:16)\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 \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 2d947a442bff..b0b0a5cb5bb3 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -1144,6 +1144,18 @@ int Camera::queueRequest(Request *request)\n> }\n> }\n> \n> + for (auto const stream : d->activeStreams_) {\n> + const StreamConfiguration &config = stream->configuration();\n> + FrameBuffer *buffer = request->findBuffer(stream);\n> +\n> + if (!buffer && config.hints & StreamConfiguration::Hint::MandatoryStream) {\n> + LOG(Camera, Error)\n> + << \"No buffer provided for mandatory stream\";\n> + request->_d()->reset();\n\nHrm ... that's interesting.\n\nShould resetting the request be left to the application? or done on\nerrors?\n\nIf we do reset here, maybe we need to reset other errors paths in this\nfunction too. (I'm planning to add a couple more checks in here, so I'm\ninterested in if it's better for apps to automatically reset or not).\n\n--\nKieran\n\n\n\n\n> + return -ENOMEM;\n> + }\n> + }\n> +\n> d->pipe_->invokeMethod(&PipelineHandler::queueRequest,\n> ConnectionTypeQueued, request);\n> \n> -- \n> 2.25.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 7AC7CBDC71\n\tfor ;\n\tFri, 27 Jan 2023 21:27:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C59B361EFA;\n\tFri, 27 Jan 2023 22:27:45 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2F3A161EF9\n\tfor ;\n\tFri, 27 Jan 2023 22:27:44 +0100 (CET)","from pendragon.ideasonboard.com\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 A4F2E4A7;\n\tFri, 27 Jan 2023 22:27:43 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674854865;\n\tbh=SRUPFD28Ldf7oX9BsY/QaItiUI0fIYkHYuqI9oPMhLY=;\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=SAs5IXK1GrM1yKozgfL/PNnwoXbGbTlv8IGwM0ncUxKBLhm+yGQwZOc8Y9TUVd7iH\n\tcoMjO8KLcE/IL5Ji/DQwPZXkiITOfB6uQvKq/0hdA/FVqBVX0fRypsgTxOvHo7L1QJ\n\tQYtFo1isWUnb+xP/x511iwWxdhjmVv/pXso3ED51yCzqT5a02hJqf664L9dFuA7abt\n\tbpts2buk4ssHEljk7EwMcX8oWkTapKqTPsJQp6r2H3sQps7pMeoMAimyP4++HHtZpj\n\tQULGdBf7DmDcJRwO+gQrhOKxITRnyxycHh9TBdSqN3/deDCBOZdgSjYCND8f9nqCps\n\t+I2gJV1iaZCag==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1674854863;\n\tbh=SRUPFD28Ldf7oX9BsY/QaItiUI0fIYkHYuqI9oPMhLY=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=bsZbDN6zVKdyrlmVk1WfJ1lJ/3do3QisTNlN1c0q1jnq9TaDiksAqaY4PRAOHYbM5\n\tnNp2yHZIVBoafDZOjP/8oQp+4/g8VTVPoELl2XielLmLdmGb9Vk6Y/z89bAiKcAfME\n\tdmHazN1cyBDzEtyC/j4ZYUBR3BayQL4Wkqqli62s="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"bsZbDN6z\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230127154322.29019-10-naush@raspberrypi.com>","References":"<20230127154322.29019-1-naush@raspberrypi.com>\n\t<20230127154322.29019-10-naush@raspberrypi.com>","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 27 Jan 2023 21:27:41 +0000","Message-ID":"<167485486104.3753862.14112194092521843644@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v6 09/15] libcamera: camera: Validate\n\tMandatoryStream in queueRequest()","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26385,"web_url":"https://patchwork.libcamera.org/comment/26385/","msgid":"","date":"2023-01-31T06:39:34","subject":"Re: [libcamera-devel] [PATCH v6 09/15] libcamera: camera: Validate\n\tMandatoryStream in queueRequest()","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Kierna,\n\nOn Fri, 27 Jan 2023 at 21:27, Kieran Bingham\n wrote:\n>\n> Quoting Naushir Patuck via libcamera-devel (2023-01-27 15:43:16)\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 \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 2d947a442bff..b0b0a5cb5bb3 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -1144,6 +1144,18 @@ int Camera::queueRequest(Request *request)\n> > }\n> > }\n> >\n> > + for (auto const stream : d->activeStreams_) {\n> > + const StreamConfiguration &config = stream->configuration();\n> > + FrameBuffer *buffer = request->findBuffer(stream);\n> > +\n> > + if (!buffer && config.hints & StreamConfiguration::Hint::MandatoryStream) {\n> > + LOG(Camera, Error)\n> > + << \"No buffer provided for mandatory stream\";\n> > + request->_d()->reset();\n>\n> Hrm ... that's interesting.\n>\n> Should resetting the request be left to the application? or done on\n> errors?\n\nYes, I wasn't 100% sure about this, but the Request destructor does a\nwhole bunch of buffer completion/cancellation signal calls, and I did\nnot think that ought to be done when the request has been rejected\nsynchronously.\n\n>\n> If we do reset here, maybe we need to reset other errors paths in this\n> function too. (I'm planning to add a couple more checks in here, so I'm\n> interested in if it's better for apps to automatically reset or not).\n\nI think it would be appropriate to do the same for the other error\npaths in this function, but I would like to confirm this is the right\nthing to do there.\n\nRegards,\nNaush\n\n>\n> --\n> Kieran\n>\n>\n>\n>\n> > + return -ENOMEM;\n> > + }\n> > + }\n> > +\n> > d->pipe_->invokeMethod(&PipelineHandler::queueRequest,\n> > ConnectionTypeQueued, request);\n> >\n> > --\n> > 2.25.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 B4F51BEFBE\n\tfor ;\n\tTue, 31 Jan 2023 06:39:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6798E625E4;\n\tTue, 31 Jan 2023 07:39:53 +0100 (CET)","from mail-yb1-xb2a.google.com (mail-yb1-xb2a.google.com\n\t[IPv6:2607:f8b0:4864:20::b2a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 684C661EF5\n\tfor ;\n\tTue, 31 Jan 2023 07:39:51 +0100 (CET)","by mail-yb1-xb2a.google.com with SMTP id 129so16977043ybb.0\n\tfor ;\n\tMon, 30 Jan 2023 22:39:51 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1675147193;\n\tbh=qcd04HOk5UH6yg2dCxoR94/Lb7+DsgaoEK3U1XJwq6c=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=PCZAmJVhQOYO5gV2nXe/62Q0o2PoObR5Zhc1dlxTWnAl5EoHUd/p/zhwZSzeHZ2r/\n\tAFvOkPnVm8y1iDe6X/PWQjJNOZq76MBhWkOGUm5LQeo0kfpQr/Q9CZ7muuvgKEFDPt\n\tAJsX9BDs27i3qOaApEUPNLw1UsygD6QFSDolP3CEpHK473gUtLWxtDlefLCMcElrzi\n\t+NaAKkdV291fw748Lr6JMF/0fqCuLSnFq+O6swHQUgWXfng0c5jHZMYc1iadJoBqfH\n\tFb/TUGEjDbGBYhM2y198PegRf/8/ACEX4OY1E885lbyjFVdjmzbmMgYO4YU0sjM8ZW\n\t7FqTOMtmnkHqQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=A3PYEXZO7noq4ZQxsBlrcc8opkjeV1CD9t9SkPottag=;\n\tb=daGhrk4T5iMtv3p61hoGW2AS4a7yhKyfQYb+wXD9uZHApA87KDvRdBZWWbxiv9FYBb\n\tzzHjTwFIljrXr0mdInqIqCZkmf4SBriy4MRf4QiY42qasE8CAeIxTLbCAGqeocF2SAZx\n\tZS7oM+tNAUvrEnTlTgRaA+4ApjeSdM1iFhp9tgyX61KGNtnmPfQnxOvVMZkMDoo7yaID\n\tDZ/Vh+f7CWC4X5wdd2lS1baCPKkS+zYqULktbqO4v+AwUY1csY6lp/3o2eaMBfvP6oQV\n\tlwK55nsNfrrJ0t6N2sUZknImw1WXrqAfe0RQxae3IrCb8njsz+VmwXfFFDxb2OZgLi4N\n\t+eoA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"daGhrk4T\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=A3PYEXZO7noq4ZQxsBlrcc8opkjeV1CD9t9SkPottag=;\n\tb=VXY9ImKMjeTmKygly6oKoUIc2jIE1f5L796I1u2pDekRk9BeI6TvWtqK165wyFsaHk\n\tq3W2xEz3mrrwSCQxyrofpoWge9hnfAdkAwZQ2HOrLclebQjKv7057kKDzmdDaf0kjqXP\n\tvRMh9DPcjlxrSh+SduvxcNVjXfDQYm/yL204qYmKuMMLQmS6x8/SKNLrCCVIXcfyDpUF\n\tFGadbR1IPMfgVxSEb0c2I+q3sQWlz5bTVMc2Q07vKE5mNYiBXM9KAtWHoW1UERv0MXn0\n\tSsj8ZGgj2/TS3ePQXpDGi8tI40e3pvXnz3ChnKWk23lzKYHYGBToHf12PYfSL1vQybwd\n\tk08g==","X-Gm-Message-State":"AFqh2kpZ0JY1pfMlgcalqHjCMHT8v9+PMJRSXmu98OH0MD44Z+N/gtf/\n\tEar9pquYBiMkoUcWBHYZNDDZPXUKsXW0Dl2AtFZoWkL3g+mARklQMZ6Ilg==","X-Google-Smtp-Source":"AMrXdXtOR94LpGoZqiN7/Oe37A6O6rHkxPgykGJxUReaBr+R5I75ghzDZ3mJSlUjJTRRNPuS1IuhHyKPp8fJ9jphxMU=","X-Received":"by 2002:a25:7583:0:b0:783:bc5e:3d67 with SMTP id\n\tq125-20020a257583000000b00783bc5e3d67mr4040382ybc.524.1675147190317;\n\tMon, 30 Jan 2023 22:39:50 -0800 (PST)","MIME-Version":"1.0","References":"<20230127154322.29019-1-naush@raspberrypi.com>\n\t<20230127154322.29019-10-naush@raspberrypi.com>\n\t<167485486104.3753862.14112194092521843644@Monstersaurus>","In-Reply-To":"<167485486104.3753862.14112194092521843644@Monstersaurus>","Date":"Tue, 31 Jan 2023 06:39:34 +0000","Message-ID":"","To":"Kieran Bingham ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v6 09/15] libcamera: camera: Validate\n\tMandatoryStream in queueRequest()","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","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]