[PATCH 2/3] apps: common: dng_writer: Add thumbnail scanline function for RawXX

[Kieran Bingham]

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 at 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 at 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
> > >