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

[Laurent Pinchart]

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 at ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at 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

-- 
Regards,

Laurent Pinchart