| Message ID | 20191230120510.938333-11-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, Thank you for the patch. On Mon, Dec 30, 2019 at 01:04:55PM +0100, Niklas Söderlund wrote: > 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 <niklas.soderlund@ragnatech.se> > --- > * 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 | 75 ++++++------------------ > src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +-- > src/libcamera/request.cpp | 2 +- > src/libcamera/v4l2_videodevice.cpp | 24 ++++---- > src/qcam/main_window.cpp | 13 ++-- > test/camera/buffer_import.cpp | 2 +- > test/camera/capture.cpp | 2 +- > test/v4l2_videodevice/buffer_sharing.cpp | 8 ++- > 13 files changed, 70 insertions(+), 103 deletions(-) > > diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h > index 8dd9f91272291648..8bd61f786748af5f 100644 > --- a/include/libcamera/buffer.h > +++ b/include/libcamera/buffer.h > @@ -93,12 +93,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; > @@ -107,11 +101,8 @@ public: > const std::array<int, 3> &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_; } > > @@ -127,11 +118,8 @@ private: > std::array<int, 3> 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 3e84068e66bb4dd7..7c58a1f50829f290 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 4a77f20751408b7c..686d0412bf54b2c1 100644 > --- a/src/libcamera/buffer.cpp > +++ b/src/libcamera/buffer.cpp > @@ -166,20 +166,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 > * > @@ -188,18 +174,19 @@ 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) > { > + metadata_.status = FrameMetadata::FrameSuccess; > + > if (metadata) { > - bytesused_ = metadata->bytesused_; > - sequence_ = metadata->sequence_; > - timestamp_ = metadata->timestamp_; > + metadata_.sequence = metadata->metadata().sequence; > + metadata_.timestamp = metadata->metadata().timestamp; > + metadata_.planes = metadata->metadata().planes; > } else { > - bytesused_ = 0; > - sequence_ = 0; > - timestamp_ = 0; > + metadata_.sequence = 0; > + metadata_.timestamp = 0; > + metadata_.planes = { { 0 } }; > } I believe this code will go away, but otherwise it could have been written as if (metadata) metadata_ = metadata->metadata(); else metadata_ = {}; metadata_.status = FrameMetadata::FrameSuccess; > } > > @@ -231,39 +218,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 update every time the buffer contained are changed, s/update/updated/ * 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. (not that it matters too much as this will go away) Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + * for example when it is dequeued from hardware. > * > - * \return The buffer status > + * \return Metadata for the buffer > */ > > /** > @@ -299,10 +260,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 bb652d0da9c6df52..46df871a51105ee4 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<utils::clock>() > - + 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) > @@ -1002,8 +1002,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 3b49e52510918eee..b17a6c2278361f00 100644 > --- a/src/libcamera/request.cpp > +++ b/src/libcamera/request.cpp > @@ -260,7 +260,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 7d585ce03ed9a45a..51112d197068cf0a 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -1015,10 +1015,12 @@ 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(); > + > + 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 +1127,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 5dec9252898100db..47793702b9aa0ee9 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<unsigned char *>(memory); > - viewfinder_->display(raw, buffer->bytesused()); > + viewfinder_->display(raw, buffer->metadata().planes[0].bytesused); > > munmap(memory, plane.length); > > 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);
diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 8dd9f91272291648..8bd61f786748af5f 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -93,12 +93,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; @@ -107,11 +101,8 @@ public: const std::array<int, 3> &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_; } @@ -127,11 +118,8 @@ private: std::array<int, 3> 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 3e84068e66bb4dd7..7c58a1f50829f290 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 4a77f20751408b7c..686d0412bf54b2c1 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -166,20 +166,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 * @@ -188,18 +174,19 @@ 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) { + metadata_.status = FrameMetadata::FrameSuccess; + if (metadata) { - bytesused_ = metadata->bytesused_; - sequence_ = metadata->sequence_; - timestamp_ = metadata->timestamp_; + metadata_.sequence = metadata->metadata().sequence; + metadata_.timestamp = metadata->metadata().timestamp; + metadata_.planes = metadata->metadata().planes; } else { - bytesused_ = 0; - sequence_ = 0; - timestamp_ = 0; + metadata_.sequence = 0; + metadata_.timestamp = 0; + metadata_.planes = { { 0 } }; } } @@ -231,39 +218,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 update every time the buffer contained are changed, + * for example when it is dequeued from hardware. * - * \return The buffer status + * \return Metadata for the buffer */ /** @@ -299,10 +260,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 bb652d0da9c6df52..46df871a51105ee4 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<utils::clock>() - + 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) @@ -1002,8 +1002,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 3b49e52510918eee..b17a6c2278361f00 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -260,7 +260,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 7d585ce03ed9a45a..51112d197068cf0a 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1015,10 +1015,12 @@ 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(); + + 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 +1127,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 5dec9252898100db..47793702b9aa0ee9 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<unsigned char *>(memory); - viewfinder_->display(raw, buffer->bytesused()); + viewfinder_->display(raw, buffer->metadata().planes[0].bytesused); munmap(memory, plane.length); 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);
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 <niklas.soderlund@ragnatech.se> --- * 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 | 75 ++++++------------------ src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +-- src/libcamera/request.cpp | 2 +- src/libcamera/v4l2_videodevice.cpp | 24 ++++---- src/qcam/main_window.cpp | 13 ++-- test/camera/buffer_import.cpp | 2 +- test/camera/capture.cpp | 2 +- test/v4l2_videodevice/buffer_sharing.cpp | 8 ++- 13 files changed, 70 insertions(+), 103 deletions(-)