[{"id":11250,"web_url":"https://patchwork.libcamera.org/comment/11250/","msgid":"<20200708152318.GG20298@pendragon.ideasonboard.com>","date":"2020-07-08T15:23:18","subject":"Re: [libcamera-devel] [PATCH v4 16/21] libcamera: rkisp1: Fill\n\tstride and frameSize at config validation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Wed, Jul 08, 2020 at 10:44:12PM +0900, Paul Elder wrote:\n> Fill the stride and frameSize fields of the StreamConfiguration at\n> configuration validation time instead of at camera configuration time.\n> This allows applications to get the stride when trying a configuration\n> without modifying the active configuration of the camera.\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> Changes in v4:\n> - mention motivation in commit message\n> - get stride and frameSize from tryFormat on the capture video node\n> \n> New in v3\n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 20 ++++++++++++++++----\n> 1 file changed, 16 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 3c3f3f3..5ac2ecc 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -115,9 +115,9 @@ public:\n> class RkISP1CameraData : public CameraData\n> {\n> public:\n> -\tRkISP1CameraData(PipelineHandler *pipe)\n> +\tRkISP1CameraData(PipelineHandler *pipe, V4L2VideoDevice *video)\n> \t\t: CameraData(pipe), sensor_(nullptr), frame_(0),\n> -\t\t frameInfo_(pipe)\n> +\t\t frameInfo_(pipe), video_(video)\n> \t{\n> \t}\n> \n> @@ -135,6 +135,8 @@ public:\n> \tRkISP1Frames frameInfo_;\n> \tRkISP1Timeline timeline_;\n> \n> +\tV4L2VideoDevice *video_;\n> +\n\nI don't think this is necessary, you can already access the pipe with\n\n\tPipelineHandlerRkISP1 *pipe =\n\t\tstatic_cast(data->pipe_);\n\nand use pipe->video_ instead of data->video_.\n\nReviewed-by: Laurent Pinchart \n\n> private:\n> \tvoid queueFrameAction(unsigned int frame,\n> \t\t\t const IPAOperationData &action);\n> @@ -535,6 +537,17 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> \n> \tcfg.bufferCount = RKISP1_BUFFER_COUNT;\n> \n> +\tV4L2DeviceFormat format = {};\n> +\tformat.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);\n> +\tformat.size = cfg.size;\n> +\n> +\tint ret = data_->video_->tryFormat(&format);\n> +\tif (ret)\n> +\t\treturn Invalid;\n> +\n> +\tcfg.stride = format.planes[0].bpl;\n> +\tcfg.frameSize = format.planes[0].size;\n> +\n> \treturn status;\n> }\n> \n> @@ -683,7 +696,6 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \t\treturn ret;\n> \n> \tcfg.setStream(&data->stream_);\n> -\tcfg.stride = outputFormat.planes[0].bpl;\n> \n> \treturn 0;\n> }\n> @@ -934,7 +946,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> \tint ret;\n> \n> \tstd::unique_ptr data =\n> -\t\tstd::make_unique(this);\n> +\t\tstd::make_unique(this, video_);\n> \n> \tControlInfoMap::Map ctrls;\n> \tctrls.emplace(std::piecewise_construct,","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 3FE61BD790\n\tfor ;\n\tWed, 8 Jul 2020 15:23:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C380161153;\n\tWed, 8 Jul 2020 17:23:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3F24561138\n\tfor ;\n\tWed, 8 Jul 2020 17:23:25 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 98D45543;\n\tWed, 8 Jul 2020 17:23:24 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"L/cxeTsW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1594221804;\n\tbh=eNHmp9WzXC/ppB1QBANjr8qb2Nk/S+p4KHaewmK8jvE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=L/cxeTsWZTGWrP55ySvAAo7EO2/u21YZBJUSy5yWwfi5BmSFrlS1mzuMI1IVw22pj\n\t6rti3R2p809kr/dWJODlJvAqf7H3rx+MaXehaBL3+AeAn7yIlxUKi1j45MPsJCL47x\n\tQXhjS1gIR+IdtaAtJg465R8c2oYtqM/KM9/uqe/8=","Date":"Wed, 8 Jul 2020 18:23:18 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"<20200708152318.GG20298@pendragon.ideasonboard.com>","References":"<20200708134417.67747-1-paul.elder@ideasonboard.com>\n\t<20200708134417.67747-17-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200708134417.67747-17-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 16/21] libcamera: rkisp1: Fill\n\tstride and frameSize at config validation","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11277,"web_url":"https://patchwork.libcamera.org/comment/11277/","msgid":"<20200709081822.GE66701@pyrite.rasen.tech>","date":"2020-07-09T08:18:22","subject":"Re: [libcamera-devel] [PATCH v4 16/21] libcamera: rkisp1: Fill\n\tstride and frameSize at config validation","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nThank you for the review.\n\nOn Wed, Jul 08, 2020 at 06:23:18PM +0300, Laurent Pinchart wrote:\n> Hi Paul,\n> \n> Thank you for the patch.\n> \n> On Wed, Jul 08, 2020 at 10:44:12PM +0900, Paul Elder wrote:\n> > Fill the stride and frameSize fields of the StreamConfiguration at\n> > configuration validation time instead of at camera configuration time.\n> > This allows applications to get the stride when trying a configuration\n> > without modifying the active configuration of the camera.\n> > \n> > Signed-off-by: Paul Elder \n> > \n> > ---\n> > Changes in v4:\n> > - mention motivation in commit message\n> > - get stride and frameSize from tryFormat on the capture video node\n> > \n> > New in v3\n> > ---\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 20 ++++++++++++++++----\n> > 1 file changed, 16 insertions(+), 4 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 3c3f3f3..5ac2ecc 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -115,9 +115,9 @@ public:\n> > class RkISP1CameraData : public CameraData\n> > {\n> > public:\n> > -\tRkISP1CameraData(PipelineHandler *pipe)\n> > +\tRkISP1CameraData(PipelineHandler *pipe, V4L2VideoDevice *video)\n> > \t\t: CameraData(pipe), sensor_(nullptr), frame_(0),\n> > -\t\t frameInfo_(pipe)\n> > +\t\t frameInfo_(pipe), video_(video)\n> > \t{\n> > \t}\n> > \n> > @@ -135,6 +135,8 @@ public:\n> > \tRkISP1Frames frameInfo_;\n> > \tRkISP1Timeline timeline_;\n> > \n> > +\tV4L2VideoDevice *video_;\n> > +\n> \n> I don't think this is necessary, you can already access the pipe with\n> \n> \tPipelineHandlerRkISP1 *pipe =\n> \t\tstatic_cast(data->pipe_);\n> \n> and use pipe->video_ instead of data->video_.\n\nvideo_ is private in PipelineHandlerRkISP1 :/\n\n> Reviewed-by: Laurent Pinchart \n> \n> > private:\n> > \tvoid queueFrameAction(unsigned int frame,\n> > \t\t\t const IPAOperationData &action);\n> > @@ -535,6 +537,17 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > \n> > \tcfg.bufferCount = RKISP1_BUFFER_COUNT;\n> > \n> > +\tV4L2DeviceFormat format = {};\n> > +\tformat.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);\n> > +\tformat.size = cfg.size;\n> > +\n> > +\tint ret = data_->video_->tryFormat(&format);\n> > +\tif (ret)\n> > +\t\treturn Invalid;\n> > +\n> > +\tcfg.stride = format.planes[0].bpl;\n> > +\tcfg.frameSize = format.planes[0].size;\n> > +\n> > \treturn status;\n> > }\n> > \n> > @@ -683,7 +696,6 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> > \t\treturn ret;\n> > \n> > \tcfg.setStream(&data->stream_);\n> > -\tcfg.stride = outputFormat.planes[0].bpl;\n> > \n> > \treturn 0;\n> > }\n> > @@ -934,7 +946,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> > \tint ret;\n> > \n> > \tstd::unique_ptr data =\n> > -\t\tstd::make_unique(this);\n> > +\t\tstd::make_unique(this, video_);\n> > \n> > \tControlInfoMap::Map ctrls;\n> > \tctrls.emplace(std::piecewise_construct,\n\n\nThanks,\n\nPaul","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 81751BD792\n\tfor ;\n\tThu, 9 Jul 2020 08:18:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 10BD661186;\n\tThu, 9 Jul 2020 10:18:30 +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 78B0461158\n\tfor ;\n\tThu, 9 Jul 2020 10:18:29 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 03D0C289;\n\tThu, 9 Jul 2020 10:18:27 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ivh3Tab4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1594282709;\n\tbh=PURZ/VbBwlMp2xjNAOTnBxin0afnAxbmRprPvNv9S34=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ivh3Tab4/ueNgn02jlt+PFCLNxFejNwgLFh7O8x9S8P5yp9AKYDbzvCklM5uk+700\n\tw5WMto2/Aj/ofrBv0+10Dt+OJJ2QX+bCiJ5KmG++nT9P+Lk3durXDbztWkVzfOzZJZ\n\tBtMh40TW7G+28tqs67V9vn+z0LEhvgunnOR5b3dw=","Date":"Thu, 9 Jul 2020 17:18:22 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart ","Message-ID":"<20200709081822.GE66701@pyrite.rasen.tech>","References":"<20200708134417.67747-1-paul.elder@ideasonboard.com>\n\t<20200708134417.67747-17-paul.elder@ideasonboard.com>\n\t<20200708152318.GG20298@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200708152318.GG20298@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 16/21] libcamera: rkisp1: Fill\n\tstride and frameSize at config validation","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]