[{"id":3378,"web_url":"https://patchwork.libcamera.org/comment/3378/","msgid":"<20200108010708.GT4871@pendragon.ideasonboard.com>","date":"2020-01-08T01:07:08","subject":"Re: [libcamera-devel] [PATCH v2 13/25] test: v4l2_videodevice:\n\tSwitch to FrameBuffer interface","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 Mon, Dec 30, 2019 at 01:04:58PM +0100, Niklas Söderlund wrote:\n> The V4L2VideoDevice class can now operate using a FrameBuffer interface,\n> switch all test cases to use it.\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> ---\n>  test/v4l2_videodevice/buffer_sharing.cpp      | 30 ++++++-------\n>  test/v4l2_videodevice/capture_async.cpp       | 20 ++++-----\n>  test/v4l2_videodevice/request_buffers.cpp     | 11 +----\n>  test/v4l2_videodevice/stream_on_off.cpp       |  6 +--\n>  test/v4l2_videodevice/v4l2_m2mdevice.cpp      | 44 ++++++++-----------\n>  test/v4l2_videodevice/v4l2_videodevice_test.h |  2 +-\n>  6 files changed, 48 insertions(+), 65 deletions(-)\n> \n> diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp\n> index fe48b2e98fdada8d..6acb06a24b47f653 100644\n> --- a/test/v4l2_videodevice/buffer_sharing.cpp\n> +++ b/test/v4l2_videodevice/buffer_sharing.cpp\n> @@ -73,16 +73,14 @@ protected:\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> -\t\tpool_.createBuffers(bufferCount);\n> -\n> -\t\tret = capture_->exportBuffers(&pool_);\n> -\t\tif (ret) {\n> -\t\t\tstd::cout << \"Failed to export buffers\" << std::endl;\n> +\t\tret = capture_->exportBuffers(bufferCount, &buffers_);\n> +\t\tif (ret < 0) {\n> +\t\t\tstd::cout << \"Failed to allocate buffers\" << std::endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> -\t\tret = output_->importBuffers(&pool_);\n> -\t\tif (ret) {\n> +\t\tret = output_->importBuffers(bufferCount);\n> +\t\tif (ret < 0) {\n>  \t\t\tstd::cout << \"Failed to import buffers\" << std::endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n> @@ -90,7 +88,7 @@ protected:\n>  \t\treturn 0;\n>  \t}\n>  \n> -\tvoid captureBufferReady(Buffer *buffer)\n> +\tvoid captureBufferReady(FrameBuffer *buffer)\n>  \t{\n>  \t\tconst FrameMetadata &metadata = buffer->metadata();\n>  \n> @@ -103,7 +101,7 @@ protected:\n>  \t\tframesCaptured_++;\n>  \t}\n>  \n> -\tvoid outputBufferReady(Buffer *buffer)\n> +\tvoid outputBufferReady(FrameBuffer *buffer)\n>  \t{\n>  \t\tconst FrameMetadata &metadata = buffer->metadata();\n>  \n> @@ -122,13 +120,15 @@ protected:\n>  \t\tTimer timeout;\n>  \t\tint ret;\n>  \n> -\t\tcapture_->bufferReady.connect(this, &BufferSharingTest::captureBufferReady);\n> -\t\toutput_->bufferReady.connect(this, &BufferSharingTest::outputBufferReady);\n> +\t\tcapture_->frameBufferReady.connect(this, &BufferSharingTest::captureBufferReady);\n> +\t\toutput_->frameBufferReady.connect(this, &BufferSharingTest::outputBufferReady);\n>  \n> -\t\tstd::vector<std::unique_ptr<Buffer>> buffers;\n> -\t\tbuffers = capture_->queueAllBuffers();\n> -\t\tif (buffers.empty())\n> -\t\t\treturn TestFail;\n> +\t\tfor (const std::unique_ptr<FrameBuffer> &buffer : buffers_) {\n> +\t\t\tif (capture_->queueBuffer(buffer.get())) {\n> +\t\t\t\tstd::cout << \"Failed to queue buffer\" << std::endl;\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n> +\t\t}\n>  \n>  \t\tret = capture_->streamOn();\n>  \t\tif (ret) {\n> diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp\n> index f62bbd837b213a0a..a57abed3bd0debc1 100644\n> --- a/test/v4l2_videodevice/capture_async.cpp\n> +++ b/test/v4l2_videodevice/capture_async.cpp\n> @@ -20,7 +20,7 @@ public:\n>  \tCaptureAsyncTest()\n>  \t\t: V4L2VideoDeviceTest(\"vimc\", \"Raw Capture 0\"), frames(0) {}\n>  \n> -\tvoid receiveBuffer(Buffer *buffer)\n> +\tvoid receiveBuffer(FrameBuffer *buffer)\n>  \t{\n>  \t\tstd::cout << \"Buffer received\" << std::endl;\n>  \t\tframes++;\n> @@ -38,18 +38,18 @@ protected:\n>  \t\tTimer timeout;\n>  \t\tint ret;\n>  \n> -\t\tpool_.createBuffers(bufferCount);\n> -\n> -\t\tret = capture_->exportBuffers(&pool_);\n> -\t\tif (ret)\n> +\t\tret = capture_->exportBuffers(bufferCount, &buffers_);\n> +\t\tif (ret < 0)\n>  \t\t\treturn TestFail;\n>  \n> -\t\tcapture_->bufferReady.connect(this, &CaptureAsyncTest::receiveBuffer);\n> +\t\tcapture_->frameBufferReady.connect(this, &CaptureAsyncTest::receiveBuffer);\n>  \n> -\t\tstd::vector<std::unique_ptr<Buffer>> buffers;\n> -\t\tbuffers = capture_->queueAllBuffers();\n> -\t\tif (buffers.empty())\n> -\t\t\treturn TestFail;\n> +\t\tfor (const std::unique_ptr<FrameBuffer> &buffer : buffers_) {\n> +\t\t\tif (capture_->queueBuffer(buffer.get())) {\n> +\t\t\t\tstd::cout << \"Failed to queue buffer\" << std::endl;\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n> +\t\t}\n>  \n>  \t\tret = capture_->streamOn();\n>  \t\tif (ret)\n> diff --git a/test/v4l2_videodevice/request_buffers.cpp b/test/v4l2_videodevice/request_buffers.cpp\n> index c4aedf7b3cd61e80..1dd65b05da430e63 100644\n> --- a/test/v4l2_videodevice/request_buffers.cpp\n> +++ b/test/v4l2_videodevice/request_buffers.cpp\n> @@ -16,17 +16,10 @@ public:\n>  protected:\n>  \tint run()\n>  \t{\n> -\t\t/*\n> -\t\t * TODO:\n> -\t\t *  Test invalid requests\n> -\t\t *  Test different buffer allocations\n> -\t\t */\n>  \t\tconst unsigned int bufferCount = 8;\n>  \n> -\t\tpool_.createBuffers(bufferCount);\n> -\n> -\t\tint ret = capture_->exportBuffers(&pool_);\n> -\t\tif (ret)\n> +\t\tint ret = capture_->exportBuffers(bufferCount, &buffers_);\n> +\t\tif (ret != bufferCount)\n>  \t\t\treturn TestFail;\n>  \n>  \t\treturn TestPass;\n> diff --git a/test/v4l2_videodevice/stream_on_off.cpp b/test/v4l2_videodevice/stream_on_off.cpp\n> index 7664adc4c1f07046..552df0963633ae4b 100644\n> --- a/test/v4l2_videodevice/stream_on_off.cpp\n> +++ b/test/v4l2_videodevice/stream_on_off.cpp\n> @@ -17,10 +17,8 @@ protected:\n>  \t{\n>  \t\tconst unsigned int bufferCount = 8;\n>  \n> -\t\tpool_.createBuffers(bufferCount);\n> -\n> -\t\tint ret = capture_->exportBuffers(&pool_);\n> -\t\tif (ret)\n> +\t\tint ret = capture_->exportBuffers(bufferCount, &buffers_);\n> +\t\tif (ret < 0)\n>  \t\t\treturn TestFail;\n>  \n>  \t\tret = capture_->streamOn();\n> diff --git a/test/v4l2_videodevice/v4l2_m2mdevice.cpp b/test/v4l2_videodevice/v4l2_m2mdevice.cpp\n> index 442bcac5dc49cc59..43b99c4f7ea9bf26 100644\n> --- a/test/v4l2_videodevice/v4l2_m2mdevice.cpp\n> +++ b/test/v4l2_videodevice/v4l2_m2mdevice.cpp\n> @@ -29,7 +29,7 @@ public:\n>  \t{\n>  \t}\n>  \n> -\tvoid outputBufferComplete(Buffer *buffer)\n> +\tvoid outputBufferComplete(FrameBuffer *buffer)\n>  \t{\n>  \t\tcout << \"Received output buffer\" << endl;\n>  \n> @@ -39,7 +39,7 @@ public:\n>  \t\tvim2m_->output()->queueBuffer(buffer);\n>  \t}\n>  \n> -\tvoid receiveCaptureBuffer(Buffer *buffer)\n> +\tvoid receiveCaptureBuffer(FrameBuffer *buffer)\n>  \t{\n>  \t\tcout << \"Received capture buffer\" << endl;\n>  \n> @@ -112,39 +112,31 @@ protected:\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> -\t\tcapturePool_.createBuffers(bufferCount);\n> -\t\toutputPool_.createBuffers(bufferCount);\n> -\n> -\t\tret = capture->exportBuffers(&capturePool_);\n> -\t\tif (ret) {\n> +\t\tret = capture->exportBuffers(bufferCount, &captureBuffers_);\n> +\t\tif (ret < 0) {\n>  \t\t\tcerr << \"Failed to export Capture Buffers\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> -\t\tret = output->exportBuffers(&outputPool_);\n> -\t\tif (ret) {\n> +\t\tret = output->exportBuffers(bufferCount, &outputBuffers_);\n> +\t\tif (ret < 0) {\n>  \t\t\tcerr << \"Failed to export Output Buffers\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> -\t\tcapture->bufferReady.connect(this, &V4L2M2MDeviceTest::receiveCaptureBuffer);\n> -\t\toutput->bufferReady.connect(this, &V4L2M2MDeviceTest::outputBufferComplete);\n> +\t\tcapture->frameBufferReady.connect(this, &V4L2M2MDeviceTest::receiveCaptureBuffer);\n> +\t\toutput->frameBufferReady.connect(this, &V4L2M2MDeviceTest::outputBufferComplete);\n>  \n> -\t\tstd::vector<std::unique_ptr<Buffer>> captureBuffers;\n> -\t\tcaptureBuffers = capture->queueAllBuffers();\n> -\t\tif (captureBuffers.empty()) {\n> -\t\t\tcerr << \"Failed to queue all Capture Buffers\" << endl;\n> -\t\t\treturn TestFail;\n> +\t\tfor (const std::unique_ptr<FrameBuffer> &buffer : captureBuffers_) {\n> +\t\t\tif (capture->queueBuffer(buffer.get())) {\n> +\t\t\t\tstd::cout << \"Failed to queue capture buffer\" << std::endl;\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n>  \t\t}\n>  \n> -\t\t/* We can't \"queueAllBuffers()\" on an output device, so we do it manually */\n> -\t\tstd::vector<std::unique_ptr<Buffer>> outputBuffers;\n> -\t\tfor (unsigned int i = 0; i < outputPool_.count(); ++i) {\n> -\t\t\tBuffer *buffer = new Buffer(i);\n> -\t\t\toutputBuffers.emplace_back(buffer);\n> -\t\t\tret = output->queueBuffer(buffer);\n> -\t\t\tif (ret) {\n> -\t\t\t\tcerr << \"Failed to queue output buffer\" << i << endl;\n> +\t\tfor (const std::unique_ptr<FrameBuffer> &buffer : outputBuffers_) {\n> +\t\t\tif (output->queueBuffer(buffer.get())) {\n> +\t\t\t\tstd::cout << \"Failed to queue output buffer\" << std::endl;\n>  \t\t\t\treturn TestFail;\n>  \t\t\t}\n>  \t\t}\n> @@ -202,8 +194,8 @@ private:\n>  \tstd::shared_ptr<MediaDevice> media_;\n>  \tV4L2M2MDevice *vim2m_;\n>  \n> -\tBufferPool capturePool_;\n> -\tBufferPool outputPool_;\n> +\tstd::vector<std::unique_ptr<FrameBuffer>> captureBuffers_;\n> +\tstd::vector<std::unique_ptr<FrameBuffer>> outputBuffers_;\n>  \n>  \tunsigned int outputFrames_;\n>  \tunsigned int captureFrames_;\n> diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.h b/test/v4l2_videodevice/v4l2_videodevice_test.h\n> index 34dd231c6d9d108d..9acaceb84fe0a12f 100644\n> --- a/test/v4l2_videodevice/v4l2_videodevice_test.h\n> +++ b/test/v4l2_videodevice/v4l2_videodevice_test.h\n> @@ -41,7 +41,7 @@ protected:\n>  \tCameraSensor *sensor_;\n>  \tV4L2Subdevice *debayer_;\n>  \tV4L2VideoDevice *capture_;\n> -\tBufferPool pool_;\n> +\tstd::vector<std::unique_ptr<FrameBuffer>> buffers_;\n>  };\n>  \n>  #endif /* __LIBCAMERA_V4L2_DEVICE_TEST_H_ */","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 39A4660612\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  8 Jan 2020 02:07:20 +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 6E41D52F;\n\tWed,  8 Jan 2020 02:07:19 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1578445639;\n\tbh=mbN3oWbE6Xl4C/uVylsBQ3Jup8XIC7z7lUZXQcb4VKo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HIpHlpEDE+7/HStB+paJ0bg/8A/Dck/yDdQrCZ+wOdahSZIzpjlALq3SYb7eWz+kL\n\t5QzTTPHkNsAhgW+sHvyRSUmgNpAKq03A+GcgDIMJRJhmTb4YXOgH1+uCtaRK0GBMI1\n\tqB4DsoerXFM2kfV3KpN8xr/BGKYkHTVV5P2YfrkU=","Date":"Wed, 8 Jan 2020 03:07:08 +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":"<20200108010708.GT4871@pendragon.ideasonboard.com>","References":"<20191230120510.938333-1-niklas.soderlund@ragnatech.se>\n\t<20191230120510.938333-14-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":"<20191230120510.938333-14-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 13/25] test: v4l2_videodevice:\n\tSwitch to FrameBuffer interface","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":"Wed, 08 Jan 2020 01:07:20 -0000"}}]