From patchwork Sun Jan 12 01:01:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2598 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 24C03606DF for ; Sun, 12 Jan 2020 02:03:12 +0100 (CET) X-Halon-ID: 4b65c7d7-34d7-11ea-b6d8-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5d7b.dip0.t-ipconnect.de [84.172.93.123]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 4b65c7d7-34d7-11ea-b6d8-005056917f90; Sun, 12 Jan 2020 02:03:07 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sun, 12 Jan 2020 02:01:56 +0100 Message-Id: <20200112010212.2609025-17-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200112010212.2609025-1-niklas.soderlund@ragnatech.se> References: <20200112010212.2609025-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 16/32] libcamera: buffer: Move captured metadata to FrameMetadata X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 12 Jan 2020 01:03:12 -0000 Move the metadata retrieved when dequeuing a V4L2 buffer into a FrameMetadata object. This is done as a step to migrate to the FrameBuffer interface as the functions added to Buffer around FrameMetadata match the ones in FrameBuffer. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- * Changes since v2 - Syntax improvement. - Improved documentation. * Changes since v1 - Rework to match FrameMetadata being a struct instead of a class - Align statements broken over multiple lines - Spiffy up bytesused printing in cam - Merged with patch who removes the old fields in Buffer. --- include/libcamera/buffer.h | 16 +---- src/android/camera_device.cpp | 4 +- src/cam/buffer_writer.cpp | 2 +- src/cam/capture.cpp | 13 +++- src/libcamera/buffer.cpp | 76 +++++------------------- src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +-- src/libcamera/request.cpp | 2 +- src/libcamera/v4l2_videodevice.cpp | 25 ++++---- src/qcam/main_window.cpp | 13 ++-- src/v4l2/v4l2_camera.cpp | 8 ++- src/v4l2/v4l2_camera.h | 4 +- src/v4l2/v4l2_camera_proxy.cpp | 4 +- test/camera/buffer_import.cpp | 2 +- test/camera/capture.cpp | 2 +- test/v4l2_videodevice/buffer_sharing.cpp | 8 ++- 16 files changed, 78 insertions(+), 113 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 55d08e278a7d0236..0eb84c32cc8570c5 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -99,12 +99,6 @@ private: class Buffer final { public: - enum Status { - BufferSuccess, - BufferError, - BufferCancelled, - }; - Buffer(unsigned int index = -1, const Buffer *metadata = nullptr); Buffer(const Buffer &) = delete; Buffer &operator=(const Buffer &) = delete; @@ -113,11 +107,8 @@ public: const std::array &dmabufs() const { return dmabuf_; } BufferMemory *mem() { return mem_; } - unsigned int bytesused() const { return bytesused_; } - uint64_t timestamp() const { return timestamp_; } - unsigned int sequence() const { return sequence_; } + const FrameMetadata &metadata() const { return metadata_; }; - Status status() const { return status_; } Request *request() const { return request_; } Stream *stream() const { return stream_; } @@ -133,11 +124,8 @@ private: std::array dmabuf_; BufferMemory *mem_; - unsigned int bytesused_; - uint64_t timestamp_; - unsigned int sequence_; + FrameMetadata metadata_; - Status status_; Request *request_; Stream *stream_; }; diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 09588c16a6301649..ebe91ea8af4f6436 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -803,11 +803,11 @@ void CameraDevice::requestComplete(Request *request) if (status == CAMERA3_BUFFER_STATUS_OK) { notifyShutter(descriptor->frameNumber, - libcameraBuffer->timestamp()); + libcameraBuffer->metadata().timestamp); captureResult.partial_result = 1; resultMetadata = getResultMetadata(descriptor->frameNumber, - libcameraBuffer->timestamp()); + libcameraBuffer->metadata().timestamp); captureResult.result = resultMetadata->get(); } diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp index 765a176238e58dff..41ef4b0a36d61b03 100644 --- a/src/cam/buffer_writer.cpp +++ b/src/cam/buffer_writer.cpp @@ -33,7 +33,7 @@ int BufferWriter::write(Buffer *buffer, const std::string &streamName) if (pos != std::string::npos) { std::stringstream ss; ss << streamName << "-" << std::setw(6) - << std::setfill('0') << buffer->sequence(); + << std::setfill('0') << buffer->metadata().sequence; filename.replace(pos, 1, ss.str()); } diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index 1a4dbe7ce4a15a2d..da942f56118983bd 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -154,9 +154,18 @@ void Capture::requestComplete(Request *request) Buffer *buffer = it->second; const std::string &name = streamName_[stream]; + const FrameMetadata &metadata = buffer->metadata(); + info << " " << name - << " seq: " << std::setw(6) << std::setfill('0') << buffer->sequence() - << " bytesused: " << buffer->bytesused(); + << " seq: " << std::setw(6) << std::setfill('0') << metadata.sequence + << " bytesused: "; + + unsigned int nplane = 0; + for (const FrameMetadata::Plane &plane : metadata.planes) { + info << plane.bytesused; + if (++nplane < metadata.planes.size()) + info << "/"; + } if (writer_) writer_->write(buffer, name); diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 8c8be4ac802735b1..92ac28387bd4c4f7 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -177,20 +177,6 @@ void BufferPool::destroyBuffers() * deleted automatically after the request complete handler returns. */ -/** - * \enum Buffer::Status - * Buffer completion status - * \var Buffer::BufferSuccess - * The buffer has completed with success and contains valid data. All its other - * metadata (such as bytesused(), timestamp() or sequence() number) are valid. - * \var Buffer::BufferError - * The buffer has completed with an error and doesn't contain valid data. Its - * other metadata are valid. - * \var Buffer::BufferCancelled - * The buffer has been cancelled due to capture stop. Its other metadata are - * invalid and shall not be used. - */ - /** * \brief Construct a buffer not associated with any stream * @@ -199,19 +185,15 @@ void BufferPool::destroyBuffers() * for a stream with Stream::createBuffer(). */ Buffer::Buffer(unsigned int index, const Buffer *metadata) - : index_(index), dmabuf_({ -1, -1, -1 }), - status_(Buffer::BufferSuccess), request_(nullptr), + : index_(index), dmabuf_({ -1, -1, -1 }), request_(nullptr), stream_(nullptr) { - if (metadata) { - bytesused_ = metadata->bytesused_; - sequence_ = metadata->sequence_; - timestamp_ = metadata->timestamp_; - } else { - bytesused_ = 0; - sequence_ = 0; - timestamp_ = 0; - } + if (metadata) + metadata_ = metadata->metadata(); + else + metadata_ = {}; + + metadata_.status = FrameMetadata::FrameSuccess; } /** @@ -242,39 +224,13 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata) */ /** - * \fn Buffer::bytesused() - * \brief Retrieve the number of bytes occupied by the data in the buffer - * \return Number of bytes occupied in the buffer - */ - -/** - * \fn Buffer::timestamp() - * \brief Retrieve the time when the buffer was processed - * - * The timestamp is expressed as a number of nanoseconds since the epoch. - * - * \return Timestamp when the buffer was processed - */ - -/** - * \fn Buffer::sequence() - * \brief Retrieve the buffer sequence number - * - * The sequence number is a monotonically increasing number assigned to the - * buffer processed by the stream. Gaps in the sequence numbers indicate - * dropped frames. - * - * \return Sequence number of the buffer - */ - -/** - * \fn Buffer::status() - * \brief Retrieve the buffer status + * \fn Buffer::metadata() + * \brief Retrieve the buffer metadata * - * The buffer status reports whether the buffer has completed successfully - * (BufferSuccess) or if an error occurred (BufferError). + * The buffer metadata is updated when the buffer contents are modified, for + * example when a frame has been captured to the buffer by the hardware. * - * \return The buffer status + * \return Metadata for the buffer */ /** @@ -310,10 +266,10 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata) */ void Buffer::cancel() { - bytesused_ = 0; - timestamp_ = 0; - sequence_ = 0; - status_ = BufferCancelled; + metadata_.status = FrameMetadata::FrameCancelled; + metadata_.sequence = 0; + metadata_.timestamp = 0; + metadata_.planes = {}; } /** diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 6d8c3fada127310e..34fc792977d151be 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -928,7 +928,7 @@ int PipelineHandlerIPU3::registerCameras() void IPU3CameraData::imguInputBufferReady(Buffer *buffer) { /* \todo Handle buffer failures when state is set to BufferError. */ - if (buffer->status() == Buffer::BufferCancelled) + if (buffer->metadata().status == FrameMetadata::FrameCancelled) return; cio2_.output_->queueBuffer(buffer); @@ -962,7 +962,7 @@ void IPU3CameraData::imguOutputBufferReady(Buffer *buffer) void IPU3CameraData::cio2BufferReady(Buffer *buffer) { /* \todo Handle buffer failures when state is set to BufferError. */ - if (buffer->status() == Buffer::BufferCancelled) + if (buffer->metadata().status == FrameMetadata::FrameCancelled) return; imgu_->input_->queueBuffer(buffer); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 1e44571589a6003d..979b670e4cb75512 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -100,10 +100,10 @@ public: ASSERT(frameOffset(SOE) == 0); utils::time_point soe = std::chrono::time_point() - + std::chrono::nanoseconds(buffer->timestamp()) + + std::chrono::nanoseconds(buffer->metadata().timestamp) + timeOffset(SOE); - notifyStartOfExposure(buffer->sequence(), soe); + notifyStartOfExposure(buffer->metadata().sequence, soe); } void setDelay(unsigned int type, int frame, int msdelay) @@ -994,8 +994,8 @@ void PipelineHandlerRkISP1::bufferReady(Buffer *buffer) data->timeline_.bufferReady(buffer); - if (data->frame_ <= buffer->sequence()) - data->frame_ = buffer->sequence() + 1; + if (data->frame_ <= buffer->metadata().sequence) + data->frame_ = buffer->metadata().sequence + 1; completeBuffer(activeCamera_, request, buffer); tryCompleteRequest(request); diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 54dfb461c58b6971..4268e31434bf4feb 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -238,7 +238,7 @@ bool Request::completeBuffer(Buffer *buffer) buffer->request_ = nullptr; - if (buffer->status() == Buffer::BufferCancelled) + if (buffer->metadata().status == FrameMetadata::FrameCancelled) cancelled_ = true; return !hasPendingBuffers(); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 4551484cfbf8c6ef..d22655c676bef1ae 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1015,10 +1015,13 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer) } if (V4L2_TYPE_IS_OUTPUT(buf.type)) { - buf.bytesused = buffer->bytesused_; - buf.sequence = buffer->sequence_; - buf.timestamp.tv_sec = buffer->timestamp_ / 1000000000; - buf.timestamp.tv_usec = (buffer->timestamp_ / 1000) % 1000000; + const FrameMetadata &metadata = buffer->metadata(); + + if (!metadata.planes.empty()) + buf.bytesused = metadata.planes[0].bytesused; + buf.sequence = metadata.sequence; + buf.timestamp.tv_sec = metadata.timestamp / 1000000000; + buf.timestamp.tv_usec = (metadata.timestamp / 1000) % 1000000; } LOG(V4L2, Debug) << "Queueing buffer " << buf.index; @@ -1125,12 +1128,14 @@ Buffer *V4L2VideoDevice::dequeueBuffer() fdEvent_->setEnabled(false); buffer->index_ = buf.index; - buffer->bytesused_ = buf.bytesused; - buffer->timestamp_ = buf.timestamp.tv_sec * 1000000000ULL - + buf.timestamp.tv_usec * 1000ULL; - buffer->sequence_ = buf.sequence; - buffer->status_ = buf.flags & V4L2_BUF_FLAG_ERROR - ? Buffer::BufferError : Buffer::BufferSuccess; + + buffer->metadata_.status = buf.flags & V4L2_BUF_FLAG_ERROR + ? FrameMetadata::FrameError + : FrameMetadata::FrameSuccess; + buffer->metadata_.sequence = buf.sequence; + buffer->metadata_.timestamp = buf.timestamp.tv_sec * 1000000000ULL + + buf.timestamp.tv_usec * 1000ULL; + buffer->metadata_.planes = { { buf.bytesused } }; return buffer; } diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 8b3d99237047a9e5..ca3464babdc1c313 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -259,14 +259,15 @@ void MainWindow::requestComplete(Request *request) framesCaptured_++; Buffer *buffer = buffers.begin()->second; + const FrameMetadata &metadata = buffer->metadata(); - double fps = buffer->timestamp() - lastBufferTime_; + double fps = metadata.timestamp - lastBufferTime_; fps = lastBufferTime_ && fps ? 1000000000.0 / fps : 0.0; - lastBufferTime_ = buffer->timestamp(); + lastBufferTime_ = metadata.timestamp; - std::cout << "seq: " << std::setw(6) << std::setfill('0') << buffer->sequence() - << " bytesused: " << buffer->bytesused() - << " timestamp: " << buffer->timestamp() + std::cout << "seq: " << std::setw(6) << std::setfill('0') << metadata.sequence + << " bytesused: " << metadata.planes[0].bytesused + << " timestamp: " << metadata.timestamp << " fps: " << std::fixed << std::setprecision(2) << fps << std::endl; @@ -306,7 +307,7 @@ int MainWindow::display(Buffer *buffer) plane.fd.fd(), 0); unsigned char *raw = static_cast(memory); - viewfinder_->display(raw, buffer->bytesused()); + viewfinder_->display(raw, buffer->metadata().planes[0].bytesused); munmap(memory, plane.length); diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index b6e0bb762bcd79fe..e4a03c414480f353 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -17,9 +17,11 @@ using namespace libcamera; LOG_DECLARE_CATEGORY(V4L2Compat); V4L2FrameMetadata::V4L2FrameMetadata(Buffer *buffer) - : index_(buffer->index()), bytesused_(buffer->bytesused()), - timestamp_(buffer->timestamp()), sequence_(buffer->sequence()), - status_(buffer->status()) + : index_(buffer->index()), + bytesused_(buffer->metadata().planes[0].bytesused), + timestamp_(buffer->metadata().timestamp), + sequence_(buffer->metadata().sequence), + status_(buffer->metadata().status) { } diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h index f760316c854fba2f..06eab0e1c93b7c8a 100644 --- a/src/v4l2/v4l2_camera.h +++ b/src/v4l2/v4l2_camera.h @@ -31,7 +31,7 @@ public: uint64_t timestamp() const { return timestamp_; } unsigned int sequence() const { return sequence_; } - Buffer::Status status() const { return status_; } + FrameMetadata::Status status() const { return status_; } private: int index_; @@ -40,7 +40,7 @@ private: uint64_t timestamp_; unsigned int sequence_; - Buffer::Status status_; + FrameMetadata::Status status_; }; class V4L2Camera : public Object diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp index e4aba33e6d33f21b..59cf360edd28f69c 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -192,7 +192,7 @@ void V4L2CameraProxy::updateBuffers() struct v4l2_buffer &buf = buffers_[fmd.index()]; switch (fmd.status()) { - case Buffer::Status::BufferSuccess: + case FrameMetadata::FrameSuccess: buf.bytesused = fmd.bytesused(); buf.field = V4L2_FIELD_NONE; buf.timestamp.tv_sec = fmd.timestamp() / 1000000000; @@ -201,7 +201,7 @@ void V4L2CameraProxy::updateBuffers() buf.flags |= V4L2_BUF_FLAG_DONE; break; - case Buffer::Status::BufferError: + case FrameMetadata::FrameError: buf.flags |= V4L2_BUF_FLAG_ERROR; break; default: diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index e5c010d81b8d6e0e..3ba6ce9690f29329 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -275,7 +275,7 @@ public: protected: void bufferComplete(Request *request, Buffer *buffer) { - if (buffer->status() != Buffer::BufferSuccess) + if (buffer->metadata().status != FrameMetadata::FrameSuccess) return; unsigned int index = buffer->index(); diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index ca1ebe419946dd4d..0d9ffc476650f414 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -28,7 +28,7 @@ protected: void bufferComplete(Request *request, Buffer *buffer) { - if (buffer->status() != Buffer::BufferSuccess) + if (buffer->metadata().status != FrameMetadata::FrameSuccess) return; completeBuffersCount_++; diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp index 3a56862cb2b77d38..fe48b2e98fdada8d 100644 --- a/test/v4l2_videodevice/buffer_sharing.cpp +++ b/test/v4l2_videodevice/buffer_sharing.cpp @@ -92,9 +92,11 @@ protected: void captureBufferReady(Buffer *buffer) { + const FrameMetadata &metadata = buffer->metadata(); + std::cout << "Received capture buffer" << std::endl; - if (buffer->status() != Buffer::BufferSuccess) + if (metadata.status != FrameMetadata::FrameSuccess) return; output_->queueBuffer(buffer); @@ -103,9 +105,11 @@ protected: void outputBufferReady(Buffer *buffer) { + const FrameMetadata &metadata = buffer->metadata(); + std::cout << "Received output buffer" << std::endl; - if (buffer->status() != Buffer::BufferSuccess) + if (metadata.status != FrameMetadata::FrameSuccess) return; capture_->queueBuffer(buffer);