| Message ID | 20240627125310.2533622-3-stefan.klug@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Stefan Klug (2024-06-27 13:51:10) > Add a thumbnail function for raw formats that are 16bit aligned. > This is needed for the upcoming RAW10 and RAW12 implemntation. > > Use the new function for RAW16 as the thumbScanlineRawXX_CSI2P produces > incorrect results for that format (it averages over adjacent bytes, > which works for the CSI formats). > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > src/apps/common/dng_writer.cpp | 32 ++++++++++++++++++++++++++++---- > 1 file changed, 28 insertions(+), 4 deletions(-) > > diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp > index 88b225d3e099..2cb320ff327b 100644 > --- a/src/apps/common/dng_writer.cpp > +++ b/src/apps/common/dng_writer.cpp > @@ -144,6 +144,30 @@ void packScanlineRaw16(void *output, const void *input, unsigned int width) > std::copy(in, in + width, out); > } > > +/* Thumbnail function for raw data aligned to 16bit words */ > +void thumbScanlineRawXX(const FormatInfo &info, void *output, Yup, I could still be tempted to call this thumbScanlineRaw ... unless ... > + const void *input, unsigned int width, > + unsigned int stride) > +{ > + const uint16_t *in = static_cast<const uint16_t *>(input); > + const uint16_t *in2 = static_cast<const uint16_t *>(input) + stride / 2; > + uint8_t *out = static_cast<uint8_t *>(output); > + > + /* shift down to 8 */ > + unsigned int shift = info.bitsPerSample - 8; > + > + /* simple averaging that produces grey RGB values */ grey rgb values? > + for (unsigned int x = 0; x < width; x++) { > + uint16_t value = (in[0] + in[1] + in2[0] + in2[1]) >> 2; > + value = value >> shift; > + *out++ = value; > + *out++ = value; > + *out++ = value; > + in += 16; > + in2 += 16; Does this code work for 8,10,12,14 bit data? Or is the implication that it only handled data algined to 16bit (per pixel?) so perhaps that's 10, 12, 14, 16? > + } > +} > + > void packScanlineRaw10_CSI2P(void *output, const void *input, unsigned int width) > { > const uint8_t *in = static_cast<const uint8_t *>(input); > @@ -321,25 +345,25 @@ const std::map<PixelFormat, FormatInfo> formatInfo = { > .bitsPerSample = 16, > .pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed }, > .packScanline = packScanlineRaw16, > - .thumbScanline = thumbScanlineRawXX_CSI2P, > + .thumbScanline = thumbScanlineRawXX, > } }, > { formats::SGBRG16, { > .bitsPerSample = 16, > .pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen }, > .packScanline = packScanlineRaw16, > - .thumbScanline = thumbScanlineRawXX_CSI2P, > + .thumbScanline = thumbScanlineRawXX, > } }, > { formats::SGRBG16, { > .bitsPerSample = 16, > .pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen }, > .packScanline = packScanlineRaw16, > - .thumbScanline = thumbScanlineRawXX_CSI2P, > + .thumbScanline = thumbScanlineRawXX, > } }, > { formats::SRGGB16, { > .bitsPerSample = 16, > .pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue }, > .packScanline = packScanlineRaw16, > - .thumbScanline = thumbScanlineRawXX_CSI2P, > + .thumbScanline = thumbScanlineRawXX, > } }, > { formats::SBGGR10_CSI2P, { > .bitsPerSample = 10, > -- > 2.43.0 >
Hi Stefan Looks good to me! On Thu, 27 Jun 2024 at 13:53, Stefan Klug <stefan.klug@ideasonboard.com> wrote: > > Add a thumbnail function for raw formats that are 16bit aligned. > This is needed for the upcoming RAW10 and RAW12 implemntation. > > Use the new function for RAW16 as the thumbScanlineRawXX_CSI2P produces > incorrect results for that format (it averages over adjacent bytes, > which works for the CSI formats). > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > src/apps/common/dng_writer.cpp | 32 ++++++++++++++++++++++++++++---- > 1 file changed, 28 insertions(+), 4 deletions(-) > > diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp > index 88b225d3e099..2cb320ff327b 100644 > --- a/src/apps/common/dng_writer.cpp > +++ b/src/apps/common/dng_writer.cpp > @@ -144,6 +144,30 @@ void packScanlineRaw16(void *output, const void *input, unsigned int width) > std::copy(in, in + width, out); > } > > +/* Thumbnail function for raw data aligned to 16bit words */ > +void thumbScanlineRawXX(const FormatInfo &info, void *output, > + const void *input, unsigned int width, > + unsigned int stride) > +{ > + const uint16_t *in = static_cast<const uint16_t *>(input); > + const uint16_t *in2 = static_cast<const uint16_t *>(input) + stride / 2; > + uint8_t *out = static_cast<uint8_t *>(output); > + > + /* shift down to 8 */ > + unsigned int shift = info.bitsPerSample - 8; > + > + /* simple averaging that produces grey RGB values */ > + for (unsigned int x = 0; x < width; x++) { > + uint16_t value = (in[0] + in[1] + in2[0] + in2[1]) >> 2; > + value = value >> shift; > + *out++ = value; > + *out++ = value; > + *out++ = value; Just a small question as to whether you might want to subtract some black level and apply some "fake" gamma? It might make the thumbnails look a bit nicer. But just a matter of taste, I think, not something I'd worry over! David > + in += 16; > + in2 += 16; > + } > +} > + > void packScanlineRaw10_CSI2P(void *output, const void *input, unsigned int width) > { > const uint8_t *in = static_cast<const uint8_t *>(input); > @@ -321,25 +345,25 @@ const std::map<PixelFormat, FormatInfo> formatInfo = { > .bitsPerSample = 16, > .pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed }, > .packScanline = packScanlineRaw16, > - .thumbScanline = thumbScanlineRawXX_CSI2P, > + .thumbScanline = thumbScanlineRawXX, > } }, > { formats::SGBRG16, { > .bitsPerSample = 16, > .pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen }, > .packScanline = packScanlineRaw16, > - .thumbScanline = thumbScanlineRawXX_CSI2P, > + .thumbScanline = thumbScanlineRawXX, > } }, > { formats::SGRBG16, { > .bitsPerSample = 16, > .pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen }, > .packScanline = packScanlineRaw16, > - .thumbScanline = thumbScanlineRawXX_CSI2P, > + .thumbScanline = thumbScanlineRawXX, > } }, > { formats::SRGGB16, { > .bitsPerSample = 16, > .pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue }, > .packScanline = packScanlineRaw16, > - .thumbScanline = thumbScanlineRawXX_CSI2P, > + .thumbScanline = thumbScanlineRawXX, > } }, > { formats::SBGGR10_CSI2P, { > .bitsPerSample = 10, > -- > 2.43.0 >
Hi Stefan, Thank you for the patch. On Thu, Jun 27, 2024 at 02:51:10PM +0200, Stefan Klug wrote: > Add a thumbnail function for raw formats that are 16bit aligned. > This is needed for the upcoming RAW10 and RAW12 implemntation. > > Use the new function for RAW16 as the thumbScanlineRawXX_CSI2P produces > incorrect results for that format (it averages over adjacent bytes, > which works for the CSI formats). > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > src/apps/common/dng_writer.cpp | 32 ++++++++++++++++++++++++++++---- > 1 file changed, 28 insertions(+), 4 deletions(-) > > diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp > index 88b225d3e099..2cb320ff327b 100644 > --- a/src/apps/common/dng_writer.cpp > +++ b/src/apps/common/dng_writer.cpp > @@ -144,6 +144,30 @@ void packScanlineRaw16(void *output, const void *input, unsigned int width) > std::copy(in, in + width, out); > } > > +/* Thumbnail function for raw data aligned to 16bit words */ > +void thumbScanlineRawXX(const FormatInfo &info, void *output, > + const void *input, unsigned int width, > + unsigned int stride) > +{ > + const uint16_t *in = static_cast<const uint16_t *>(input); > + const uint16_t *in2 = static_cast<const uint16_t *>(input) + stride / 2; > + uint8_t *out = static_cast<uint8_t *>(output); > + > + /* shift down to 8 */ /* Shift down to 8. */ > + unsigned int shift = info.bitsPerSample - 8; > + > + /* simple averaging that produces grey RGB values */ Similarly here. > + for (unsigned int x = 0; x < width; x++) { > + uint16_t value = (in[0] + in[1] + in2[0] + in2[1]) >> 2; > + value = value >> shift; > + *out++ = value; > + *out++ = value; > + *out++ = value; > + in += 16; > + in2 += 16; > + } > +} Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + > void packScanlineRaw10_CSI2P(void *output, const void *input, unsigned int width) > { > const uint8_t *in = static_cast<const uint8_t *>(input); > @@ -321,25 +345,25 @@ const std::map<PixelFormat, FormatInfo> formatInfo = { > .bitsPerSample = 16, > .pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed }, > .packScanline = packScanlineRaw16, > - .thumbScanline = thumbScanlineRawXX_CSI2P, > + .thumbScanline = thumbScanlineRawXX, > } }, > { formats::SGBRG16, { > .bitsPerSample = 16, > .pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen }, > .packScanline = packScanlineRaw16, > - .thumbScanline = thumbScanlineRawXX_CSI2P, > + .thumbScanline = thumbScanlineRawXX, > } }, > { formats::SGRBG16, { > .bitsPerSample = 16, > .pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen }, > .packScanline = packScanlineRaw16, > - .thumbScanline = thumbScanlineRawXX_CSI2P, > + .thumbScanline = thumbScanlineRawXX, > } }, > { formats::SRGGB16, { > .bitsPerSample = 16, > .pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue }, > .packScanline = packScanlineRaw16, > - .thumbScanline = thumbScanlineRawXX_CSI2P, > + .thumbScanline = thumbScanlineRawXX, > } }, > { formats::SBGGR10_CSI2P, { > .bitsPerSample = 10,
On Thu, Jun 27, 2024 at 02:10:18PM +0100, Kieran Bingham wrote: > Quoting Stefan Klug (2024-06-27 13:51:10) > > Add a thumbnail function for raw formats that are 16bit aligned. > > This is needed for the upcoming RAW10 and RAW12 implemntation. > > > > Use the new function for RAW16 as the thumbScanlineRawXX_CSI2P produces > > incorrect results for that format (it averages over adjacent bytes, > > which works for the CSI formats). > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > --- > > src/apps/common/dng_writer.cpp | 32 ++++++++++++++++++++++++++++---- > > 1 file changed, 28 insertions(+), 4 deletions(-) > > > > diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp > > index 88b225d3e099..2cb320ff327b 100644 > > --- a/src/apps/common/dng_writer.cpp > > +++ b/src/apps/common/dng_writer.cpp > > @@ -144,6 +144,30 @@ void packScanlineRaw16(void *output, const void *input, unsigned int width) > > std::copy(in, in + width, out); > > } > > > > +/* Thumbnail function for raw data aligned to 16bit words */ > > +void thumbScanlineRawXX(const FormatInfo &info, void *output, > > Yup, I could still be tempted to call this thumbScanlineRaw ... unless > ... > > > + const void *input, unsigned int width, > > + unsigned int stride) > > +{ > > + const uint16_t *in = static_cast<const uint16_t *>(input); > > + const uint16_t *in2 = static_cast<const uint16_t *>(input) + stride / 2; > > + uint8_t *out = static_cast<uint8_t *>(output); > > + > > + /* shift down to 8 */ > > + unsigned int shift = info.bitsPerSample - 8; > > + > > + /* simple averaging that produces grey RGB values */ > > grey rgb values? I was looking for the rigt words here. We produce rgb thumbnails, but to ease the averaging and to prevent the need for debayering only grey is output. See commit 2223579dfee > > > + for (unsigned int x = 0; x < width; x++) { > > + uint16_t value = (in[0] + in[1] + in2[0] + in2[1]) >> 2; > > + value = value >> shift; > > + *out++ = value; > > + *out++ = value; > > + *out++ = value; > > + in += 16; > > + in2 += 16; > > Does this code work for 8,10,12,14 bit data? Or is the implication that > it only handled data algined to 16bit (per pixel?) so perhaps that's 10, > 12, 14, 16? It only handles formats that are aligned 16bit per pixel (so Raw8 is excluded) Proposals for a better name are very welcome :-) Cheers, Stefan > > > > > + } > > +} > > + > > void packScanlineRaw10_CSI2P(void *output, const void *input, unsigned int width) > > { > > const uint8_t *in = static_cast<const uint8_t *>(input); > > @@ -321,25 +345,25 @@ const std::map<PixelFormat, FormatInfo> formatInfo = { > > .bitsPerSample = 16, > > .pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed }, > > .packScanline = packScanlineRaw16, > > - .thumbScanline = thumbScanlineRawXX_CSI2P, > > + .thumbScanline = thumbScanlineRawXX, > > } }, > > { formats::SGBRG16, { > > .bitsPerSample = 16, > > .pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen }, > > .packScanline = packScanlineRaw16, > > - .thumbScanline = thumbScanlineRawXX_CSI2P, > > + .thumbScanline = thumbScanlineRawXX, > > } }, > > { formats::SGRBG16, { > > .bitsPerSample = 16, > > .pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen }, > > .packScanline = packScanlineRaw16, > > - .thumbScanline = thumbScanlineRawXX_CSI2P, > > + .thumbScanline = thumbScanlineRawXX, > > } }, > > { formats::SRGGB16, { > > .bitsPerSample = 16, > > .pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue }, > > .packScanline = packScanlineRaw16, > > - .thumbScanline = thumbScanlineRawXX_CSI2P, > > + .thumbScanline = thumbScanlineRawXX, > > } }, > > { formats::SBGGR10_CSI2P, { > > .bitsPerSample = 10, > > -- > > 2.43.0 > >
Hi David, On Thu, Jun 27, 2024 at 02:27:57PM +0100, David Plowman wrote: > Hi Stefan > > Looks good to me! Thanks > > On Thu, 27 Jun 2024 at 13:53, Stefan Klug <stefan.klug@ideasonboard.com> wrote: > > > > Add a thumbnail function for raw formats that are 16bit aligned. > > This is needed for the upcoming RAW10 and RAW12 implemntation. > > > > Use the new function for RAW16 as the thumbScanlineRawXX_CSI2P produces > > incorrect results for that format (it averages over adjacent bytes, > > which works for the CSI formats). > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > --- > > src/apps/common/dng_writer.cpp | 32 ++++++++++++++++++++++++++++---- > > 1 file changed, 28 insertions(+), 4 deletions(-) > > > > diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp > > index 88b225d3e099..2cb320ff327b 100644 > > --- a/src/apps/common/dng_writer.cpp > > +++ b/src/apps/common/dng_writer.cpp > > @@ -144,6 +144,30 @@ void packScanlineRaw16(void *output, const void *input, unsigned int width) > > std::copy(in, in + width, out); > > } > > > > +/* Thumbnail function for raw data aligned to 16bit words */ > > +void thumbScanlineRawXX(const FormatInfo &info, void *output, > > + const void *input, unsigned int width, > > + unsigned int stride) > > +{ > > + const uint16_t *in = static_cast<const uint16_t *>(input); > > + const uint16_t *in2 = static_cast<const uint16_t *>(input) + stride / 2; > > + uint8_t *out = static_cast<uint8_t *>(output); > > + > > + /* shift down to 8 */ > > + unsigned int shift = info.bitsPerSample - 8; > > + > > + /* simple averaging that produces grey RGB values */ > > + for (unsigned int x = 0; x < width; x++) { > > + uint16_t value = (in[0] + in[1] + in2[0] + in2[1]) >> 2; > > + value = value >> shift; > > + *out++ = value; > > + *out++ = value; > > + *out++ = value; > > Just a small question as to whether you might want to subtract some > black level and apply some "fake" gamma? It might make the thumbnails > look a bit nicer. But just a matter of taste, I think, not something > I'd worry over! That would surely be nicer. I'd also like to have some colors in there. Do you see the thumbnails a lot? I actually had to dig where the thumbnails get displayed (in my case only the overview in RawTherapy) because in my daily use I never saw them. Cheers, Stefan > > David > > > + in += 16; > > + in2 += 16; > > + } > > +} > > + > > void packScanlineRaw10_CSI2P(void *output, const void *input, unsigned int width) > > { > > const uint8_t *in = static_cast<const uint8_t *>(input); > > @@ -321,25 +345,25 @@ const std::map<PixelFormat, FormatInfo> formatInfo = { > > .bitsPerSample = 16, > > .pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed }, > > .packScanline = packScanlineRaw16, > > - .thumbScanline = thumbScanlineRawXX_CSI2P, > > + .thumbScanline = thumbScanlineRawXX, > > } }, > > { formats::SGBRG16, { > > .bitsPerSample = 16, > > .pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen }, > > .packScanline = packScanlineRaw16, > > - .thumbScanline = thumbScanlineRawXX_CSI2P, > > + .thumbScanline = thumbScanlineRawXX, > > } }, > > { formats::SGRBG16, { > > .bitsPerSample = 16, > > .pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen }, > > .packScanline = packScanlineRaw16, > > - .thumbScanline = thumbScanlineRawXX_CSI2P, > > + .thumbScanline = thumbScanlineRawXX, > > } }, > > { formats::SRGGB16, { > > .bitsPerSample = 16, > > .pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue }, > > .packScanline = packScanlineRaw16, > > - .thumbScanline = thumbScanlineRawXX_CSI2P, > > + .thumbScanline = thumbScanlineRawXX, > > } }, > > { formats::SBGGR10_CSI2P, { > > .bitsPerSample = 10, > > -- > > 2.43.0 > >
Quoting Stefan Klug (2024-06-27 14:50:33) > On Thu, Jun 27, 2024 at 02:10:18PM +0100, Kieran Bingham wrote: > > Quoting Stefan Klug (2024-06-27 13:51:10) > > > Add a thumbnail function for raw formats that are 16bit aligned. > > > This is needed for the upcoming RAW10 and RAW12 implemntation. > > > > > > Use the new function for RAW16 as the thumbScanlineRawXX_CSI2P produces > > > incorrect results for that format (it averages over adjacent bytes, > > > which works for the CSI formats). > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > --- > > > src/apps/common/dng_writer.cpp | 32 ++++++++++++++++++++++++++++---- > > > 1 file changed, 28 insertions(+), 4 deletions(-) > > > > > > diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp > > > index 88b225d3e099..2cb320ff327b 100644 > > > --- a/src/apps/common/dng_writer.cpp > > > +++ b/src/apps/common/dng_writer.cpp > > > @@ -144,6 +144,30 @@ void packScanlineRaw16(void *output, const void *input, unsigned int width) > > > std::copy(in, in + width, out); > > > } > > > > > > +/* Thumbnail function for raw data aligned to 16bit words */ > > > +void thumbScanlineRawXX(const FormatInfo &info, void *output, > > > > Yup, I could still be tempted to call this thumbScanlineRaw ... unless > > ... > > > > > + const void *input, unsigned int width, > > > + unsigned int stride) > > > +{ > > > + const uint16_t *in = static_cast<const uint16_t *>(input); > > > + const uint16_t *in2 = static_cast<const uint16_t *>(input) + stride / 2; > > > + uint8_t *out = static_cast<uint8_t *>(output); > > > + > > > + /* shift down to 8 */ > > > + unsigned int shift = info.bitsPerSample - 8; > > > + > > > + /* simple averaging that produces grey RGB values */ > > > > grey rgb values? > > I was looking for the rigt words here. We produce rgb thumbnails, but to > ease the averaging and to prevent the need for debayering only grey is > output. See commit 2223579dfee Aha you mean the output is in greyscale ? Oh - right that's what the averaging is doing. I see now. > > > > > > > + for (unsigned int x = 0; x < width; x++) { > > > + uint16_t value = (in[0] + in[1] + in2[0] + in2[1]) >> 2; > > > + value = value >> shift; > > > + *out++ = value; > > > + *out++ = value; > > > + *out++ = value; And storing the average value in each of RGB gives 'greyscale' only. I'd misunderstood this on the first pass. > > > + in += 16; > > > + in2 += 16; > > > > Does this code work for 8,10,12,14 bit data? Or is the implication that > > it only handled data algined to 16bit (per pixel?) so perhaps that's 10, > > 12, 14, 16? > > It only handles formats that are aligned 16bit per pixel (so Raw8 is > excluded) Proposals for a better name are very welcome :-) I guess that would be the special case as thumbScanlineRaw8 This is fine for me. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Cheers, > Stefan > > > > > > > > > > + } > > > +} > > > + > > > void packScanlineRaw10_CSI2P(void *output, const void *input, unsigned int width) > > > { > > > const uint8_t *in = static_cast<const uint8_t *>(input); > > > @@ -321,25 +345,25 @@ const std::map<PixelFormat, FormatInfo> formatInfo = { > > > .bitsPerSample = 16, > > > .pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed }, > > > .packScanline = packScanlineRaw16, > > > - .thumbScanline = thumbScanlineRawXX_CSI2P, > > > + .thumbScanline = thumbScanlineRawXX, > > > } }, > > > { formats::SGBRG16, { > > > .bitsPerSample = 16, > > > .pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen }, > > > .packScanline = packScanlineRaw16, > > > - .thumbScanline = thumbScanlineRawXX_CSI2P, > > > + .thumbScanline = thumbScanlineRawXX, > > > } }, > > > { formats::SGRBG16, { > > > .bitsPerSample = 16, > > > .pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen }, > > > .packScanline = packScanlineRaw16, > > > - .thumbScanline = thumbScanlineRawXX_CSI2P, > > > + .thumbScanline = thumbScanlineRawXX, > > > } }, > > > { formats::SRGGB16, { > > > .bitsPerSample = 16, > > > .pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue }, > > > .packScanline = packScanlineRaw16, > > > - .thumbScanline = thumbScanlineRawXX_CSI2P, > > > + .thumbScanline = thumbScanlineRawXX, > > > } }, > > > { formats::SBGGR10_CSI2P, { > > > .bitsPerSample = 10, > > > -- > > > 2.43.0 > > >
Hi Stefan On Thu, 27 Jun 2024 at 15:02, Stefan Klug <stefan.klug@ideasonboard.com> wrote: > > Hi David, > > On Thu, Jun 27, 2024 at 02:27:57PM +0100, David Plowman wrote: > > Hi Stefan > > > > Looks good to me! > > Thanks > > > > > On Thu, 27 Jun 2024 at 13:53, Stefan Klug <stefan.klug@ideasonboard.com> wrote: > > > > > > Add a thumbnail function for raw formats that are 16bit aligned. > > > This is needed for the upcoming RAW10 and RAW12 implemntation. > > > > > > Use the new function for RAW16 as the thumbScanlineRawXX_CSI2P produces > > > incorrect results for that format (it averages over adjacent bytes, > > > which works for the CSI formats). > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > --- > > > src/apps/common/dng_writer.cpp | 32 ++++++++++++++++++++++++++++---- > > > 1 file changed, 28 insertions(+), 4 deletions(-) > > > > > > diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp > > > index 88b225d3e099..2cb320ff327b 100644 > > > --- a/src/apps/common/dng_writer.cpp > > > +++ b/src/apps/common/dng_writer.cpp > > > @@ -144,6 +144,30 @@ void packScanlineRaw16(void *output, const void *input, unsigned int width) > > > std::copy(in, in + width, out); > > > } > > > > > > +/* Thumbnail function for raw data aligned to 16bit words */ > > > +void thumbScanlineRawXX(const FormatInfo &info, void *output, > > > + const void *input, unsigned int width, > > > + unsigned int stride) > > > +{ > > > + const uint16_t *in = static_cast<const uint16_t *>(input); > > > + const uint16_t *in2 = static_cast<const uint16_t *>(input) + stride / 2; > > > + uint8_t *out = static_cast<uint8_t *>(output); > > > + > > > + /* shift down to 8 */ > > > + unsigned int shift = info.bitsPerSample - 8; > > > + > > > + /* simple averaging that produces grey RGB values */ > > > + for (unsigned int x = 0; x < width; x++) { > > > + uint16_t value = (in[0] + in[1] + in2[0] + in2[1]) >> 2; > > > + value = value >> shift; > > > + *out++ = value; > > > + *out++ = value; > > > + *out++ = value; > > > > Just a small question as to whether you might want to subtract some > > black level and apply some "fake" gamma? It might make the thumbnails > > look a bit nicer. But just a matter of taste, I think, not something > > I'd worry over! > > That would surely be nicer. I'd also like to have some colors in there. > Do you see the thumbnails a lot? I actually had to dig where the > thumbnails get displayed (in my case only the overview in RawTherapy) > because in my daily use I never saw them. Colours are more of a hassle because you have white balance and a colour matrix to apply. You have all the information so it's doable. But I think it's mostly a question of whether you can be bothered... There are indeed ever fewer places where a thumbnail seems necessary. Some older versions of operating systems maybe, but these days not so much. So not changing anything at all seems fair enough to me! David > > Cheers, > Stefan > > > > > David > > > > > + in += 16; > > > + in2 += 16; > > > + } > > > +} > > > + > > > void packScanlineRaw10_CSI2P(void *output, const void *input, unsigned int width) > > > { > > > const uint8_t *in = static_cast<const uint8_t *>(input); > > > @@ -321,25 +345,25 @@ const std::map<PixelFormat, FormatInfo> formatInfo = { > > > .bitsPerSample = 16, > > > .pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed }, > > > .packScanline = packScanlineRaw16, > > > - .thumbScanline = thumbScanlineRawXX_CSI2P, > > > + .thumbScanline = thumbScanlineRawXX, > > > } }, > > > { formats::SGBRG16, { > > > .bitsPerSample = 16, > > > .pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen }, > > > .packScanline = packScanlineRaw16, > > > - .thumbScanline = thumbScanlineRawXX_CSI2P, > > > + .thumbScanline = thumbScanlineRawXX, > > > } }, > > > { formats::SGRBG16, { > > > .bitsPerSample = 16, > > > .pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen }, > > > .packScanline = packScanlineRaw16, > > > - .thumbScanline = thumbScanlineRawXX_CSI2P, > > > + .thumbScanline = thumbScanlineRawXX, > > > } }, > > > { formats::SRGGB16, { > > > .bitsPerSample = 16, > > > .pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue }, > > > .packScanline = packScanlineRaw16, > > > - .thumbScanline = thumbScanlineRawXX_CSI2P, > > > + .thumbScanline = thumbScanlineRawXX, > > > } }, > > > { formats::SBGGR10_CSI2P, { > > > .bitsPerSample = 10, > > > -- > > > 2.43.0 > > >
diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp index 88b225d3e099..2cb320ff327b 100644 --- a/src/apps/common/dng_writer.cpp +++ b/src/apps/common/dng_writer.cpp @@ -144,6 +144,30 @@ void packScanlineRaw16(void *output, const void *input, unsigned int width) std::copy(in, in + width, out); } +/* Thumbnail function for raw data aligned to 16bit words */ +void thumbScanlineRawXX(const FormatInfo &info, void *output, + const void *input, unsigned int width, + unsigned int stride) +{ + const uint16_t *in = static_cast<const uint16_t *>(input); + const uint16_t *in2 = static_cast<const uint16_t *>(input) + stride / 2; + uint8_t *out = static_cast<uint8_t *>(output); + + /* shift down to 8 */ + unsigned int shift = info.bitsPerSample - 8; + + /* simple averaging that produces grey RGB values */ + for (unsigned int x = 0; x < width; x++) { + uint16_t value = (in[0] + in[1] + in2[0] + in2[1]) >> 2; + value = value >> shift; + *out++ = value; + *out++ = value; + *out++ = value; + in += 16; + in2 += 16; + } +} + void packScanlineRaw10_CSI2P(void *output, const void *input, unsigned int width) { const uint8_t *in = static_cast<const uint8_t *>(input); @@ -321,25 +345,25 @@ const std::map<PixelFormat, FormatInfo> formatInfo = { .bitsPerSample = 16, .pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed }, .packScanline = packScanlineRaw16, - .thumbScanline = thumbScanlineRawXX_CSI2P, + .thumbScanline = thumbScanlineRawXX, } }, { formats::SGBRG16, { .bitsPerSample = 16, .pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen }, .packScanline = packScanlineRaw16, - .thumbScanline = thumbScanlineRawXX_CSI2P, + .thumbScanline = thumbScanlineRawXX, } }, { formats::SGRBG16, { .bitsPerSample = 16, .pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen }, .packScanline = packScanlineRaw16, - .thumbScanline = thumbScanlineRawXX_CSI2P, + .thumbScanline = thumbScanlineRawXX, } }, { formats::SRGGB16, { .bitsPerSample = 16, .pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue }, .packScanline = packScanlineRaw16, - .thumbScanline = thumbScanlineRawXX_CSI2P, + .thumbScanline = thumbScanlineRawXX, } }, { formats::SBGGR10_CSI2P, { .bitsPerSample = 10,
Add a thumbnail function for raw formats that are 16bit aligned. This is needed for the upcoming RAW10 and RAW12 implemntation. Use the new function for RAW16 as the thumbScanlineRawXX_CSI2P produces incorrect results for that format (it averages over adjacent bytes, which works for the CSI formats). Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- src/apps/common/dng_writer.cpp | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-)