[{"id":3151,"web_url":"https://patchwork.libcamera.org/comment/3151/","msgid":"<20191127145947.zrqb24ul7kweobso@uno.localdomain>","date":"2019-11-27T14:59:47","subject":"Re: [libcamera-devel] [PATCH 14/30] libcamera: request: In\n\taddBuffer() do not fetch stream from Buffer","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Wed, Nov 27, 2019 at 12:36:04AM +0100, Niklas Söderlund wrote:\n> In the FrameBuffer interface the stream will not be available from the\n> buffer object as the buffer might be allocated externally. The\n> application needs to explicitly state which stream the buffer is being\n> added for to the request.\n>\n> Extend the addBuffer() function to get this information explicitly from\n> the caller.\n\nLooks good!\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n>\n> Signed-off-by: Niklas Söderlund \n> ---\n> include/libcamera/request.h | 2 +-\n> src/android/camera_device.cpp | 2 +-\n> src/cam/capture.cpp | 4 ++--\n> src/libcamera/request.cpp | 4 ++--\n> src/qcam/main_window.cpp | 4 ++--\n> test/camera/buffer_import.cpp | 2 +-\n> test/camera/capture.cpp | 4 ++--\n> test/camera/statemachine.cpp | 2 +-\n> 8 files changed, 12 insertions(+), 12 deletions(-)\n>\n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index 2d5a5964e99eb75f..a8708010ec1247a2 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -39,7 +39,7 @@ public:\n> \tControlList &controls() { return *controls_; }\n> \tControlList &metadata() { return *metadata_; }\n> \tconst std::map &buffers() const { return bufferMap_; }\n> -\tint addBuffer(std::unique_ptr buffer);\n> +\tint addBuffer(Stream *stream, std::unique_ptr buffer);\n> \tBuffer *findBuffer(Stream *stream) const;\n>\n> \tuint64_t cookie() const { return cookie_; }\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 065e0292e186c2ad..09588c16a6301649 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -754,7 +754,7 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque\n>\n> \tRequest *request =\n> \t\tcamera_->createRequest(reinterpret_cast(descriptor));\n> -\trequest->addBuffer(std::move(buffer));\n> +\trequest->addBuffer(stream, std::move(buffer));\n>\n> \tint ret = camera_->queueRequest(request);\n> \tif (ret) {\n> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> index 4b65b1d0a2dbed35..1a4dbe7ce4a15a2d 100644\n> --- a/src/cam/capture.cpp\n> +++ b/src/cam/capture.cpp\n> @@ -95,7 +95,7 @@ int Capture::capture(EventLoop *loop)\n> \t\t\tStream *stream = cfg.stream();\n> \t\t\tstd::unique_ptr buffer = stream->createBuffer(i);\n>\n> -\t\t\tret = request->addBuffer(std::move(buffer));\n> +\t\t\tret = request->addBuffer(stream, std::move(buffer));\n> \t\t\tif (ret < 0) {\n> \t\t\t\tstd::cerr << \"Can't set buffer for request\"\n> \t\t\t\t\t << std::endl;\n> @@ -185,7 +185,7 @@ void Capture::requestComplete(Request *request)\n> \t\t\treturn;\n> \t\t}\n>\n> -\t\trequest->addBuffer(std::move(newBuffer));\n> +\t\trequest->addBuffer(stream, std::move(newBuffer));\n> \t}\n>\n> \tcamera_->queueRequest(request);\n> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> index c2854dc2e8caab2e..3b49e52510918eee 100644\n> --- a/src/libcamera/request.cpp\n> +++ b/src/libcamera/request.cpp\n> @@ -113,6 +113,7 @@ Request::~Request()\n>\n> /**\n> * \\brief Store a Buffer with its associated Stream in the Request\n> + * \\param[in] stream The stream the buffer belongs to\n> * \\param[in] buffer The Buffer to store in the request\n> *\n> * Ownership of the buffer is passed to the request. It will be deleted when\n> @@ -125,9 +126,8 @@ Request::~Request()\n> * \\retval -EEXIST The request already contains a buffer for the stream\n> * \\retval -EINVAL The buffer does not reference a valid Stream\n> */\n> -int Request::addBuffer(std::unique_ptr buffer)\n> +int Request::addBuffer(Stream *stream, std::unique_ptr buffer)\n> {\n> -\tStream *stream = buffer->stream();\n> \tif (!stream) {\n> \t\tLOG(Request, Error) << \"Invalid stream reference\";\n> \t\treturn -EINVAL;\n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index a828a176cbbf4854..4cecf7e351214f3d 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -190,7 +190,7 @@ int MainWindow::startCapture()\n> \t\t\tgoto error;\n> \t\t}\n>\n> -\t\tret = request->addBuffer(std::move(buffer));\n> +\t\tret = request->addBuffer(stream, std::move(buffer));\n> \t\tif (ret < 0) {\n> \t\t\tstd::cerr << \"Can't set buffer for request\" << std::endl;\n> \t\t\tgoto error;\n> @@ -288,7 +288,7 @@ void MainWindow::requestComplete(Request *request)\n> \t\t\treturn;\n> \t\t}\n>\n> -\t\trequest->addBuffer(std::move(newBuffer));\n> +\t\trequest->addBuffer(stream, std::move(newBuffer));\n> \t}\n>\n> \tcamera_->queueRequest(request);\n> diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp\n> index 8b0d1c167bd2bbe8..dae907f9f41841c9 100644\n> --- a/test/camera/buffer_import.cpp\n> +++ b/test/camera/buffer_import.cpp\n> @@ -268,7 +268,7 @@ public:\n> \t\tRequest *request = camera_->createRequest(cookie);\n>\n> \t\tstd::unique_ptr buffer = stream_->createBuffer({ dmabuf, -1, -1 });\n> -\t\trequest->addBuffer(move(buffer));\n> +\t\trequest->addBuffer(stream_, move(buffer));\n> \t\tcamera_->queueRequest(request);\n> \t}\n>\n> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp\n> index 7cb76038f557d461..ca1ebe419946dd4d 100644\n> --- a/test/camera/capture.cpp\n> +++ b/test/camera/capture.cpp\n> @@ -49,7 +49,7 @@ protected:\n> \t\tstd::unique_ptr newBuffer = stream->createBuffer(buffer->index());\n>\n> \t\trequest = camera_->createRequest();\n> -\t\trequest->addBuffer(std::move(newBuffer));\n> +\t\trequest->addBuffer(stream, std::move(newBuffer));\n> \t\tcamera_->queueRequest(request);\n> \t}\n>\n> @@ -101,7 +101,7 @@ protected:\n> \t\t\t\treturn TestFail;\n> \t\t\t}\n>\n> -\t\t\tif (request->addBuffer(std::move(buffer))) {\n> +\t\t\tif (request->addBuffer(stream, std::move(buffer))) {\n> \t\t\t\tcout << \"Failed to associating buffer with request\" << endl;\n> \t\t\t\treturn TestFail;\n> \t\t\t}\n> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp\n> index afa13ba77b0b7fe0..f627b8f37422350e 100644\n> --- a/test/camera/statemachine.cpp\n> +++ b/test/camera/statemachine.cpp\n> @@ -219,7 +219,7 @@ protected:\n>\n> \t\tStream *stream = *camera_->streams().begin();\n> \t\tstd::unique_ptr buffer = stream->createBuffer(0);\n> -\t\tif (request->addBuffer(std::move(buffer)))\n> +\t\tif (request->addBuffer(stream, std::move(buffer)))\n> \t\t\treturn TestFail;\n>\n> \t\tif (camera_->queueRequest(request))\n> --\n> 2.24.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 87D2260C33\n\tfor ;\n\tWed, 27 Nov 2019 15:57:41 +0100 (CET)","from uno.localdomain (93-34-114-233.ip49.fastwebnet.it\n\t[93.34.114.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id F408740012;\n\tWed, 27 Nov 2019 14:57:40 +0000 (UTC)"],"X-Originating-IP":"93.34.114.233","Date":"Wed, 27 Nov 2019 15:59:47 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191127145947.zrqb24ul7kweobso@uno.localdomain>","References":"<20191126233620.1695316-1-niklas.soderlund@ragnatech.se>\n\t<20191126233620.1695316-15-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"24dzewfx7aevuzyr\"","Content-Disposition":"inline","In-Reply-To":"<20191126233620.1695316-15-niklas.soderlund@ragnatech.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 14/30] libcamera: request: In\n\taddBuffer() do not fetch stream from Buffer","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":"Wed, 27 Nov 2019 14:57:41 -0000"}},{"id":3235,"web_url":"https://patchwork.libcamera.org/comment/3235/","msgid":"<20191209184145.GE18060@pendragon.ideasonboard.com>","date":"2019-12-09T18:41:45","subject":"Re: [libcamera-devel] [PATCH 14/30] libcamera: request: In\n\taddBuffer() do not fetch stream from Buffer","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 Wed, Nov 27, 2019 at 12:36:04AM +0100, Niklas Söderlund wrote:\n> In the FrameBuffer interface the stream will not be available from the\n> buffer object as the buffer might be allocated externally. The\n> application needs to explicitly state which stream the buffer is being\n> added for to the request.\n> \n> Extend the addBuffer() function to get this information explicitly from\n> the caller.\n> \n> Signed-off-by: Niklas Söderlund \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> include/libcamera/request.h | 2 +-\n> src/android/camera_device.cpp | 2 +-\n> src/cam/capture.cpp | 4 ++--\n> src/libcamera/request.cpp | 4 ++--\n> src/qcam/main_window.cpp | 4 ++--\n> test/camera/buffer_import.cpp | 2 +-\n> test/camera/capture.cpp | 4 ++--\n> test/camera/statemachine.cpp | 2 +-\n> 8 files changed, 12 insertions(+), 12 deletions(-)\n> \n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index 2d5a5964e99eb75f..a8708010ec1247a2 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -39,7 +39,7 @@ public:\n> \tControlList &controls() { return *controls_; }\n> \tControlList &metadata() { return *metadata_; }\n> \tconst std::map &buffers() const { return bufferMap_; }\n> -\tint addBuffer(std::unique_ptr buffer);\n> +\tint addBuffer(Stream *stream, std::unique_ptr buffer);\n> \tBuffer *findBuffer(Stream *stream) const;\n> \n> \tuint64_t cookie() const { return cookie_; }\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 065e0292e186c2ad..09588c16a6301649 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -754,7 +754,7 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque\n> \n> \tRequest *request =\n> \t\tcamera_->createRequest(reinterpret_cast(descriptor));\n> -\trequest->addBuffer(std::move(buffer));\n> +\trequest->addBuffer(stream, std::move(buffer));\n> \n> \tint ret = camera_->queueRequest(request);\n> \tif (ret) {\n> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> index 4b65b1d0a2dbed35..1a4dbe7ce4a15a2d 100644\n> --- a/src/cam/capture.cpp\n> +++ b/src/cam/capture.cpp\n> @@ -95,7 +95,7 @@ int Capture::capture(EventLoop *loop)\n> \t\t\tStream *stream = cfg.stream();\n> \t\t\tstd::unique_ptr buffer = stream->createBuffer(i);\n> \n> -\t\t\tret = request->addBuffer(std::move(buffer));\n> +\t\t\tret = request->addBuffer(stream, std::move(buffer));\n> \t\t\tif (ret < 0) {\n> \t\t\t\tstd::cerr << \"Can't set buffer for request\"\n> \t\t\t\t\t << std::endl;\n> @@ -185,7 +185,7 @@ void Capture::requestComplete(Request *request)\n> \t\t\treturn;\n> \t\t}\n> \n> -\t\trequest->addBuffer(std::move(newBuffer));\n> +\t\trequest->addBuffer(stream, std::move(newBuffer));\n> \t}\n> \n> \tcamera_->queueRequest(request);\n> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> index c2854dc2e8caab2e..3b49e52510918eee 100644\n> --- a/src/libcamera/request.cpp\n> +++ b/src/libcamera/request.cpp\n> @@ -113,6 +113,7 @@ Request::~Request()\n> \n> /**\n> * \\brief Store a Buffer with its associated Stream in the Request\n> + * \\param[in] stream The stream the buffer belongs to\n> * \\param[in] buffer The Buffer to store in the request\n> *\n> * Ownership of the buffer is passed to the request. It will be deleted when\n> @@ -125,9 +126,8 @@ Request::~Request()\n> * \\retval -EEXIST The request already contains a buffer for the stream\n> * \\retval -EINVAL The buffer does not reference a valid Stream\n> */\n> -int Request::addBuffer(std::unique_ptr buffer)\n> +int Request::addBuffer(Stream *stream, std::unique_ptr buffer)\n> {\n> -\tStream *stream = buffer->stream();\n> \tif (!stream) {\n> \t\tLOG(Request, Error) << \"Invalid stream reference\";\n> \t\treturn -EINVAL;\n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index a828a176cbbf4854..4cecf7e351214f3d 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -190,7 +190,7 @@ int MainWindow::startCapture()\n> \t\t\tgoto error;\n> \t\t}\n> \n> -\t\tret = request->addBuffer(std::move(buffer));\n> +\t\tret = request->addBuffer(stream, std::move(buffer));\n> \t\tif (ret < 0) {\n> \t\t\tstd::cerr << \"Can't set buffer for request\" << std::endl;\n> \t\t\tgoto error;\n> @@ -288,7 +288,7 @@ void MainWindow::requestComplete(Request *request)\n> \t\t\treturn;\n> \t\t}\n> \n> -\t\trequest->addBuffer(std::move(newBuffer));\n> +\t\trequest->addBuffer(stream, std::move(newBuffer));\n> \t}\n> \n> \tcamera_->queueRequest(request);\n> diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp\n> index 8b0d1c167bd2bbe8..dae907f9f41841c9 100644\n> --- a/test/camera/buffer_import.cpp\n> +++ b/test/camera/buffer_import.cpp\n> @@ -268,7 +268,7 @@ public:\n> \t\tRequest *request = camera_->createRequest(cookie);\n> \n> \t\tstd::unique_ptr buffer = stream_->createBuffer({ dmabuf, -1, -1 });\n> -\t\trequest->addBuffer(move(buffer));\n> +\t\trequest->addBuffer(stream_, move(buffer));\n> \t\tcamera_->queueRequest(request);\n> \t}\n> \n> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp\n> index 7cb76038f557d461..ca1ebe419946dd4d 100644\n> --- a/test/camera/capture.cpp\n> +++ b/test/camera/capture.cpp\n> @@ -49,7 +49,7 @@ protected:\n> \t\tstd::unique_ptr newBuffer = stream->createBuffer(buffer->index());\n> \n> \t\trequest = camera_->createRequest();\n> -\t\trequest->addBuffer(std::move(newBuffer));\n> +\t\trequest->addBuffer(stream, std::move(newBuffer));\n> \t\tcamera_->queueRequest(request);\n> \t}\n> \n> @@ -101,7 +101,7 @@ protected:\n> \t\t\t\treturn TestFail;\n> \t\t\t}\n> \n> -\t\t\tif (request->addBuffer(std::move(buffer))) {\n> +\t\t\tif (request->addBuffer(stream, std::move(buffer))) {\n> \t\t\t\tcout << \"Failed to associating buffer with request\" << endl;\n> \t\t\t\treturn TestFail;\n> \t\t\t}\n> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp\n> index afa13ba77b0b7fe0..f627b8f37422350e 100644\n> --- a/test/camera/statemachine.cpp\n> +++ b/test/camera/statemachine.cpp\n> @@ -219,7 +219,7 @@ protected:\n> \n> \t\tStream *stream = *camera_->streams().begin();\n> \t\tstd::unique_ptr buffer = stream->createBuffer(0);\n> -\t\tif (request->addBuffer(std::move(buffer)))\n> +\t\tif (request->addBuffer(stream, std::move(buffer)))\n> \t\t\treturn TestFail;\n> \n> \t\tif (camera_->queueRequest(request))","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9D8E560BDB\n\tfor ;\n\tMon, 9 Dec 2019 19:41: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 1715311B7;\n\tMon, 9 Dec 2019 19:41:52 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1575916912;\n\tbh=fdXuHJjNjvW29tDnYsAZtXqf45JpLLSL/JIpowMlHDQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=b66LPDk9WUKznaHyAMtqKdqxflk7OyMfISff9Pyca7ewJBTeaWGboVarhPm0Zl7jH\n\tSouN2Vs3Y7AObSLYQhfAY/DbWCqEQfpF/rfI5//Kq5km4zVy2OU6c+hfifag/Qy1mE\n\tbAA+dn94AE57n1x9FF066f+8Rcx/BjdlP/Kj3BHA=","Date":"Mon, 9 Dec 2019 20:41:45 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191209184145.GE18060@pendragon.ideasonboard.com>","References":"<20191126233620.1695316-1-niklas.soderlund@ragnatech.se>\n\t<20191126233620.1695316-15-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":"<20191126233620.1695316-15-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 14/30] libcamera: request: In\n\taddBuffer() do not fetch stream from Buffer","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":"Mon, 09 Dec 2019 18:41:52 -0000"}}]