[libcamera-devel] [PATCH v4 2/3] libcamera: pixel_format: Add a function to return format based on string

[Kieran Bingham]

Hi Kaaira,

On 27/07/2020 17:21, Kaaira Gupta wrote:
> Add a function which retrieves pixel format corrsponding to its name

s/corrsponding/corresponding/

> from PixelFormatInfo.
> 
> Signed-off-by: Kaaira Gupta <kgupta at es.iitr.ac.in>
> ---
>  include/libcamera/pixel_format.h | 2 ++
>  src/libcamera/pixel_format.cpp   | 9 +++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/include/libcamera/pixel_format.h b/include/libcamera/pixel_format.h
> index 6727315..c4ae088 100644
> --- a/include/libcamera/pixel_format.h
> +++ b/include/libcamera/pixel_format.h
> @@ -38,6 +38,8 @@ public:
>  
>  	std::string toString() const;
>  
> +	static PixelFormat fromString(const std::string &name);
> +
>  private:
>  	uint32_t fourcc_;
>  	uint64_t modifier_;
> diff --git a/src/libcamera/pixel_format.cpp b/src/libcamera/pixel_format.cpp
> index 14addb5..9b07781 100644
> --- a/src/libcamera/pixel_format.cpp
> +++ b/src/libcamera/pixel_format.cpp
> @@ -130,4 +130,13 @@ std::string PixelFormat::toString() const
>  	return info.name;
>  }
>  
> +/**
> + * \brief Retrive pixel format corresponding to the string

s/Retrive/Retrieve/



> + * \return Pixel format
> + */

We might want to express a bit more about what is returned, especially
as this function is in the user-facing API.

"\return The PixelFormat represented by the \a name if known, or an
invalid pixel format otherwise."


Those can also be fixed on applying if there's nothing else.

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


> +PixelFormat PixelFormat::fromString(const std::string &name)

Actually, could this return a const reference? (which will save a copy).

const PixelFormat &PixelFormat::fromString....

> +{
> +	return PixelFormatInfo::info(name).format;
> +}
> +
>  } /* namespace libcamera */
> 

-- 
Regards
--
Kieran