[libcamera-devel] [RFC PATCH 1/2] libcamera: bayer_format: Add toMbusCode method

[Jacopo Mondi]

Hi David,

On Thu, Nov 03, 2022 at 10:40:26AM +0000, David Plowman via libcamera-devel wrote:
> This makes it easier to perform transformations on Bayer type mbus
> codes by converting them to a BayerFormat, doing the transform, and
> then converting them back again.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  include/libcamera/internal/bayer_format.h |  1 +
>  src/libcamera/bayer_format.cpp            | 12 ++++++++++++
>  2 files changed, 13 insertions(+)
>
> diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h
> index 78ba3969..09f756c4 100644
> --- a/include/libcamera/internal/bayer_format.h
> +++ b/include/libcamera/internal/bayer_format.h
> @@ -47,6 +47,7 @@ public:
>  	}
>
>  	static const BayerFormat &fromMbusCode(unsigned int mbusCode);
> +	unsigned int toMbusCode(bool &valid) const;
>  	bool isValid() const { return bitDepth != 0; }
>
>  	std::string toString() const;
> diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp
> index f27cc166..a03c58c0 100644
> --- a/src/libcamera/bayer_format.cpp
> +++ b/src/libcamera/bayer_format.cpp
> @@ -226,6 +226,18 @@ const BayerFormat &BayerFormat::fromMbusCode(unsigned int mbusCode)
>  		return it->second;
>  }
>
> +/**
> + * \brief Retrieve the media bus code corresponding this this BayerFormat
> + * \param[out] valid Set to true if a matching media bus code was found, else false
> + */
> +unsigned int BayerFormat::toMbusCode(bool &valid) const

Do we need valid ?
Valid media bus code are > 0, can we return 0 in the case the
BayerForamt is not supported ?

> +{
> +	auto it = std::find_if(mbusCodeToBayer.begin(), mbusCodeToBayer.end(),
> +			       [this](const auto &i) { return i.second == *this; });

To avoid iterating on the full map could store the media bus code as a BayerFormat
class member at construction ? It will require a large update of the
two maps though..

const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{
	{ MEDIA_BUS_FMT_SBGGR8_1X8,
                { MEDIA_BUS_FMT_SBGGR8_1X8, BayerFormat::BGGR, 8, BayerFormat::Packing::None } },
        ...
}
const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{
	{ { MEDIA_BUS_FMT_SBGGR8_1X8, BayerFormat::BGGR, 8, BayerFormat::Packing::None },
		{ formats::SBGGR8, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) } },
        ...
}

Or maybe we can expand

struct Formats {
	PixelFormat pixelFormat;
	V4L2PixelFormat v4l2Format;
};

to

struct Formats {
	PixelFormat pixelFormat;
	V4L2PixelFormat v4l2Format;
        uint32_t mbusCode;
};

which would require to expand

const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{
	{ { BayerFormat::BGGR, 8, BayerFormat::Packing::None },
		{ formats::SBGGR8, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8), MEDIA_BUS_FMT_SBGGR8_1X8 } },
        ...
}

so that we can lookup in bayerToFormat map.

Not a big deal, we already have a similar construct in other functions
in the class...

> +	valid = it != mbusCodeToBayer.end();
> +	return valid ? it->first : 0;
> +}
> +
>  /**
>   * \fn BayerFormat::isValid()
>   * \brief Return whether a BayerFormat is valid
> --
> 2.30.2
>