[libcamera-devel] [PATCH v2 14/25] test: camera: buffer_import: Update to FrameBuffer restrictions
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jan 8 02:17:41 CET 2020
Hi Niklas,
Thank you for the patch.
On Mon, Dec 30, 2019 at 01:04:59PM +0100, Niklas Söderlund wrote:
> With the FrameBuffer interface the V4L2 buffer indexes are not visible
> outside the V4L2VideoDevice so it's not possible to to test that the
> expected indexes are used when using external buffers (allocated
> directly from a V4L2 video device) with a Camera. Rewrite the test to
> this limitation.
>
> The idea of the test stays the same, test that buffers allocated from a
> V4L2 video device (vivid) can be imported and used on a Camera (vimc).
> As an added bonus the rewrite makes us of the FrameBuffer interface for
s/us/use/
> the allocation of buffers at the source (vivid) and imports them to the
> Camera which still uses the Buffer interface internally.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> test/camera/buffer_import.cpp | 390 ++++++++++------------------------
> 1 file changed, 110 insertions(+), 280 deletions(-)
>
> diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
> index 3ba6ce9690f29329..6559a89ce914a183 100644
> --- a/test/camera/buffer_import.cpp
> +++ b/test/camera/buffer_import.cpp
> @@ -22,19 +22,32 @@
>
> using namespace libcamera;
>
> -/* Keep SINK_BUFFER_COUNT > CAMERA_BUFFER_COUNT + 1 */
> -static constexpr unsigned int SINK_BUFFER_COUNT = 8;
> -static constexpr unsigned int CAMERA_BUFFER_COUNT = 4;
> +namespace {
>
> -class FrameSink
> +/* A provider of external buffers, suitable for import by a Camera. */
> +class BufferSource
> {
> public:
> - FrameSink()
> + BufferSource()
> : video_(nullptr)
> {
> }
>
> - int init()
> + ~BufferSource()
> + {
> + if (video_) {
> + video_->releaseBuffers();
> + video_->close();
> + }
> +
> + delete video_;
> + video_ = nullptr;
> +
> + if (media_)
> + media_->release();
> + }
> +
> + int allocate(const StreamConfiguration &config)
> {
> int ret;
>
> @@ -72,187 +85,27 @@ public:
> return TestFail;
>
> /* Configure the format. */
> - ret = video_->getFormat(&format_);
> + V4L2DeviceFormat format;
> + ret = video_->getFormat(&format);
> if (ret) {
> std::cout << "Failed to get format on output device" << std::endl;
> return ret;
> }
>
> - format_.size.width = 1920;
> - format_.size.height = 1080;
> - format_.fourcc = V4L2_PIX_FMT_RGB24;
> - format_.planesCount = 1;
> - format_.planes[0].size = 1920 * 1080 * 3;
> - format_.planes[0].bpl = 1920 * 3;
> -
> - if (video_->setFormat(&format_)) {
> - cleanup();
> - return TestFail;
> - }
> -
> - /* Export the buffers to a pool. */
> - pool_.createBuffers(SINK_BUFFER_COUNT);
> - ret = video_->exportBuffers(&pool_);
> - if (ret) {
> - std::cout << "Failed to export buffers" << std::endl;
> - cleanup();
> + format.size = config.size;
> + format.fourcc = V4L2VideoDevice::toV4L2Fourcc(config.pixelFormat, false);
> + if (video_->setFormat(&format))
> return TestFail;
> - }
> -
> - /* Only use the first CAMERA_BUFFER_COUNT buffers to start with. */
> - availableBuffers_.resize(CAMERA_BUFFER_COUNT);
> - std::iota(availableBuffers_.begin(), availableBuffers_.end(), 0);
> -
> - /* Connect the buffer ready signal. */
> - video_->bufferReady.connect(this, &FrameSink::bufferComplete);
> -
> - return TestPass;
> - }
> -
> - void cleanup()
> - {
> - if (video_) {
> - video_->streamOff();
> - video_->releaseBuffers();
> - video_->close();
> -
> - delete video_;
> - video_ = nullptr;
> - }
>
> - if (media_)
> - media_->release();
> + return video_->exportBuffers(config.bufferCount, &buffers_);
> }
>
> - int start()
> - {
> - requestsCount_ = 0;
> - done_ = false;
> -
> - int ret = video_->streamOn();
> - if (ret < 0)
> - return ret;
> -
> - /* Queue all the initial requests. */
> - for (unsigned int index = 0; index < CAMERA_BUFFER_COUNT; ++index)
> - queueRequest(index);
> -
> - return 0;
> - }
> -
> - int stop()
> - {
> - return video_->streamOff();
> - }
> -
> - void requestComplete(uint64_t cookie, const Buffer *metadata)
> - {
> - unsigned int index = cookie;
> -
> - Buffer *buffer = new Buffer(index, metadata);
> - int ret = video_->queueBuffer(buffer);
> - if (ret < 0)
> - std::cout << "Failed to queue buffer to sink" << std::endl;
> - }
> -
> - bool done() const { return done_; }
> -
> - PixelFormat format() const
> - {
> - return video_->toPixelFormat(format_.fourcc);
> - }
> -
> - const Size &size() const
> - {
> - return format_.size;
> - }
> -
> - Signal<uint64_t, int> requestReady;
> + const std::vector<std::unique_ptr<FrameBuffer>> &buffers() { return buffers_; };
s/;$//
and I'd wrap this as
{
return buffers_;
}
to keep the line shorter.
>
> private:
> - void queueRequest(unsigned int index)
> - {
> - auto it = std::find(availableBuffers_.begin(),
> - availableBuffers_.end(), index);
> - availableBuffers_.erase(it);
> -
> - uint64_t cookie = index;
> - BufferMemory &mem = pool_.buffers()[index];
> - int dmabuf = mem.planes()[0].fd.fd();
> -
> - requestReady.emit(cookie, dmabuf);
> -
> - requestsCount_++;
> - }
> -
> - void bufferComplete(Buffer *buffer)
> - {
> - availableBuffers_.push_back(buffer->index());
> -
> - /*
> - * Pick the buffer for the next request among the available
> - * buffers.
> - *
> - * For the first 20 frames, select the buffer that has just
> - * completed to keep the mapping of dmabuf fds to buffers
> - * unchanged in the camera.
> - *
> - * For the next 20 frames, cycle randomly over the available
> - * buffers. The mapping should still be kept unchanged, as the
> - * camera should map using the cached fds.
> - *
> - * For the last 20 frames, cycles through all buffers, which
> - * should trash the mappings.
> - */
> - unsigned int index = buffer->index();
> - delete buffer;
> -
> - std::cout << "Completed buffer, request=" << requestsCount_
> - << ", available buffers=" << availableBuffers_.size()
> - << std::endl;
> -
> - if (requestsCount_ >= 60) {
> - if (availableBuffers_.size() == SINK_BUFFER_COUNT)
> - done_ = true;
> - return;
> - }
> -
> - if (requestsCount_ == 40) {
> - /* Add the remaining of the buffers. */
> - for (unsigned int i = CAMERA_BUFFER_COUNT;
> - i < SINK_BUFFER_COUNT; ++i)
> - availableBuffers_.push_back(i);
> - }
> -
> - if (requestsCount_ >= 20) {
> - /*
> - * Wait until we have enough buffers to make this
> - * meaningful. Preferably half of the camera buffers,
> - * but no less than 2 in any case.
> - */
> - const unsigned int min_pool_size =
> - std::min(CAMERA_BUFFER_COUNT / 2, 2U);
> - if (availableBuffers_.size() < min_pool_size)
> - return;
> -
> - /* Pick a buffer at random. */
> - unsigned int pos = random_() % availableBuffers_.size();
> - index = availableBuffers_[pos];
> - }
> -
> - queueRequest(index);
> - }
> -
> std::shared_ptr<MediaDevice> media_;
> V4L2VideoDevice *video_;
> - BufferPool pool_;
> - V4L2DeviceFormat format_;
> -
> - unsigned int requestsCount_;
> - std::vector<int> availableBuffers_;
> - std::random_device random_;
> -
> - bool done_;
> + std::vector<std::unique_ptr<FrameBuffer>> buffers_;
> };
>
> class BufferImportTest : public CameraTest, public Test
> @@ -263,175 +116,152 @@ public:
> {
> }
>
> - void queueRequest(uint64_t cookie, int dmabuf)
> - {
> - Request *request = camera_->createRequest(cookie);
> -
> - std::unique_ptr<Buffer> buffer = stream_->createBuffer({ dmabuf, -1, -1 });
> - request->addBuffer(stream_, move(buffer));
> - camera_->queueRequest(request);
> - }
> -
> protected:
> void bufferComplete(Request *request, Buffer *buffer)
> {
> if (buffer->metadata().status != FrameMetadata::FrameSuccess)
> return;
>
> - unsigned int index = buffer->index();
> - int dmabuf = buffer->dmabufs()[0];
> -
> - /* Record dmabuf to index remappings. */
> - bool remapped = false;
> - if (bufferMappings_.find(index) != bufferMappings_.end()) {
> - if (bufferMappings_[index] != dmabuf)
> - remapped = true;
> - }
> + completeBuffersCount_++;
> + }
>
> - std::cout << "Completed request " << framesCaptured_
> - << ": dmabuf fd " << dmabuf
> - << " -> index " << index
> - << " (" << (remapped ? 'R' : '-') << ")"
> - << std::endl;
> + void requestComplete(Request *request)
> + {
> + if (request->status() != Request::RequestComplete)
> + return;
>
> - if (remapped)
> - bufferRemappings_.push_back(framesCaptured_);
> + const std::map<Stream *, Buffer *> &buffers = request->buffers();
>
> - bufferMappings_[index] = dmabuf;
> - framesCaptured_++;
> + completeRequestsCount_++;
>
> - sink_.requestComplete(request->cookie(), buffer);
> + /* Create a new request. */
> + Stream *stream = buffers.begin()->first;
> + int dmabuf = buffers.begin()->second->dmabufs()[0];
> + std::unique_ptr<Buffer> buffer = stream->createBuffer({ dmabuf, -1, -1 });
>
> - if (framesCaptured_ == 60)
> - sink_.stop();
> + request = camera_->createRequest();
> + request->addBuffer(stream, std::move(buffer));
> + camera_->queueRequest(request);
> }
>
> - int initCamera()
> + int init() override
> {
> - if (camera_->acquire()) {
> - std::cout << "Failed to acquire the camera" << std::endl;
> - return TestFail;
> - }
> + if (status_ != TestPass)
> + return status_;
>
> - /*
> - * Configure the Stream to work with externally allocated
> - * buffers by setting the memoryType to ExternalMemory.
> - */
> - std::unique_ptr<CameraConfiguration> config;
> - config = camera_->generateConfiguration({ StreamRole::VideoRecording });
> - if (!config || config->size() != 1) {
> - std::cout << "Failed to generate configuration" << std::endl;
> + config_ = camera_->generateConfiguration({ StreamRole::VideoRecording });
> + if (!config_ || config_->size() != 1) {
> + std::cout << "Failed to generate default configuration" << std::endl;
> return TestFail;
> }
>
> - StreamConfiguration &cfg = config->at(0);
> - cfg.size = sink_.size();
> - cfg.pixelFormat = sink_.format();
> - cfg.bufferCount = CAMERA_BUFFER_COUNT;
> + StreamConfiguration &cfg = config_->at(0);
> cfg.memoryType = ExternalMemory;
>
> - if (camera_->configure(config.get())) {
> - std::cout << "Failed to set configuration" << std::endl;
> + return TestPass;
> + }
> +
> + int run() override
> + {
> + StreamConfiguration &cfg = config_->at(0);
> +
> + if (camera_->acquire()) {
> + std::cout << "Failed to acquire the camera" << std::endl;
> return TestFail;
> }
>
> - stream_ = cfg.stream();
> + if (camera_->configure(config_.get())) {
> + std::cout << "Failed to set default configuration" << std::endl;
> + return TestFail;
> + }
>
> - /* Allocate buffers. */
> if (camera_->allocateBuffers()) {
> std::cout << "Failed to allocate buffers" << std::endl;
> return TestFail;
> }
>
> - /* Connect the buffer completed signal. */
> - camera_->bufferCompleted.connect(this, &BufferImportTest::bufferComplete);
> + Stream *stream = cfg.stream();
>
> - return TestPass;
> - }
> + BufferSource source;
> + int ret = source.allocate(cfg);
> + if (ret < 0)
> + return TestFail;
>
> - int init()
> - {
> - if (status_ != TestPass)
> - return status_;
> + std::vector<Request *> requests;
> + for (const std::unique_ptr<FrameBuffer> &framebuffer : source.buffers()) {
> + int dmabuf = framebuffer->planes()[0].fd.fd();
>
> - int ret = sink_.init();
> - if (ret != TestPass) {
> - cleanup();
> - return ret;
> - }
> + Request *request = camera_->createRequest();
> + if (!request) {
> + std::cout << "Failed to create request" << std::endl;
> + return TestFail;
> + }
>
> - ret = initCamera();
> - if (ret != TestPass) {
> - cleanup();
> - return ret;
> - }
> + std::unique_ptr<Buffer> buffer = stream->createBuffer({ dmabuf, -1, -1 });
> + if (request->addBuffer(stream, std::move(buffer))) {
> + std::cout << "Failed to associating buffer with request" << std::endl;
> + return TestFail;
> + }
>
> - sink_.requestReady.connect(this, &BufferImportTest::queueRequest);
> - return TestPass;
> - }
> + requests.push_back(request);
> + }
>
> - int run()
> - {
> - int ret;
> + completeRequestsCount_ = 0;
> + completeBuffersCount_ = 0;
>
> - framesCaptured_ = 0;
> + camera_->bufferCompleted.connect(this, &BufferImportTest::bufferComplete);
> + camera_->requestCompleted.connect(this, &BufferImportTest::requestComplete);
>
> if (camera_->start()) {
> std::cout << "Failed to start camera" << std::endl;
> return TestFail;
> }
>
> - ret = sink_.start();
> - if (ret < 0) {
> - std::cout << "Failed to start sink" << std::endl;
> - return TestFail;
> + for (Request *request : requests) {
> + if (camera_->queueRequest(request)) {
> + std::cout << "Failed to queue request" << std::endl;
> + return TestFail;
> + }
> }
>
> EventDispatcher *dispatcher = cm_->eventDispatcher();
>
> Timer timer;
> - timer.start(5000);
> - while (timer.isRunning() && !sink_.done())
> + timer.start(1000);
> + while (timer.isRunning())
> dispatcher->processEvents();
>
> - std::cout << framesCaptured_ << " frames captured, "
> - << bufferRemappings_.size() << " buffers remapped"
> - << std::endl;
> + if (completeRequestsCount_ <= cfg.bufferCount * 2) {
> + std::cout << "Failed to capture enough frames (got "
> + << completeRequestsCount_ << " expected at least "
> + << cfg.bufferCount * 2 << ")" << std::endl;
> + return TestFail;
> + }
>
> - if (framesCaptured_ < 60) {
> - std::cout << "Too few frames captured" << std::endl;
> + if (completeRequestsCount_ != completeBuffersCount_) {
> + std::cout << "Number of completed buffers and requests differ" << std::endl;
> return TestFail;
> }
>
> - if (bufferRemappings_.empty()) {
> - std::cout << "No buffer remappings" << std::endl;
> + if (camera_->stop()) {
> + std::cout << "Failed to stop camera" << std::endl;
> return TestFail;
> }
>
> - if (bufferRemappings_[0] < 40) {
> - std::cout << "Early buffer remapping" << std::endl;
> + if (camera_->freeBuffers()) {
> + std::cout << "Failed to free buffers" << std::endl;
> return TestFail;
> }
>
> return TestPass;
> }
>
> - void cleanup()
> - {
> - sink_.cleanup();
> -
> - camera_->stop();
> - camera_->freeBuffers();
> - }
> -
> private:
> - Stream *stream_;
> -
> - std::map<unsigned int, int> bufferMappings_;
> - std::vector<unsigned int> bufferRemappings_;
> - unsigned int framesCaptured_;
> -
> - FrameSink sink_;
> + unsigned int completeBuffersCount_;
> + unsigned int completeRequestsCount_;
> + std::unique_ptr<CameraConfiguration> config_;
> };
>
> +} /* namespace */
> +
> TEST_REGISTER(BufferImportTest);
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list