Patch Detail
Show a patch.
GET /api/1.1/patches/3100/?format=api
{ "id": 3100, "url": "https://patchwork.libcamera.org/api/1.1/patches/3100/?format=api", "web_url": "https://patchwork.libcamera.org/patch/3100/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/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": "<20200314235728.15495-8-laurent.pinchart@ideasonboard.com>", "date": "2020-03-14T23:57:26", "name": "[libcamera-devel,7/9] libcamera: pipeline_handler: Decouple buffer import and export", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "4e16d9f997e21fe2bae06ffe6c6d838e8e65926c", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/3100/mbox/", "series": [ { "id": 719, "url": "https://patchwork.libcamera.org/api/1.1/series/719/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=719", "date": "2020-03-14T23:57:19", "name": "Simplify buffer management with V4L2 buffer orphaning", "version": 1, "mbox": "https://patchwork.libcamera.org/series/719/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/3100/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/3100/checks/", "tags": {}, "headers": { "Return-Path": "<laurent.pinchart@ideasonboard.com>", "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 AE9BF62948\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 15 Mar 2020 00:57:42 +0100 (CET)", "from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 48AD21163\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 15 Mar 2020 00:57:42 +0100 (CET)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1584230262;\n\tbh=4AldzKHX3fs3lMoI0VJvST0CJIkkeFe56cH0WOUeeHU=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=alVadJvC+MM8KMQiICJhMxJBJ5j8YjuTAo+odOQj4LL9Yr/4LsyZhElfJZaN1ywID\n\trJ18EB1oOoVR5SbZ5XdY8WKZwVdREKDHyRliVSnsptZyvNVtjEkRBfimfE6eThPwWm\n\tQOovo1A4zYE+80EK+Q9TWz3jsoYHS5y0TyeVC48U=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Sun, 15 Mar 2020 01:57:26 +0200", "Message-Id": "<20200314235728.15495-8-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.24.1", "In-Reply-To": "<20200314235728.15495-1-laurent.pinchart@ideasonboard.com>", "References": "<20200314235728.15495-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH 7/9] libcamera: pipeline_handler: Decouple\n\tbuffer import and export", "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, 14 Mar 2020 23:57:43 -0000" }, "content": "Use the V4L2 buffer orphaning feature, exposed through\nV4L2VideoDevice::exportBuffers(), to decouple buffer import and export.\nThe PipelineHandler::importFrameBuffers() function is now called for all\nstreams regardless of whether exportFrameBuffers() has been called or\nnot. This simplifies the Camera implementation slightly, and opens the\ndoor to additional simplifications.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/camera.h | 1 -\n src/libcamera/camera.cpp | 21 +-------------------\n src/libcamera/framebuffer_allocator.cpp | 9 ---------\n src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-\n src/libcamera/pipeline/uvcvideo.cpp | 2 +-\n src/libcamera/pipeline/vimc.cpp | 2 +-\n src/libcamera/pipeline_handler.cpp | 25 +++++-------------------\n 8 files changed, 10 insertions(+), 54 deletions(-)", "diff": "diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\nindex c453b95228a7..a5e2b49f0f25 100644\n--- a/include/libcamera/camera.h\n+++ b/include/libcamera/camera.h\n@@ -113,7 +113,6 @@ private:\n \tfriend class FrameBufferAllocator;\n \tint exportFrameBuffers(Stream *stream,\n \t\t\t std::vector<std::unique_ptr<FrameBuffer>> *buffers);\n-\tint freeFrameBuffers(Stream *stream);\n \t/* \\todo Remove allocator_ from the exposed API */\n \tFrameBufferAllocator *allocator_;\n };\ndiff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex 9c432adb1d97..3192dfb42d01 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -554,18 +554,6 @@ int Camera::exportFrameBuffers(Stream *stream,\n \t\t\t\t buffers);\n }\n \n-int Camera::freeFrameBuffers(Stream *stream)\n-{\n-\tint ret = p_->isAccessAllowed(Private::CameraConfigured, true);\n-\tif (ret < 0)\n-\t\treturn ret;\n-\n-\tp_->pipe_->invokeMethod(&PipelineHandler::freeFrameBuffers,\n-\t\t\t\tConnectionTypeBlocking, this, stream);\n-\n-\treturn 0;\n-}\n-\n /**\n * \\brief Acquire the camera device for exclusive access\n *\n@@ -928,9 +916,6 @@ int Camera::start()\n \tLOG(Camera, Debug) << \"Starting capture\";\n \n \tfor (Stream *stream : p_->activeStreams_) {\n-\t\tif (allocator_ && !allocator_->buffers(stream).empty())\n-\t\t\tcontinue;\n-\n \t\tret = p_->pipe_->invokeMethod(&PipelineHandler::importFrameBuffers,\n \t\t\t\t\t ConnectionTypeDirect, this, stream);\n \t\tif (ret < 0)\n@@ -974,13 +959,9 @@ int Camera::stop()\n \tp_->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking,\n \t\t\t\tthis);\n \n-\tfor (Stream *stream : p_->activeStreams_) {\n-\t\tif (allocator_ && !allocator_->buffers(stream).empty())\n-\t\t\tcontinue;\n-\n+\tfor (Stream *stream : p_->activeStreams_)\n \t\tp_->pipe_->invokeMethod(&PipelineHandler::freeFrameBuffers,\n \t\t\t\t\tConnectionTypeBlocking, this, stream);\n-\t}\n \n \treturn 0;\n }\ndiff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp\nindex e79f4a8f65c8..6f7a2e90b08a 100644\n--- a/src/libcamera/framebuffer_allocator.cpp\n+++ b/src/libcamera/framebuffer_allocator.cpp\n@@ -87,11 +87,6 @@ FrameBufferAllocator::FrameBufferAllocator(std::shared_ptr<Camera> camera)\n \n FrameBufferAllocator::~FrameBufferAllocator()\n {\n-\tfor (auto &value : buffers_) {\n-\t\tStream *stream = value.first;\n-\t\tcamera_->freeFrameBuffers(stream);\n-\t}\n-\n \tbuffers_.clear();\n \n \tcamera_->allocator_ = nullptr;\n@@ -148,10 +143,6 @@ int FrameBufferAllocator::free(Stream *stream)\n \tif (iter == buffers_.end())\n \t\treturn -EINVAL;\n \n-\tint ret = camera_->freeFrameBuffers(stream);\n-\tif (ret < 0)\n-\t\treturn ret;\n-\n \tstd::vector<std::unique_ptr<FrameBuffer>> &buffers = iter->second;\n \tbuffers.clear();\n \tbuffers_.erase(iter);\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex 10a2698bad09..b6db8d567ea4 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -622,7 +622,7 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,\n \tV4L2VideoDevice *video = ipu3stream->device_->dev;\n \tunsigned int count = stream->configuration().bufferCount;\n \n-\treturn video->allocateBuffers(count, buffers);\n+\treturn video->exportBuffers(count, buffers);\n }\n \n int PipelineHandlerIPU3::importFrameBuffers(Camera *camera, Stream *stream)\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex f6934324c5a3..1ad53fbde112 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -665,7 +665,7 @@ 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_->allocateBuffers(count, buffers);\n+\treturn video_->exportBuffers(count, buffers);\n }\n \n int PipelineHandlerRkISP1::importFrameBuffers(Camera *camera, Stream *stream)\ndiff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\nindex d81627c224ea..29afb121aa46 100644\n--- a/src/libcamera/pipeline/uvcvideo.cpp\n+++ b/src/libcamera/pipeline/uvcvideo.cpp\n@@ -199,7 +199,7 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,\n \tUVCCameraData *data = cameraData(camera);\n \tunsigned int count = stream->configuration().bufferCount;\n \n-\treturn data->video_->allocateBuffers(count, buffers);\n+\treturn data->video_->exportBuffers(count, buffers);\n }\n \n int PipelineHandlerUVC::importFrameBuffers(Camera *camera, Stream *stream)\ndiff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\nindex bb94ef7fd38d..5d3d12fef30b 100644\n--- a/src/libcamera/pipeline/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc.cpp\n@@ -265,7 +265,7 @@ int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream,\n \tVimcCameraData *data = cameraData(camera);\n \tunsigned int count = stream->configuration().bufferCount;\n \n-\treturn data->video_->allocateBuffers(count, buffers);\n+\treturn data->video_->exportBuffers(count, buffers);\n }\n \n int PipelineHandlerVimc::importFrameBuffers(Camera *camera, Stream *stream)\ndiff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\nindex 8d623f5431fa..e5034c54e2fb 100644\n--- a/src/libcamera/pipeline_handler.cpp\n+++ b/src/libcamera/pipeline_handler.cpp\n@@ -337,16 +337,7 @@ const ControlList &PipelineHandler::properties(Camera *camera)\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+ * streams that are part of the active camera configuration.\n *\n * The only intended caller is Camera::exportFrameBuffers().\n *\n@@ -371,12 +362,7 @@ const ControlList &PipelineHandler::properties(Camera *camera)\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+ * pipeline handler for the stream to prepare for starting the Camera.\n *\n * The only intended caller is Camera::start().\n *\n@@ -391,10 +377,9 @@ const ControlList &PipelineHandler::properties(Camera *camera)\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+ * This method shall release all resources allocated for the \\a stream by\n+ * importFrameBuffers(). It shall be called only after a successful call that\n+ * method, and only once per stream.\n *\n * The only intended callers are Camera::stop() and Camera::freeFrameBuffers().\n *\n", "prefixes": [ "libcamera-devel", "7/9" ] }