[libcamera-devel,v3,07/33] libcamera: buffer: Add FrameMetadata container for metadata information

Message ID 20200110193808.2266294-8-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamera: Rework buffer API
Related show

Commit Message

Niklas Söderlund Jan. 10, 2020, 7:37 p.m. UTC
With the introduction of FrameBuffer objects, the metadata information
related to captured frames will not be stored directly in the frame
buffer object. Add a new FrameMetadata class to hold this information.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
* Changes since v2
- Clarify documentation for values in enum FrameMetadata::Status members.
- Improved documentation.
- Added todo to improve FrameMetadata::timestamp descripton.

* Changes since v1:
- Rename BufferInfo to FrameMetadata
- Rename prefix for BufferInfo::Status s/Buffer/Frame/
- Make FrameMetadata a struct with public data members instead of a
  class with accessors methods.
- Dropped FrameMetadata::update()
- Documentation updates
---
 include/libcamera/buffer.h | 17 +++++++++
 src/libcamera/buffer.cpp   | 73 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+)

Comments

Laurent Pinchart Jan. 10, 2020, 11:40 p.m. UTC | #1
Hi Niklas,

Thank you for the patch.

On Fri, Jan 10, 2020 at 08:37:42PM +0100, Niklas Söderlund wrote:
> With the introduction of FrameBuffer objects, the metadata information
> related to captured frames will not be stored directly in the frame
> buffer object. Add a new FrameMetadata class to hold this information.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> * Changes since v2
> - Clarify documentation for values in enum FrameMetadata::Status members.
> - Improved documentation.
> - Added todo to improve FrameMetadata::timestamp descripton.
> 
> * Changes since v1:
> - Rename BufferInfo to FrameMetadata
> - Rename prefix for BufferInfo::Status s/Buffer/Frame/
> - Make FrameMetadata a struct with public data members instead of a
>   class with accessors methods.
> - Dropped FrameMetadata::update()
> - Documentation updates
> ---
>  include/libcamera/buffer.h | 17 +++++++++
>  src/libcamera/buffer.cpp   | 73 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 90 insertions(+)
> 
> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h
> index 80602124f24be4a0..0b95e41a2dd205b2 100644
> --- a/include/libcamera/buffer.h
> +++ b/include/libcamera/buffer.h
> @@ -16,6 +16,23 @@ namespace libcamera {
>  class Request;
>  class Stream;
>  
> +struct FrameMetadata {
> +	enum Status {
> +		FrameSuccess,
> +		FrameError,
> +		FrameCancelled,
> +	};
> +
> +	struct Plane {
> +		unsigned int bytesused;
> +	};
> +
> +	Status status;
> +	unsigned int sequence;
> +	uint64_t timestamp;
> +	std::vector<Plane> planes;
> +};
> +
>  class Plane final
>  {
>  public:
> diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp
> index 960eeb8f7d193ddd..b7bc948c80748b18 100644
> --- a/src/libcamera/buffer.cpp
> +++ b/src/libcamera/buffer.cpp
> @@ -23,6 +23,79 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(Buffer)
>  
> +/**
> + * \struct FrameMetadata
> + * \brief Metadata related to a captured frame
> + *
> + * The FrameMetadata structure stores all metadata related to a captured frame,
> + * as stored in a FrameBuffer, such as capture status, timestamp and bytesused.
> + */
> +
> +/**
> + * \enum FrameMetadata::Status
> + * \brief Define the frame completion status
> + * \var FrameMetadata::FrameSuccess
> + * The frame has been captured with success and contains valid data. All fields
> + * of the FrameMetadata structure are valid.
> + * \var FrameMetadata::FrameError
> + * An error occurred during capture of the frame. The frame data may be partly
> + * or fully invalid. The sequence and timestamp fields of the FrameMetadata
> + * structure is valid, the other fields may be invalid.
> + * \var FrameMetadata::FrameCancelled
> + * Capture stopped before the frame completed. The frame data is not valid. All
> + * fields of the FrameMetadata structure are invalid.

"all fields of the FrameMetadata structure but the status field are
invalid."

Please keep my tag.

> + */
> +
> +/**
> + * \struct FrameMetadata::Plane
> + * \brief Per-plane frame metadata
> + *
> + * Frames are stored in memory in one or multiple planes. The
> + * FrameMetadata::Plane structure stores per-plane metadata.
> + */
> +
> +/**
> + * \var FrameMetadata::Plane::bytesused
> + * \brief Number of bytes occupied by the data in the plane, including line
> + * padding
> + *
> + * This value may vary per frame for compressed formats. For uncompressed
> + * formats it will be constant for all frames, but may be smaller than the
> + * FrameBuffer size.
> + */
> +
> +/**
> + * \var FrameMetadata::status
> + * \brief Status of the frame
> + *
> + * The validity of other fields of the FrameMetadata structure depends on the
> + * status value.
> + */
> +
> +/**
> + * \var FrameMetadata::sequence
> + * \brief Frame sequence number
> + *
> + * The sequence number is a monotonically increasing number assigned to the
> + * frames captured by the stream. The value is increased by one for each frame.
> + * Gaps in the sequence numbers indicate dropped frames.
> + */
> +
> +/**
> + * \var FrameMetadata::timestamp
> + * \brief Time when the frame was captured
> + *
> + * The timestamp is expressed as a number of nanoseconds relative to the system
> + * clock since an unspecified time point.
> + *
> + * \todo Be more precise on what timestamps refer to.
> + */
> +
> +/**
> + * \var FrameMetadata::planes
> + * \brief Array of per-plane metadata
> + */
> +
>  /**
>   * \class Plane
>   * \brief A memory region to store a single plane of a frame

Patch

diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h
index 80602124f24be4a0..0b95e41a2dd205b2 100644
--- a/include/libcamera/buffer.h
+++ b/include/libcamera/buffer.h
@@ -16,6 +16,23 @@  namespace libcamera {
 class Request;
 class Stream;
 
+struct FrameMetadata {
+	enum Status {
+		FrameSuccess,
+		FrameError,
+		FrameCancelled,
+	};
+
+	struct Plane {
+		unsigned int bytesused;
+	};
+
+	Status status;
+	unsigned int sequence;
+	uint64_t timestamp;
+	std::vector<Plane> planes;
+};
+
 class Plane final
 {
 public:
diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp
index 960eeb8f7d193ddd..b7bc948c80748b18 100644
--- a/src/libcamera/buffer.cpp
+++ b/src/libcamera/buffer.cpp
@@ -23,6 +23,79 @@  namespace libcamera {
 
 LOG_DEFINE_CATEGORY(Buffer)
 
+/**
+ * \struct FrameMetadata
+ * \brief Metadata related to a captured frame
+ *
+ * The FrameMetadata structure stores all metadata related to a captured frame,
+ * as stored in a FrameBuffer, such as capture status, timestamp and bytesused.
+ */
+
+/**
+ * \enum FrameMetadata::Status
+ * \brief Define the frame completion status
+ * \var FrameMetadata::FrameSuccess
+ * The frame has been captured with success and contains valid data. All fields
+ * of the FrameMetadata structure are valid.
+ * \var FrameMetadata::FrameError
+ * An error occurred during capture of the frame. The frame data may be partly
+ * or fully invalid. The sequence and timestamp fields of the FrameMetadata
+ * structure is valid, the other fields may be invalid.
+ * \var FrameMetadata::FrameCancelled
+ * Capture stopped before the frame completed. The frame data is not valid. All
+ * fields of the FrameMetadata structure are invalid.
+ */
+
+/**
+ * \struct FrameMetadata::Plane
+ * \brief Per-plane frame metadata
+ *
+ * Frames are stored in memory in one or multiple planes. The
+ * FrameMetadata::Plane structure stores per-plane metadata.
+ */
+
+/**
+ * \var FrameMetadata::Plane::bytesused
+ * \brief Number of bytes occupied by the data in the plane, including line
+ * padding
+ *
+ * This value may vary per frame for compressed formats. For uncompressed
+ * formats it will be constant for all frames, but may be smaller than the
+ * FrameBuffer size.
+ */
+
+/**
+ * \var FrameMetadata::status
+ * \brief Status of the frame
+ *
+ * The validity of other fields of the FrameMetadata structure depends on the
+ * status value.
+ */
+
+/**
+ * \var FrameMetadata::sequence
+ * \brief Frame sequence number
+ *
+ * The sequence number is a monotonically increasing number assigned to the
+ * frames captured by the stream. The value is increased by one for each frame.
+ * Gaps in the sequence numbers indicate dropped frames.
+ */
+
+/**
+ * \var FrameMetadata::timestamp
+ * \brief Time when the frame was captured
+ *
+ * The timestamp is expressed as a number of nanoseconds relative to the system
+ * clock since an unspecified time point.
+ *
+ * \todo Be more precise on what timestamps refer to.
+ */
+
+/**
+ * \var FrameMetadata::planes
+ * \brief Array of per-plane metadata
+ */
+
 /**
  * \class Plane
  * \brief A memory region to store a single plane of a frame