[PATCH v3 08/16] libcamera: software_isp: Add DebayerCpu class

[Hans de Goede]

Hi,

On 2/15/24 16:33, Milan Zamazal wrote:
> Hans de Goede <hdegoede at redhat.com> writes:
> 
>> Add CPU based debayering implementation. This initial implementation
>> only supports debayering packed 10 bits per pixel bayer data in
>> the 4 standard bayer orders.

<snip>

>> +SizeRange DebayerCpu::sizes(PixelFormat inputFormat, const Size &inputSize)
>> +{
>> +	Size pattern_size = patternSize(inputFormat);
>> +	unsigned int border_height = pattern_size.height;
>> +
>> +	if (pattern_size.isNull())
>> +		return {};
>> +
>> +	/* No need for top/bottom border with a pattern height of 2 */
>> +	if (pattern_size.height == 2)
>> +		border_height = 0;
>> +
>> +	/*
>> +	 * For debayer interpolation a border is kept around the entire image
>> +	 * and the minimum output size is pattern-height x pattern-width.
>> +	 */
> 
> What if the output size is larger?  The border is quite impractical because it
> forces (or not?) the output size to be non-standard, assuming the camera
> provides common resolutions.  Consider e.g. full-HD camera resolution not being
> able to be output 1:1 to a full-HD display.

Hardware ISPs also need a similar border, because including special hardware
to deal with the edges would be quite expensive both in silicon area as
well as in power consumption. So sensors typically have a slightly bigger
resolution then the standard resolutions. E.g. the ov2680 sensor has a
pixelarray of 1616x1216 pixels and the ov2740 used in ThinkPads has
1932x1092 pixels.

HW ISPs use the extra pixels both for interpolation near the border as
well as to be able to use a crop window starting at 0x1 1x0 or 1x1 to
shift the bayer pattern so that the hw only needs to support a single
bayer pattern.

<snip>

>> +private:
>> +	/**
>> +	 * \brief Called to debayer 1 line of Bayer input data to output format
>> +	 * \param[out] dst Pointer to the start of the output line to write
>> +	 * \param[in] src The input data
>> +	 *
>> +	 * Input data is an array of (patternSize_.height + 1) src
>> +	 * pointers each pointing to a line in the Bayer source. The middle
>> +	 * element of the array will point to the actual line being processed.
>> +	 * Earlier element(s) will point to the previous line(s) and later
>> +	 * element(s) to the next line(s).
>> +	 *
>> +	 * These functions take an array of src pointers, rather then
>> +	 * a single src pointer + a stride for the source, so that when the src
>> +	 * is slow uncached memory it can be copied to faster memory before
>> +	 * debayering. Debayering a standard 2x2 Bayer pattern requires access
>> +	 * to the previous and next src lines for interpolating the missing
>> +	 * colors. To allow copying the src lines only once 3 buffers each
> 
> I'd avoid using the term "buffer" here to avoid any confusion with input and
> output buffers.

But they are buffers, I have added temporary there now so that this now
reads "To allow copying the src lines only once 3 temporary buffers each ..."
to make clear these are not the input / output buffers.

> 
>> +	 * holding a single line are used, re-using the oldest buffer for
>> +	 * the next line and the pointers are swizzled so that:
>> +	 * src[0] = previous-line, src[1] = currrent-line, src[2] = next-line.
>> +	 * This way the 3 pointers passed to the debayer functions form
>> +	 * a sliding window over the src avoiding the need to copy each
>> +	 * line more then once.
> 
> then -> than
> 
>> +	 *
>> +	 * Similarly for bayer patterns which repeat every 4 lines, 5 src
>> +	 * pointers are passed holding: src[0] = 2-lines-up, src[1] = 1-line-up
>> +	 * src[2] = current-line, src[3] = 1-line-down, src[4] = 2-lines-down.
>> +	 */

Thanks, both fixed in my personal tree.

Regards,

Hans