[{"id":3411,"web_url":"https://patchwork.libcamera.org/comment/3411/","msgid":"<20200111010739.GN4859@pendragon.ideasonboard.com>","date":"2020-01-11T01:07:39","subject":"Re: [libcamera-devel] [PATCH v3 24/33] libcamera: pipeline: Add\n\tFrameBuffer handlers","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, Jan 10, 2020 at 08:37:59PM +0100, Niklas Söderlund wrote:\n> Extend the pipeline handlers to support the FrameBuffer API with three\n> new methods to handle allocation, importing and freeing of buffers. The\n> new methods will replace allocateBuffers() and freeBuffers().\n> \n> The FrameBuffer API will use the methods on a stream level and either\n> allocate or import buffers for each active stream controlled from the\n> Camera class and an upcoming FrameBufferAllocator helper. With this new\n> API the implementation in pipeline handlers can be made simpler as all\n> streams don't need to be handled in allocateBuffers().\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n> * Changes since v2\n> - In the subject line, s/pipelines/pipeline/\n> - Rename allocateFrameBuffers() to exportFrameBuffers()\n> - Rewrite of documentation, thanks Laurent!\n> - Remove count argument from exportFrameBuffers() and\n>   importFrameBuffers() and instead get the buffer count from the stream\n>   configuration.\n> ---\n>  src/libcamera/include/pipeline_handler.h |  5 ++\n>  src/libcamera/pipeline/ipu3/ipu3.cpp     | 32 +++++++++++\n>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 23 ++++++++\n>  src/libcamera/pipeline/uvcvideo.cpp      | 29 ++++++++++\n>  src/libcamera/pipeline/vimc.cpp          | 29 ++++++++++\n>  src/libcamera/pipeline_handler.cpp       | 67 ++++++++++++++++++++++++\n>  6 files changed, 185 insertions(+)\n> \n> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> index 067baef56278b577..560be3bad8d59b20 100644\n> --- a/src/libcamera/include/pipeline_handler.h\n> +++ b/src/libcamera/include/pipeline_handler.h\n> @@ -70,6 +70,11 @@ public:\n>  \t\tconst StreamRoles &roles) = 0;\n>  \tvirtual int configure(Camera *camera, CameraConfiguration *config) = 0;\n>  \n> +\tvirtual int exportFrameBuffers(Camera *camera, Stream *stream,\n> +\t\t\t\t       std::vector<std::unique_ptr<FrameBuffer>> *buffers) = 0;\n> +\tvirtual int importFrameBuffers(Camera *camera, Stream *stream) = 0;\n> +\tvirtual void freeFrameBuffers(Camera *camera, Stream *stream) = 0;\n> +\n>  \tvirtual int allocateBuffers(Camera *camera,\n>  \t\t\t\t    const std::set<Stream *> &streams) = 0;\n>  \tvirtual int freeBuffers(Camera *camera,\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 7c4539d488bd2d26..b73f0f0725ee2613 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -210,6 +210,11 @@ public:\n>  \t\tconst StreamRoles &roles) override;\n>  \tint configure(Camera *camera, CameraConfiguration *config) override;\n>  \n> +\tint exportFrameBuffers(Camera *camera, Stream *stream,\n> +\t\t\t       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;\n> +\tint importFrameBuffers(Camera *camera, Stream *stream) override;\n> +\tvoid freeFrameBuffers(Camera *camera, Stream *stream) override;\n> +\n>  \tint allocateBuffers(Camera *camera,\n>  \t\t\t    const std::set<Stream *> &streams) override;\n>  \tint freeBuffers(Camera *camera,\n> @@ -616,6 +621,33 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n>  \treturn 0;\n>  }\n>  \n> +int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,\n> +\t\t\t\t\t    std::vector<std::unique_ptr<FrameBuffer>> *buffers)\n> +{\n> +\tIPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream);\n> +\tV4L2VideoDevice *video = ipu3stream->device_->dev;\n> +\tunsigned int count = stream->configuration().bufferCount;\n> +\n> +\treturn video->exportBuffers(count, buffers);\n> +}\n> +\n> +int PipelineHandlerIPU3::importFrameBuffers(Camera *camera, Stream *stream)\n> +{\n> +\tIPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream);\n> +\tV4L2VideoDevice *video = ipu3stream->device_->dev;\n> +\tunsigned int count = stream->configuration().bufferCount;\n> +\n> +\treturn video->importBuffers(count);\n> +}\n> +\n> +void PipelineHandlerIPU3::freeFrameBuffers(Camera *camera, Stream *stream)\n> +{\n> +\tIPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream);\n> +\tV4L2VideoDevice *video = ipu3stream->device_->dev;\n> +\n> +\tvideo->releaseBuffers();\n> +}\n> +\n>  /**\n>   * \\todo Clarify if 'viewfinder' and 'stat' nodes have to be set up and\n>   * started even if not in use. As of now, if not properly configured and\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index dbc5df801f30e76b..ba8f93e8584c97a9 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -174,6 +174,11 @@ public:\n>  \t\tconst StreamRoles &roles) override;\n>  \tint configure(Camera *camera, CameraConfiguration *config) override;\n>  \n> +\tint exportFrameBuffers(Camera *camera, Stream *stream,\n> +\t\t\t       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;\n> +\tint importFrameBuffers(Camera *camera, Stream *stream) override;\n> +\tvoid freeFrameBuffers(Camera *camera, Stream *stream) override;\n> +\n>  \tint allocateBuffers(Camera *camera,\n>  \t\tconst std::set<Stream *> &streams) override;\n>  \tint freeBuffers(Camera *camera,\n> @@ -665,6 +670,24 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n>  \treturn 0;\n>  }\n>  \n> +int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream,\n> +\t\t\t\t\t      std::vector<std::unique_ptr<FrameBuffer>> *buffers)\n> +{\n> +\tunsigned int count = stream->configuration().bufferCount;\n> +\treturn video_->exportBuffers(count, buffers);\n> +}\n> +\n> +int PipelineHandlerRkISP1::importFrameBuffers(Camera *camera, Stream *stream)\n> +{\n> +\tunsigned int count = stream->configuration().bufferCount;\n> +\treturn video_->importBuffers(count);\n> +}\n> +\n> +void PipelineHandlerRkISP1::freeFrameBuffers(Camera *camera, Stream *stream)\n> +{\n> +\tvideo_->releaseBuffers();\n> +}\n> +\n>  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,\n>  \t\t\t\t\t   const std::set<Stream *> &streams)\n>  {\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index ce38445d2a48ca82..b72841edee572f99 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -65,6 +65,11 @@ public:\n>  \t\tconst StreamRoles &roles) override;\n>  \tint configure(Camera *camera, CameraConfiguration *config) override;\n>  \n> +\tint exportFrameBuffers(Camera *camera, Stream *stream,\n> +\t\t\t       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;\n> +\tint importFrameBuffers(Camera *camera, Stream *stream) override;\n> +\tvoid freeFrameBuffers(Camera *camera, Stream *stream) override;\n> +\n>  \tint allocateBuffers(Camera *camera,\n>  \t\t\t    const std::set<Stream *> &streams) override;\n>  \tint freeBuffers(Camera *camera,\n> @@ -193,6 +198,30 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)\n>  \treturn 0;\n>  }\n>  \n> +int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,\n> +\t\t\t\t\t   std::vector<std::unique_ptr<FrameBuffer>> *buffers)\n> +{\n> +\tUVCCameraData *data = cameraData(camera);\n> +\tunsigned int count = stream->configuration().bufferCount;\n> +\n> +\treturn data->video_->exportBuffers(count, buffers);\n> +}\n> +\n> +int PipelineHandlerUVC::importFrameBuffers(Camera *camera, Stream *stream)\n> +{\n> +\tUVCCameraData *data = cameraData(camera);\n> +\tunsigned int count = stream->configuration().bufferCount;\n> +\n> +\treturn data->video_->importBuffers(count);\n> +}\n> +\n> +void PipelineHandlerUVC::freeFrameBuffers(Camera *camera, Stream *stream)\n> +{\n> +\tUVCCameraData *data = cameraData(camera);\n> +\n> +\tdata->video_->releaseBuffers();\n> +}\n> +\n>  int PipelineHandlerUVC::allocateBuffers(Camera *camera,\n>  \t\t\t\t\tconst std::set<Stream *> &streams)\n>  {\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index 292900bcf8d1b359..3fb5cacde0dab5b6 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -82,6 +82,11 @@ public:\n>  \t\tconst StreamRoles &roles) override;\n>  \tint configure(Camera *camera, CameraConfiguration *config) override;\n>  \n> +\tint exportFrameBuffers(Camera *camera, Stream *stream,\n> +\t\t\t       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;\n> +\tint importFrameBuffers(Camera *camera, Stream *stream) override;\n> +\tvoid freeFrameBuffers(Camera *camera, Stream *stream) override;\n> +\n>  \tint allocateBuffers(Camera *camera,\n>  \t\t\t    const std::set<Stream *> &streams) override;\n>  \tint freeBuffers(Camera *camera,\n> @@ -259,6 +264,30 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n>  \treturn 0;\n>  }\n>  \n> +int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream,\n> +\t\t\t\t\t    std::vector<std::unique_ptr<FrameBuffer>> *buffers)\n> +{\n> +\tVimcCameraData *data = cameraData(camera);\n> +\tunsigned int count = stream->configuration().bufferCount;\n> +\n> +\treturn data->video_->exportBuffers(count, buffers);\n> +}\n> +\n> +int PipelineHandlerVimc::importFrameBuffers(Camera *camera, Stream *stream)\n> +{\n> +\tVimcCameraData *data = cameraData(camera);\n> +\tunsigned int count = stream->configuration().bufferCount;\n> +\n> +\treturn data->video_->importBuffers(count);\n> +}\n> +\n> +void PipelineHandlerVimc::freeFrameBuffers(Camera *camera, Stream *stream)\n> +{\n> +\tVimcCameraData *data = cameraData(camera);\n> +\n> +\tdata->video_->releaseBuffers();\n> +}\n> +\n>  int PipelineHandlerVimc::allocateBuffers(Camera *camera,\n>  \t\t\t\t\t const std::set<Stream *> &streams)\n>  {\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 698dd52560797d7c..572f751b5217eb5f 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -289,6 +289,73 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera)\n>   * \\return 0 on success or a negative error code otherwise\n>   */\n>  \n> +/**\n> + * \\fn PipelineHandler::exportFrameBuffers()\n> + * \\brief Allocate buffers for \\a stream\n> + * \\param[in] camera The camera\n> + * \\param[in] stream The stream to allocate buffers for\n> + * \\param[out] buffers Array of buffers successfully allocated\n> + *\n> + * This method allocates buffers for the \\a stream from the devices associated\n> + * with the stream in the corresponding pipeline handler. Those buffers shall be\n> + * suitable to be added to a Request for the stream, and shall be mappable to\n> + * the CPU through their associated dmabufs with mmap().\n> + *\n\nIs there a reason why you haven't included the following paragraph,\nproposed in the review of v2 ?\n\n * The method may only be called after the Camera has been configured and before\n * it gets started, or after it gets stopped. It shall be called only for\n * streams that are part of the active camera configuration, and at most once\n * per stream until buffers for the stream are freed with freeFrameBuffers().\n\n> + * exportFrameBuffers() shall also allocate all other resources required by\n> + * the pipeline handler for the stream to prepare for starting the Camera. This\n> + * responsibility is shared with importFrameBuffers(), and one and only one of\n> + * those two methods shall be called for each stream until the buffers are\n> + * freed. The pipeline handler shall support all combinations of\n> + * exportFrameBuffers() and importFrameBuffers() for the streams contained in\n> + * any camera configuration.\n> + *\n> + * The only intended caller is the FrameBufferAllocator helper.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +\n> +/**\n> + * \\fn PipelineHandler::importFrameBuffers()\n> + * \\brief Prepare \\a stream to use external buffers\n> + * \\param[in] camera The camera\n> + * \\param[in] stream The stream to prepare for import\n> + *\n> + * This method prepares the pipeline handler to use buffers provided by the\n> + * application for the \\a stream.\n> + *\n> + * The method may only be called after the Camera has been configured and before\n> + * it gets started, or after it gets stopped. It shall be called only for\n> + * streams that are part of the active camera configuration, and at most once\n> + * per stream until buffers for the stream are freed with freeFrameBuffers().\n> + *\n> + * importFrameBuffers() shall also allocate all other resources required by the\n> + * pipeline handler for the stream to prepare for starting the Camera. This\n> + * responsibility is shared with exportFrameBuffers(), and one and only one of\n> + * those two methods shall be called for each stream until the buffers are\n> + * freed. The pipeline handler shall support all combinations of\n> + * exportFrameBuffers() and importFrameBuffers() for the streams contained in\n> + * any camera configuration.\n> + *\n> + * The only intended caller is Camera::start().\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +\n> +/**\n> + * \\fn PipelineHandler::freeFrameBuffers()\n> + * \\brief Free buffers allocated from the stream\n> + * \\param[in] camera The camera\n> + * \\param[in] stream The stream to free buffers for\n> + *\n> + * This method shall free all buffers and all other resources allocated for the\n> + * \\a stream by exportFrameBuffers() or importFrameBuffers(). It shall be\n> + * called only after a successful call to either of these two methods, and only\n> + * once per stream.\n> + *\n> + * The only intended callers are Camera::stop() and the FrameBufferAllocator\n> + * helper.\n> + */\n> +\n>  /**\n>   * \\fn PipelineHandler::allocateBuffers()\n>   * \\brief Allocate buffers for a stream","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E9571606AC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 11 Jan 2020 02:07:52 +0100 (CET)","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 6566952F;\n\tSat, 11 Jan 2020 02:07:52 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1578704872;\n\tbh=21b0Y6ccoqq2KemTaW39TORjfWPQwZJDByoKhK1P+Bc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jAv+uy/1QM7aaWqrRQAf+wusasp9jlJthtSCqO5CxlOpEUJBm9u3jarLldAjK+/b1\n\tpyCbqXS9ltD/BinlxYD7qNUdl8B8d0NjIJJNcVQkedhckahu7JZ9jE4Fg9X2rrdNqA\n\tGc0POYEmrZkfd+xPMEBHJypsqSFTUXE7qrekJ52Q=","Date":"Sat, 11 Jan 2020 03:07:39 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200111010739.GN4859@pendragon.ideasonboard.com>","References":"<20200110193808.2266294-1-niklas.soderlund@ragnatech.se>\n\t<20200110193808.2266294-25-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":"<20200110193808.2266294-25-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 24/33] libcamera: pipeline: Add\n\tFrameBuffer handlers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Sat, 11 Jan 2020 01:07:53 -0000"}},{"id":3423,"web_url":"https://patchwork.libcamera.org/comment/3423/","msgid":"<20200111233839.GC697401@oden.dyn.berto.se>","date":"2020-01-11T23:38:39","subject":"Re: [libcamera-devel] [PATCH v3 24/33] libcamera: pipeline: Add\n\tFrameBuffer handlers","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 feedback.\n\nOn 2020-01-11 03:07:39 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Fri, Jan 10, 2020 at 08:37:59PM +0100, Niklas Söderlund wrote:\n> > Extend the pipeline handlers to support the FrameBuffer API with three\n> > new methods to handle allocation, importing and freeing of buffers. The\n> > new methods will replace allocateBuffers() and freeBuffers().\n> > \n> > The FrameBuffer API will use the methods on a stream level and either\n> > allocate or import buffers for each active stream controlled from the\n> > Camera class and an upcoming FrameBufferAllocator helper. With this new\n> > API the implementation in pipeline handlers can be made simpler as all\n> > streams don't need to be handled in allocateBuffers().\n> > \n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> > * Changes since v2\n> > - In the subject line, s/pipelines/pipeline/\n> > - Rename allocateFrameBuffers() to exportFrameBuffers()\n> > - Rewrite of documentation, thanks Laurent!\n> > - Remove count argument from exportFrameBuffers() and\n> >   importFrameBuffers() and instead get the buffer count from the stream\n> >   configuration.\n> > ---\n> >  src/libcamera/include/pipeline_handler.h |  5 ++\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp     | 32 +++++++++++\n> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 23 ++++++++\n> >  src/libcamera/pipeline/uvcvideo.cpp      | 29 ++++++++++\n> >  src/libcamera/pipeline/vimc.cpp          | 29 ++++++++++\n> >  src/libcamera/pipeline_handler.cpp       | 67 ++++++++++++++++++++++++\n> >  6 files changed, 185 insertions(+)\n> > \n> > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> > index 067baef56278b577..560be3bad8d59b20 100644\n> > --- a/src/libcamera/include/pipeline_handler.h\n> > +++ b/src/libcamera/include/pipeline_handler.h\n> > @@ -70,6 +70,11 @@ public:\n> >  \t\tconst StreamRoles &roles) = 0;\n> >  \tvirtual int configure(Camera *camera, CameraConfiguration *config) = 0;\n> >  \n> > +\tvirtual int exportFrameBuffers(Camera *camera, Stream *stream,\n> > +\t\t\t\t       std::vector<std::unique_ptr<FrameBuffer>> *buffers) = 0;\n> > +\tvirtual int importFrameBuffers(Camera *camera, Stream *stream) = 0;\n> > +\tvirtual void freeFrameBuffers(Camera *camera, Stream *stream) = 0;\n> > +\n> >  \tvirtual int allocateBuffers(Camera *camera,\n> >  \t\t\t\t    const std::set<Stream *> &streams) = 0;\n> >  \tvirtual int freeBuffers(Camera *camera,\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 7c4539d488bd2d26..b73f0f0725ee2613 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -210,6 +210,11 @@ public:\n> >  \t\tconst StreamRoles &roles) override;\n> >  \tint configure(Camera *camera, CameraConfiguration *config) override;\n> >  \n> > +\tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > +\t\t\t       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;\n> > +\tint importFrameBuffers(Camera *camera, Stream *stream) override;\n> > +\tvoid freeFrameBuffers(Camera *camera, Stream *stream) override;\n> > +\n> >  \tint allocateBuffers(Camera *camera,\n> >  \t\t\t    const std::set<Stream *> &streams) override;\n> >  \tint freeBuffers(Camera *camera,\n> > @@ -616,6 +621,33 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> >  \treturn 0;\n> >  }\n> >  \n> > +int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,\n> > +\t\t\t\t\t    std::vector<std::unique_ptr<FrameBuffer>> *buffers)\n> > +{\n> > +\tIPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream);\n> > +\tV4L2VideoDevice *video = ipu3stream->device_->dev;\n> > +\tunsigned int count = stream->configuration().bufferCount;\n> > +\n> > +\treturn video->exportBuffers(count, buffers);\n> > +}\n> > +\n> > +int PipelineHandlerIPU3::importFrameBuffers(Camera *camera, Stream *stream)\n> > +{\n> > +\tIPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream);\n> > +\tV4L2VideoDevice *video = ipu3stream->device_->dev;\n> > +\tunsigned int count = stream->configuration().bufferCount;\n> > +\n> > +\treturn video->importBuffers(count);\n> > +}\n> > +\n> > +void PipelineHandlerIPU3::freeFrameBuffers(Camera *camera, Stream *stream)\n> > +{\n> > +\tIPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream);\n> > +\tV4L2VideoDevice *video = ipu3stream->device_->dev;\n> > +\n> > +\tvideo->releaseBuffers();\n> > +}\n> > +\n> >  /**\n> >   * \\todo Clarify if 'viewfinder' and 'stat' nodes have to be set up and\n> >   * started even if not in use. As of now, if not properly configured and\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index dbc5df801f30e76b..ba8f93e8584c97a9 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -174,6 +174,11 @@ public:\n> >  \t\tconst StreamRoles &roles) override;\n> >  \tint configure(Camera *camera, CameraConfiguration *config) override;\n> >  \n> > +\tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > +\t\t\t       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;\n> > +\tint importFrameBuffers(Camera *camera, Stream *stream) override;\n> > +\tvoid freeFrameBuffers(Camera *camera, Stream *stream) override;\n> > +\n> >  \tint allocateBuffers(Camera *camera,\n> >  \t\tconst std::set<Stream *> &streams) override;\n> >  \tint freeBuffers(Camera *camera,\n> > @@ -665,6 +670,24 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> >  \treturn 0;\n> >  }\n> >  \n> > +int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream,\n> > +\t\t\t\t\t      std::vector<std::unique_ptr<FrameBuffer>> *buffers)\n> > +{\n> > +\tunsigned int count = stream->configuration().bufferCount;\n> > +\treturn video_->exportBuffers(count, buffers);\n> > +}\n> > +\n> > +int PipelineHandlerRkISP1::importFrameBuffers(Camera *camera, Stream *stream)\n> > +{\n> > +\tunsigned int count = stream->configuration().bufferCount;\n> > +\treturn video_->importBuffers(count);\n> > +}\n> > +\n> > +void PipelineHandlerRkISP1::freeFrameBuffers(Camera *camera, Stream *stream)\n> > +{\n> > +\tvideo_->releaseBuffers();\n> > +}\n> > +\n> >  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,\n> >  \t\t\t\t\t   const std::set<Stream *> &streams)\n> >  {\n> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > index ce38445d2a48ca82..b72841edee572f99 100644\n> > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > @@ -65,6 +65,11 @@ public:\n> >  \t\tconst StreamRoles &roles) override;\n> >  \tint configure(Camera *camera, CameraConfiguration *config) override;\n> >  \n> > +\tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > +\t\t\t       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;\n> > +\tint importFrameBuffers(Camera *camera, Stream *stream) override;\n> > +\tvoid freeFrameBuffers(Camera *camera, Stream *stream) override;\n> > +\n> >  \tint allocateBuffers(Camera *camera,\n> >  \t\t\t    const std::set<Stream *> &streams) override;\n> >  \tint freeBuffers(Camera *camera,\n> > @@ -193,6 +198,30 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)\n> >  \treturn 0;\n> >  }\n> >  \n> > +int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,\n> > +\t\t\t\t\t   std::vector<std::unique_ptr<FrameBuffer>> *buffers)\n> > +{\n> > +\tUVCCameraData *data = cameraData(camera);\n> > +\tunsigned int count = stream->configuration().bufferCount;\n> > +\n> > +\treturn data->video_->exportBuffers(count, buffers);\n> > +}\n> > +\n> > +int PipelineHandlerUVC::importFrameBuffers(Camera *camera, Stream *stream)\n> > +{\n> > +\tUVCCameraData *data = cameraData(camera);\n> > +\tunsigned int count = stream->configuration().bufferCount;\n> > +\n> > +\treturn data->video_->importBuffers(count);\n> > +}\n> > +\n> > +void PipelineHandlerUVC::freeFrameBuffers(Camera *camera, Stream *stream)\n> > +{\n> > +\tUVCCameraData *data = cameraData(camera);\n> > +\n> > +\tdata->video_->releaseBuffers();\n> > +}\n> > +\n> >  int PipelineHandlerUVC::allocateBuffers(Camera *camera,\n> >  \t\t\t\t\tconst std::set<Stream *> &streams)\n> >  {\n> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> > index 292900bcf8d1b359..3fb5cacde0dab5b6 100644\n> > --- a/src/libcamera/pipeline/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc.cpp\n> > @@ -82,6 +82,11 @@ public:\n> >  \t\tconst StreamRoles &roles) override;\n> >  \tint configure(Camera *camera, CameraConfiguration *config) override;\n> >  \n> > +\tint exportFrameBuffers(Camera *camera, Stream *stream,\n> > +\t\t\t       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;\n> > +\tint importFrameBuffers(Camera *camera, Stream *stream) override;\n> > +\tvoid freeFrameBuffers(Camera *camera, Stream *stream) override;\n> > +\n> >  \tint allocateBuffers(Camera *camera,\n> >  \t\t\t    const std::set<Stream *> &streams) override;\n> >  \tint freeBuffers(Camera *camera,\n> > @@ -259,6 +264,30 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> >  \treturn 0;\n> >  }\n> >  \n> > +int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream,\n> > +\t\t\t\t\t    std::vector<std::unique_ptr<FrameBuffer>> *buffers)\n> > +{\n> > +\tVimcCameraData *data = cameraData(camera);\n> > +\tunsigned int count = stream->configuration().bufferCount;\n> > +\n> > +\treturn data->video_->exportBuffers(count, buffers);\n> > +}\n> > +\n> > +int PipelineHandlerVimc::importFrameBuffers(Camera *camera, Stream *stream)\n> > +{\n> > +\tVimcCameraData *data = cameraData(camera);\n> > +\tunsigned int count = stream->configuration().bufferCount;\n> > +\n> > +\treturn data->video_->importBuffers(count);\n> > +}\n> > +\n> > +void PipelineHandlerVimc::freeFrameBuffers(Camera *camera, Stream *stream)\n> > +{\n> > +\tVimcCameraData *data = cameraData(camera);\n> > +\n> > +\tdata->video_->releaseBuffers();\n> > +}\n> > +\n> >  int PipelineHandlerVimc::allocateBuffers(Camera *camera,\n> >  \t\t\t\t\t const std::set<Stream *> &streams)\n> >  {\n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index 698dd52560797d7c..572f751b5217eb5f 100644\n> > --- a/src/libcamera/pipeline_handler.cpp\n> > +++ b/src/libcamera/pipeline_handler.cpp\n> > @@ -289,6 +289,73 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera)\n> >   * \\return 0 on success or a negative error code otherwise\n> >   */\n> >  \n> > +/**\n> > + * \\fn PipelineHandler::exportFrameBuffers()\n> > + * \\brief Allocate buffers for \\a stream\n> > + * \\param[in] camera The camera\n> > + * \\param[in] stream The stream to allocate buffers for\n> > + * \\param[out] buffers Array of buffers successfully allocated\n> > + *\n> > + * This method allocates buffers for the \\a stream from the devices associated\n> > + * with the stream in the corresponding pipeline handler. Those buffers shall be\n> > + * suitable to be added to a Request for the stream, and shall be mappable to\n> > + * the CPU through their associated dmabufs with mmap().\n> > + *\n> \n> Is there a reason why you haven't included the following paragraph,\n> proposed in the review of v2 ?\n\nI must have missed it :-( Sorry about that, will add for v4.\n\n> \n>  * The method may only be called after the Camera has been configured and before\n>  * it gets started, or after it gets stopped. It shall be called only for\n>  * streams that are part of the active camera configuration, and at most once\n>  * per stream until buffers for the stream are freed with freeFrameBuffers().\n> \n> > + * exportFrameBuffers() shall also allocate all other resources required by\n> > + * the pipeline handler for the stream to prepare for starting the Camera. This\n> > + * responsibility is shared with importFrameBuffers(), and one and only one of\n> > + * those two methods shall be called for each stream until the buffers are\n> > + * freed. The pipeline handler shall support all combinations of\n> > + * exportFrameBuffers() and importFrameBuffers() for the streams contained in\n> > + * any camera configuration.\n> > + *\n> > + * The only intended caller is the FrameBufferAllocator helper.\n> > + *\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +\n> > +/**\n> > + * \\fn PipelineHandler::importFrameBuffers()\n> > + * \\brief Prepare \\a stream to use external buffers\n> > + * \\param[in] camera The camera\n> > + * \\param[in] stream The stream to prepare for import\n> > + *\n> > + * This method prepares the pipeline handler to use buffers provided by the\n> > + * application for the \\a stream.\n> > + *\n> > + * The method may only be called after the Camera has been configured and before\n> > + * it gets started, or after it gets stopped. It shall be called only for\n> > + * streams that are part of the active camera configuration, and at most once\n> > + * per stream until buffers for the stream are freed with freeFrameBuffers().\n> > + *\n> > + * importFrameBuffers() shall also allocate all other resources required by the\n> > + * pipeline handler for the stream to prepare for starting the Camera. This\n> > + * responsibility is shared with exportFrameBuffers(), and one and only one of\n> > + * those two methods shall be called for each stream until the buffers are\n> > + * freed. The pipeline handler shall support all combinations of\n> > + * exportFrameBuffers() and importFrameBuffers() for the streams contained in\n> > + * any camera configuration.\n> > + *\n> > + * The only intended caller is Camera::start().\n> > + *\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +\n> > +/**\n> > + * \\fn PipelineHandler::freeFrameBuffers()\n> > + * \\brief Free buffers allocated from the stream\n> > + * \\param[in] camera The camera\n> > + * \\param[in] stream The stream to free buffers for\n> > + *\n> > + * This method shall free all buffers and all other resources allocated for the\n> > + * \\a stream by exportFrameBuffers() or importFrameBuffers(). It shall be\n> > + * called only after a successful call to either of these two methods, and only\n> > + * once per stream.\n> > + *\n> > + * The only intended callers are Camera::stop() and the FrameBufferAllocator\n> > + * helper.\n> > + */\n> > +\n> >  /**\n> >   * \\fn PipelineHandler::allocateBuffers()\n> >   * \\brief Allocate buffers for a stream\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lf1-x144.google.com (mail-lf1-x144.google.com\n\t[IPv6:2a00:1450:4864:20::144])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9C9CB6045B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 12 Jan 2020 00:38:42 +0100 (CET)","by mail-lf1-x144.google.com with SMTP id n25so4267284lfl.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 11 Jan 2020 15:38:42 -0800 (PST)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\td4sm3319566lfn.42.2020.01.11.15.38.40\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 11 Jan 2020 15:38:40 -0800 (PST)"],"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=18eLixqKlHjMw8Fg+mhCIn0WTIaGTUo7/32OGKH7oAI=;\n\tb=R2GJPT/obY9ogsi8GJ49Hn4G1iEGNO0wmxQjs7fXmW5MSMxXcRY3y2Qg150uILLdFu\n\tBvgbqATvaIoz9pTnV3Wj2jFZbCdh5cwU3r+OglKoUZdxx+P1d+rCt1gAIzWkq1xFXhkl\n\tJoFutkZzWN6tDQbaKRBogHJ1yZXEodjJV9Jfur24c2p1Jtm+Ic2B34WCstLeo4hxWMit\n\tqHkL+K423eMh/BVld1pT9s01SsCIiUUzx9rJER6GxVVE/nxlRIIa0t+K/90GEdnfZ554\n\tM03R1tyhzyLsgIFojdmfmy8dhn3T9aqYZiIJgJeOwpOY5mNp9acEBaT6kZB24MSw4lsq\n\tGGJw==","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=18eLixqKlHjMw8Fg+mhCIn0WTIaGTUo7/32OGKH7oAI=;\n\tb=pHd7xEK9HVWvFNppEVscp3FhyZuK0QM7b69P2VvyvJW9NtVjiroqXHcRPC/pKHc35+\n\tWgEVmPzrAy+ruFYLI1sb0uU6JuLMfVPJxiiWKAa+EZ96YR4g4NF10WMV6Q9n/6A+pvVm\n\t7quMA2fwVGnJvZa7RjbodX9HvkTPpHbz1gPWqbLtsKc/78w7nl6chq/VRwKuB7WMqBf/\n\tVNqTu3iPzRNvzTLvNr441tESNVYdekGMk675LbhJkGjYmIk+0gecFIKRkuJQbMD5ZDsA\n\tbfJsYim0FUB8Qo/5m6WNrzbf2NTZO6xnf5YrptyuWk1uotf6jwsOZyYRtKPr8m7k7DT/\n\t9lsA==","X-Gm-Message-State":"APjAAAV0Kr+anaLb9aIg/HWdD+xBM3xUzCaCCcwe/wujKC6g+dbGFh7C\n\tzLaxMpYOVXhw5D2R/eJlBRdUyQ==","X-Google-Smtp-Source":"APXvYqwD8aFBWleQx0xRrnYSecPzpP6v+B9/uvyXcFFJ4v2Qg8oR+3QIAIxPAG7GzQ+tx8ngLuLXSw==","X-Received":"by 2002:ac2:4add:: with SMTP id\n\tm29mr6128625lfp.190.1578785921666; \n\tSat, 11 Jan 2020 15:38:41 -0800 (PST)","Date":"Sun, 12 Jan 2020 00:38:39 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200111233839.GC697401@oden.dyn.berto.se>","References":"<20200110193808.2266294-1-niklas.soderlund@ragnatech.se>\n\t<20200110193808.2266294-25-niklas.soderlund@ragnatech.se>\n\t<20200111010739.GN4859@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":"<20200111010739.GN4859@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 24/33] libcamera: pipeline: Add\n\tFrameBuffer handlers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Sat, 11 Jan 2020 23:38:42 -0000"}}]