[{"id":1488,"web_url":"https://patchwork.libcamera.org/comment/1488/","msgid":"<20190419121423.GG7006@pendragon.ideasonboard.com>","date":"2019-04-19T12:14:23","subject":"Re: [libcamera-devel] [PATCH v7 4/8] libcamera: ipu3: Use roles in\n\tstream configuration","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Fri, Apr 19, 2019 at 12:18:35PM +0200, Jacopo Mondi wrote:\n> Use and inspect the stream roles provided by the application to\n> streamConfiguration() to assign streams to their intended roles and\n> return a default configuration associated with them.\n> \n> Support a limited number of usages, with the viewfinder stream able to\n> capture both continuous video streams and still images, and the main\n> output stream supporting still images images. This is an artificial\n\ns/images images/images/\n\n> limitation until we figure out the exact capabilities of the hardware.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 113 ++++++++++++++++++++-------\n> 1 file changed, 83 insertions(+), 30 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 9b2d96f22b2b..45edc24b5d4c 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -5,6 +5,7 @@\n> * ipu3.cpp - Pipeline handler for Intel IPU3\n> */\n> \n> +#include \n> #include \n> #include \n> #include \n> @@ -222,38 +223,90 @@ CameraConfiguration\n> PipelineHandlerIPU3::streamConfiguration(Camera *camera,\n> \t\t\t\t\t const std::vector &usages)\n> {\n> -\tCameraConfiguration configs;\n> \tIPU3CameraData *data = cameraData(camera);\n> -\tStreamConfiguration config = {};\n> +\tCameraConfiguration cameraConfig;\n> +\tstd::set streams = {\n> +\t\t&data->outStream_,\n> +\t\t&data->vfStream_,\n> +\t};\n> \n> -\t/*\n> -\t * FIXME: Soraka: the maximum resolution reported by both sensors\n> -\t * (2592x1944 for ov5670 and 4224x3136 for ov13858) are returned as\n> -\t * default configurations but they're not correctly processed by the\n> -\t * ImgU. Resolutions up tp 2560x1920 have been validated.\n> -\t *\n> -\t * \\todo Clarify ImgU alignement requirements.\n> -\t */\n> -\tconfig.width = 2560;\n> -\tconfig.height = 1920;\n> -\tconfig.pixelFormat = V4L2_PIX_FMT_NV12;\n> -\tconfig.bufferCount = IPU3_BUFFER_COUNT;\n> -\n> -\tconfigs[&data->outStream_] = config;\n> -\tLOG(IPU3, Debug)\n> -\t\t<< \"Stream '\" << data->outStream_.name_ << \"' set to \"\n> -\t\t<< config.width << \"x\" << config.height << \"-0x\"\n> -\t\t<< std::hex << std::setfill('0') << std::setw(8)\n> -\t\t<< config.pixelFormat;\n> -\n> -\tconfigs[&data->vfStream_] = config;\n> -\tLOG(IPU3, Debug)\n> -\t\t<< \"Stream '\" << data->vfStream_.name_ << \"' set to \"\n> -\t\t<< config.width << \"x\" << config.height << \"-0x\"\n> -\t\t<< std::hex << std::setfill('0') << std::setw(8)\n> -\t\t<< config.pixelFormat;\n> -\n> -\treturn configs;\n> +\tfor (const StreamUsage &usage : usages) {\n> +\t\tstd::vector::iterator found;\n\nThis variable isn't used anymore. No warning from the compiler ?\n\n> +\t\tStreamUsage::Role role = usage.role();\n> +\t\tStreamConfiguration streamConfig = {};\n> +\t\tIPU3Stream *stream = nullptr;\n> +\n> +\t\tif (role == StreamUsage::Role::StillCapture) {\n> +\t\t\t/*\n> +\t\t\t * Don't allow viewfinder or video capture on the\n> +\t\t\t * 'output stream. This is an artificial limitation\n> +\t\t\t * until we figure out the capabilities of the\n> +\t\t\t * hardware.\n> +\t\t\t */\n> +\t\t\tif (streams.find(&data->outStream_) != streams.end())\n> +\t\t\t\tstream = &data->outStream_;\n> +\t\t\telse if (streams.find(&data->vfStream_) != streams.end())\n> +\t\t\t\tstream = &data->vfStream_;\n> +\t\t\telse\n> +\t\t\t\tgoto error;\n> +\n> +\t\t\t/*\n> +\t\t\t * FIXME: Soraka: the maximum resolution reported by\n> +\t\t\t * both sensors (2592x1944 for ov5670 and 4224x3136 for\n> +\t\t\t * ov13858) are returned as default configurations but\n> +\t\t\t * they're not correctly processed by the ImgU.\n> +\t\t\t * Resolutions up tp 2560x1920 have been validated.\n> +\t\t\t *\n> +\t\t\t * \\todo Clarify ImgU alignment requirements.\n> +\t\t\t */\n> +\t\t\tstreamConfig.width = 2560;\n> +\t\t\tstreamConfig.height = 1920;\n> +\t\t} else if (role == StreamUsage::Role::Viewfinder ||\n> +\t\t\t role == StreamUsage::Role::VideoRecording) {\n> +\t\t\t/*\n> +\t\t\t * We can't use the 'output' stream for viewfinder or\n> +\t\t\t * video capture usages.\n> +\t\t\t */\n\nI'd duplicate here the information from the commit message explaining\nthat the limitation is artificial, maybe with a \\todo\n\n> +\t\t\tif (streams.find(&data->vfStream_) == streams.end())\n> +\t\t\t\tgoto error;\n> +\n> +\t\t\tstream = &data->vfStream_;\n> +\n> +\n\nExtra blank line.\n\n> +\t\t\t/*\n> +\t\t\t * Align the requested viewfinder size to the\n> +\t\t\t * maximum available sensor resolution and to the\n> +\t\t\t * IPU3 alignment constraints.\n> +\t\t\t */\n> +\t\t\tconst Size &res = data->cio2_.sensor_->resolution();\n> +\t\t\tunsigned int width = std::min(usage.size().width,\n> +\t\t\t\t\t\t res.width);\n> +\t\t\tunsigned int height = std::min(usage.size().height,\n> +\t\t\t\t\t\t res.height);\n\nShould we set a minimum size too ?\n\nWe need to figure out a policy for the streamsConfiguration() operation.\nIt doesn't have to be solved as part of this series, but we need to\ndocument it.\n\n> +\t\t\tstreamConfig.width = width & ~7;\n> +\t\t\tstreamConfig.height = height & ~3;\n> +\t\t}\n> +\n> +\t\tstreams.erase(stream);\n> +\n> +\t\tstreamConfig.pixelFormat = V4L2_PIX_FMT_NV12;\n> +\t\tstreamConfig.bufferCount = IPU3_BUFFER_COUNT;\n> +\n> +\t\tcameraConfig[stream] = streamConfig;\n> +\n> +\t\tLOG(IPU3, Debug)\n> +\t\t\t<< \"Stream '\" << stream->name_ << \"' format set to \"\n> +\t\t\t<< streamConfig.width << \"x\" << streamConfig.height\n> +\t\t\t<< \"-0x\" << std::hex << std::setfill('0')\n> +\t\t\t<< std::setw(8) << streamConfig.pixelFormat;\n> +\t}\n> +\n> +\treturn cameraConfig;\n> +\n> +error:\n> +\tLOG(IPU3, Error) << \"Requested stream roles not supported\";\n> +\n> +\treturn CameraConfiguration{};\n> }\n> \n> int PipelineHandlerIPU3::configureStreams(Camera *camera,","headers":{"Return-Path":"","Received":["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 8D31160B2E\n\tfor ;\n\tFri, 19 Apr 2019 14:14:32 +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 00BBF31A;\n\tFri, 19 Apr 2019 14:14:31 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1555676072;\n\tbh=eBs+8P9idsJB7eZimM6+/h0Tfzc8gf5nl/y5yRMoG48=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ktWaKUPV1dXmwR4FGm0MQn27YcD9Tfb7LLNLBwCgjyOy0egcgFvlJO+U6+VA3mnmG\n\tJzpf9TSd/0ngXZGNKQ2Tm4y6CyF1zangyNKijcZcr3SuOBY30KttiGXzsUPIW5kufX\n\tNxGRq9idugrHWwbLZ3yDx3+P97zlQRI9le2s2CGk=","Date":"Fri, 19 Apr 2019 15:14:23 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190419121423.GG7006@pendragon.ideasonboard.com>","References":"<20190419101839.10337-1-jacopo@jmondi.org>\n\t<20190419101839.10337-5-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190419101839.10337-5-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v7 4/8] libcamera: ipu3: Use roles in\n\tstream configuration","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 19 Apr 2019 12:14:32 -0000"}}]