[libcamera-devel] [PATCH v5 03/23] libcamera: StreamConfiguration: Add frameSize field

[Kieran Bingham]

Hi Paul,

On 09/07/2020 14:28, Paul Elder wrote:
> In addition to the stride field, we want the pipeline handler to be able
> to declare the frame size for the configuration. Add a frameSize field
> to StreamConfiguration for this purpose.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> ---
> No change in v5
> 
> No change in v4
> 
> New in v3
> ---
>  include/libcamera/stream.h |  1 +
>  src/libcamera/stream.cpp   | 17 ++++++++++++++---
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> index 1a68bd2..f502b35 100644
> --- a/include/libcamera/stream.h
> +++ b/include/libcamera/stream.h
> @@ -43,6 +43,7 @@ struct StreamConfiguration {
>  	PixelFormat pixelFormat;
>  	Size size;
>  	unsigned int stride;
> +	unsigned int frameSize;
>  
>  	unsigned int bufferCount;
>  
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index 6df5882..6d6e279 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -279,7 +279,8 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const
>   * handlers provide StreamFormats.
>   */
>  StreamConfiguration::StreamConfiguration()
> -	: pixelFormat(0), stride(0), bufferCount(0), stream_(nullptr)
> +	: pixelFormat(0), stride(0), frameSize(0), bufferCount(0),
> +	  stream_(nullptr)
>  {
>  }
>  
> @@ -287,8 +288,8 @@ StreamConfiguration::StreamConfiguration()
>   * \brief Construct a configuration with stream formats
>   */
>  StreamConfiguration::StreamConfiguration(const StreamFormats &formats)
> -	: pixelFormat(0), stride(0), bufferCount(0), stream_(nullptr),
> -	  formats_(formats)
> +	: pixelFormat(0), stride(0), frameSize(0), bufferCount(0),
> +	  stream_(nullptr), formats_(formats)
>  {
>  }
>  
> @@ -315,6 +316,16 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)
>   * the camera is configured.
>   */
>  
> +/**
> + * \var StreamConfiguration::frameSize
> + * \brief Frame size for the stream, in bytes
> + *
> + * The frameSize value reports the number of bytes necessary to contain one
> + * frame of an image buffer for this stream. The value is valid after
> + * successfully validating the configuration with a call to
> + * CameraConfiguration::validate().

How will we handle multi-planar formats? Will this always be the total
of all planes? Should we state if this total includes bytes required for
all image planes explicitly?

In fact, will there be a planeSize function too?

Still, that can be later either way.

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


Though, should this be squashed with "libcamera: PixelFormatInfo: Add
functions stride and frameSize" ? Otherwise this value becomes available
at this point but always zero.

It's only used first in that patch, so I'd at least put this patch
directly before so that the closely-related patches are adjacent.



> + */
> +
>  /**
>   * \var StreamConfiguration::bufferCount
>   * \brief Requested number of buffers to allocate for the stream
> 

-- 
Regards
--
Kieran