[{"id":4698,"web_url":"https://patchwork.libcamera.org/comment/4698/","msgid":"<20200501144607.GH5951@pendragon.ideasonboard.com>","date":"2020-05-01T14:46:07","subject":"Re: [libcamera-devel] [PATCH] libcamera: stream: Expose stride value","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Fri, May 01, 2020 at 04:30:21PM +0200, Niklas Söderlund wrote:\n> Expose the image stride which may be retrieved after a video device\n> have been configured. It may only be retrieved at that point as the\n\ns/have/has/\n\n> assignment of video devices takes place at this point.\n> \n> In the future video devices should be assign at configuration validation\n\ns/assign/assigned/\n\n> time and the stride value retrieved at that point.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> include/libcamera/stream.h | 1 +\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 42 ++++++++++++--------\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 1 +\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 1 +\n> src/libcamera/pipeline/vimc/vimc.cpp | 1 +\n> src/libcamera/stream.cpp | 7 ++++\n> 6 files changed, 36 insertions(+), 17 deletions(-)\n> \n> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> index 18142dc997bb8885..b3cbea3d52294846 100644\n> --- a/include/libcamera/stream.h\n> +++ b/include/libcamera/stream.h\n> @@ -42,6 +42,7 @@ struct StreamConfiguration {\n> \n> \tPixelFormat pixelFormat;\n> \tSize size;\n> +\tunsigned int stride;\n> \n> \tunsigned int bufferCount;\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index ff33f15f9eac3b68..0f9ffd411a680ad9 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -77,7 +77,8 @@ public:\n> \tint configureInput(const Size &size,\n> \t\t\t V4L2DeviceFormat *inputFormat);\n> \tint configureOutput(ImgUOutput *output,\n> -\t\t\t const StreamConfiguration &cfg);\n> +\t\t\t const StreamConfiguration &cfg,\n> +\t\t\t V4L2DeviceFormat *outputFormat);\n> \n> \tint allocateBuffers(IPU3CameraData *data, unsigned int bufferCount);\n> \tvoid freeBuffers(IPU3CameraData *data);\n> @@ -546,6 +547,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \tIPU3Stream *vfStream = &data->vfStream_;\n> \tCIO2Device *cio2 = &data->cio2_;\n> \tImgUDevice *imgu = data->imgu_;\n> +\tV4L2DeviceFormat outputFormat;\n> \tint ret;\n> \n> \t/*\n> @@ -625,12 +627,15 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \t\t * The RAW still capture stream just copies buffers from the\n> \t\t * internal queue and doesn't need any specific configuration.\n> \t\t */\n> -\t\tif (stream->raw_)\n> -\t\t\tcontinue;\n> -\n> -\t\tret = imgu->configureOutput(stream->device_, cfg);\n> -\t\tif (ret)\n> -\t\t\treturn ret;\n> +\t\tif (stream->raw_) {\n> +\t\t\tcfg.stride = cio2Format.planes[0].bpl;\n> +\t\t} else {\n> +\t\t\tret = imgu->configureOutput(stream->device_, cfg,\n> +\t\t\t\t\t\t &outputFormat);\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n> +\t\t\tcfg.stride = outputFormat.planes[0].bpl;\n> +\t\t}\n> \t}\n> \n> \t/*\n> @@ -639,13 +644,15 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \t * be at least one active stream in the configuration request).\n> \t */\n> \tif (!outStream->active_) {\n> -\t\tret = imgu->configureOutput(outStream->device_, config->at(0));\n> +\t\tret = imgu->configureOutput(outStream->device_, config->at(0),\n> +\t\t\t\t\t &outputFormat);\n> \t\tif (ret)\n> \t\t\treturn ret;\n> \t}\n> \n> \tif (!vfStream->active_) {\n> -\t\tret = imgu->configureOutput(vfStream->device_, config->at(0));\n> +\t\tret = imgu->configureOutput(vfStream->device_, config->at(0),\n> +\t\t\t\t\t &outputFormat);\n> \t\tif (ret)\n> \t\t\treturn ret;\n> \t}\n> @@ -657,7 +664,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \tStreamConfiguration statCfg = {};\n> \tstatCfg.size = cio2Format.size;\n> \n> -\tret = imgu->configureOutput(&imgu->stat_, statCfg);\n> +\tret = imgu->configureOutput(&imgu->stat_, statCfg, &outputFormat);\n> \tif (ret)\n> \t\treturn ret;\n> \n\nThis isn't very nice, but will do for now as we know we'll have to\nrework the code anyway.\n\n> @@ -1166,7 +1173,8 @@ int ImgUDevice::configureInput(const Size &size,\n> * \\return 0 on success or a negative error code otherwise\n> */\n> int ImgUDevice::configureOutput(ImgUOutput *output,\n> -\t\t\t\tconst StreamConfiguration &cfg)\n> +\t\t\t\tconst StreamConfiguration &cfg,\n> +\t\t\t\tV4L2DeviceFormat *outputFormat)\n> {\n> \tV4L2VideoDevice *dev = output->dev;\n> \tunsigned int pad = output->pad;\n> @@ -1183,17 +1191,17 @@ int ImgUDevice::configureOutput(ImgUOutput *output,\n> \tif (output == &stat_)\n> \t\treturn 0;\n> \n> -\tV4L2DeviceFormat outputFormat = {};\n> -\toutputFormat.fourcc = dev->toV4L2PixelFormat(PixelFormat(DRM_FORMAT_NV12));\n> -\toutputFormat.size = cfg.size;\n> -\toutputFormat.planesCount = 2;\n> +\t*outputFormat = {};\n> +\toutputFormat->fourcc = dev->toV4L2PixelFormat(PixelFormat(DRM_FORMAT_NV12));\n> +\toutputFormat->size = cfg.size;\n> +\toutputFormat->planesCount = 2;\n> \n> -\tret = dev->setFormat(&outputFormat);\n> +\tret = dev->setFormat(outputFormat);\n> \tif (ret)\n> \t\treturn ret;\n> \n> \tLOG(IPU3, Debug) << \"ImgU \" << output->name << \" format = \"\n> -\t\t\t << outputFormat.toString();\n> +\t\t\t << outputFormat->toString();\n> \n> \treturn 0;\n> }\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 6aa3178669b40a37..1e81a0048f093d8f 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -681,6 +681,7 @@ 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> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 455693264c2e3e67..f0c1337de8623a6b 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -204,6 +204,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)\n> \t\treturn -EINVAL;\n> \n> \tcfg.setStream(&data->stream_);\n> +\tcfg.stride = format.planes[0].bpl;\n> \n> \treturn 0;\n> }\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index ccfd7f86d15860c5..7019ac322e6c2c59 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -253,6 +253,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> \t\treturn ret;\n> \n> \tcfg.setStream(&data->stream_);\n> +\tcfg.stride = format.planes[0].bpl;\n> \n> \treturn 0;\n> }\n> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> index ef16aaa1b2f4586a..0a2468d92039676c 100644\n> --- a/src/libcamera/stream.cpp\n> +++ b/src/libcamera/stream.cpp\n> @@ -301,6 +301,13 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)\n> * \\brief Stream pixel format\n> */\n> \n> +/**\n> + * \\var StreamConfiguration::stride\n> + * \\brief Stream stride in bytes\n\nMaybe \"Image stride for the stream, in bytes\" ?\n\nI would add\n\n * The stride value reports the number of bytes between the beginning of\n * successive lines in an image buffer for this stream. The value is\n * valid after successfully configuring the camera with this\n * configuration with a call to Camera::Configure().\n\nReviewed-by: Laurent Pinchart \n\n> + * \\todo Update this value when configuration is validated instead of when\n> + * the camera is configured.\n> + */\n> +\n> /**\n> * \\var StreamConfiguration::bufferCount\n> * \\brief Requested number of buffers to allocate for the stream","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 E3C84603F2\n\tfor ;\n\tFri, 1 May 2020 16:46:10 +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 5DA2D72C;\n\tFri, 1 May 2020 16:46:10 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"R5kveRag\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1588344370;\n\tbh=tPzSElD+CZSz0jZqJabaK31mDHEYj6pjVL5ZEgV1b9o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=R5kveRag3WI8nccZ8TXIkdZr8x+MMTmJb3JrBqGfbHtRFD0fAFgstaNOooUxl5ASH\n\tw1wE1JAu9flJUuaz8WsD3Y5jvgjLjDaaJy+V588/bQcA8rjGy9VFJxDLg1nIswHfFe\n\tgjDl5OAhGCjhunnttFZXbjIAvJDNzlytnFRFwn1g=","Date":"Fri, 1 May 2020 17:46:07 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200501144607.GH5951@pendragon.ideasonboard.com>","References":"<20200501143021.404090-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200501143021.404090-1-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: stream: Expose stride value","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","X-List-Received-Date":"Fri, 01 May 2020 14:46:11 -0000"}},{"id":4699,"web_url":"https://patchwork.libcamera.org/comment/4699/","msgid":"<8c32dc76af5e500dde4c3032cc796be2f320a6dd.camel@ndufresne.ca>","date":"2020-05-01T15:05:47","subject":"Re: [libcamera-devel] [PATCH] libcamera: stream: Expose stride value","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Le vendredi 01 mai 2020 à 16:30 +0200, Niklas Söderlund a écrit :\n> Expose the image stride which may be retrieved after a video device\n> have been configured. It may only be retrieved at that point as the\n> assignment of video devices takes place at this point.\n> \n> In the future video devices should be assign at configuration validation\n> time and the stride value retrieved at that point.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> include/libcamera/stream.h | 1 +\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 42 ++++++++++++--------\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 1 +\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 1 +\n> src/libcamera/pipeline/vimc/vimc.cpp | 1 +\n> src/libcamera/stream.cpp | 7 ++++\n> 6 files changed, 36 insertions(+), 17 deletions(-)\n> \n> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> index 18142dc997bb8885..b3cbea3d52294846 100644\n> --- a/include/libcamera/stream.h\n> +++ b/include/libcamera/stream.h\n> @@ -42,6 +42,7 @@ struct StreamConfiguration {\n> \n> \tPixelFormat pixelFormat;\n> \tSize size;\n> +\tunsigned int stride;\n\nIn my opinion, as we have full knowledge of it already, we should\ndirectly start with an array of up to 4 strides. We can implement\nextrapolation for non-mplane V4L2 formats. This way we don't need to\nport the userspace bits twice.\n\n> \n> \tunsigned int bufferCount;\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index ff33f15f9eac3b68..0f9ffd411a680ad9 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -77,7 +77,8 @@ public:\n> \tint configureInput(const Size &size,\n> \t\t\t V4L2DeviceFormat *inputFormat);\n> \tint configureOutput(ImgUOutput *output,\n> -\t\t\t const StreamConfiguration &cfg);\n> +\t\t\t const StreamConfiguration &cfg,\n> +\t\t\t V4L2DeviceFormat *outputFormat);\n> \n> \tint allocateBuffers(IPU3CameraData *data, unsigned int bufferCount);\n> \tvoid freeBuffers(IPU3CameraData *data);\n> @@ -546,6 +547,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \tIPU3Stream *vfStream = &data->vfStream_;\n> \tCIO2Device *cio2 = &data->cio2_;\n> \tImgUDevice *imgu = data->imgu_;\n> +\tV4L2DeviceFormat outputFormat;\n> \tint ret;\n> \n> \t/*\n> @@ -625,12 +627,15 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \t\t * The RAW still capture stream just copies buffers from the\n> \t\t * internal queue and doesn't need any specific configuration.\n> \t\t */\n> -\t\tif (stream->raw_)\n> -\t\t\tcontinue;\n> -\n> -\t\tret = imgu->configureOutput(stream->device_, cfg);\n> -\t\tif (ret)\n> -\t\t\treturn ret;\n> +\t\tif (stream->raw_) {\n> +\t\t\tcfg.stride = cio2Format.planes[0].bpl;\n> +\t\t} else {\n> +\t\t\tret = imgu->configureOutput(stream->device_, cfg,\n> +\t\t\t\t\t\t &outputFormat);\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n> +\t\t\tcfg.stride = outputFormat.planes[0].bpl;\n> +\t\t}\n> \t}\n> \n> \t/*\n> @@ -639,13 +644,15 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \t * be at least one active stream in the configuration request).\n> \t */\n> \tif (!outStream->active_) {\n> -\t\tret = imgu->configureOutput(outStream->device_, config->at(0));\n> +\t\tret = imgu->configureOutput(outStream->device_, config->at(0),\n> +\t\t\t\t\t &outputFormat);\n> \t\tif (ret)\n> \t\t\treturn ret;\n> \t}\n> \n> \tif (!vfStream->active_) {\n> -\t\tret = imgu->configureOutput(vfStream->device_, config->at(0));\n> +\t\tret = imgu->configureOutput(vfStream->device_, config->at(0),\n> +\t\t\t\t\t &outputFormat);\n> \t\tif (ret)\n> \t\t\treturn ret;\n> \t}\n> @@ -657,7 +664,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \tStreamConfiguration statCfg = {};\n> \tstatCfg.size = cio2Format.size;\n> \n> -\tret = imgu->configureOutput(&imgu->stat_, statCfg);\n> +\tret = imgu->configureOutput(&imgu->stat_, statCfg, &outputFormat);\n> \tif (ret)\n> \t\treturn ret;\n> \n> @@ -1166,7 +1173,8 @@ int ImgUDevice::configureInput(const Size &size,\n> * \\return 0 on success or a negative error code otherwise\n> */\n> int ImgUDevice::configureOutput(ImgUOutput *output,\n> -\t\t\t\tconst StreamConfiguration &cfg)\n> +\t\t\t\tconst StreamConfiguration &cfg,\n> +\t\t\t\tV4L2DeviceFormat *outputFormat)\n> {\n> \tV4L2VideoDevice *dev = output->dev;\n> \tunsigned int pad = output->pad;\n> @@ -1183,17 +1191,17 @@ int ImgUDevice::configureOutput(ImgUOutput *output,\n> \tif (output == &stat_)\n> \t\treturn 0;\n> \n> -\tV4L2DeviceFormat outputFormat = {};\n> -\toutputFormat.fourcc = dev->toV4L2PixelFormat(PixelFormat(DRM_FORMAT_NV12));\n> -\toutputFormat.size = cfg.size;\n> -\toutputFormat.planesCount = 2;\n> +\t*outputFormat = {};\n> +\toutputFormat->fourcc = dev->toV4L2PixelFormat(PixelFormat(DRM_FORMAT_NV12));\n> +\toutputFormat->size = cfg.size;\n> +\toutputFormat->planesCount = 2;\n> \n> -\tret = dev->setFormat(&outputFormat);\n> +\tret = dev->setFormat(outputFormat);\n> \tif (ret)\n> \t\treturn ret;\n> \n> \tLOG(IPU3, Debug) << \"ImgU \" << output->name << \" format = \"\n> -\t\t\t << outputFormat.toString();\n> +\t\t\t << outputFormat->toString();\n> \n> \treturn 0;\n> }\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 6aa3178669b40a37..1e81a0048f093d8f 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -681,6 +681,7 @@ 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> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 455693264c2e3e67..f0c1337de8623a6b 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -204,6 +204,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)\n> \t\treturn -EINVAL;\n> \n> \tcfg.setStream(&data->stream_);\n> +\tcfg.stride = format.planes[0].bpl;\n> \n> \treturn 0;\n> }\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index ccfd7f86d15860c5..7019ac322e6c2c59 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -253,6 +253,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> \t\treturn ret;\n> \n> \tcfg.setStream(&data->stream_);\n> +\tcfg.stride = format.planes[0].bpl;\n> \n> \treturn 0;\n> }\n> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> index ef16aaa1b2f4586a..0a2468d92039676c 100644\n> --- a/src/libcamera/stream.cpp\n> +++ b/src/libcamera/stream.cpp\n> @@ -301,6 +301,13 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)\n> * \\brief Stream pixel format\n> */\n> \n> +/**\n> + * \\var StreamConfiguration::stride\n> + * \\brief Stream stride in bytes\n> + * \\todo Update this value when configuration is validated instead of when\n> + * the camera is configured.\n> + */\n> +\n> /**\n> * \\var StreamConfiguration::bufferCount\n> * \\brief Requested number of buffers to allocate for the stream","headers":{"Return-Path":"","Received":["from mail-qt1-x843.google.com (mail-qt1-x843.google.com\n\t[IPv6:2607:f8b0:4864:20::843])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9E8F4601B8\n\tfor ;\n\tFri, 1 May 2020 17:05:50 +0200 (CEST)","by mail-qt1-x843.google.com with SMTP id k12so8126240qtm.4\n\tfor ;\n\tFri, 01 May 2020 08:05:50 -0700 (PDT)","from skullcanyon ([192.222.193.21])\n\tby smtp.gmail.com with ESMTPSA id\n\tw18sm2796741qkw.113.2020.05.01.08.05.48\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 01 May 2020 08:05:48 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ndufresne-ca.20150623.gappssmtp.com\n\theader.i=@ndufresne-ca.20150623.gappssmtp.com header.b=\"nq0SVQ10\"; \n\tdkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20150623.gappssmtp.com; s=20150623;\n\th=message-id:subject:from:to:date:in-reply-to:references:user-agent\n\t:mime-version:content-transfer-encoding;\n\tbh=hLl5L9zSBwtebmO7KPxEJtqj9K5Kd0c/qn1vLQMisns=;\n\tb=nq0SVQ10QX5i0j4DTDYwNsHfClSgSYkC/QDcJ+5Z3faWr5VlL0/CfbS7NW9zu3oCsw\n\t8N2OEFuT782cFGpZADljBe52Bgf6DWRV2E/sS7c+AkpeyWAnpjTfYdYAWF7Lyt0pOP8j\n\tTOW+6ZL1hY062HoVHgQqdnvaXhO0eF5CZEuFRgMiMMVbMjUmIKiMCNKueP/NxkHgV8yG\n\tnxNoxOIEx1Fw1bYbX6AQQvvZxwwWwOCJx065QyU1ESmutWLvfezJ4xXRRPSGce955B4r\n\tU/ELpaOqVqbCg2OEj1+R7WZLAeDYotur7FNyTUAqBdn9tWXZMLDDz4pvz9qu0vM/MQvg\n\tCd9g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:message-id:subject:from:to:date:in-reply-to\n\t:references:user-agent:mime-version:content-transfer-encoding;\n\tbh=hLl5L9zSBwtebmO7KPxEJtqj9K5Kd0c/qn1vLQMisns=;\n\tb=aw0Ayg134GHSaVIEeOZvo7ID+dUJEgC8CqCT+JIv3e1IvLthnRqkJ5EqfsWXGXcOrk\n\tuBuwFNLQXU8eJ3QtwbceYhMPDbsLwZHweNOhF225gjR3Kba5c0u0Riso9Qs5OdvWk9jD\n\twQt3KfRi6/aM1rzyv5jSISyLBQJNq+UHQ+kcG0r3PKne3HTcKjcghCyYk11teUVvcQX0\n\tagbcjKvzsHLYI6mZxqJ8MrK0H9nZojJGenKdxWJc+OeqpnKppDIhOfN4jYllV7JQ+MIJ\n\tmBrXPmCW6KeQ20L31m5EuF5heweGiKTdZiRvav5uMX6Y24nY8bPjD15h80c5iOoM8x/Q\n\ttOzQ==","X-Gm-Message-State":"AGi0PuaEaHXKZKiau9CrwiUJHW/0CD2L2EwfZDPAY9qCgPCnYjUGAkYa\n\tVLodqxqg16d8cgyJOAAzjW1MiQ==","X-Google-Smtp-Source":"APiQypIzn+CcXg1+WmPpXPywkcC5T4sA2JbgUYaocEHCu5yPNVj0t3EnPv1GkAUtRRt3ko9zqUhiOg==","X-Received":"by 2002:ac8:4c9d:: with SMTP id\n\tj29mr4392188qtv.297.1588345549047; \n\tFri, 01 May 2020 08:05:49 -0700 (PDT)","Message-ID":"<8c32dc76af5e500dde4c3032cc796be2f320a6dd.camel@ndufresne.ca>","From":"Nicolas Dufresne ","To":"Niklas =?iso-8859-1?q?S=F6derlund?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 01 May 2020 11:05:47 -0400","In-Reply-To":"<20200501143021.404090-1-niklas.soderlund@ragnatech.se>","References":"<20200501143021.404090-1-niklas.soderlund@ragnatech.se>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.36.1 (3.36.1-1.fc32) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: stream: Expose stride value","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","X-List-Received-Date":"Fri, 01 May 2020 15:05:50 -0000"}},{"id":4700,"web_url":"https://patchwork.libcamera.org/comment/4700/","msgid":"<895c30768d32ded90a534c3d3e3de08978353b71.camel@ndufresne.ca>","date":"2020-05-01T15:08:13","subject":"Re: [libcamera-devel] [PATCH] libcamera: stream: Expose stride value","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Le vendredi 01 mai 2020 à 17:46 +0300, Laurent Pinchart a écrit :\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Fri, May 01, 2020 at 04:30:21PM +0200, Niklas Söderlund wrote:\n> > Expose the image stride which may be retrieved after a video device\n> > have been configured. It may only be retrieved at that point as the\n> \n> s/have/has/\n> \n> > assignment of video devices takes place at this point.\n> > \n> > In the future video devices should be assign at configuration validation\n> \n> s/assign/assigned/\n> \n> > time and the stride value retrieved at that point.\n> > \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > include/libcamera/stream.h | 1 +\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 42 ++++++++++++--------\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 1 +\n> > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 1 +\n> > src/libcamera/pipeline/vimc/vimc.cpp | 1 +\n> > src/libcamera/stream.cpp | 7 ++++\n> > 6 files changed, 36 insertions(+), 17 deletions(-)\n> > \n> > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> > index 18142dc997bb8885..b3cbea3d52294846 100644\n> > --- a/include/libcamera/stream.h\n> > +++ b/include/libcamera/stream.h\n> > @@ -42,6 +42,7 @@ struct StreamConfiguration {\n> > \n> > \tPixelFormat pixelFormat;\n> > \tSize size;\n> > +\tunsigned int stride;\n> > \n> > \tunsigned int bufferCount;\n> > \n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index ff33f15f9eac3b68..0f9ffd411a680ad9 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -77,7 +77,8 @@ public:\n> > \tint configureInput(const Size &size,\n> > \t\t\t V4L2DeviceFormat *inputFormat);\n> > \tint configureOutput(ImgUOutput *output,\n> > -\t\t\t const StreamConfiguration &cfg);\n> > +\t\t\t const StreamConfiguration &cfg,\n> > +\t\t\t V4L2DeviceFormat *outputFormat);\n> > \n> > \tint allocateBuffers(IPU3CameraData *data, unsigned int bufferCount);\n> > \tvoid freeBuffers(IPU3CameraData *data);\n> > @@ -546,6 +547,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > \tIPU3Stream *vfStream = &data->vfStream_;\n> > \tCIO2Device *cio2 = &data->cio2_;\n> > \tImgUDevice *imgu = data->imgu_;\n> > +\tV4L2DeviceFormat outputFormat;\n> > \tint ret;\n> > \n> > \t/*\n> > @@ -625,12 +627,15 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > \t\t * The RAW still capture stream just copies buffers from the\n> > \t\t * internal queue and doesn't need any specific configuration.\n> > \t\t */\n> > -\t\tif (stream->raw_)\n> > -\t\t\tcontinue;\n> > -\n> > -\t\tret = imgu->configureOutput(stream->device_, cfg);\n> > -\t\tif (ret)\n> > -\t\t\treturn ret;\n> > +\t\tif (stream->raw_) {\n> > +\t\t\tcfg.stride = cio2Format.planes[0].bpl;\n> > +\t\t} else {\n> > +\t\t\tret = imgu->configureOutput(stream->device_, cfg,\n> > +\t\t\t\t\t\t &outputFormat);\n> > +\t\t\tif (ret)\n> > +\t\t\t\treturn ret;\n> > +\t\t\tcfg.stride = outputFormat.planes[0].bpl;\n> > +\t\t}\n> > \t}\n> > \n> > \t/*\n> > @@ -639,13 +644,15 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > \t * be at least one active stream in the configuration request).\n> > \t */\n> > \tif (!outStream->active_) {\n> > -\t\tret = imgu->configureOutput(outStream->device_, config->at(0));\n> > +\t\tret = imgu->configureOutput(outStream->device_, config->at(0),\n> > +\t\t\t\t\t &outputFormat);\n> > \t\tif (ret)\n> > \t\t\treturn ret;\n> > \t}\n> > \n> > \tif (!vfStream->active_) {\n> > -\t\tret = imgu->configureOutput(vfStream->device_, config->at(0));\n> > +\t\tret = imgu->configureOutput(vfStream->device_, config->at(0),\n> > +\t\t\t\t\t &outputFormat);\n> > \t\tif (ret)\n> > \t\t\treturn ret;\n> > \t}\n> > @@ -657,7 +664,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > \tStreamConfiguration statCfg = {};\n> > \tstatCfg.size = cio2Format.size;\n> > \n> > -\tret = imgu->configureOutput(&imgu->stat_, statCfg);\n> > +\tret = imgu->configureOutput(&imgu->stat_, statCfg, &outputFormat);\n> > \tif (ret)\n> > \t\treturn ret;\n> > \n> \n> This isn't very nice, but will do for now as we know we'll have to\n> rework the code anyway.\n> \n> > @@ -1166,7 +1173,8 @@ int ImgUDevice::configureInput(const Size &size,\n> > * \\return 0 on success or a negative error code otherwise\n> > */\n> > int ImgUDevice::configureOutput(ImgUOutput *output,\n> > -\t\t\t\tconst StreamConfiguration &cfg)\n> > +\t\t\t\tconst StreamConfiguration &cfg,\n> > +\t\t\t\tV4L2DeviceFormat *outputFormat)\n> > {\n> > \tV4L2VideoDevice *dev = output->dev;\n> > \tunsigned int pad = output->pad;\n> > @@ -1183,17 +1191,17 @@ int ImgUDevice::configureOutput(ImgUOutput *output,\n> > \tif (output == &stat_)\n> > \t\treturn 0;\n> > \n> > -\tV4L2DeviceFormat outputFormat = {};\n> > -\toutputFormat.fourcc = dev->toV4L2PixelFormat(PixelFormat(DRM_FORMAT_NV12));\n> > -\toutputFormat.size = cfg.size;\n> > -\toutputFormat.planesCount = 2;\n> > +\t*outputFormat = {};\n> > +\toutputFormat->fourcc = dev->toV4L2PixelFormat(PixelFormat(DRM_FORMAT_NV12));\n> > +\toutputFormat->size = cfg.size;\n> > +\toutputFormat->planesCount = 2;\n> > \n> > -\tret = dev->setFormat(&outputFormat);\n> > +\tret = dev->setFormat(outputFormat);\n> > \tif (ret)\n> > \t\treturn ret;\n> > \n> > \tLOG(IPU3, Debug) << \"ImgU \" << output->name << \" format = \"\n> > -\t\t\t << outputFormat.toString();\n> > +\t\t\t << outputFormat->toString();\n> > \n> > \treturn 0;\n> > }\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 6aa3178669b40a37..1e81a0048f093d8f 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -681,6 +681,7 @@ 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> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > index 455693264c2e3e67..f0c1337de8623a6b 100644\n> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > @@ -204,6 +204,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)\n> > \t\treturn -EINVAL;\n> > \n> > \tcfg.setStream(&data->stream_);\n> > +\tcfg.stride = format.planes[0].bpl;\n> > \n> > \treturn 0;\n> > }\n> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > index ccfd7f86d15860c5..7019ac322e6c2c59 100644\n> > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > @@ -253,6 +253,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> > \t\treturn ret;\n> > \n> > \tcfg.setStream(&data->stream_);\n> > +\tcfg.stride = format.planes[0].bpl;\n> > \n> > \treturn 0;\n> > }\n> > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > index ef16aaa1b2f4586a..0a2468d92039676c 100644\n> > --- a/src/libcamera/stream.cpp\n> > +++ b/src/libcamera/stream.cpp\n> > @@ -301,6 +301,13 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)\n> > * \\brief Stream pixel format\n> > */\n> > \n> > +/**\n> > + * \\var StreamConfiguration::stride\n> > + * \\brief Stream stride in bytes\n> \n> Maybe \"Image stride for the stream, in bytes\" ?\n> \n> I would add\n> \n> * The stride value reports the number of bytes between the beginning of\n> * successive lines in an image buffer for this stream. The value is\n> * valid after successfully configuring the camera with this\n> * configuration with a call to Camera::Configure().\n\nThis does not apply to an image, but to one plane (well first plane) of\nan image. I really think we should do this right on first go and expose\nan array of strides, and while at it, offsets.\n\n> \n> Reviewed-by: Laurent Pinchart \n> \n> > + * \\todo Update this value when configuration is validated instead of when\n> > + * the camera is configured.\n> > + */\n> > +\n> > /**\n> > * \\var StreamConfiguration::bufferCount\n> > * \\brief Requested number of buffers to allocate for the stream","headers":{"Return-Path":"","Received":["from mail-qk1-x744.google.com (mail-qk1-x744.google.com\n\t[IPv6:2607:f8b0:4864:20::744])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 199A2603F2\n\tfor ;\n\tFri, 1 May 2020 17:08:16 +0200 (CEST)","by mail-qk1-x744.google.com with SMTP id f83so2108697qke.13\n\tfor ;\n\tFri, 01 May 2020 08:08:16 -0700 (PDT)","from skullcanyon ([192.222.193.21])\n\tby smtp.gmail.com with ESMTPSA id\n\ts8sm2883530qtb.0.2020.05.01.08.08.13\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 01 May 2020 08:08:14 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ndufresne-ca.20150623.gappssmtp.com\n\theader.i=@ndufresne-ca.20150623.gappssmtp.com header.b=\"eHvyVuSq\"; \n\tdkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20150623.gappssmtp.com; s=20150623;\n\th=message-id:subject:from:to:cc:date:in-reply-to:references\n\t:user-agent:mime-version:content-transfer-encoding;\n\tbh=ZYUzXZCW8brpdv/qQtLs6smCYpHgO3V+3DuQPiqUaWo=;\n\tb=eHvyVuSqokU8zi2gmsebrvYGsrTvXRu4YC0LT3g3l9dfLgoyzHOjS5xf4+Y4pDZTM0\n\t79k723u2VKovZrVjTKYTsJQnkuwqRpkJIicNX0GMQCuZXrcQhxyUgUX8PqiOzLBCddDq\n\tMy1tslStWIsit1xpeMhf3fmpeIHjqH+Zk07n/40MyMvUDFs9kMRzqwD3CShN/nMcPvzY\n\tkqxyc9/Tg+XIMvpuiWFoiuUmUX8p/Y80xhjFa+FAXvrMajN96OsPVUddQxUPqICmEeEf\n\tcF9AZ8U5LU9n6W+9cF6wsqxwCPdpu/25EXboF9aZloB4I1yKawlP3RgcF4heJAyFzPV7\n\tDsPw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to\n\t:references:user-agent:mime-version:content-transfer-encoding;\n\tbh=ZYUzXZCW8brpdv/qQtLs6smCYpHgO3V+3DuQPiqUaWo=;\n\tb=kRjAXmWH59KE7CxCEZmNUIcO/KKvhUyZSbFI8IBe+s32CuQyfOkVhIXLd81mBxW4di\n\tIa/uiC2S065zmVQ7CxQ9s9CCJYW/SsOqzDWVEsYxAkr7c6y7OyPIR26negTO+utzEsqm\n\tT8KntRtu1O+K1T+uwiYNGwDIryTUQd3pqZkN6QJtsiqIpGZgalA7sff79c8EoxO4V0Nm\n\tBK8nS9/Vk7ALW+Zb/EaO/Dfxp4rOM2n0mKOjdwkpqBF2H84g2nXnw8WFprqrD7AsmgOH\n\tKynrhzXfe3cNBA/VyCAbv9Q094GWt+M9FS45Mu7P/ZWOkHWNG/0Luxu2tHgfMEYTSScl\n\tLiOQ==","X-Gm-Message-State":"AGi0PuaSFf/pVpCkiOo4C7tuYENwKeWR9reEy5MkWf35gU2aT+4ARgU5\n\tAuBs/CEhNN9n6v2Mx+ZELF6Gig==","X-Google-Smtp-Source":"APiQypL2TJvkwEAa9XvxECNiSk1c6pk7dsvnpEnbyc4prCgWvVODPvN8xpCJRf/u+kJrDVHYpVY7Aw==","X-Received":"by 2002:a37:4953:: with SMTP id\n\tw80mr4078658qka.237.1588345694652; \n\tFri, 01 May 2020 08:08:14 -0700 (PDT)","Message-ID":"<895c30768d32ded90a534c3d3e3de08978353b71.camel@ndufresne.ca>","From":"Nicolas Dufresne ","To":"Laurent Pinchart , Niklas\n\t=?iso-8859-1?q?S=F6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Date":"Fri, 01 May 2020 11:08:13 -0400","In-Reply-To":"<20200501144607.GH5951@pendragon.ideasonboard.com>","References":"<20200501143021.404090-1-niklas.soderlund@ragnatech.se>\n\t<20200501144607.GH5951@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.36.1 (3.36.1-1.fc32) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: stream: Expose stride value","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","X-List-Received-Date":"Fri, 01 May 2020 15:08:16 -0000"}},{"id":4701,"web_url":"https://patchwork.libcamera.org/comment/4701/","msgid":"<20200501150841.GA2520266@oden.dyn.berto.se>","date":"2020-05-01T15:08:41","subject":"Re: [libcamera-devel] [PATCH] libcamera: stream: Expose stride value","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your comments. I have taken them all and pushed this to \nmaster.\n\nOn 2020-05-01 17:46:07 +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Fri, May 01, 2020 at 04:30:21PM +0200, Niklas Söderlund wrote:\n> > Expose the image stride which may be retrieved after a video device\n> > have been configured. It may only be retrieved at that point as the\n> \n> s/have/has/\n> \n> > assignment of video devices takes place at this point.\n> > \n> > In the future video devices should be assign at configuration validation\n> \n> s/assign/assigned/\n> \n> > time and the stride value retrieved at that point.\n> > \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > include/libcamera/stream.h | 1 +\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 42 ++++++++++++--------\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 1 +\n> > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 1 +\n> > src/libcamera/pipeline/vimc/vimc.cpp | 1 +\n> > src/libcamera/stream.cpp | 7 ++++\n> > 6 files changed, 36 insertions(+), 17 deletions(-)\n> > \n> > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> > index 18142dc997bb8885..b3cbea3d52294846 100644\n> > --- a/include/libcamera/stream.h\n> > +++ b/include/libcamera/stream.h\n> > @@ -42,6 +42,7 @@ struct StreamConfiguration {\n> > \n> > \tPixelFormat pixelFormat;\n> > \tSize size;\n> > +\tunsigned int stride;\n> > \n> > \tunsigned int bufferCount;\n> > \n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index ff33f15f9eac3b68..0f9ffd411a680ad9 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -77,7 +77,8 @@ public:\n> > \tint configureInput(const Size &size,\n> > \t\t\t V4L2DeviceFormat *inputFormat);\n> > \tint configureOutput(ImgUOutput *output,\n> > -\t\t\t const StreamConfiguration &cfg);\n> > +\t\t\t const StreamConfiguration &cfg,\n> > +\t\t\t V4L2DeviceFormat *outputFormat);\n> > \n> > \tint allocateBuffers(IPU3CameraData *data, unsigned int bufferCount);\n> > \tvoid freeBuffers(IPU3CameraData *data);\n> > @@ -546,6 +547,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > \tIPU3Stream *vfStream = &data->vfStream_;\n> > \tCIO2Device *cio2 = &data->cio2_;\n> > \tImgUDevice *imgu = data->imgu_;\n> > +\tV4L2DeviceFormat outputFormat;\n> > \tint ret;\n> > \n> > \t/*\n> > @@ -625,12 +627,15 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > \t\t * The RAW still capture stream just copies buffers from the\n> > \t\t * internal queue and doesn't need any specific configuration.\n> > \t\t */\n> > -\t\tif (stream->raw_)\n> > -\t\t\tcontinue;\n> > -\n> > -\t\tret = imgu->configureOutput(stream->device_, cfg);\n> > -\t\tif (ret)\n> > -\t\t\treturn ret;\n> > +\t\tif (stream->raw_) {\n> > +\t\t\tcfg.stride = cio2Format.planes[0].bpl;\n> > +\t\t} else {\n> > +\t\t\tret = imgu->configureOutput(stream->device_, cfg,\n> > +\t\t\t\t\t\t &outputFormat);\n> > +\t\t\tif (ret)\n> > +\t\t\t\treturn ret;\n> > +\t\t\tcfg.stride = outputFormat.planes[0].bpl;\n> > +\t\t}\n> > \t}\n> > \n> > \t/*\n> > @@ -639,13 +644,15 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > \t * be at least one active stream in the configuration request).\n> > \t */\n> > \tif (!outStream->active_) {\n> > -\t\tret = imgu->configureOutput(outStream->device_, config->at(0));\n> > +\t\tret = imgu->configureOutput(outStream->device_, config->at(0),\n> > +\t\t\t\t\t &outputFormat);\n> > \t\tif (ret)\n> > \t\t\treturn ret;\n> > \t}\n> > \n> > \tif (!vfStream->active_) {\n> > -\t\tret = imgu->configureOutput(vfStream->device_, config->at(0));\n> > +\t\tret = imgu->configureOutput(vfStream->device_, config->at(0),\n> > +\t\t\t\t\t &outputFormat);\n> > \t\tif (ret)\n> > \t\t\treturn ret;\n> > \t}\n> > @@ -657,7 +664,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > \tStreamConfiguration statCfg = {};\n> > \tstatCfg.size = cio2Format.size;\n> > \n> > -\tret = imgu->configureOutput(&imgu->stat_, statCfg);\n> > +\tret = imgu->configureOutput(&imgu->stat_, statCfg, &outputFormat);\n> > \tif (ret)\n> > \t\treturn ret;\n> > \n> \n> This isn't very nice, but will do for now as we know we'll have to\n> rework the code anyway.\n> \n> > @@ -1166,7 +1173,8 @@ int ImgUDevice::configureInput(const Size &size,\n> > * \\return 0 on success or a negative error code otherwise\n> > */\n> > int ImgUDevice::configureOutput(ImgUOutput *output,\n> > -\t\t\t\tconst StreamConfiguration &cfg)\n> > +\t\t\t\tconst StreamConfiguration &cfg,\n> > +\t\t\t\tV4L2DeviceFormat *outputFormat)\n> > {\n> > \tV4L2VideoDevice *dev = output->dev;\n> > \tunsigned int pad = output->pad;\n> > @@ -1183,17 +1191,17 @@ int ImgUDevice::configureOutput(ImgUOutput *output,\n> > \tif (output == &stat_)\n> > \t\treturn 0;\n> > \n> > -\tV4L2DeviceFormat outputFormat = {};\n> > -\toutputFormat.fourcc = dev->toV4L2PixelFormat(PixelFormat(DRM_FORMAT_NV12));\n> > -\toutputFormat.size = cfg.size;\n> > -\toutputFormat.planesCount = 2;\n> > +\t*outputFormat = {};\n> > +\toutputFormat->fourcc = dev->toV4L2PixelFormat(PixelFormat(DRM_FORMAT_NV12));\n> > +\toutputFormat->size = cfg.size;\n> > +\toutputFormat->planesCount = 2;\n> > \n> > -\tret = dev->setFormat(&outputFormat);\n> > +\tret = dev->setFormat(outputFormat);\n> > \tif (ret)\n> > \t\treturn ret;\n> > \n> > \tLOG(IPU3, Debug) << \"ImgU \" << output->name << \" format = \"\n> > -\t\t\t << outputFormat.toString();\n> > +\t\t\t << outputFormat->toString();\n> > \n> > \treturn 0;\n> > }\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 6aa3178669b40a37..1e81a0048f093d8f 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -681,6 +681,7 @@ 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> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > index 455693264c2e3e67..f0c1337de8623a6b 100644\n> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > @@ -204,6 +204,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)\n> > \t\treturn -EINVAL;\n> > \n> > \tcfg.setStream(&data->stream_);\n> > +\tcfg.stride = format.planes[0].bpl;\n> > \n> > \treturn 0;\n> > }\n> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > index ccfd7f86d15860c5..7019ac322e6c2c59 100644\n> > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > @@ -253,6 +253,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> > \t\treturn ret;\n> > \n> > \tcfg.setStream(&data->stream_);\n> > +\tcfg.stride = format.planes[0].bpl;\n> > \n> > \treturn 0;\n> > }\n> > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > index ef16aaa1b2f4586a..0a2468d92039676c 100644\n> > --- a/src/libcamera/stream.cpp\n> > +++ b/src/libcamera/stream.cpp\n> > @@ -301,6 +301,13 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)\n> > * \\brief Stream pixel format\n> > */\n> > \n> > +/**\n> > + * \\var StreamConfiguration::stride\n> > + * \\brief Stream stride in bytes\n> \n> Maybe \"Image stride for the stream, in bytes\" ?\n> \n> I would add\n> \n> * The stride value reports the number of bytes between the beginning of\n> * successive lines in an image buffer for this stream. The value is\n> * valid after successfully configuring the camera with this\n> * configuration with a call to Camera::Configure().\n> \n> Reviewed-by: Laurent Pinchart \n> \n> > + * \\todo Update this value when configuration is validated instead of when\n> > + * the camera is configured.\n> > + */\n> > +\n> > /**\n> > * \\var StreamConfiguration::bufferCount\n> > * \\brief Requested number of buffers to allocate for the stream\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E6B17603F2\n\tfor ;\n\tFri, 1 May 2020 17:08:42 +0200 (CEST)","by mail-lj1-x242.google.com with SMTP id a21so2835796ljj.11\n\tfor ;\n\tFri, 01 May 2020 08:08:42 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\ta28sm2376933lfr.4.2020.05.01.08.08.41\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 01 May 2020 08:08:41 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com header.b=\"0zHDVTGS\"; \n\tdkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=tkak8m057GIhWhIlmuB1yL6DylighZHQ9398ar4RrOM=;\n\tb=0zHDVTGSyHsONEoACv/bZasqU+oHMiBUX+eTEoSFxjkm7usvIHayumTpwwUX5RDg3K\n\to3bS3UfG0pba7gu/Idp1qvOTQK3vYL7g3VNRPGEMLMCPWOlnmKMAMfhZElfkQFaLDS1u\n\tfLoAFbiSA7YPJUvEUjkSWzU+SDgWBQRUy/yhp8fYKGfIGkE8hiZS99YFtPZ4OAbxW7Zr\n\tT20hLEj31Gp/BvX5w7EuOv1L3jrwsDGHZidVOUX8DikCMww9fq2P48CIQAOOLvXWGalz\n\t6rOm47c9ROomMhzcicP9zN7ouOz/G0Sb4SPIM0fFofVyLDuBUf0NXIuzSji+xo0dgGjO\n\txOXA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=tkak8m057GIhWhIlmuB1yL6DylighZHQ9398ar4RrOM=;\n\tb=qaFQJRjPqpJIWGJKkpPOo5duzWhXFzCm5Hh/EIwCp8gviIjciB1Ol4b9bcWKhTM8kb\n\tBHAWO3f7E6lVbZ/AlrcuT2irXHv2UZZmuZIqXrURVCBWoktNouzB9+NLfB4v/j6bXTm2\n\tdksXcuJV/l1cKfMDoZDPzaD9Hp0Ja7fn+PhNnW4ASCJsleSRwALuZUxmneHUK25bERqB\n\tTyqUgmrJ8r3FQIdR+ukTq9U5lJdRwDQgpZUlQe8LZkB0UGLFsN2K5MEj04XDaGLT+ruT\n\tCLtPKuTU4Yp2H4JsyOLCnW4MHKxiUHpYDf+zVKXWmuOzzzuONxLHB/7Oc2aWPlrPdPX1\n\to7mA==","X-Gm-Message-State":"AGi0PuZ2hXlerBQUsoUN2qX4ZQkEKAprwzlxBoAzqGj/Z3EdvtOjekHk\n\tVLVaUy0+lByvWUmphBptaz8Ktw==","X-Google-Smtp-Source":"APiQypKcfk32hAXnjua3ve8LNprttshgSFKpgwblJLSXyrWRJ0HJjBLTPOEUfmM4aStbi0k2y1fRJQ==","X-Received":"by 2002:a2e:90cd:: with SMTP id\n\to13mr2804932ljg.220.1588345722021; \n\tFri, 01 May 2020 08:08:42 -0700 (PDT)","Date":"Fri, 1 May 2020 17:08:41 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200501150841.GA2520266@oden.dyn.berto.se>","References":"<20200501143021.404090-1-niklas.soderlund@ragnatech.se>\n\t<20200501144607.GH5951@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200501144607.GH5951@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: stream: Expose stride value","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","X-List-Received-Date":"Fri, 01 May 2020 15:08:43 -0000"}},{"id":4703,"web_url":"https://patchwork.libcamera.org/comment/4703/","msgid":"<20200501153433.GA2569889@oden.dyn.berto.se>","date":"2020-05-01T15:34:33","subject":"Re: [libcamera-devel] [PATCH] libcamera: stream: Expose stride value","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Nicolas,\n\nSorry for jumping the gun and merging this minuets after your comments. \nI had not seen them at the time :-(\n\nOn 2020-05-01 11:08:13 -0400, Nicolas Dufresne wrote:\n> Le vendredi 01 mai 2020 à 17:46 +0300, Laurent Pinchart a écrit :\n> > Hi Niklas,\n> > \n> > Thank you for the patch.\n> > \n> > On Fri, May 01, 2020 at 04:30:21PM +0200, Niklas Söderlund wrote:\n> > > Expose the image stride which may be retrieved after a video device\n> > > have been configured. It may only be retrieved at that point as the\n> > \n> > s/have/has/\n> > \n> > > assignment of video devices takes place at this point.\n> > > \n> > > In the future video devices should be assign at configuration validation\n> > \n> > s/assign/assigned/\n> > \n> > > time and the stride value retrieved at that point.\n> > > \n> > > Signed-off-by: Niklas Söderlund \n> > > ---\n> > > include/libcamera/stream.h | 1 +\n> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 42 ++++++++++++--------\n> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 1 +\n> > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 1 +\n> > > src/libcamera/pipeline/vimc/vimc.cpp | 1 +\n> > > src/libcamera/stream.cpp | 7 ++++\n> > > 6 files changed, 36 insertions(+), 17 deletions(-)\n> > > \n> > > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> > > index 18142dc997bb8885..b3cbea3d52294846 100644\n> > > --- a/include/libcamera/stream.h\n> > > +++ b/include/libcamera/stream.h\n> > > @@ -42,6 +42,7 @@ struct StreamConfiguration {\n> > > \n> > > \tPixelFormat pixelFormat;\n> > > \tSize size;\n> > > +\tunsigned int stride;\n> > > \n> > > \tunsigned int bufferCount;\n> > > \n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index ff33f15f9eac3b68..0f9ffd411a680ad9 100644\n> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > @@ -77,7 +77,8 @@ public:\n> > > \tint configureInput(const Size &size,\n> > > \t\t\t V4L2DeviceFormat *inputFormat);\n> > > \tint configureOutput(ImgUOutput *output,\n> > > -\t\t\t const StreamConfiguration &cfg);\n> > > +\t\t\t const StreamConfiguration &cfg,\n> > > +\t\t\t V4L2DeviceFormat *outputFormat);\n> > > \n> > > \tint allocateBuffers(IPU3CameraData *data, unsigned int bufferCount);\n> > > \tvoid freeBuffers(IPU3CameraData *data);\n> > > @@ -546,6 +547,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > > \tIPU3Stream *vfStream = &data->vfStream_;\n> > > \tCIO2Device *cio2 = &data->cio2_;\n> > > \tImgUDevice *imgu = data->imgu_;\n> > > +\tV4L2DeviceFormat outputFormat;\n> > > \tint ret;\n> > > \n> > > \t/*\n> > > @@ -625,12 +627,15 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > > \t\t * The RAW still capture stream just copies buffers from the\n> > > \t\t * internal queue and doesn't need any specific configuration.\n> > > \t\t */\n> > > -\t\tif (stream->raw_)\n> > > -\t\t\tcontinue;\n> > > -\n> > > -\t\tret = imgu->configureOutput(stream->device_, cfg);\n> > > -\t\tif (ret)\n> > > -\t\t\treturn ret;\n> > > +\t\tif (stream->raw_) {\n> > > +\t\t\tcfg.stride = cio2Format.planes[0].bpl;\n> > > +\t\t} else {\n> > > +\t\t\tret = imgu->configureOutput(stream->device_, cfg,\n> > > +\t\t\t\t\t\t &outputFormat);\n> > > +\t\t\tif (ret)\n> > > +\t\t\t\treturn ret;\n> > > +\t\t\tcfg.stride = outputFormat.planes[0].bpl;\n> > > +\t\t}\n> > > \t}\n> > > \n> > > \t/*\n> > > @@ -639,13 +644,15 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > > \t * be at least one active stream in the configuration request).\n> > > \t */\n> > > \tif (!outStream->active_) {\n> > > -\t\tret = imgu->configureOutput(outStream->device_, config->at(0));\n> > > +\t\tret = imgu->configureOutput(outStream->device_, config->at(0),\n> > > +\t\t\t\t\t &outputFormat);\n> > > \t\tif (ret)\n> > > \t\t\treturn ret;\n> > > \t}\n> > > \n> > > \tif (!vfStream->active_) {\n> > > -\t\tret = imgu->configureOutput(vfStream->device_, config->at(0));\n> > > +\t\tret = imgu->configureOutput(vfStream->device_, config->at(0),\n> > > +\t\t\t\t\t &outputFormat);\n> > > \t\tif (ret)\n> > > \t\t\treturn ret;\n> > > \t}\n> > > @@ -657,7 +664,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > > \tStreamConfiguration statCfg = {};\n> > > \tstatCfg.size = cio2Format.size;\n> > > \n> > > -\tret = imgu->configureOutput(&imgu->stat_, statCfg);\n> > > +\tret = imgu->configureOutput(&imgu->stat_, statCfg, &outputFormat);\n> > > \tif (ret)\n> > > \t\treturn ret;\n> > > \n> > \n> > This isn't very nice, but will do for now as we know we'll have to\n> > rework the code anyway.\n> > \n> > > @@ -1166,7 +1173,8 @@ int ImgUDevice::configureInput(const Size &size,\n> > > * \\return 0 on success or a negative error code otherwise\n> > > */\n> > > int ImgUDevice::configureOutput(ImgUOutput *output,\n> > > -\t\t\t\tconst StreamConfiguration &cfg)\n> > > +\t\t\t\tconst StreamConfiguration &cfg,\n> > > +\t\t\t\tV4L2DeviceFormat *outputFormat)\n> > > {\n> > > \tV4L2VideoDevice *dev = output->dev;\n> > > \tunsigned int pad = output->pad;\n> > > @@ -1183,17 +1191,17 @@ int ImgUDevice::configureOutput(ImgUOutput *output,\n> > > \tif (output == &stat_)\n> > > \t\treturn 0;\n> > > \n> > > -\tV4L2DeviceFormat outputFormat = {};\n> > > -\toutputFormat.fourcc = dev->toV4L2PixelFormat(PixelFormat(DRM_FORMAT_NV12));\n> > > -\toutputFormat.size = cfg.size;\n> > > -\toutputFormat.planesCount = 2;\n> > > +\t*outputFormat = {};\n> > > +\toutputFormat->fourcc = dev->toV4L2PixelFormat(PixelFormat(DRM_FORMAT_NV12));\n> > > +\toutputFormat->size = cfg.size;\n> > > +\toutputFormat->planesCount = 2;\n> > > \n> > > -\tret = dev->setFormat(&outputFormat);\n> > > +\tret = dev->setFormat(outputFormat);\n> > > \tif (ret)\n> > > \t\treturn ret;\n> > > \n> > > \tLOG(IPU3, Debug) << \"ImgU \" << output->name << \" format = \"\n> > > -\t\t\t << outputFormat.toString();\n> > > +\t\t\t << outputFormat->toString();\n> > > \n> > > \treturn 0;\n> > > }\n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index 6aa3178669b40a37..1e81a0048f093d8f 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > @@ -681,6 +681,7 @@ 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> > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > index 455693264c2e3e67..f0c1337de8623a6b 100644\n> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > @@ -204,6 +204,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)\n> > > \t\treturn -EINVAL;\n> > > \n> > > \tcfg.setStream(&data->stream_);\n> > > +\tcfg.stride = format.planes[0].bpl;\n> > > \n> > > \treturn 0;\n> > > }\n> > > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > > index ccfd7f86d15860c5..7019ac322e6c2c59 100644\n> > > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > > @@ -253,6 +253,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> > > \t\treturn ret;\n> > > \n> > > \tcfg.setStream(&data->stream_);\n> > > +\tcfg.stride = format.planes[0].bpl;\n> > > \n> > > \treturn 0;\n> > > }\n> > > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > > index ef16aaa1b2f4586a..0a2468d92039676c 100644\n> > > --- a/src/libcamera/stream.cpp\n> > > +++ b/src/libcamera/stream.cpp\n> > > @@ -301,6 +301,13 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)\n> > > * \\brief Stream pixel format\n> > > */\n> > > \n> > > +/**\n> > > + * \\var StreamConfiguration::stride\n> > > + * \\brief Stream stride in bytes\n> > \n> > Maybe \"Image stride for the stream, in bytes\" ?\n> > \n> > I would add\n> > \n> > * The stride value reports the number of bytes between the beginning of\n> > * successive lines in an image buffer for this stream. The value is\n> > * valid after successfully configuring the camera with this\n> > * configuration with a call to Camera::Configure().\n> \n> This does not apply to an image, but to one plane (well first plane) of\n> an image. I really think we should do this right on first go and expose\n> an array of strides, and while at it, offsets.\n\nI agree it would be best to do this correct on the first implementation. \nI just don't know what solution covers all future use-cases.\n\nI agree more work is needed here in to cover all needs as \nStreamConfiguration should be made plane aware for other reasons then \nstride but also offsets as you point out. What else could be needed? \nInformation of the usage of each plane?\n\n> \n> > \n> > Reviewed-by: Laurent Pinchart \n> > \n> > > + * \\todo Update this value when configuration is validated instead of when\n> > > + * the camera is configured.\n> > > + */\n> > > +\n> > > /**\n> > > * \\var StreamConfiguration::bufferCount\n> > > * \\brief Requested number of buffers to allocate for the stream\n>","headers":{"Return-Path":"","Received":["from mail-lj1-x236.google.com (mail-lj1-x236.google.com\n\t[IPv6:2a00:1450:4864:20::236])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D0096603F3\n\tfor ;\n\tFri, 1 May 2020 17:34:35 +0200 (CEST)","by mail-lj1-x236.google.com with SMTP id w20so2984938ljj.0\n\tfor ;\n\tFri, 01 May 2020 08:34:35 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\ti3sm2128767ljg.82.2020.05.01.08.34.34\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 01 May 2020 08:34:34 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com header.b=\"Y8aymk3D\"; \n\tdkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=/BcTZZHGT+oUgmupvgu8+BrTpzSGaBSHXHc2Wyc7DhE=;\n\tb=Y8aymk3Dhma5Utc+aGocGvjlepD7nyTwxLUXNniYe/CCO/8JVZhXfgrp5d/8ViR1sd\n\tAfFkfxoi3yUHEyJ52NxBBf6/ry8cYHK2r42iaEuXpUNK9iM+bTtSSF8qjDk32gZvk9SM\n\tzZKobkke6KDLYp9WTDtR0tpRhUzfvrZri6afLastnIvw5wIxfAFav6ov4NJ7hrucU2kY\n\tGgs6YlUD+HyZecfssRVN+tdoPgCUnNME23BtHXnK+RRpSobXGdsx0DKgweJQcFcTdset\n\tPymQppdFtmzUII2tai2nxupBmsRrMNEV6qe6HJuFPwybeRJ0LAQ0GQgNmFKqdVSnGXTs\n\tOzng==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=/BcTZZHGT+oUgmupvgu8+BrTpzSGaBSHXHc2Wyc7DhE=;\n\tb=RKrCuAjV3khl0jk9eouWYYmmu5klfMmUs7jfLQrLO8jpxu4F21gqL1RzG49TLTG9YJ\n\tubBwfJVnXb/GKnXCcknSH+m5aC+GaZvhaQcH3F+VQYoiESlSI+jZeR+UvZ5x3UP7Ag8S\n\tgIGsZ4j0CFPdmQ6FQYSgIWKIWLQBm3eGcpETMpWmJS4Ec2+taBEV4yfLX1zm5z9MyWYI\n\ttOiYI+VUsmVVZQMVMK0N9Q+nD3TYMoNhmlOD/MG8cgiTqN99x6gGnlCBJVp+OvSb8xpO\n\tcyLZHT+l//CeCx052PUx1PWJi4EYjanAvk7yKrDQCa18/nuOX2MEVNI1dtAWTWTDHWlh\n\tAUlQ==","X-Gm-Message-State":"AGi0PuZT+vlGM4H6tJPsJjWL11Gd5Eu/r/B3AXbQxVmGe8XKf/k5lgZB\n\t9gRGelz+G2ywD+9lsPhRLloAqWgRr38=","X-Google-Smtp-Source":"APiQypLgRhPEoRIwhs+Wnat1/ycPVwcKjRecR2egR5lrMNFYPj7QjkHL+1o4rGLYvagnQZ+4UkNLxg==","X-Received":"by 2002:a05:651c:549:: with SMTP id\n\tq9mr2814624ljp.236.1588347275028; \n\tFri, 01 May 2020 08:34:35 -0700 (PDT)","Date":"Fri, 1 May 2020 17:34:33 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Nicolas Dufresne ","Cc":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200501153433.GA2569889@oden.dyn.berto.se>","References":"<20200501143021.404090-1-niklas.soderlund@ragnatech.se>\n\t<20200501144607.GH5951@pendragon.ideasonboard.com>\n\t<895c30768d32ded90a534c3d3e3de08978353b71.camel@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<895c30768d32ded90a534c3d3e3de08978353b71.camel@ndufresne.ca>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: stream: Expose stride value","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","X-List-Received-Date":"Fri, 01 May 2020 15:34:36 -0000"}},{"id":4704,"web_url":"https://patchwork.libcamera.org/comment/4704/","msgid":"<20200501161716.GI5951@pendragon.ideasonboard.com>","date":"2020-05-01T16:17:16","subject":"Re: [libcamera-devel] [PATCH] libcamera: stream: Expose stride value","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas,\n\nOn Fri, May 01, 2020 at 11:05:47AM -0400, Nicolas Dufresne wrote:\n> Le vendredi 01 mai 2020 à 16:30 +0200, Niklas Söderlund a écrit :\n> > Expose the image stride which may be retrieved after a video device\n> > have been configured. It may only be retrieved at that point as the\n> > assignment of video devices takes place at this point.\n> > \n> > In the future video devices should be assign at configuration validation\n> > time and the stride value retrieved at that point.\n> > \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > include/libcamera/stream.h | 1 +\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 42 ++++++++++++--------\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 1 +\n> > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 1 +\n> > src/libcamera/pipeline/vimc/vimc.cpp | 1 +\n> > src/libcamera/stream.cpp | 7 ++++\n> > 6 files changed, 36 insertions(+), 17 deletions(-)\n> > \n> > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> > index 18142dc997bb8885..b3cbea3d52294846 100644\n> > --- a/include/libcamera/stream.h\n> > +++ b/include/libcamera/stream.h\n> > @@ -42,6 +42,7 @@ struct StreamConfiguration {\n> > \n> > \tPixelFormat pixelFormat;\n> > \tSize size;\n> > +\tunsigned int stride;\n> \n> In my opinion, as we have full knowledge of it already, we should\n> directly start with an array of up to 4 strides. We can implement\n> extrapolation for non-mplane V4L2 formats. This way we don't need to\n> port the userspace bits twice.\n\nI agree with you. Let's not use the stride in the gstreamer element yet,\nwe'll update that to 4 values. I would however like to also move stride\ncomputation to CameraConfiguration::validate() time too, and that\nrequires a bit of work on the pipeline handler side. Please bear with us\nuntil we complete that :-)\n\n> > \tunsigned int bufferCount;\n> > \n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index ff33f15f9eac3b68..0f9ffd411a680ad9 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -77,7 +77,8 @@ public:\n> > \tint configureInput(const Size &size,\n> > \t\t\t V4L2DeviceFormat *inputFormat);\n> > \tint configureOutput(ImgUOutput *output,\n> > -\t\t\t const StreamConfiguration &cfg);\n> > +\t\t\t const StreamConfiguration &cfg,\n> > +\t\t\t V4L2DeviceFormat *outputFormat);\n> > \n> > \tint allocateBuffers(IPU3CameraData *data, unsigned int bufferCount);\n> > \tvoid freeBuffers(IPU3CameraData *data);\n> > @@ -546,6 +547,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > \tIPU3Stream *vfStream = &data->vfStream_;\n> > \tCIO2Device *cio2 = &data->cio2_;\n> > \tImgUDevice *imgu = data->imgu_;\n> > +\tV4L2DeviceFormat outputFormat;\n> > \tint ret;\n> > \n> > \t/*\n> > @@ -625,12 +627,15 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > \t\t * The RAW still capture stream just copies buffers from the\n> > \t\t * internal queue and doesn't need any specific configuration.\n> > \t\t */\n> > -\t\tif (stream->raw_)\n> > -\t\t\tcontinue;\n> > -\n> > -\t\tret = imgu->configureOutput(stream->device_, cfg);\n> > -\t\tif (ret)\n> > -\t\t\treturn ret;\n> > +\t\tif (stream->raw_) {\n> > +\t\t\tcfg.stride = cio2Format.planes[0].bpl;\n> > +\t\t} else {\n> > +\t\t\tret = imgu->configureOutput(stream->device_, cfg,\n> > +\t\t\t\t\t\t &outputFormat);\n> > +\t\t\tif (ret)\n> > +\t\t\t\treturn ret;\n> > +\t\t\tcfg.stride = outputFormat.planes[0].bpl;\n> > +\t\t}\n> > \t}\n> > \n> > \t/*\n> > @@ -639,13 +644,15 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > \t * be at least one active stream in the configuration request).\n> > \t */\n> > \tif (!outStream->active_) {\n> > -\t\tret = imgu->configureOutput(outStream->device_, config->at(0));\n> > +\t\tret = imgu->configureOutput(outStream->device_, config->at(0),\n> > +\t\t\t\t\t &outputFormat);\n> > \t\tif (ret)\n> > \t\t\treturn ret;\n> > \t}\n> > \n> > \tif (!vfStream->active_) {\n> > -\t\tret = imgu->configureOutput(vfStream->device_, config->at(0));\n> > +\t\tret = imgu->configureOutput(vfStream->device_, config->at(0),\n> > +\t\t\t\t\t &outputFormat);\n> > \t\tif (ret)\n> > \t\t\treturn ret;\n> > \t}\n> > @@ -657,7 +664,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > \tStreamConfiguration statCfg = {};\n> > \tstatCfg.size = cio2Format.size;\n> > \n> > -\tret = imgu->configureOutput(&imgu->stat_, statCfg);\n> > +\tret = imgu->configureOutput(&imgu->stat_, statCfg, &outputFormat);\n> > \tif (ret)\n> > \t\treturn ret;\n> > \n> > @@ -1166,7 +1173,8 @@ int ImgUDevice::configureInput(const Size &size,\n> > * \\return 0 on success or a negative error code otherwise\n> > */\n> > int ImgUDevice::configureOutput(ImgUOutput *output,\n> > -\t\t\t\tconst StreamConfiguration &cfg)\n> > +\t\t\t\tconst StreamConfiguration &cfg,\n> > +\t\t\t\tV4L2DeviceFormat *outputFormat)\n> > {\n> > \tV4L2VideoDevice *dev = output->dev;\n> > \tunsigned int pad = output->pad;\n> > @@ -1183,17 +1191,17 @@ int ImgUDevice::configureOutput(ImgUOutput *output,\n> > \tif (output == &stat_)\n> > \t\treturn 0;\n> > \n> > -\tV4L2DeviceFormat outputFormat = {};\n> > -\toutputFormat.fourcc = dev->toV4L2PixelFormat(PixelFormat(DRM_FORMAT_NV12));\n> > -\toutputFormat.size = cfg.size;\n> > -\toutputFormat.planesCount = 2;\n> > +\t*outputFormat = {};\n> > +\toutputFormat->fourcc = dev->toV4L2PixelFormat(PixelFormat(DRM_FORMAT_NV12));\n> > +\toutputFormat->size = cfg.size;\n> > +\toutputFormat->planesCount = 2;\n> > \n> > -\tret = dev->setFormat(&outputFormat);\n> > +\tret = dev->setFormat(outputFormat);\n> > \tif (ret)\n> > \t\treturn ret;\n> > \n> > \tLOG(IPU3, Debug) << \"ImgU \" << output->name << \" format = \"\n> > -\t\t\t << outputFormat.toString();\n> > +\t\t\t << outputFormat->toString();\n> > \n> > \treturn 0;\n> > }\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 6aa3178669b40a37..1e81a0048f093d8f 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -681,6 +681,7 @@ 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> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > index 455693264c2e3e67..f0c1337de8623a6b 100644\n> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > @@ -204,6 +204,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)\n> > \t\treturn -EINVAL;\n> > \n> > \tcfg.setStream(&data->stream_);\n> > +\tcfg.stride = format.planes[0].bpl;\n> > \n> > \treturn 0;\n> > }\n> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > index ccfd7f86d15860c5..7019ac322e6c2c59 100644\n> > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > @@ -253,6 +253,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> > \t\treturn ret;\n> > \n> > \tcfg.setStream(&data->stream_);\n> > +\tcfg.stride = format.planes[0].bpl;\n> > \n> > \treturn 0;\n> > }\n> > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > index ef16aaa1b2f4586a..0a2468d92039676c 100644\n> > --- a/src/libcamera/stream.cpp\n> > +++ b/src/libcamera/stream.cpp\n> > @@ -301,6 +301,13 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)\n> > * \\brief Stream pixel format\n> > */\n> > \n> > +/**\n> > + * \\var StreamConfiguration::stride\n> > + * \\brief Stream stride in bytes\n> > + * \\todo Update this value when configuration is validated instead of when\n> > + * the camera is configured.\n> > + */\n> > +\n> > /**\n> > * \\var StreamConfiguration::bufferCount\n> > * \\brief Requested number of buffers to allocate for the stream","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 617D7603F3\n\tfor ;\n\tFri, 1 May 2020 18:17:20 +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 ADEF472C;\n\tFri, 1 May 2020 18:17:19 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"JWawMgLO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1588349839;\n\tbh=sb0LH3eWnTCM+P0UVx8KC40ge5Yv0WOQcZrYnyDCZ6Q=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JWawMgLOgOErEvHq5FhU0p4eIm//+DQTgsVDyycoJW8UypdVqY5FUu49aY3coi+0b\n\tqt1ZZWWIPjIkB/neWqCD1krNgAfm85FcZw/az+JmPMeP6E4KYDGjtoU/hxqvIKvVKJ\n\tQZ828s3DEOMNkRxva83yCFWa/dyqXskTt91nYFVQ=","Date":"Fri, 1 May 2020 19:17:16 +0300","From":"Laurent Pinchart ","To":"Nicolas Dufresne ","Cc":"Niklas =?utf-8?q?S=C3=B6derlund?= ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200501161716.GI5951@pendragon.ideasonboard.com>","References":"<20200501143021.404090-1-niklas.soderlund@ragnatech.se>\n\t<8c32dc76af5e500dde4c3032cc796be2f320a6dd.camel@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<8c32dc76af5e500dde4c3032cc796be2f320a6dd.camel@ndufresne.ca>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: stream: Expose stride value","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","X-List-Received-Date":"Fri, 01 May 2020 16:17:20 -0000"}}]