Patch Detail
Show a patch.
GET /api/patches/2579/?format=api
{ "id": 2579, "url": "https://patchwork.libcamera.org/api/patches/2579/?format=api", "web_url": "https://patchwork.libcamera.org/patch/2579/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20200110193808.2266294-33-niklas.soderlund@ragnatech.se>", "date": "2020-01-10T19:38:07", "name": "[libcamera-devel,v3,32/33] libcamera: pipeline: Remove explicit buffer handling", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "120edb7ebc078677425427799ae246d47cfe5d51", "submitter": { "id": 5, "url": "https://patchwork.libcamera.org/api/people/5/?format=api", "name": "Niklas Söderlund", "email": "niklas.soderlund@ragnatech.se" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/2579/mbox/", "series": [ { "id": 615, "url": "https://patchwork.libcamera.org/api/series/615/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=615", "date": "2020-01-10T19:37:35", "name": "libcamera: Rework buffer API", "version": 3, "mbox": "https://patchwork.libcamera.org/series/615/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/2579/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/2579/checks/", "tags": {}, "headers": { "Return-Path": "<niklas.soderlund@ragnatech.se>", "Received": [ "from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net\n\t[195.74.38.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 48873606B0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 10 Jan 2020 20:39:11 +0100 (CET)", "from bismarck.berto.se (p54ac5d7b.dip0.t-ipconnect.de\n\t[84.172.93.123]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA\n\tid dd9a08e9-33e0-11ea-b6d8-005056917f90;\n\tFri, 10 Jan 2020 20:39:07 +0100 (CET)" ], "X-Halon-ID": "dd9a08e9-33e0-11ea-b6d8-005056917f90", "Authorized-sender": "niklas@soderlund.pp.se", "From": "=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Fri, 10 Jan 2020 20:38:07 +0100", "Message-Id": "<20200110193808.2266294-33-niklas.soderlund@ragnatech.se>", "X-Mailer": "git-send-email 2.24.1", "In-Reply-To": "<20200110193808.2266294-1-niklas.soderlund@ragnatech.se>", "References": "<20200110193808.2266294-1-niklas.soderlund@ragnatech.se>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v3 32/33] libcamera: pipeline: Remove\n\texplicit buffer handling", "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": "Fri, 10 Jan 2020 19:39:11 -0000" }, "content": "With the FrameBuffer interface in place there is no need for the Camera\nto call into the specific pipelines allocation and freeing of buffers as\nit no longer needs to be synchronized with buffer allocation by the\napplication.\n\nRemove the function prototypes in the pipeline handler base class and\nfold the functionality in the pipelines start() and stop() functions\nwhere needed. A follow up patch will remove the now no-op\nCamera::allocateBuffers() and Camera::freeBuffers().\n\nSigned-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/libcamera/camera.cpp | 8 +------\n src/libcamera/include/pipeline_handler.h | 5 ----\n src/libcamera/pipeline/ipu3/ipu3.cpp | 24 ++++++++++++--------\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 24 ++++++++++++--------\n src/libcamera/pipeline/uvcvideo.cpp | 17 --------------\n src/libcamera/pipeline/vimc.cpp | 17 --------------\n src/libcamera/pipeline_handler.cpp | 29 ------------------------\n 7 files changed, 30 insertions(+), 94 deletions(-)", "diff": "diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex 963b78d87f8eba22..8c3ebb2c17666c52 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -726,12 +726,6 @@ int Camera::allocateBuffers()\n \t\treturn -EINVAL;\n \t}\n \n-\tint ret = pipe_->allocateBuffers(this, activeStreams_);\n-\tif (ret) {\n-\t\tLOG(Camera, Error) << \"Failed to allocate buffers\";\n-\t\treturn ret;\n-\t}\n-\n \tstate_ = CameraPrepared;\n \n \treturn 0;\n@@ -752,7 +746,7 @@ int Camera::freeBuffers()\n \n \tstate_ = CameraConfigured;\n \n-\treturn pipe_->freeBuffers(this, activeStreams_);\n+\treturn 0;\n }\n \n /**\ndiff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\nindex 27f3852c6c87843f..a6c1e1fbae384405 100644\n--- a/src/libcamera/include/pipeline_handler.h\n+++ b/src/libcamera/include/pipeline_handler.h\n@@ -75,11 +75,6 @@ public:\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-\t\t\t\tconst std::set<Stream *> &streams) = 0;\n-\n \tvirtual int start(Camera *camera) = 0;\n \tvirtual void stop(Camera *camera) = 0;\n \ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex 1ea4d938ad88ae21..7894084a025e9946 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -211,11 +211,6 @@ public:\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-\t\t\tconst std::set<Stream *> &streams) override;\n-\n \tint start(Camera *camera) override;\n \tvoid stop(Camera *camera) override;\n \n@@ -232,6 +227,9 @@ private:\n \n \tint registerCameras();\n \n+\tint allocateBuffers(Camera *camera);\n+\tint freeBuffers(Camera *camera);\n+\n \tImgUDevice imgu0_;\n \tImgUDevice imgu1_;\n \tMediaDevice *cio2MediaDev_;\n@@ -652,8 +650,7 @@ void PipelineHandlerIPU3::freeFrameBuffers(Camera *camera, Stream *stream)\n * In order to be able to start the 'viewfinder' and 'stat' nodes, we need\n * memory to be reserved.\n */\n-int PipelineHandlerIPU3::allocateBuffers(Camera *camera,\n-\t\t\t\t\t const std::set<Stream *> &streams)\n+int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n {\n \tIPU3CameraData *data = cameraData(camera);\n \tIPU3Stream *outStream = &data->outStream_;\n@@ -716,13 +713,12 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera,\n \treturn 0;\n \n error:\n-\tfreeBuffers(camera, streams);\n+\tfreeBuffers(camera);\n \n \treturn ret;\n }\n \n-int PipelineHandlerIPU3::freeBuffers(Camera *camera,\n-\t\t\t\t const std::set<Stream *> &streams)\n+int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n {\n \tIPU3CameraData *data = cameraData(camera);\n \n@@ -739,6 +735,11 @@ int PipelineHandlerIPU3::start(Camera *camera)\n \tImgUDevice *imgu = data->imgu_;\n \tint ret;\n \n+\t/* Allocate buffers for internal pipeline usage. */\n+\tret = allocateBuffers(camera);\n+\tif (ret)\n+\t\treturn ret;\n+\n \t/*\n \t * Start the ImgU video devices, buffers will be queued to the\n \t * ImgU output and viewfinder when requests will be queued.\n@@ -757,6 +758,7 @@ int PipelineHandlerIPU3::start(Camera *camera)\n \treturn 0;\n \n error:\n+\tfreeBuffers(camera);\n \tLOG(IPU3, Error) << \"Failed to start camera \" << camera->name();\n \n \treturn ret;\n@@ -772,6 +774,8 @@ void PipelineHandlerIPU3::stop(Camera *camera)\n \tif (ret)\n \t\tLOG(IPU3, Warning) << \"Failed to stop camera \"\n \t\t\t\t << camera->name();\n+\n+\tfreeBuffers(camera);\n }\n \n int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex da6e079f51620234..389a99cf52bd1cea 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -178,11 +178,6 @@ public:\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-\t\tconst std::set<Stream *> &streams) override;\n-\n \tint start(Camera *camera) override;\n \tvoid stop(Camera *camera) override;\n \n@@ -208,6 +203,9 @@ private:\n \tvoid paramReady(FrameBuffer *buffer);\n \tvoid statReady(FrameBuffer *buffer);\n \n+\tint allocateBuffers(Camera *camera);\n+\tint freeBuffers(Camera *camera);\n+\n \tMediaDevice *media_;\n \tV4L2Subdevice *dphy_;\n \tV4L2Subdevice *isp_;\n@@ -675,8 +673,7 @@ void PipelineHandlerRkISP1::freeFrameBuffers(Camera *camera, Stream *stream)\n \tvideo_->releaseBuffers();\n }\n \n-int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,\n-\t\t\t\t\t const std::set<Stream *> &streams)\n+int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n {\n \tRkISP1CameraData *data = cameraData(camera);\n \tunsigned int count = 1;\n@@ -720,8 +717,7 @@ error:\n \treturn ret;\n }\n \n-int PipelineHandlerRkISP1::freeBuffers(Camera *camera,\n-\t\t\t\t const std::set<Stream *> &streams)\n+int PipelineHandlerRkISP1::freeBuffers(Camera *camera)\n {\n \tRkISP1CameraData *data = cameraData(camera);\n \n@@ -755,10 +751,16 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n \tRkISP1CameraData *data = cameraData(camera);\n \tint ret;\n \n+\t/* Allocate buffers for internal pipeline usage. */\n+\tret = allocateBuffers(camera);\n+\tif (ret)\n+\t\treturn ret;\n+\n \tdata->frame_ = 0;\n \n \tret = param_->streamOn();\n \tif (ret) {\n+\t\tfreeBuffers(camera);\n \t\tLOG(RkISP1, Error)\n \t\t\t<< \"Failed to start parameters \" << camera->name();\n \t\treturn ret;\n@@ -767,6 +769,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n \tret = stat_->streamOn();\n \tif (ret) {\n \t\tparam_->streamOff();\n+\t\tfreeBuffers(camera);\n \t\tLOG(RkISP1, Error)\n \t\t\t<< \"Failed to start statistics \" << camera->name();\n \t\treturn ret;\n@@ -776,6 +779,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n \tif (ret) {\n \t\tparam_->streamOff();\n \t\tstat_->streamOff();\n+\t\tfreeBuffers(camera);\n \n \t\tLOG(RkISP1, Error)\n \t\t\t<< \"Failed to start camera \" << camera->name();\n@@ -820,6 +824,8 @@ void PipelineHandlerRkISP1::stop(Camera *camera)\n \n \tdata->timeline_.reset();\n \n+\tfreeBuffers(camera);\n+\n \tactiveCamera_ = nullptr;\n }\n \ndiff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\nindex 67d29b7919e46801..47916ffb453672d4 100644\n--- a/src/libcamera/pipeline/uvcvideo.cpp\n+++ b/src/libcamera/pipeline/uvcvideo.cpp\n@@ -70,11 +70,6 @@ public:\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-\t\t\tconst std::set<Stream *> &streams) override;\n-\n \tint start(Camera *camera) override;\n \tvoid stop(Camera *camera) override;\n \n@@ -222,18 +217,6 @@ void PipelineHandlerUVC::freeFrameBuffers(Camera *camera, Stream *stream)\n \tdata->video_->releaseBuffers();\n }\n \n-int PipelineHandlerUVC::allocateBuffers(Camera *camera,\n-\t\t\t\t\tconst std::set<Stream *> &streams)\n-{\n-\treturn 0;\n-}\n-\n-int PipelineHandlerUVC::freeBuffers(Camera *camera,\n-\t\t\t\t const std::set<Stream *> &streams)\n-{\n-\treturn 0;\n-}\n-\n int PipelineHandlerUVC::start(Camera *camera)\n {\n \tUVCCameraData *data = cameraData(camera);\ndiff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\nindex 4cfdb5ae79ce5779..1700ac967299b106 100644\n--- a/src/libcamera/pipeline/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc.cpp\n@@ -87,11 +87,6 @@ public:\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-\t\t\tconst std::set<Stream *> &streams) override;\n-\n \tint start(Camera *camera) override;\n \tvoid stop(Camera *camera) override;\n \n@@ -288,18 +283,6 @@ void PipelineHandlerVimc::freeFrameBuffers(Camera *camera, Stream *stream)\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-\treturn 0;\n-}\n-\n-int PipelineHandlerVimc::freeBuffers(Camera *camera,\n-\t\t\t\t const std::set<Stream *> &streams)\n-{\n-\treturn 0;\n-}\n-\n int PipelineHandlerVimc::start(Camera *camera)\n {\n \tVimcCameraData *data = cameraData(camera);\ndiff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\nindex 0348e3cfa68ed6ec..2fd65b468b9cd84f 100644\n--- a/src/libcamera/pipeline_handler.cpp\n+++ b/src/libcamera/pipeline_handler.cpp\n@@ -356,35 +356,6 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera)\n * helper.\n */\n \n-/**\n- * \\fn PipelineHandler::allocateBuffers()\n- * \\brief Allocate buffers for a stream\n- * \\param[in] camera The camera the \\a stream belongs to\n- * \\param[in] streams The set of streams to allocate buffers for\n- *\n- * This method allocates buffers internally in the pipeline handler for each\n- * stream in the \\a streams buffer set, and associates them with the stream's\n- * buffer pool.\n- *\n- * The intended caller of this method is the Camera class.\n- *\n- * \\return 0 on success or a negative error code otherwise\n- */\n-\n-/**\n- * \\fn PipelineHandler::freeBuffers()\n- * \\brief Free all buffers associated with a stream\n- * \\param[in] camera The camera the \\a stream belongs to\n- * \\param[in] streams The set of streams to free buffers from\n- *\n- * After a capture session has been stopped all buffers associated with each\n- * stream shall be freed.\n- *\n- * The intended caller of this method is the Camera class.\n- *\n- * \\return 0 on success or a negative error code otherwise\n- */\n-\n /**\n * \\fn PipelineHandler::start()\n * \\brief Start capturing from a group of streams\n", "prefixes": [ "libcamera-devel", "v3", "32/33" ] }