[{"id":18914,"web_url":"https://patchwork.libcamera.org/comment/18914/","msgid":"<20210818113909.GK1733965@pyrite.rasen.tech>","date":"2021-08-18T11:39:09","subject":"Re: [libcamera-devel] [RFC PATCH v3 2/4] libcamera: swisp: The\n\tsoftware ISP class","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Siyuan,\n\nOn Tue, Aug 17, 2021 at 04:42:19PM +0100, Siyuan Fan wrote:\n> From: Fan Siyuan \n> \n> Currently class ISPCPU only supports to output RGB888 and BGR888(640x480).\n> Based on format set by application, using getOutputPixelFormat() to match\n> output format and compressAndTransformFormat() to transform corresponding format.\n> \n> Signed-off-by: Fan Siyuan \n> ---\n> src/libcamera/swisp/isp.cpp | 726 ++++++++++++++++++++++++++++++++++++\n> src/libcamera/swisp/isp.h | 125 +++++++\n> 2 files changed, 851 insertions(+)\n> create mode 100644 src/libcamera/swisp/isp.cpp\n> create mode 100644 src/libcamera/swisp/isp.h\n> \n> diff --git a/src/libcamera/swisp/isp.cpp b/src/libcamera/swisp/isp.cpp\n> new file mode 100644\n> index 00000000..b0f801e9\n> --- /dev/null\n> +++ b/src/libcamera/swisp/isp.cpp\n> @@ -0,0 +1,726 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Siyuan Fan \n> + *\n> + * isp.cpp - The software ISP class\n> + */\n> +\n> +#include \"isp.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \"libcamera/base/log.h\"\n> +\n> +namespace libcamera{\n\nnamespace libcamera {\n\n> +\n> +LOG_DECLARE_CATEGORY(ISP)\n> +\n> +void ISPCPU::autoContrast(uint16_t *data, float lowCut, float highCut, int width, int height)\n> +{\n> + int blue, gr, gb, red;\n> + int histBlue[1024] = {0}, histGb[1024] = {0}, histGr[1024] = {0}, histRed[1024] = {0};\n> +\n> + int index;\n> + for (int i = 0; i < height; i++) {\n> + index = i * width;\n> + for (int j = 0; j < width; j++) {\n> + if (i % 2 == 0 && j % 2 == 0) {\n> + blue = data[index];\n> + histBlue[blue]++;\n> + }\n> + else if ((i % 2 == 0 && j % 2 == 1)) {\n> + gb = data[index];\n> + histGb[gb]++;\n> + }\n> + else if ((i % 2 == 1 && j % 2 == 0)) {\n> + gr = data[index];\n> + histGr[gr]++;\n> + }\n> + else {\n> + red = data[index];\n> + histRed[red]++;\n> + }\n> + index++;\n> + } \n> + }\n> +\n> + int pixelAmount = width * height;\n> + int sum = 0;\n> + int minBlue;\n> + for (int i = 0; i < 1024; i++){\n> + sum = sum + histBlue[i];\n> + if (sum >= pixelAmount * lowCut * 0.01) {\n> + minBlue = i;\n> + break;\n> + }\n> + }\n> +\n> + sum = 0;\n> + int maxBlue;\n> + for (int i = 1023; i >= 0; i--){\n> + sum = sum + histBlue[i];\n> + if (sum >= pixelAmount * highCut * 0.01) {\n> + maxBlue = i;\n> + break;\n> + }\n> + }\n> +\n> + sum = 0;\n> + int minGb;\n> + for (int i = 0; i < 1024; i++){\n> + sum = sum + histGb[i];\n> + if (sum >= pixelAmount * lowCut * 0.01) {\n> + minGb = i;\n> + break;\n> + }\n> + }\n> +\n> + sum = 0;\n> + int maxGb;\n> + for (int i = 1023; i >= 0; i--){\n> + sum = sum + histGb[i];\n> + if (sum >= pixelAmount * highCut * 0.01) {\n> + maxGb = i;\n> + break;\n> + }\n> + }\n> +\n> + sum = 0;\n> + int minGr;\n> + for (int i = 0; i < 1024; i++){\n> + sum = sum + histGr[i];\n> + if (sum >= pixelAmount * lowCut * 0.01) {\n> + minGr = i;\n> + break;\n> + }\n> + }\n> +\n> + sum = 0;\n> + int maxGr;\n> + for (int i = 1023; i >= 0; i--){\n> + sum = sum + histGr[i];\n> + if (sum >= pixelAmount * highCut * 0.01) {\n> + maxGr = i;\n> + break;\n> + }\n> + }\n> +\n> + sum = 0;\n> + int minRed;\n> + for (int i = 0; i < 1024; i++){\n> + sum = sum + histRed[i];\n> + if (sum >= pixelAmount * lowCut * 0.01) {\n> + minRed = i;\n> + break;\n> + }\n> + }\n> +\n> + sum = 0;\n> + int maxRed;\n> + for (int i = 1023; i >= 0; i--){\n> + sum = sum + histRed[i];\n> + if (sum >= pixelAmount * highCut * 0.01) {\n> + maxRed = i;\n> + break;\n> + }\n> + }\n> +\n> + int blueMap[1024];\n> + float norb = 1.0 / (maxBlue - minBlue);\n> + for (int i = 0; i < 1024; i++) {\n> + if (i < minBlue) {\n> + blueMap[i] = 0;\n> + }\n> + else if (i > maxBlue) {\n> + blueMap[i] = 1023;\n> + }\n> + else {\n> + blueMap[i] = (i - minBlue) * norb * 1023;\n> + }\n> + if (blueMap[i] > 1023) blueMap[i] = 1023;\n> + }\n> +\n> + int gbMap[1024];\n> + float norgb = 1.0 / (maxGb - minGb);\n> + for (int i = 0; i < 1024; i++) {\n> + if (i < minGb) {\n> + gbMap[i] = 0;\n> + }\n> + else if (i > maxGb) {\n> + gbMap[i] = 1023;\n> + }\n> + else {\n> + gbMap[i] = (i - minGb) * norgb * 1023;\n> + }\n> + if (gbMap[i] > 1023) gbMap[i] = 1023;\n> + }\n> +\n> + int grMap[1024];\n> + float norgr = 1.0 / (maxGr - minGr);\n> + for (int i = 0; i < 1024; i++) {\n> + if (i < minGr) {\n> + grMap[i] = 0;\n> + }\n> + else if (i > maxGr) {\n> + grMap[i] = 1023;\n> + }\n> + else {\n> + grMap[i] = (i - minGr) * norgr * 1023;\n> + }\n> + if (grMap[i] > 1023) grMap[i] = 1023;\n> + }\n> +\n> + int redMap[1024];\n> + float norr = 1.0 / (maxRed - minRed);\n> + for (int i = 0; i < 1024; i++) {\n> + if (i < minRed) {\n> + redMap[i] = 0;\n> + }\n> + else if (i > maxRed) {\n> + redMap[i] = 1023;\n> + }\n> + else{\n> + redMap[i] = (i - minRed) * norr * 1023;\n> + }\n> + if (redMap[i] > 1023) redMap[i] = 1023;\n> + }\n> +\n> + for (int i = 0;i < height; i++) {\n> + for (int j = 0; j < width; j++){\n> + index = i * width;\n> + if (i % 2 == 0 && j % 2 == 0) {\n> + data[index] = blueMap[data[index]];\n> + } \n> + else if (i % 2 == 0 && j % 2 == 1) {\n> + data[index] = gbMap[data[index]];\n> + } \n> + else if (i % 2 == 1 && j % 2 == 0) {\n> + data[index] = grMap[data[index]];\n> + } \n> + else {\n> + data[index] = redMap[data[index]];\n> + }\n> + index++;\n> + }\n> + }\n> +}\n> +\n> +void ISPCPU::blackLevelCorrect(uint16_t *data, uint16_t offset, int width, int height)\n> +{\n> + int len = width * height;\n> + for(int i = 0; i < len; i++) {\n> + if (data[i] < offset){\n> + data[i] = 0;\n> + }\n> + else {\n> + data[i] -= offset;\n> + }\n> + }\n> +}\n> +\n> +void ISPCPU::readChannels(uint16_t *data, uint16_t *R, uint16_t *G, uint16_t *B,\n> + int width, int height)\n> +{\n> + int index;\n> + for (int i = 0; i < height; i++) {\n> + index = i * width;\n> + for (int j = 0; j < width; j++) {\n> + if (i % 2 == 0 && j % 2 == 0) {\n> + B[index] = data[index];\n> + }\n> + else if ((i % 2 == 0 && j % 2 == 1) || (i % 2 == 1 && j % 2 == 0)){\n> + G[index] = data[index];\n> + }\n> + else {\n> + R[index] = data[index];\n> + }\n> + index++;\n> + }\n> + }\n> +}\n> +\n> +void ISPCPU::firstPixelInsert(uint16_t *src, uint16_t *dst, int width, int height)\n> +{\n> + int index;\n> + for (int i = 0; i < height; i++) {\n> + index = i * width;\n> + for (int j = 0; j < width; j++){\n> + if (i % 2 == 0 && j % 2 == 1) {\n> + if (j == (width - 1)) {\n> + dst[index] = src[index - 1];\n> + }\n> + else {\n> + dst[index] = (src[index - 1] +\n> + src[index + 1]) >> 1;\n> + }\n> + }\n> +\n> + if (i % 2 == 1 && j % 2 == 0) {\n> + if(i == height - 1) {\n> + dst[index] = src[index - width];\n> + }\n> + else {\n> + dst[index] = (src[index - width]+\n> + src[index + width]) >> 1;\n> + }\n> + }\n> + \n> + if (i % 2 == 1 && j % 2 == 1) {\n> + if (j < width - 1 && i < height - 1) {\n> + dst[index] = (src[index - width - 1] +\n> + src[index - width + 1] +\n> + src[index + width - 1] +\n> + src[index + width + 1]) >> 2;\n> + }\n> + else if (i == height - 1 && j < width - 1) {\n> + dst[index] = (src[index - width - 1] +\n> + src[index - width + 1]) >> 1;\n> + }\n> + else if (i < height - 1 && j == width - 1) {\n> + dst[index] = (src[index - width - 1] +\n> + src[index + width - 1]) >> 1;\n> + }\n> + else {\n> + dst[index] = src[index - width - 1];\n> + } \n> + }\n> + index++;\n> + }\n> + }\n> +}\n> +\n> +void ISPCPU::twoPixelInsert(uint16_t *src, uint16_t *dst, int width, int height)\n> +{\n> + int index;\n> + for (int i = 0; i < height; i++) {\n> + index = i * width;\n> + for (int j = 0; j < width; j++) {\n> + if (i == 0 && j == 0) {\n> + dst[index] = (src[index + width] +\n> + src[index + 1]) >> 1;\n> + }\n> + else if (i == 0 && j > 0 && j % 2 == 0) {\n> + dst[index] = (src[index - 1] +\n> + src[index + width] +\n> + src[index + 1]) / 3;\n> + }\n> + else if (i > 0 && j == 0 && i % 2 == 0) {\n> + dst[index] = (src[index - width] +\n> + src[index + 1] +\n> + src[index + width]) / 3;\n> + }\n> + else if (i == (height - 1) && j < (width - 1) && j % 2 == 1) {\n> + dst[index] = (src[index - 1] +\n> + src[index - width] +\n> + src[index + 1]) / 3;\n> + }\n> + else if (i < (height - 1) && j == (width - 1) && i % 2 == 1) {\n> + dst[index] = (src[index - width] +\n> + src[index - 1] +\n> + src[index + width]) / 3;\n> + }\n> + else if (i == (height - 1) && j == (width - 1)) {\n> + dst[index] = (src[index - width] +\n> + src[index - 1]) >> 1;\n> + }\n> + else if ((i % 2 == 0 && j % 2 == 0) || (i % 2 == 1 && j % 2 == 1)) {\n> + dst[index] = (src[index - 1] +\n> + src[index + 1] +\n> + src[index - width] +\n> + src[index + width]) / 4;\n> + }\n> + index++;\n> + }\n> + }\n> +}\n> +\n> +void ISPCPU::lastPixelInsert(uint16_t *src, uint16_t *dst, int width, int height)\n> +{\n> + int index;\n> + for (int i = 0; i < height; i++) {\n> + index = i * width;\n> + for (int j = 0; j < width; j++){\n> + if (i % 2 == 1 && j % 2 == 0) {\n> + if (j == 0) {\n> + dst[index] = src[index + 1];\n> + }\n> + else {\n> + dst[index] = (src[index - 1] +\n> + src[index + 1]) >> 1;\n> + }\n> + }\n> +\n> + if (i % 2 == 0 && j % 2 == 1) {\n> + if(i == 0) {\n> + dst[index] = src[index + width];\n> + }\n> + else {\n> + dst[index] = (src[index - width]+\n> + src[index + width]) >> 1;\n> + }\n> + }\n> +\n> + if (i % 2 == 0 && j % 2 == 0) {\n> + if (i > 0 && j > 0) {\n> + dst[index] = (src[index - width - 1] +\n> + src[index - width + 1] +\n> + src[index + width - 1] +\n> + src[index + width + 1]) >> 2;\n> + }\n> + else if (i == 0 && j > 0) {\n> + dst[index] = (src[index + width - 1] +\n> + src[index + width + 1]) >> 1;\n> + }\n> + else if (i > 0 && j == 0) {\n> + dst[index] = (src[index - width + 1] +\n> + src[index + width + 1]) >> 1;\n> + }\n> + else {\n> + dst[index] = src[index + width + 1];\n> + }\n> + }\n> + index++;\n> + }\n> + }\n> +}\n> +\n> +void ISPCPU::demosaic(uint16_t *data, uint16_t *R, uint16_t *G, uint16_t *B,\n> + int width, int height)\n> +{\n> + firstPixelInsert(data, B, width, height);\n> + twoPixelInsert(data, G, width, height);\n> + lastPixelInsert(data, R, width, height);\n> +}\n> +\n> +void ISPCPU::autoWhiteBalance(uint16_t *R, uint16_t *G, uint16_t *B, int width, int height)\n> +{\n> + float aveB = 0, aveG = 0, aveR = 0;\n> + float Kb, Kg, Kr;\n> +\n> + for (int i = 0; i < width * height; i++) {\n> + aveB += 1.0 * B[i];\n> + aveG += 1.0 * G[i];\n> + aveR += 1.0 * R[i];\n> + }\n> +\n> + aveB *= (1.0 / (width * height));\n> + aveG *= (1.0 / (width * height));\n> + aveR *= (1.0 / (width * height));\n> +\n> + Kr = (aveB + aveG + aveR) / aveR * (1.0 / 3.0);\n> + Kg = (aveB + aveG + aveR) / aveG * (1.0 / 3.0);\n> + Kb = (aveB + aveG + aveR) / aveB * (1.0 / 3.0);\n> +\n> + for (int i = 0; i < width * height; i++) {\n> + B[i] = B[i] * Kb;\n> + G[i] = G[i] * Kg;\n> + R[i] = R[i] * Kr;\n> +\n> + if (R[i] > 1023) R[i] = 1023;\n> + if (G[i] > 1023) G[i] = 1023;\n> + if (R[i] > 1023) B[i] = 1023;\n> + }\n> +}\n> +\n> +void ISPCPU::gammaCorrect(uint16_t *R, uint16_t *G, uint16_t *B, float val, int width, int height)\n> +{\n> + float nor = 1.0 / 1023.0;\n> + float gamma = 1.0 / val;\n> + for (int i = 0; i < width * height; i++) {\n> + R[i] = pow(R[i] * nor, gamma) * 1023;\n> + G[i] = pow(G[i] * nor, gamma) * 1023;\n> + B[i] = pow(B[i] * nor, gamma) * 1023;\n> +\n> + if (R[i] > 1023) R[i] = 1023;\n> + if (G[i] > 1023) G[i] = 1023;\n> + if (B[i] > 1023) B[i] = 1023;\n> + }\n> +}\n> +\n> +void ISPCPU::compressAndTransformFormat(uint16_t *src, uint8_t *dst, int width, int height)\n\nUsually the order of dst and src is reversed (dst first, src next).\n\n> +{\n> + switch(outputpixelformat)\n> + {\n> + case RGB888: {\n> + int j = 0;\n> + for (int i = 0; i < width * height; i++, j += 3) {\n> + dst[i] = src[j] >> 2 & 0xff;\n\nYou have dst and src reversed.\n\n> + }\n> + \n> + j = 1;\n> + for (int i = 0; i < width * height; i++, j += 3) {\n> + dst[i + width * height] = src[j] >> 2 & 0xff;\n> + }\n> +\n> + j = 2;\n> + for (int i = 0; i < width * height; i++, j += 3) {\n> + dst[i + width * height *2] = src[j] >> 2 & 0xff;\n> + }\n> + break;\n> + }\n> +\n> + case BGR888: {\n> + int j = 2;\n> + for (int i = 0; i < width * height; i++, j += 3) {\n> + dst[i] = src[j] >> 2 & 0xff;\n> + }\n> + \n> + j = 1;\n> + for (int i = 0; i < width * height; i++, j += 3) {\n> + dst[i + width * height] = src[j] >> 2 & 0xff;\n> + }\n> +\n> + j = 0;\n> + for (int i = 0; i < width * height; i++, j += 3) {\n> + dst[i + width * height *2] = src[j] >> 2 & 0xff;\n> + }\n> + break;\n> + }\n> + }\n> +}\n> +\n> +float ISPCPU::distance(int x, int y, int i, int j)\n> +{\n> + return float(sqrt(pow(x - i, 2) + pow(y - j, 2)));\n> +}\n> +\n> +double ISPCPU::gaussian(float x, double sigma)\n> +{\n> + return exp(-(pow(x, 2)) / (2 * pow(sigma, 2))) / (2 * 3.1415926 * pow(sigma, 2));\n> +}\n> +\n> +void ISPCPU::bilateralFilter(uint16_t *R, uint16_t *G, uint16_t *B,\n> + int diameter, double sigmaI,\n> + double sigmaS, int width, int height)\n> +{\n> + for (int i = 2; i < height - 2; i++) {\n> + for (int j = 2; j < width - 2; j++) {\n> + double iFiltered = 0;\n> + double wp = 0;\n> + int neighbor_x = 0;\n> + int neighbor_y = 0;\n> + int half = diameter / 2;\n> +\n> + for (int k = 0; k < diameter; k++) {\n> + for (int l = 0; l < diameter; l++) {\n> + neighbor_x = i - (half - k);\n> + neighbor_y = j - (half - l);\n> + double gi = gaussian(R[neighbor_x * width + neighbor_y] - R[i * width +j], sigmaI);\n> + double gs = gaussian(distance(i, j, neighbor_x, neighbor_y), sigmaS);\n> + double w = gi * gs;\n> + iFiltered = iFiltered + R[neighbor_x * width + neighbor_y] * w;\n> + wp = wp + w;\n> + } \n> + }\n> +\n> + iFiltered = iFiltered / wp;\n> + R[i * width + j] = iFiltered;\n> + }\n> + }\n> +\n> + for (int i = 2; i < height - 2; i++) {\n> + for (int j = 2; j < width - 2; j++) {\n> + double iFiltered = 0;\n> + double wp = 0;\n> + int neighbor_x = 0;\n> + int neighbor_y = 0;\n> + int half = diameter / 2;\n> +\n> + for (int k = 0; k < diameter; k++) {\n> + for (int l = 0; l < diameter; l++) {\n> + neighbor_x = i - (half - k);\n> + neighbor_y = j - (half - l);\n> + double gi = gaussian(G[neighbor_x * width + neighbor_y] - G[i * width +j], sigmaI);\n> + double gs = gaussian(distance(i, j, neighbor_x, neighbor_y), sigmaS);\n> + double w = gi * gs;\n> + iFiltered = iFiltered + G[neighbor_x * width + neighbor_y] * w;\n> + wp = wp + w;\n> + } \n> + }\n> +\n> + iFiltered = iFiltered / wp;\n> + G[i * width + j] = iFiltered;\n> + }\n> + }\n> +\n> + for (int i = 2; i < height - 2; i++) {\n> + for (int j = 2; j < width - 2; j++) {\n> + double iFiltered = 0;\n> + double wp = 0;\n> + int neighbor_x = 0;\n> + int neighbor_y = 0;\n> + int half = diameter / 2;\n> +\n> + for (int k = 0; k < diameter; k++) {\n> + for (int l = 0; l < diameter; l++) {\n> + neighbor_x = i - (half - k);\n> + neighbor_y = j - (half - l);\n> + double gi = gaussian(B[neighbor_x * width + neighbor_y] - B[i * width +j], sigmaI);\n> + double gs = gaussian(distance(i, j, neighbor_x, neighbor_y), sigmaS);\n> + double w = gi * gs;\n> + iFiltered = iFiltered + B[neighbor_x * width + neighbor_y] * w;\n> + wp = wp + w;\n> + } \n> + }\n> +\n> + iFiltered = iFiltered / wp;\n> + B[i * width + j] = iFiltered;\n> + }\n> + }\n> +}\n> +\n> +void ISPCPU::noiseReduction(uint16_t *R, uint16_t *G, uint16_t *B, int width, int height)\n> +{\n> + bilateralFilter(R, G, B, 5, 24.0, 32.0, width, height);\n> +}\n> +\n> +void ISPCPU::processing(FrameBuffer *srcBuffer, FrameBuffer *dstBuffer, int width, int height)\n> +{\n\nYou'll have to change this after you change the interface, as discussed\nbelow (by the headers).\n\n> + uint8_t *rgb_buf;\n> + uint16_t *rawData;\n> + uint16_t *rgbData = new std::uint16_t[width * height * 3];\n> +\n> + uint16_t *rData = rgbData;\n> + uint16_t *gData = rData + width * height;\n> + uint16_t *bData = gData + width * height;\n> + memset(rgbData, 0x0, width * height * 3);\n\nIt would be nice to rename these variables to make it clear which buffer\nis which :/\n\n> +\n> + const FrameBuffer::Plane &plane = srcBuffer->planes()[0]; \n> + rawData = (uint16_t *)mmap(NULL, plane.length, PROT_READ|PROT_WRITE, MAP_SHARED, plane.fd.fd(), 0);\n> + if (rawData == MAP_FAILED) {\n> + LOG(ISP, Error) << \"Read raw data failed\";\n> + ispCompleted.emit(srcBuffer, dstBuffer);\n> + }\n> +\n> + blackLevelCorrect(rawData, 16, width, height);\n> + readChannels(rawData, rData, gData, bData, width, height);\n> + demosaic(rawData, rData, gData, bData, width, height);\n> + autoWhiteBalance(rData, gData, bData, width, height);\n> + autoContrast(rData, 0.01, 0.01, width, height);\n> + autoContrast(gData, 0.01, 0.01, width, height);\n> + autoContrast(bData, 0.01, 0.01, width, height);\n> + gammaCorrect(rData, gData, bData, 2.2, width, height);\n> + //bilateralFilter(rData, gData, bData, 5, 24.0, 32.0, width, height);\n> + \n> + const FrameBuffer::Plane &rgbPlane = dstBuffer->planes()[0];\n> + rgb_buf = (uint8_t *)mmap(NULL, rgbPlane.length, PROT_READ|PROT_WRITE, MAP_SHARED, rgbPlane.fd.fd(), 0);\n> + if (rgb_buf == MAP_FAILED) {\n> + LOG(ISP, Error) << \"Read rgb data failed\";\n> + ispCompleted.emit(srcBuffer, dstBuffer);\n> + }\n> +\n> + compressAndTransformFormat(rgbData, rgb_buf, width, height);\n> +\n> + dstBuffer->metadata_.status = srcBuffer->metadata().status;\n> + dstBuffer->metadata_.sequence = srcBuffer->metadata().sequence;\n> + dstBuffer->metadata_.timestamp = srcBuffer->metadata().timestamp;\n> +\n> + dstBuffer->metadata_.planes.clear();\n> + dstBuffer->metadata_.planes.push_back({rgbPlane.length});\n> +\n> + delete[] rgbData;\n> +\n> + ispCompleted.emit(srcBuffer, dstBuffer);\n> +}\n> +\n> +ISPCPU::outputPixelFormat ISPCPU::getOutputPixelFormat(PixelFormat format)\n> +{\n> + static const std::map transform {\n> + {formats::RGB888, RGB888},\n> + {formats::BGR888, BGR888},\n> + };\n> +\n> + auto itr = transform.find(format);\n> + return itr->second;\n> +\n\nI think you don't need this.\n\n> +\n> +std::map> ISPCPU::pixelFormatConfiguration()\n> +{\n> + SizeRange sizeRange({640, 480});\n> + std::vector sizeRanges({std::move(sizeRange)});\n> + ispFormat.insert({formats::RGB888, sizeRanges});\n> + ispFormat.insert({formats::BGR888, sizeRanges});\n> +\n> + return ispFormat;\n> +}\n\nI think you don't need this.\n\n> +\n> +void ISPCPU::paramConfiguration()\n> +{\n> + struct BLC_PARAM blc = {16};\n> +\n> + struct LSC_PARAM lsc_grid = {\n> + {{1.4305, 1.4355, 1.4390, 1.4440, 1.4530, 1.4640, 1.4740, 1.4800, 1.4810, 1.4800, 1.4710, 1.4615, 1.4525, 1.4480, 1.4410, 1.4405},\n> + {1.4315, 1.4370, 1.4425, 1.4520, 1.4635, 1.4760, 1.4855, 1.4955, 1.4955, 1.4920, 1.4830, 1.4695, 1.4590, 1.4510, 1.4445, 1.4405},\n> + {1.4335, 1.4410, 1.4500, 1.4625, 1.4755, 1.4920, 1.5055, 1.5155, 1.5170, 1.5165, 1.4975, 1.4830, 1.4680, 1.4540, 1.4475, 1.4425},\n> + {1.4325, 1.4430, 1.4550, 1.4705, 1.4920, 1.5070, 1.5250, 1.5370, 1.5380, 1.5325, 1.5165, 1.4975, 1.4750, 1.4575, 1.4490, 1.4455},\n> + {1.4325, 1.4425, 1.4575, 1.4805, 1.5050, 1.5250, 1.5380, 1.5490, 1.5495, 1.5410, 1.5320, 1.5070, 1.4825, 1.4600, 1.4485, 1.4450},\n> + {1.4315, 1.4425, 1.4575, 1.4805, 1.5055, 1.5270, 1.5470, 1.5550, 1.5550, 1.5465, 1.5325, 1.5080, 1.4825, 1.4600, 1.4455, 1.4430},\n> + {1.4300, 1.4400, 1.4555, 1.4785, 1.5050, 1.5260, 1.5435, 1.5485, 1.5495, 1.5380, 1.5270, 1.5075, 1.4795, 1.4580, 1.4430, 1.4390},\n> + {1.4275, 1.4345, 1.4480, 1.4690, 1.4965, 1.5135, 1.5275, 1.5370, 1.5365, 1.5270, 1.5105, 1.4965, 1.4725, 1.4525, 1.4390, 1.4335},\n> + {1.4215, 1.4285, 1.4395, 1.4580, 1.4795, 1.4980, 1.5135, 1.5205, 1.5205, 1.5090, 1.4965, 1.4780, 1.4600, 1.4435, 1.4330, 1.4290},\n> + {1.4165, 1.4230, 1.4300, 1.4410, 1.4590, 1.4795, 1.4955, 1.5005, 1.5005, 1.4885, 1.4780, 1.4600, 1.4500, 1.4360, 1.4310, 1.4250},\n> + {1.4125, 1.4160, 1.4230, 1.4290, 1.4410, 1.4575, 1.4705, 1.4760, 1.4760, 1.4690, 1.4545, 1.4495, 1.4355, 1.4300, 1.4250, 1.4250},\n> + {1.4100, 1.4135, 1.4175, 1.4230, 1.4290, 1.4410, 1.4545, 1.4560, 1.4560, 1.4525, 1.4485, 1.4365, 1.4305, 1.4235, 1.4230, 1.4250}},\n> +\n> + {{1.2955, 1.2935, 1.2805, 1.2660, 1.2490, 1.234, 1.2320, 1.2320, 1.2325, 1.2365, 1.2425, 1.2550, 1.2690, 1.2810, 1.2875, 1.2905},\n> + {1.2935, 1.2840, 1.2690, 1.2515, 1.2320, 1.2160, 1.2060, 1.2060, 1.2090, 1.2130, 1.2255, 1.2390, 1.2565, 1.2700, 1.2805, 1.2835},\n> + {1.2860, 1.2710, 1.2525, 1.2320, 1.2160, 1.2030, 1.1890, 1.1860, 1.1865, 1.1955, 1.2055, 1.2240, 1.2370, 1.2550, 1.2715, 1.2780},\n> + {1.2815, 1.2590, 1.2390, 1.2200, 1.2030, 1.1890, 1.1785, 1.1740, 1.1740, 1.1830, 1.1950, 1.2055, 1.2235, 1.2425, 1.2625, 1.2770},\n> + {1.2805, 1.2560, 1.2330, 1.2125, 1.1960, 1.1795, 1.1735, 1.1660, 1.1660, 1.1730, 1.1830, 1.1960, 1.2145, 1.2360, 1.2575, 1.2730},\n> + {1.2795, 1.2510, 1.2280, 1.2080, 1.1910, 1.1770, 1.1670, 1.1640, 1.1635, 1.1655, 1.1750, 1.1895, 1.2080, 1.2315, 1.2550, 1.2720},\n> + {1.2795, 1.2510, 1.2265, 1.2070, 1.1910, 1.1770, 1.1680, 1.1640, 1.1630, 1.1645, 1.1740, 1.1870, 1.2060, 1.2315, 1.2550, 1.2715},\n> + {1.2805, 1.2520, 1.2265, 1.2105, 1.1950, 1.1865, 1.1765, 1.1680, 1.1665, 1.1725, 1.1795, 1.1905, 1.2075, 1.2320, 1.2565, 1.2720},\n> + {1.2815, 1.2585, 1.2350, 1.2195, 1.2090, 1.1975, 1.1880, 1.1820, 1.1805, 1.1810, 1.1905, 1.2025, 1.2185, 1.2385, 1.2625, 1.2750},\n> + {1.2825, 1.2675, 1.2495, 1.2325, 1.2220, 1.2135, 1.2060, 1.2020, 1.2000, 1.1995, 1.2050, 1.2170, 1.2315, 1.2495, 1.2725, 1.2785},\n> + {1.2825, 1.2740, 1.2640, 1.2460, 1.2360, 1.2290, 1.2235, 1.2215, 1.2200, 1.2185, 1.2195, 1.2285, 1.2415, 1.2565, 1.2750, 1.2850},\n> + {1.2825, 1.2765, 1.2700, 1.2605, 1.2450, 1.2380, 1.2350, 1.2350, 1.2350, 1.2310, 1.2315, 1.2390, 1.2500, 1.2575, 1.2740, 1.2875}},\n> + };\t\n\nThese default values should go to a tuning file.\n\n> +\n> +}\n> +\n> +int ISPCPU::exportBuffers(std::vector> *buffers,\n> + unsigned int count, int width, int height)\n> +{\n> + int bufferByte = width * height * 3;\n> +\n> + for (unsigned int i = 0; i < count; i++) {\n> + std::string name = \"frame-\" + std::to_string(i);\n> +\n> + const int isp_fd = memfd_create(name.c_str(), 0);\n> + int ret = ftruncate(isp_fd, bufferByte);\n> + if (ret < 0) {\n> + LOG(ISP, Error) << \"Failed to resize memfd\" << strerror(-ret);\n> + return ret;\n> + }\n> +\n> + FrameBuffer::Plane rgbPlane;\n> + rgbPlane.fd = FileDescriptor(std::move(isp_fd));\n> + rgbPlane.length = bufferByte;\n> +\n> + std::vector planes{rgbPlane};\n> + buffers->emplace_back(std::make_unique(std::move(planes)));\n> + }\n> +\n> + return count;\n> +}\n> +\n> +void ISPCPU::startThreadISP()\n> +{\n> + moveToThread(&thread_);\n> + thread_.start();\n> +}\n> +\n> +void ISPCPU::stopThreadISP()\n> +{\n> + thread_.exit();\n> + thread_.wait();\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/swisp/isp.h b/src/libcamera/swisp/isp.h\n\nThis should go do include/libcamera/internal/isp.h\n\n> new file mode 100644\n> index 00000000..535f1b61\n> --- /dev/null\n> +++ b/src/libcamera/swisp/isp.h\n> @@ -0,0 +1,125 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Siyuan Fan \n> + *\n> + * isp.h - The software ISP class\n> + */\n> +#ifndef __LIBCAMERA_SWISP_ISP_H__\n> +#define __LIBCAMERA_SWISP_ISP_H__\n> +\n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \"libcamera/base/object.h\"\n> +#include \"libcamera/base/signal.h\"\n> +#include \"libcamera/base/thread.h\"\n> +\n> +namespace libcamera{\n> +\n> +using std::uint16_t;\n> +using std::uint8_t;\n\nWhy are these here?\n\n> +\n> +class ISP : public Object\n> +{\n> +public:\n> + ISP() {}\n> +\n> + virtual ~ISP() {}\n> +\n> + enum outputPixelFormat {\n> + RGB888,\n> + BGR888,\n> + };\n\nNo need for this enum, just use libcamera's format::RGB888 etc directly.\n\n> +\n> + virtual outputPixelFormat getOutputPixelFormat(PixelFormat format) = 0;\n\nYou don't need this.\n\n> +\n> + virtual void processing(FrameBuffer *srcBuffer, FrameBuffer *dstBuffer, int width, int height) = 0;\n\nJust some ideas (as we discussed in the meeting).\n\nYou want to support big-ish parameters, like the lens shading table. We\nmight need a control for this...? (Laurent, what do you think?)\n\nstruct ISPOp {\n\tOpCode,\n\tparams\n}\n\nvirtual void processing(std::vector ops, src, dst, width height);\n\nisp->processing({ Demosaic, AWB, ToneMapping, NR, Contrast(params) }, src, dst, width, height)\n\nThis might be good?\n\nisp->processing(Demosaic, src, dst, w, h)\nisp->processing(Contrast, src, dst, w, h, threshold)\n\nThis would have a lot of thread switching.\n\n> +\n> + virtual std::map> pixelFormatConfiguration() = 0;\n\nI think you can remove this.\n\n> +\n> + virtual void paramConfiguration() = 0;\n\nThis should be removed obviously; we'll discuss where to move its\ncontents to.\n\n> +\n> + virtual int exportBuffers(std::vector> *buffers,\n> + unsigned int count, int width, int height) = 0;\n\nI think you should have a configure() function.\n\n\tvirtual int configure(PixelFormat inputFormat,\n\t\t\t PixelFormat outputFormat,\n\t\t\t Size inputSize,\n\t\t\t Size outputSize) = 0;\n\nThis asks the ISP to use the specified formats and sizes, and the ISP\nreplies with if it is able to do so.\n\n> +\n> + virtual void startThreadISP() = 0;\n> + virtual void stopThreadISP() = 0;\n\nThese can be start(), stop(); I think it's cleaner.\n\n> +\n> + Signal ispCompleted;\n\nGood.\n\n> +\n> + std::map> ispFormat;\n\nI don't think you need this.\n\n> +};\n\nOnly everything above here will go into isp.h, as it is the API for all\nISPs.\n\n\nEverything below here is a header for an ISP implementation. It should\ngo in src/libcamera/swisp/cpu/cpu.h\n\nYou'll also need src/libcamera/swisp/cpu/meson.build.\n\nI think you only need to add cpu.h and cpu.cpp to libcamera_sources\n(like src/libcamera/pipeline/raspberrypi/meson.build).\n\nsrc/libcamera/swisp/meson.build will just subdir() every swisp\nimplementation (like src/libcamera/pipeline/meson.build).\n\n\nPaul\n\n> +\n> +class ISPCPU : public ISP\n> +{\n> +public:\n> + struct BLC_PARAM {\n> + uint16_t black_level;\n> + };\n> +\n> + struct LSC_PARAM {\n> + float bGain[12][16];\n> + float rGain[12][16];\n> + };\n> +\n> + outputPixelFormat getOutputPixelFormat(PixelFormat format) override;\n> +\n> + void processing(FrameBuffer *srcBuffer, FrameBuffer *dstBuffer, int width, int height) override;\n> +\n> + std::map> pixelFormatConfiguration() override;\n> +\n> + void paramConfiguration() override;\n> +\n> + int exportBuffers(std::vector> *buffers,\n> + unsigned int count, int width, int height) override;\n> +\n> + void startThreadISP() override;\n> +\n> + void stopThreadISP() override;\n> +\n> + enum outputPixelFormat outputpixelformat;\n> +\n> +private:\n> + void autoContrast(uint16_t *data, float lowCut, float highCut, int width, int height);\n> +\n> + void blackLevelCorrect(uint16_t *data, uint16_t offset, int width, int height);\n> +\n> + void readChannels(uint16_t *data, uint16_t *R, uint16_t *G, uint16_t *B,\n> + int width, int height);\n> +\n> + void firstPixelInsert(uint16_t *src, uint16_t *dst, int width, int height);\n> +\n> + void twoPixelInsert(uint16_t *src, uint16_t *dst, int width, int height);\n> +\n> + void lastPixelInsert(uint16_t *src, uint16_t *dst, int width, int height);\n> +\n> + void demosaic(uint16_t *data, uint16_t *R, uint16_t *G, uint16_t *B,\n> + int width, int height);\n> +\n> + void autoWhiteBalance(uint16_t *R, uint16_t *G, uint16_t *B, int width, int height);\n> +\n> + void gammaCorrect(uint16_t *R, uint16_t *G, uint16_t *B, float val, int width, int height);\n> +\n> + float distance(int x, int y, int i, int j);\n> +\n> + double gaussian(float x, double sigma);\n> +\n> + void bilateralFilter(uint16_t *R, uint16_t *G, uint16_t *B,\n> + int diameter, double sigmaI, double sigmaS,\n> + int width, int height);\n> +\n> + void noiseReduction(uint16_t *R, uint16_t *G, uint16_t *B, int width, int height);\n> +\n> + void compressAndTransformFormat(uint16_t *src, uint8_t *dst, int width, int height); \n> +\n> + Thread thread_;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_SWISP_ISP_H__ */\n> -- \n> 2.20.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3953FBD87D\n\tfor ;\n\tWed, 18 Aug 2021 11:39:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A21C5688A3;\n\tWed, 18 Aug 2021 13:39:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1BF256888A\n\tfor ;\n\tWed, 18 Aug 2021 13:39:18 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 64E0DEE;\n\tWed, 18 Aug 2021 13:39:16 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Hbz+93xr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629286757;\n\tbh=jHDWeCz0fCeRVXlr/lVfayqv8/8XMR1R4NHR8SYim8w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Hbz+93xr7vUeHjvgtovaQpoXCrJ3h5sMWcnrECI/mILuQJ8445s5E+EEaOKcnkynP\n\tmRmRBBenF7uPuHVWqXD69W963G04wPt7rvV8gAv2dNeBPuPgb0iuZ4bfvBtrtIDmAL\n\t6YXyOos+Shwxf8mT2ZRCGTMNn6ODnFoCOHAShRYI=","Date":"Wed, 18 Aug 2021 20:39:09 +0900","From":"paul.elder@ideasonboard.com","To":"Siyuan Fan ","Message-ID":"<20210818113909.GK1733965@pyrite.rasen.tech>","References":"","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH v3 2/4] libcamera: swisp: The\n\tsoftware ISP class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18915,"web_url":"https://patchwork.libcamera.org/comment/18915/","msgid":"","date":"2021-08-18T12:08:08","subject":"Re: [libcamera-devel] [RFC PATCH v3 2/4] libcamera: swisp: The\n\tsoftware ISP class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Aug 18, 2021 at 08:39:09PM +0900, paul.elder@ideasonboard.com wrote:\n> Hi Siyuan,\n> \n> On Tue, Aug 17, 2021 at 04:42:19PM +0100, Siyuan Fan wrote:\n> > From: Fan Siyuan \n> > \n> > Currently class ISPCPU only supports to output RGB888 and BGR888(640x480).\n> > Based on format set by application, using getOutputPixelFormat() to match\n> > output format and compressAndTransformFormat() to transform corresponding format.\n> > \n> > Signed-off-by: Fan Siyuan \n> > ---\n> > src/libcamera/swisp/isp.cpp | 726 ++++++++++++++++++++++++++++++++++++\n> > src/libcamera/swisp/isp.h | 125 +++++++\n> > 2 files changed, 851 insertions(+)\n> > create mode 100644 src/libcamera/swisp/isp.cpp\n> > create mode 100644 src/libcamera/swisp/isp.h\n> > \n> > diff --git a/src/libcamera/swisp/isp.cpp b/src/libcamera/swisp/isp.cpp\n> > new file mode 100644\n> > index 00000000..b0f801e9\n> > --- /dev/null\n> > +++ b/src/libcamera/swisp/isp.cpp\n> > @@ -0,0 +1,726 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Siyuan Fan \n> > + *\n> > + * isp.cpp - The software ISP class\n> > + */\n> > +\n> > +#include \"isp.h\"\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \"libcamera/base/log.h\"\n> > +\n> > +namespace libcamera{\n> \n> namespace libcamera {\n\nEven better,\n\ncp utils/hooks/post-commit .git/hooks/\n\n> > +\n> > +LOG_DECLARE_CATEGORY(ISP)\n> > +\n> > +void ISPCPU::autoContrast(uint16_t *data, float lowCut, float highCut, int width, int height)\n> > +{\n> > + int blue, gr, gb, red;\n> > + int histBlue[1024] = {0}, histGb[1024] = {0}, histGr[1024] = {0}, histRed[1024] = {0};\n> > +\n> > + int index;\n> > + for (int i = 0; i < height; i++) {\n> > + index = i * width;\n> > + for (int j = 0; j < width; j++) {\n> > + if (i % 2 == 0 && j % 2 == 0) {\n> > + blue = data[index];\n> > + histBlue[blue]++;\n> > + }\n> > + else if ((i % 2 == 0 && j % 2 == 1)) {\n> > + gb = data[index];\n> > + histGb[gb]++;\n> > + }\n> > + else if ((i % 2 == 1 && j % 2 == 0)) {\n> > + gr = data[index];\n> > + histGr[gr]++;\n> > + }\n> > + else {\n> > + red = data[index];\n> > + histRed[red]++;\n> > + }\n> > + index++;\n> > + } \n> > + }\n> > +\n> > + int pixelAmount = width * height;\n> > + int sum = 0;\n> > + int minBlue;\n> > + for (int i = 0; i < 1024; i++){\n> > + sum = sum + histBlue[i];\n> > + if (sum >= pixelAmount * lowCut * 0.01) {\n> > + minBlue = i;\n> > + break;\n> > + }\n> > + }\n> > +\n> > + sum = 0;\n> > + int maxBlue;\n> > + for (int i = 1023; i >= 0; i--){\n> > + sum = sum + histBlue[i];\n> > + if (sum >= pixelAmount * highCut * 0.01) {\n> > + maxBlue = i;\n> > + break;\n> > + }\n> > + }\n> > +\n> > + sum = 0;\n> > + int minGb;\n> > + for (int i = 0; i < 1024; i++){\n> > + sum = sum + histGb[i];\n> > + if (sum >= pixelAmount * lowCut * 0.01) {\n> > + minGb = i;\n> > + break;\n> > + }\n> > + }\n> > +\n> > + sum = 0;\n> > + int maxGb;\n> > + for (int i = 1023; i >= 0; i--){\n> > + sum = sum + histGb[i];\n> > + if (sum >= pixelAmount * highCut * 0.01) {\n> > + maxGb = i;\n> > + break;\n> > + }\n> > + }\n> > +\n> > + sum = 0;\n> > + int minGr;\n> > + for (int i = 0; i < 1024; i++){\n> > + sum = sum + histGr[i];\n> > + if (sum >= pixelAmount * lowCut * 0.01) {\n> > + minGr = i;\n> > + break;\n> > + }\n> > + }\n> > +\n> > + sum = 0;\n> > + int maxGr;\n> > + for (int i = 1023; i >= 0; i--){\n> > + sum = sum + histGr[i];\n> > + if (sum >= pixelAmount * highCut * 0.01) {\n> > + maxGr = i;\n> > + break;\n> > + }\n> > + }\n> > +\n> > + sum = 0;\n> > + int minRed;\n> > + for (int i = 0; i < 1024; i++){\n> > + sum = sum + histRed[i];\n> > + if (sum >= pixelAmount * lowCut * 0.01) {\n> > + minRed = i;\n> > + break;\n> > + }\n> > + }\n> > +\n> > + sum = 0;\n> > + int maxRed;\n> > + for (int i = 1023; i >= 0; i--){\n> > + sum = sum + histRed[i];\n> > + if (sum >= pixelAmount * highCut * 0.01) {\n> > + maxRed = i;\n> > + break;\n> > + }\n> > + }\n> > +\n> > + int blueMap[1024];\n> > + float norb = 1.0 / (maxBlue - minBlue);\n> > + for (int i = 0; i < 1024; i++) {\n> > + if (i < minBlue) {\n> > + blueMap[i] = 0;\n> > + }\n> > + else if (i > maxBlue) {\n> > + blueMap[i] = 1023;\n> > + }\n> > + else {\n> > + blueMap[i] = (i - minBlue) * norb * 1023;\n> > + }\n> > + if (blueMap[i] > 1023) blueMap[i] = 1023;\n> > + }\n> > +\n> > + int gbMap[1024];\n> > + float norgb = 1.0 / (maxGb - minGb);\n> > + for (int i = 0; i < 1024; i++) {\n> > + if (i < minGb) {\n> > + gbMap[i] = 0;\n> > + }\n> > + else if (i > maxGb) {\n> > + gbMap[i] = 1023;\n> > + }\n> > + else {\n> > + gbMap[i] = (i - minGb) * norgb * 1023;\n> > + }\n> > + if (gbMap[i] > 1023) gbMap[i] = 1023;\n> > + }\n> > +\n> > + int grMap[1024];\n> > + float norgr = 1.0 / (maxGr - minGr);\n> > + for (int i = 0; i < 1024; i++) {\n> > + if (i < minGr) {\n> > + grMap[i] = 0;\n> > + }\n> > + else if (i > maxGr) {\n> > + grMap[i] = 1023;\n> > + }\n> > + else {\n> > + grMap[i] = (i - minGr) * norgr * 1023;\n> > + }\n> > + if (grMap[i] > 1023) grMap[i] = 1023;\n> > + }\n> > +\n> > + int redMap[1024];\n> > + float norr = 1.0 / (maxRed - minRed);\n> > + for (int i = 0; i < 1024; i++) {\n> > + if (i < minRed) {\n> > + redMap[i] = 0;\n> > + }\n> > + else if (i > maxRed) {\n> > + redMap[i] = 1023;\n> > + }\n> > + else{\n> > + redMap[i] = (i - minRed) * norr * 1023;\n> > + }\n> > + if (redMap[i] > 1023) redMap[i] = 1023;\n> > + }\n> > +\n> > + for (int i = 0;i < height; i++) {\n> > + for (int j = 0; j < width; j++){\n> > + index = i * width;\n> > + if (i % 2 == 0 && j % 2 == 0) {\n> > + data[index] = blueMap[data[index]];\n> > + } \n> > + else if (i % 2 == 0 && j % 2 == 1) {\n> > + data[index] = gbMap[data[index]];\n> > + } \n> > + else if (i % 2 == 1 && j % 2 == 0) {\n> > + data[index] = grMap[data[index]];\n> > + } \n> > + else {\n> > + data[index] = redMap[data[index]];\n> > + }\n> > + index++;\n> > + }\n> > + }\n> > +}\n> > +\n> > +void ISPCPU::blackLevelCorrect(uint16_t *data, uint16_t offset, int width, int height)\n> > +{\n> > + int len = width * height;\n> > + for(int i = 0; i < len; i++) {\n> > + if (data[i] < offset){\n> > + data[i] = 0;\n> > + }\n> > + else {\n> > + data[i] -= offset;\n> > + }\n> > + }\n> > +}\n> > +\n> > +void ISPCPU::readChannels(uint16_t *data, uint16_t *R, uint16_t *G, uint16_t *B,\n> > + int width, int height)\n> > +{\n> > + int index;\n> > + for (int i = 0; i < height; i++) {\n> > + index = i * width;\n> > + for (int j = 0; j < width; j++) {\n> > + if (i % 2 == 0 && j % 2 == 0) {\n> > + B[index] = data[index];\n> > + }\n> > + else if ((i % 2 == 0 && j % 2 == 1) || (i % 2 == 1 && j % 2 == 0)){\n> > + G[index] = data[index];\n> > + }\n> > + else {\n> > + R[index] = data[index];\n> > + }\n> > + index++;\n> > + }\n> > + }\n> > +}\n> > +\n> > +void ISPCPU::firstPixelInsert(uint16_t *src, uint16_t *dst, int width, int height)\n> > +{\n> > + int index;\n> > + for (int i = 0; i < height; i++) {\n> > + index = i * width;\n> > + for (int j = 0; j < width; j++){\n> > + if (i % 2 == 0 && j % 2 == 1) {\n> > + if (j == (width - 1)) {\n> > + dst[index] = src[index - 1];\n> > + }\n> > + else {\n> > + dst[index] = (src[index - 1] +\n> > + src[index + 1]) >> 1;\n> > + }\n> > + }\n> > +\n> > + if (i % 2 == 1 && j % 2 == 0) {\n> > + if(i == height - 1) {\n> > + dst[index] = src[index - width];\n> > + }\n> > + else {\n> > + dst[index] = (src[index - width]+\n> > + src[index + width]) >> 1;\n> > + }\n> > + }\n> > + \n> > + if (i % 2 == 1 && j % 2 == 1) {\n> > + if (j < width - 1 && i < height - 1) {\n> > + dst[index] = (src[index - width - 1] +\n> > + src[index - width + 1] +\n> > + src[index + width - 1] +\n> > + src[index + width + 1]) >> 2;\n> > + }\n> > + else if (i == height - 1 && j < width - 1) {\n> > + dst[index] = (src[index - width - 1] +\n> > + src[index - width + 1]) >> 1;\n> > + }\n> > + else if (i < height - 1 && j == width - 1) {\n> > + dst[index] = (src[index - width - 1] +\n> > + src[index + width - 1]) >> 1;\n> > + }\n> > + else {\n> > + dst[index] = src[index - width - 1];\n> > + } \n> > + }\n> > + index++;\n> > + }\n> > + }\n> > +}\n> > +\n> > +void ISPCPU::twoPixelInsert(uint16_t *src, uint16_t *dst, int width, int height)\n> > +{\n> > + int index;\n> > + for (int i = 0; i < height; i++) {\n> > + index = i * width;\n> > + for (int j = 0; j < width; j++) {\n> > + if (i == 0 && j == 0) {\n> > + dst[index] = (src[index + width] +\n> > + src[index + 1]) >> 1;\n> > + }\n> > + else if (i == 0 && j > 0 && j % 2 == 0) {\n> > + dst[index] = (src[index - 1] +\n> > + src[index + width] +\n> > + src[index + 1]) / 3;\n> > + }\n> > + else if (i > 0 && j == 0 && i % 2 == 0) {\n> > + dst[index] = (src[index - width] +\n> > + src[index + 1] +\n> > + src[index + width]) / 3;\n> > + }\n> > + else if (i == (height - 1) && j < (width - 1) && j % 2 == 1) {\n> > + dst[index] = (src[index - 1] +\n> > + src[index - width] +\n> > + src[index + 1]) / 3;\n> > + }\n> > + else if (i < (height - 1) && j == (width - 1) && i % 2 == 1) {\n> > + dst[index] = (src[index - width] +\n> > + src[index - 1] +\n> > + src[index + width]) / 3;\n> > + }\n> > + else if (i == (height - 1) && j == (width - 1)) {\n> > + dst[index] = (src[index - width] +\n> > + src[index - 1]) >> 1;\n> > + }\n> > + else if ((i % 2 == 0 && j % 2 == 0) || (i % 2 == 1 && j % 2 == 1)) {\n> > + dst[index] = (src[index - 1] +\n> > + src[index + 1] +\n> > + src[index - width] +\n> > + src[index + width]) / 4;\n> > + }\n> > + index++;\n> > + }\n> > + }\n> > +}\n> > +\n> > +void ISPCPU::lastPixelInsert(uint16_t *src, uint16_t *dst, int width, int height)\n> > +{\n> > + int index;\n> > + for (int i = 0; i < height; i++) {\n> > + index = i * width;\n> > + for (int j = 0; j < width; j++){\n> > + if (i % 2 == 1 && j % 2 == 0) {\n> > + if (j == 0) {\n> > + dst[index] = src[index + 1];\n> > + }\n> > + else {\n> > + dst[index] = (src[index - 1] +\n> > + src[index + 1]) >> 1;\n> > + }\n> > + }\n> > +\n> > + if (i % 2 == 0 && j % 2 == 1) {\n> > + if(i == 0) {\n> > + dst[index] = src[index + width];\n> > + }\n> > + else {\n> > + dst[index] = (src[index - width]+\n> > + src[index + width]) >> 1;\n> > + }\n> > + }\n> > +\n> > + if (i % 2 == 0 && j % 2 == 0) {\n> > + if (i > 0 && j > 0) {\n> > + dst[index] = (src[index - width - 1] +\n> > + src[index - width + 1] +\n> > + src[index + width - 1] +\n> > + src[index + width + 1]) >> 2;\n> > + }\n> > + else if (i == 0 && j > 0) {\n> > + dst[index] = (src[index + width - 1] +\n> > + src[index + width + 1]) >> 1;\n> > + }\n> > + else if (i > 0 && j == 0) {\n> > + dst[index] = (src[index - width + 1] +\n> > + src[index + width + 1]) >> 1;\n> > + }\n> > + else {\n> > + dst[index] = src[index + width + 1];\n> > + }\n> > + }\n> > + index++;\n> > + }\n> > + }\n> > +}\n> > +\n> > +void ISPCPU::demosaic(uint16_t *data, uint16_t *R, uint16_t *G, uint16_t *B,\n> > + int width, int height)\n> > +{\n> > + firstPixelInsert(data, B, width, height);\n> > + twoPixelInsert(data, G, width, height);\n> > + lastPixelInsert(data, R, width, height);\n> > +}\n> > +\n> > +void ISPCPU::autoWhiteBalance(uint16_t *R, uint16_t *G, uint16_t *B, int width, int height)\n> > +{\n> > + float aveB = 0, aveG = 0, aveR = 0;\n> > + float Kb, Kg, Kr;\n> > +\n> > + for (int i = 0; i < width * height; i++) {\n> > + aveB += 1.0 * B[i];\n> > + aveG += 1.0 * G[i];\n> > + aveR += 1.0 * R[i];\n> > + }\n> > +\n> > + aveB *= (1.0 / (width * height));\n> > + aveG *= (1.0 / (width * height));\n> > + aveR *= (1.0 / (width * height));\n> > +\n> > + Kr = (aveB + aveG + aveR) / aveR * (1.0 / 3.0);\n> > + Kg = (aveB + aveG + aveR) / aveG * (1.0 / 3.0);\n> > + Kb = (aveB + aveG + aveR) / aveB * (1.0 / 3.0);\n> > +\n> > + for (int i = 0; i < width * height; i++) {\n> > + B[i] = B[i] * Kb;\n> > + G[i] = G[i] * Kg;\n> > + R[i] = R[i] * Kr;\n> > +\n> > + if (R[i] > 1023) R[i] = 1023;\n> > + if (G[i] > 1023) G[i] = 1023;\n> > + if (R[i] > 1023) B[i] = 1023;\n> > + }\n> > +}\n> > +\n> > +void ISPCPU::gammaCorrect(uint16_t *R, uint16_t *G, uint16_t *B, float val, int width, int height)\n> > +{\n> > + float nor = 1.0 / 1023.0;\n> > + float gamma = 1.0 / val;\n> > + for (int i = 0; i < width * height; i++) {\n> > + R[i] = pow(R[i] * nor, gamma) * 1023;\n> > + G[i] = pow(G[i] * nor, gamma) * 1023;\n> > + B[i] = pow(B[i] * nor, gamma) * 1023;\n> > +\n> > + if (R[i] > 1023) R[i] = 1023;\n> > + if (G[i] > 1023) G[i] = 1023;\n> > + if (B[i] > 1023) B[i] = 1023;\n> > + }\n> > +}\n> > +\n> > +void ISPCPU::compressAndTransformFormat(uint16_t *src, uint8_t *dst, int width, int height)\n> \n> Usually the order of dst and src is reversed (dst first, src next).\n> \n> > +{\n> > + switch(outputpixelformat)\n> > + {\n> > + case RGB888: {\n> > + int j = 0;\n> > + for (int i = 0; i < width * height; i++, j += 3) {\n> > + dst[i] = src[j] >> 2 & 0xff;\n> \n> You have dst and src reversed.\n> \n> > + }\n> > + \n> > + j = 1;\n> > + for (int i = 0; i < width * height; i++, j += 3) {\n> > + dst[i + width * height] = src[j] >> 2 & 0xff;\n> > + }\n> > +\n> > + j = 2;\n> > + for (int i = 0; i < width * height; i++, j += 3) {\n> > + dst[i + width * height *2] = src[j] >> 2 & 0xff;\n> > + }\n> > + break;\n> > + }\n> > +\n> > + case BGR888: {\n> > + int j = 2;\n> > + for (int i = 0; i < width * height; i++, j += 3) {\n> > + dst[i] = src[j] >> 2 & 0xff;\n> > + }\n> > + \n> > + j = 1;\n> > + for (int i = 0; i < width * height; i++, j += 3) {\n> > + dst[i + width * height] = src[j] >> 2 & 0xff;\n> > + }\n> > +\n> > + j = 0;\n> > + for (int i = 0; i < width * height; i++, j += 3) {\n> > + dst[i + width * height *2] = src[j] >> 2 & 0xff;\n> > + }\n> > + break;\n> > + }\n> > + }\n> > +}\n> > +\n> > +float ISPCPU::distance(int x, int y, int i, int j)\n> > +{\n> > + return float(sqrt(pow(x - i, 2) + pow(y - j, 2)));\n> > +}\n> > +\n> > +double ISPCPU::gaussian(float x, double sigma)\n> > +{\n> > + return exp(-(pow(x, 2)) / (2 * pow(sigma, 2))) / (2 * 3.1415926 * pow(sigma, 2));\n> > +}\n> > +\n> > +void ISPCPU::bilateralFilter(uint16_t *R, uint16_t *G, uint16_t *B,\n> > + int diameter, double sigmaI,\n> > + double sigmaS, int width, int height)\n> > +{\n> > + for (int i = 2; i < height - 2; i++) {\n> > + for (int j = 2; j < width - 2; j++) {\n> > + double iFiltered = 0;\n> > + double wp = 0;\n> > + int neighbor_x = 0;\n> > + int neighbor_y = 0;\n> > + int half = diameter / 2;\n> > +\n> > + for (int k = 0; k < diameter; k++) {\n> > + for (int l = 0; l < diameter; l++) {\n> > + neighbor_x = i - (half - k);\n> > + neighbor_y = j - (half - l);\n> > + double gi = gaussian(R[neighbor_x * width + neighbor_y] - R[i * width +j], sigmaI);\n> > + double gs = gaussian(distance(i, j, neighbor_x, neighbor_y), sigmaS);\n> > + double w = gi * gs;\n> > + iFiltered = iFiltered + R[neighbor_x * width + neighbor_y] * w;\n> > + wp = wp + w;\n> > + } \n> > + }\n> > +\n> > + iFiltered = iFiltered / wp;\n> > + R[i * width + j] = iFiltered;\n> > + }\n> > + }\n> > +\n> > + for (int i = 2; i < height - 2; i++) {\n> > + for (int j = 2; j < width - 2; j++) {\n> > + double iFiltered = 0;\n> > + double wp = 0;\n> > + int neighbor_x = 0;\n> > + int neighbor_y = 0;\n> > + int half = diameter / 2;\n> > +\n> > + for (int k = 0; k < diameter; k++) {\n> > + for (int l = 0; l < diameter; l++) {\n> > + neighbor_x = i - (half - k);\n> > + neighbor_y = j - (half - l);\n> > + double gi = gaussian(G[neighbor_x * width + neighbor_y] - G[i * width +j], sigmaI);\n> > + double gs = gaussian(distance(i, j, neighbor_x, neighbor_y), sigmaS);\n> > + double w = gi * gs;\n> > + iFiltered = iFiltered + G[neighbor_x * width + neighbor_y] * w;\n> > + wp = wp + w;\n> > + } \n> > + }\n> > +\n> > + iFiltered = iFiltered / wp;\n> > + G[i * width + j] = iFiltered;\n> > + }\n> > + }\n> > +\n> > + for (int i = 2; i < height - 2; i++) {\n> > + for (int j = 2; j < width - 2; j++) {\n> > + double iFiltered = 0;\n> > + double wp = 0;\n> > + int neighbor_x = 0;\n> > + int neighbor_y = 0;\n> > + int half = diameter / 2;\n> > +\n> > + for (int k = 0; k < diameter; k++) {\n> > + for (int l = 0; l < diameter; l++) {\n> > + neighbor_x = i - (half - k);\n> > + neighbor_y = j - (half - l);\n> > + double gi = gaussian(B[neighbor_x * width + neighbor_y] - B[i * width +j], sigmaI);\n> > + double gs = gaussian(distance(i, j, neighbor_x, neighbor_y), sigmaS);\n> > + double w = gi * gs;\n> > + iFiltered = iFiltered + B[neighbor_x * width + neighbor_y] * w;\n> > + wp = wp + w;\n> > + } \n> > + }\n> > +\n> > + iFiltered = iFiltered / wp;\n> > + B[i * width + j] = iFiltered;\n> > + }\n> > + }\n> > +}\n> > +\n> > +void ISPCPU::noiseReduction(uint16_t *R, uint16_t *G, uint16_t *B, int width, int height)\n> > +{\n> > + bilateralFilter(R, G, B, 5, 24.0, 32.0, width, height);\n> > +}\n> > +\n> > +void ISPCPU::processing(FrameBuffer *srcBuffer, FrameBuffer *dstBuffer, int width, int height)\n> > +{\n> \n> You'll have to change this after you change the interface, as discussed\n> below (by the headers).\n> \n> > + uint8_t *rgb_buf;\n> > + uint16_t *rawData;\n> > + uint16_t *rgbData = new std::uint16_t[width * height * 3];\n> > +\n> > + uint16_t *rData = rgbData;\n> > + uint16_t *gData = rData + width * height;\n> > + uint16_t *bData = gData + width * height;\n> > + memset(rgbData, 0x0, width * height * 3);\n> \n> It would be nice to rename these variables to make it clear which buffer\n> is which :/\n> \n> > +\n> > + const FrameBuffer::Plane &plane = srcBuffer->planes()[0]; \n> > + rawData = (uint16_t *)mmap(NULL, plane.length, PROT_READ|PROT_WRITE, MAP_SHARED, plane.fd.fd(), 0);\n> > + if (rawData == MAP_FAILED) {\n> > + LOG(ISP, Error) << \"Read raw data failed\";\n> > + ispCompleted.emit(srcBuffer, dstBuffer);\n> > + }\n> > +\n> > + blackLevelCorrect(rawData, 16, width, height);\n> > + readChannels(rawData, rData, gData, bData, width, height);\n> > + demosaic(rawData, rData, gData, bData, width, height);\n> > + autoWhiteBalance(rData, gData, bData, width, height);\n> > + autoContrast(rData, 0.01, 0.01, width, height);\n> > + autoContrast(gData, 0.01, 0.01, width, height);\n> > + autoContrast(bData, 0.01, 0.01, width, height);\n> > + gammaCorrect(rData, gData, bData, 2.2, width, height);\n> > + //bilateralFilter(rData, gData, bData, 5, 24.0, 32.0, width, height);\n> > + \n> > + const FrameBuffer::Plane &rgbPlane = dstBuffer->planes()[0];\n> > + rgb_buf = (uint8_t *)mmap(NULL, rgbPlane.length, PROT_READ|PROT_WRITE, MAP_SHARED, rgbPlane.fd.fd(), 0);\n> > + if (rgb_buf == MAP_FAILED) {\n> > + LOG(ISP, Error) << \"Read rgb data failed\";\n> > + ispCompleted.emit(srcBuffer, dstBuffer);\n> > + }\n> > +\n> > + compressAndTransformFormat(rgbData, rgb_buf, width, height);\n> > +\n> > + dstBuffer->metadata_.status = srcBuffer->metadata().status;\n> > + dstBuffer->metadata_.sequence = srcBuffer->metadata().sequence;\n> > + dstBuffer->metadata_.timestamp = srcBuffer->metadata().timestamp;\n> > +\n> > + dstBuffer->metadata_.planes.clear();\n> > + dstBuffer->metadata_.planes.push_back({rgbPlane.length});\n> > +\n> > + delete[] rgbData;\n> > +\n> > + ispCompleted.emit(srcBuffer, dstBuffer);\n> > +}\n> > +\n> > +ISPCPU::outputPixelFormat ISPCPU::getOutputPixelFormat(PixelFormat format)\n> > +{\n> > + static const std::map transform {\n> > + {formats::RGB888, RGB888},\n> > + {formats::BGR888, BGR888},\n> > + };\n> > +\n> > + auto itr = transform.find(format);\n> > + return itr->second;\n> > +\n> \n> I think you don't need this.\n> \n> > +\n> > +std::map> ISPCPU::pixelFormatConfiguration()\n> > +{\n> > + SizeRange sizeRange({640, 480});\n> > + std::vector sizeRanges({std::move(sizeRange)});\n> > + ispFormat.insert({formats::RGB888, sizeRanges});\n> > + ispFormat.insert({formats::BGR888, sizeRanges});\n> > +\n> > + return ispFormat;\n> > +}\n> \n> I think you don't need this.\n> \n> > +\n> > +void ISPCPU::paramConfiguration()\n> > +{\n> > + struct BLC_PARAM blc = {16};\n> > +\n> > + struct LSC_PARAM lsc_grid = {\n> > + {{1.4305, 1.4355, 1.4390, 1.4440, 1.4530, 1.4640, 1.4740, 1.4800, 1.4810, 1.4800, 1.4710, 1.4615, 1.4525, 1.4480, 1.4410, 1.4405},\n> > + {1.4315, 1.4370, 1.4425, 1.4520, 1.4635, 1.4760, 1.4855, 1.4955, 1.4955, 1.4920, 1.4830, 1.4695, 1.4590, 1.4510, 1.4445, 1.4405},\n> > + {1.4335, 1.4410, 1.4500, 1.4625, 1.4755, 1.4920, 1.5055, 1.5155, 1.5170, 1.5165, 1.4975, 1.4830, 1.4680, 1.4540, 1.4475, 1.4425},\n> > + {1.4325, 1.4430, 1.4550, 1.4705, 1.4920, 1.5070, 1.5250, 1.5370, 1.5380, 1.5325, 1.5165, 1.4975, 1.4750, 1.4575, 1.4490, 1.4455},\n> > + {1.4325, 1.4425, 1.4575, 1.4805, 1.5050, 1.5250, 1.5380, 1.5490, 1.5495, 1.5410, 1.5320, 1.5070, 1.4825, 1.4600, 1.4485, 1.4450},\n> > + {1.4315, 1.4425, 1.4575, 1.4805, 1.5055, 1.5270, 1.5470, 1.5550, 1.5550, 1.5465, 1.5325, 1.5080, 1.4825, 1.4600, 1.4455, 1.4430},\n> > + {1.4300, 1.4400, 1.4555, 1.4785, 1.5050, 1.5260, 1.5435, 1.5485, 1.5495, 1.5380, 1.5270, 1.5075, 1.4795, 1.4580, 1.4430, 1.4390},\n> > + {1.4275, 1.4345, 1.4480, 1.4690, 1.4965, 1.5135, 1.5275, 1.5370, 1.5365, 1.5270, 1.5105, 1.4965, 1.4725, 1.4525, 1.4390, 1.4335},\n> > + {1.4215, 1.4285, 1.4395, 1.4580, 1.4795, 1.4980, 1.5135, 1.5205, 1.5205, 1.5090, 1.4965, 1.4780, 1.4600, 1.4435, 1.4330, 1.4290},\n> > + {1.4165, 1.4230, 1.4300, 1.4410, 1.4590, 1.4795, 1.4955, 1.5005, 1.5005, 1.4885, 1.4780, 1.4600, 1.4500, 1.4360, 1.4310, 1.4250},\n> > + {1.4125, 1.4160, 1.4230, 1.4290, 1.4410, 1.4575, 1.4705, 1.4760, 1.4760, 1.4690, 1.4545, 1.4495, 1.4355, 1.4300, 1.4250, 1.4250},\n> > + {1.4100, 1.4135, 1.4175, 1.4230, 1.4290, 1.4410, 1.4545, 1.4560, 1.4560, 1.4525, 1.4485, 1.4365, 1.4305, 1.4235, 1.4230, 1.4250}},\n> > +\n> > + {{1.2955, 1.2935, 1.2805, 1.2660, 1.2490, 1.234, 1.2320, 1.2320, 1.2325, 1.2365, 1.2425, 1.2550, 1.2690, 1.2810, 1.2875, 1.2905},\n> > + {1.2935, 1.2840, 1.2690, 1.2515, 1.2320, 1.2160, 1.2060, 1.2060, 1.2090, 1.2130, 1.2255, 1.2390, 1.2565, 1.2700, 1.2805, 1.2835},\n> > + {1.2860, 1.2710, 1.2525, 1.2320, 1.2160, 1.2030, 1.1890, 1.1860, 1.1865, 1.1955, 1.2055, 1.2240, 1.2370, 1.2550, 1.2715, 1.2780},\n> > + {1.2815, 1.2590, 1.2390, 1.2200, 1.2030, 1.1890, 1.1785, 1.1740, 1.1740, 1.1830, 1.1950, 1.2055, 1.2235, 1.2425, 1.2625, 1.2770},\n> > + {1.2805, 1.2560, 1.2330, 1.2125, 1.1960, 1.1795, 1.1735, 1.1660, 1.1660, 1.1730, 1.1830, 1.1960, 1.2145, 1.2360, 1.2575, 1.2730},\n> > + {1.2795, 1.2510, 1.2280, 1.2080, 1.1910, 1.1770, 1.1670, 1.1640, 1.1635, 1.1655, 1.1750, 1.1895, 1.2080, 1.2315, 1.2550, 1.2720},\n> > + {1.2795, 1.2510, 1.2265, 1.2070, 1.1910, 1.1770, 1.1680, 1.1640, 1.1630, 1.1645, 1.1740, 1.1870, 1.2060, 1.2315, 1.2550, 1.2715},\n> > + {1.2805, 1.2520, 1.2265, 1.2105, 1.1950, 1.1865, 1.1765, 1.1680, 1.1665, 1.1725, 1.1795, 1.1905, 1.2075, 1.2320, 1.2565, 1.2720},\n> > + {1.2815, 1.2585, 1.2350, 1.2195, 1.2090, 1.1975, 1.1880, 1.1820, 1.1805, 1.1810, 1.1905, 1.2025, 1.2185, 1.2385, 1.2625, 1.2750},\n> > + {1.2825, 1.2675, 1.2495, 1.2325, 1.2220, 1.2135, 1.2060, 1.2020, 1.2000, 1.1995, 1.2050, 1.2170, 1.2315, 1.2495, 1.2725, 1.2785},\n> > + {1.2825, 1.2740, 1.2640, 1.2460, 1.2360, 1.2290, 1.2235, 1.2215, 1.2200, 1.2185, 1.2195, 1.2285, 1.2415, 1.2565, 1.2750, 1.2850},\n> > + {1.2825, 1.2765, 1.2700, 1.2605, 1.2450, 1.2380, 1.2350, 1.2350, 1.2350, 1.2310, 1.2315, 1.2390, 1.2500, 1.2575, 1.2740, 1.2875}},\n> > + };\t\n> \n> These default values should go to a tuning file.\n> \n> > +\n> > +}\n> > +\n> > +int ISPCPU::exportBuffers(std::vector> *buffers,\n> > + unsigned int count, int width, int height)\n> > +{\n> > + int bufferByte = width * height * 3;\n> > +\n> > + for (unsigned int i = 0; i < count; i++) {\n> > + std::string name = \"frame-\" + std::to_string(i);\n> > +\n> > + const int isp_fd = memfd_create(name.c_str(), 0);\n> > + int ret = ftruncate(isp_fd, bufferByte);\n> > + if (ret < 0) {\n> > + LOG(ISP, Error) << \"Failed to resize memfd\" << strerror(-ret);\n> > + return ret;\n> > + }\n> > +\n> > + FrameBuffer::Plane rgbPlane;\n> > + rgbPlane.fd = FileDescriptor(std::move(isp_fd));\n> > + rgbPlane.length = bufferByte;\n> > +\n> > + std::vector planes{rgbPlane};\n> > + buffers->emplace_back(std::make_unique(std::move(planes)));\n> > + }\n> > +\n> > + return count;\n> > +}\n> > +\n> > +void ISPCPU::startThreadISP()\n> > +{\n> > + moveToThread(&thread_);\n> > + thread_.start();\n> > +}\n> > +\n> > +void ISPCPU::stopThreadISP()\n> > +{\n> > + thread_.exit();\n> > + thread_.wait();\n> > +}\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/swisp/isp.h b/src/libcamera/swisp/isp.h\n> \n> This should go do include/libcamera/internal/isp.h\n> \n> > new file mode 100644\n> > index 00000000..535f1b61\n> > --- /dev/null\n> > +++ b/src/libcamera/swisp/isp.h\n> > @@ -0,0 +1,125 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Siyuan Fan \n> > + *\n> > + * isp.h - The software ISP class\n> > + */\n> > +#ifndef __LIBCAMERA_SWISP_ISP_H__\n> > +#define __LIBCAMERA_SWISP_ISP_H__\n> > +\n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \"libcamera/base/object.h\"\n> > +#include \"libcamera/base/signal.h\"\n> > +#include \"libcamera/base/thread.h\"\n> > +\n> > +namespace libcamera{\n> > +\n> > +using std::uint16_t;\n> > +using std::uint8_t;\n> \n> Why are these here?\n> \n> > +\n> > +class ISP : public Object\n> > +{\n> > +public:\n> > + ISP() {}\n> > +\n> > + virtual ~ISP() {}\n> > +\n> > + enum outputPixelFormat {\n> > + RGB888,\n> > + BGR888,\n> > + };\n> \n> No need for this enum, just use libcamera's format::RGB888 etc directly.\n> \n> > +\n> > + virtual outputPixelFormat getOutputPixelFormat(PixelFormat format) = 0;\n> \n> You don't need this.\n> \n> > +\n> > + virtual void processing(FrameBuffer *srcBuffer, FrameBuffer *dstBuffer, int width, int height) = 0;\n> \n> Just some ideas (as we discussed in the meeting).\n> \n> You want to support big-ish parameters, like the lens shading table. We\n> might need a control for this...? (Laurent, what do you think?)\n> \n> struct ISPOp {\n> \tOpCode,\n> \tparams\n> }\n> \n> virtual void processing(std::vector ops, src, dst, width height);\n> \n> isp->processing({ Demosaic, AWB, ToneMapping, NR, Contrast(params) }, src, dst, width, height)\n> \n> This might be good?\n> \n> isp->processing(Demosaic, src, dst, w, h)\n> isp->processing(Contrast, src, dst, w, h, threshold)\n> \n> This would have a lot of thread switching.\n> \n> > +\n> > + virtual std::map> pixelFormatConfiguration() = 0;\n> \n> I think you can remove this.\n> \n> > +\n> > + virtual void paramConfiguration() = 0;\n> \n> This should be removed obviously; we'll discuss where to move its\n> contents to.\n> \n> > +\n> > + virtual int exportBuffers(std::vector> *buffers,\n> > + unsigned int count, int width, int height) = 0;\n> \n> I think you should have a configure() function.\n> \n> \tvirtual int configure(PixelFormat inputFormat,\n> \t\t\t PixelFormat outputFormat,\n> \t\t\t Size inputSize,\n> \t\t\t Size outputSize) = 0;\n> \n> This asks the ISP to use the specified formats and sizes, and the ISP\n> replies with if it is able to do so.\n> \n> > +\n> > + virtual void startThreadISP() = 0;\n> > + virtual void stopThreadISP() = 0;\n> \n> These can be start(), stop(); I think it's cleaner.\n> \n> > +\n> > + Signal ispCompleted;\n> \n> Good.\n> \n> > +\n> > + std::map> ispFormat;\n> \n> I don't think you need this.\n> \n> > +};\n> \n> Only everything above here will go into isp.h, as it is the API for all\n> ISPs.\n> \n> \n> Everything below here is a header for an ISP implementation. It should\n> go in src/libcamera/swisp/cpu/cpu.h\n> \n> You'll also need src/libcamera/swisp/cpu/meson.build.\n> \n> I think you only need to add cpu.h and cpu.cpp to libcamera_sources\n> (like src/libcamera/pipeline/raspberrypi/meson.build).\n> \n> src/libcamera/swisp/meson.build will just subdir() every swisp\n> implementation (like src/libcamera/pipeline/meson.build).\n> \n> \n> Paul\n> \n> > +\n> > +class ISPCPU : public ISP\n> > +{\n> > +public:\n> > + struct BLC_PARAM {\n> > + uint16_t black_level;\n> > + };\n> > +\n> > + struct LSC_PARAM {\n> > + float bGain[12][16];\n> > + float rGain[12][16];\n> > + };\n> > +\n> > + outputPixelFormat getOutputPixelFormat(PixelFormat format) override;\n> > +\n> > + void processing(FrameBuffer *srcBuffer, FrameBuffer *dstBuffer, int width, int height) override;\n> > +\n> > + std::map> pixelFormatConfiguration() override;\n> > +\n> > + void paramConfiguration() override;\n> > +\n> > + int exportBuffers(std::vector> *buffers,\n> > + unsigned int count, int width, int height) override;\n> > +\n> > + void startThreadISP() override;\n> > +\n> > + void stopThreadISP() override;\n> > +\n> > + enum outputPixelFormat outputpixelformat;\n> > +\n> > +private:\n> > + void autoContrast(uint16_t *data, float lowCut, float highCut, int width, int height);\n> > +\n> > + void blackLevelCorrect(uint16_t *data, uint16_t offset, int width, int height);\n> > +\n> > + void readChannels(uint16_t *data, uint16_t *R, uint16_t *G, uint16_t *B,\n> > + int width, int height);\n> > +\n> > + void firstPixelInsert(uint16_t *src, uint16_t *dst, int width, int height);\n> > +\n> > + void twoPixelInsert(uint16_t *src, uint16_t *dst, int width, int height);\n> > +\n> > + void lastPixelInsert(uint16_t *src, uint16_t *dst, int width, int height);\n> > +\n> > + void demosaic(uint16_t *data, uint16_t *R, uint16_t *G, uint16_t *B,\n> > + int width, int height);\n> > +\n> > + void autoWhiteBalance(uint16_t *R, uint16_t *G, uint16_t *B, int width, int height);\n> > +\n> > + void gammaCorrect(uint16_t *R, uint16_t *G, uint16_t *B, float val, int width, int height);\n> > +\n> > + float distance(int x, int y, int i, int j);\n> > +\n> > + double gaussian(float x, double sigma);\n> > +\n> > + void bilateralFilter(uint16_t *R, uint16_t *G, uint16_t *B,\n> > + int diameter, double sigmaI, double sigmaS,\n> > + int width, int height);\n> > +\n> > + void noiseReduction(uint16_t *R, uint16_t *G, uint16_t *B, int width, int height);\n> > +\n> > + void compressAndTransformFormat(uint16_t *src, uint8_t *dst, int width, int height); \n> > +\n> > + Thread thread_;\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > +\n> > +#endif /* __LIBCAMERA_SWISP_ISP_H__ */","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0DC64BD87C\n\tfor ;\n\tWed, 18 Aug 2021 12:08:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5D24B68895;\n\tWed, 18 Aug 2021 14:08:17 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F09346888A\n\tfor ;\n\tWed, 18 Aug 2021 14:08:15 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4A3F5EE;\n\tWed, 18 Aug 2021 14:08:15 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"A5Lez5Jx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629288495;\n\tbh=ttjADgSgXSre4m32slWxictZIyL8AdKMohep4g6kSD4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=A5Lez5JxstXGOYJ80qfr2SEZ+jH+1upmALyVCW9yAUukow6e8BIcITtkjXQ11lIP1\n\troU2wC5+r8o0/3chK7gVmAXapxVJs2JM6qS7KZz9X5RjEfhRcJh3QCI+Q/XwoRCRiU\n\tjzSFdgvm/dWla/ZKQPkboHSi6Kxlc1AEd1Gu9d9Q=","Date":"Wed, 18 Aug 2021 15:08:08 +0300","From":"Laurent Pinchart ","To":"paul.elder@ideasonboard.com","Message-ID":"","References":"\n\t<20210818113909.GK1733965@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210818113909.GK1733965@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [RFC PATCH v3 2/4] libcamera: swisp: The\n\tsoftware ISP class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]