apps: common: dng_writer: Support RAW16 formats
diff mbox series

Message ID 20240613150950.1035299-1-dan.scally@ideasonboard.com
State Accepted
Commit 5155150bbfd8a76fcc4dab5b7a9a88b614767bc6
Headers show
Series
  • apps: common: dng_writer: Support RAW16 formats
Related show

Commit Message

Dan Scally June 13, 2024, 3:09 p.m. UTC
Add support for RAW16 formats to the DNGWriter helpers so that we can
produce dng files from the mali-c55.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 src/apps/common/dng_writer.cpp | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

Comments

Laurent Pinchart June 13, 2024, 4:08 p.m. UTC | #1
Hi Dan,

Thank you for the patch.

On Thu, Jun 13, 2024 at 04:09:49PM +0100, Daniel Scally wrote:
> Add support for RAW16 formats to the DNGWriter helpers so that we can
> produce dng files from the mali-c55.
> 
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/apps/common/dng_writer.cpp | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp
> index 59f1fa23..150782e8 100644
> --- a/src/apps/common/dng_writer.cpp
> +++ b/src/apps/common/dng_writer.cpp
> @@ -134,6 +134,14 @@ void packScanlineSBGGR8(void *output, const void *input, unsigned int width)
>  	std::copy(in, in + width, out);
>  }
>  
> +void packScanlineSBGGR16(void *output, const void *input, unsigned int width)
> +{
> +	const uint16_t *in = static_cast<const uint16_t *>(input);
> +	uint16_t *out = static_cast<uint16_t *>(output);
> +
> +	std::copy(in, in + width, out);
> +}
> +
>  void packScanlineSBGGR10P(void *output, const void *input, unsigned int width)
>  {
>  	const uint8_t *in = static_cast<const uint8_t *>(input);
> @@ -307,6 +315,30 @@ static const std::map<PixelFormat, FormatInfo> formatInfo = {
>  		.packScanline = packScanlineSBGGR8,
>  		.thumbScanline = thumbScanlineSBGGRxxP,
>  	} },
> +	{ formats::SBGGR16, {
> +		.bitsPerSample = 16,
> +		.pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },
> +		.packScanline = packScanlineSBGGR16,
> +		.thumbScanline = thumbScanlineSBGGRxxP,
> +	} },
> +	{ formats::SGBRG16, {
> +		.bitsPerSample = 16,
> +		.pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen },
> +		.packScanline = packScanlineSBGGR16,
> +		.thumbScanline = thumbScanlineSBGGRxxP,
> +	} },
> +	{ formats::SGRBG16, {
> +		.bitsPerSample = 16,
> +		.pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen },
> +		.packScanline = packScanlineSBGGR16,
> +		.thumbScanline = thumbScanlineSBGGRxxP,
> +	} },
> +	{ formats::SRGGB16, {
> +		.bitsPerSample = 16,
> +		.pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue },
> +		.packScanline = packScanlineSBGGR16,
> +		.thumbScanline = thumbScanlineSBGGRxxP,
> +	} },
>  	{ formats::SBGGR10_CSI2P, {
>  		.bitsPerSample = 10,
>  		.pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },
Stefan Klug June 13, 2024, 4:14 p.m. UTC | #2
Hi Dan,

thank you for the patch.

On Thu, Jun 13, 2024 at 04:09:49PM +0100, Daniel Scally wrote:
> Add support for RAW16 formats to the DNGWriter helpers so that we can
> produce dng files from the mali-c55.
> 
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
>  src/apps/common/dng_writer.cpp | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp
> index 59f1fa23..150782e8 100644
> --- a/src/apps/common/dng_writer.cpp
> +++ b/src/apps/common/dng_writer.cpp
> @@ -134,6 +134,14 @@ void packScanlineSBGGR8(void *output, const void *input, unsigned int width)
>  	std::copy(in, in + width, out);
>  }
>  
> +void packScanlineSBGGR16(void *output, const void *input, unsigned int width)
> +{
> +	const uint16_t *in = static_cast<const uint16_t *>(input);
> +	uint16_t *out = static_cast<uint16_t *>(output);
> +
> +	std::copy(in, in + width, out);
> +}
> +
>  void packScanlineSBGGR10P(void *output, const void *input, unsigned int width)
>  {
>  	const uint8_t *in = static_cast<const uint8_t *>(input);
> @@ -307,6 +315,30 @@ static const std::map<PixelFormat, FormatInfo> formatInfo = {
>  		.packScanline = packScanlineSBGGR8,
>  		.thumbScanline = thumbScanlineSBGGRxxP,
>  	} },
> +	{ formats::SBGGR16, {
> +		.bitsPerSample = 16,
> +		.pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },
> +		.packScanline = packScanlineSBGGR16,
> +		.thumbScanline = thumbScanlineSBGGRxxP,

I had to look twice to understand why the BGGR16 function is used for
every pattern. For me a "packScanlineRaw16" would be easier to
understand.

Whatever you prefer.

Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> 

> +	} },
> +	{ formats::SGBRG16, {
> +		.bitsPerSample = 16,
> +		.pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen },
> +		.packScanline = packScanlineSBGGR16,
> +		.thumbScanline = thumbScanlineSBGGRxxP,
> +	} },
> +	{ formats::SGRBG16, {
> +		.bitsPerSample = 16,
> +		.pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen },
> +		.packScanline = packScanlineSBGGR16,
> +		.thumbScanline = thumbScanlineSBGGRxxP,
> +	} },
> +	{ formats::SRGGB16, {
> +		.bitsPerSample = 16,
> +		.pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue },
> +		.packScanline = packScanlineSBGGR16,
> +		.thumbScanline = thumbScanlineSBGGRxxP,
> +	} },
>  	{ formats::SBGGR10_CSI2P, {
>  		.bitsPerSample = 10,
>  		.pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },
> -- 
> 2.30.2
>
Laurent Pinchart June 13, 2024, 4:30 p.m. UTC | #3
On Thu, Jun 13, 2024 at 06:14:11PM +0200, Stefan Klug wrote:
> On Thu, Jun 13, 2024 at 04:09:49PM +0100, Daniel Scally wrote:
> > Add support for RAW16 formats to the DNGWriter helpers so that we can
> > produce dng files from the mali-c55.
> > 
> > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> > ---
> >  src/apps/common/dng_writer.cpp | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> > 
> > diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp
> > index 59f1fa23..150782e8 100644
> > --- a/src/apps/common/dng_writer.cpp
> > +++ b/src/apps/common/dng_writer.cpp
> > @@ -134,6 +134,14 @@ void packScanlineSBGGR8(void *output, const void *input, unsigned int width)
> >  	std::copy(in, in + width, out);
> >  }
> >  
> > +void packScanlineSBGGR16(void *output, const void *input, unsigned int width)
> > +{
> > +	const uint16_t *in = static_cast<const uint16_t *>(input);
> > +	uint16_t *out = static_cast<uint16_t *>(output);
> > +
> > +	std::copy(in, in + width, out);
> > +}
> > +
> >  void packScanlineSBGGR10P(void *output, const void *input, unsigned int width)
> >  {
> >  	const uint8_t *in = static_cast<const uint8_t *>(input);
> > @@ -307,6 +315,30 @@ static const std::map<PixelFormat, FormatInfo> formatInfo = {
> >  		.packScanline = packScanlineSBGGR8,
> >  		.thumbScanline = thumbScanlineSBGGRxxP,
> >  	} },
> > +	{ formats::SBGGR16, {
> > +		.bitsPerSample = 16,
> > +		.pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },
> > +		.packScanline = packScanlineSBGGR16,
> > +		.thumbScanline = thumbScanlineSBGGRxxP,
> 
> I had to look twice to understand why the BGGR16 function is used for
> every pattern. For me a "packScanlineRaw16" would be easier to
> understand.

Good idea.

> Whatever you prefer.
> 
> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> 
> 
> > +	} },
> > +	{ formats::SGBRG16, {
> > +		.bitsPerSample = 16,
> > +		.pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen },
> > +		.packScanline = packScanlineSBGGR16,
> > +		.thumbScanline = thumbScanlineSBGGRxxP,
> > +	} },
> > +	{ formats::SGRBG16, {
> > +		.bitsPerSample = 16,
> > +		.pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen },
> > +		.packScanline = packScanlineSBGGR16,
> > +		.thumbScanline = thumbScanlineSBGGRxxP,
> > +	} },
> > +	{ formats::SRGGB16, {
> > +		.bitsPerSample = 16,
> > +		.pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue },
> > +		.packScanline = packScanlineSBGGR16,
> > +		.thumbScanline = thumbScanlineSBGGRxxP,
> > +	} },
> >  	{ formats::SBGGR10_CSI2P, {
> >  		.bitsPerSample = 10,
> >  		.pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },
Dan Scally June 13, 2024, 9:44 p.m. UTC | #4
On 13/06/2024 17:30, Laurent Pinchart wrote:
> On Thu, Jun 13, 2024 at 06:14:11PM +0200, Stefan Klug wrote:
>> On Thu, Jun 13, 2024 at 04:09:49PM +0100, Daniel Scally wrote:
>>> Add support for RAW16 formats to the DNGWriter helpers so that we can
>>> produce dng files from the mali-c55.
>>>
>>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
>>> ---
>>>   src/apps/common/dng_writer.cpp | 32 ++++++++++++++++++++++++++++++++
>>>   1 file changed, 32 insertions(+)
>>>
>>> diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp
>>> index 59f1fa23..150782e8 100644
>>> --- a/src/apps/common/dng_writer.cpp
>>> +++ b/src/apps/common/dng_writer.cpp
>>> @@ -134,6 +134,14 @@ void packScanlineSBGGR8(void *output, const void *input, unsigned int width)
>>>   	std::copy(in, in + width, out);
>>>   }
>>>   
>>> +void packScanlineSBGGR16(void *output, const void *input, unsigned int width)
>>> +{
>>> +	const uint16_t *in = static_cast<const uint16_t *>(input);
>>> +	uint16_t *out = static_cast<uint16_t *>(output);
>>> +
>>> +	std::copy(in, in + width, out);
>>> +}
>>> +
>>>   void packScanlineSBGGR10P(void *output, const void *input, unsigned int width)
>>>   {
>>>   	const uint8_t *in = static_cast<const uint8_t *>(input);
>>> @@ -307,6 +315,30 @@ static const std::map<PixelFormat, FormatInfo> formatInfo = {
>>>   		.packScanline = packScanlineSBGGR8,
>>>   		.thumbScanline = thumbScanlineSBGGRxxP,
>>>   	} },
>>> +	{ formats::SBGGR16, {
>>> +		.bitsPerSample = 16,
>>> +		.pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },
>>> +		.packScanline = packScanlineSBGGR16,
>>> +		.thumbScanline = thumbScanlineSBGGRxxP,
>> I had to look twice to understand why the BGGR16 function is used for
>> every pattern. For me a "packScanlineRaw16" would be easier to
>> understand.
> Good idea.

Ack! I was kinda following the crowd here, we also have packScanlineSBGGR8, packScanlineSBGGR10P and 
packScanlineSBGGR12P that are used regardless of the format's Bayer Order. Shall I rename those too?
>
>> Whatever you prefer.
>>
>> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
>>
>>> +	} },
>>> +	{ formats::SGBRG16, {
>>> +		.bitsPerSample = 16,
>>> +		.pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen },
>>> +		.packScanline = packScanlineSBGGR16,
>>> +		.thumbScanline = thumbScanlineSBGGRxxP,
>>> +	} },
>>> +	{ formats::SGRBG16, {
>>> +		.bitsPerSample = 16,
>>> +		.pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen },
>>> +		.packScanline = packScanlineSBGGR16,
>>> +		.thumbScanline = thumbScanlineSBGGRxxP,
>>> +	} },
>>> +	{ formats::SRGGB16, {
>>> +		.bitsPerSample = 16,
>>> +		.pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue },
>>> +		.packScanline = packScanlineSBGGR16,
>>> +		.thumbScanline = thumbScanlineSBGGRxxP,
>>> +	} },
>>>   	{ formats::SBGGR10_CSI2P, {
>>>   		.bitsPerSample = 10,
>>>   		.pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },
Laurent Pinchart June 13, 2024, 9:53 p.m. UTC | #5
On Thu, Jun 13, 2024 at 10:44:08PM +0100, Daniel Scally wrote:
> 
> On 13/06/2024 17:30, Laurent Pinchart wrote:
> > On Thu, Jun 13, 2024 at 06:14:11PM +0200, Stefan Klug wrote:
> >> On Thu, Jun 13, 2024 at 04:09:49PM +0100, Daniel Scally wrote:
> >>> Add support for RAW16 formats to the DNGWriter helpers so that we can
> >>> produce dng files from the mali-c55.
> >>>
> >>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> >>> ---
> >>>   src/apps/common/dng_writer.cpp | 32 ++++++++++++++++++++++++++++++++
> >>>   1 file changed, 32 insertions(+)
> >>>
> >>> diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp
> >>> index 59f1fa23..150782e8 100644
> >>> --- a/src/apps/common/dng_writer.cpp
> >>> +++ b/src/apps/common/dng_writer.cpp
> >>> @@ -134,6 +134,14 @@ void packScanlineSBGGR8(void *output, const void *input, unsigned int width)
> >>>   	std::copy(in, in + width, out);
> >>>   }
> >>>   
> >>> +void packScanlineSBGGR16(void *output, const void *input, unsigned int width)
> >>> +{
> >>> +	const uint16_t *in = static_cast<const uint16_t *>(input);
> >>> +	uint16_t *out = static_cast<uint16_t *>(output);
> >>> +
> >>> +	std::copy(in, in + width, out);
> >>> +}
> >>> +
> >>>   void packScanlineSBGGR10P(void *output, const void *input, unsigned int width)
> >>>   {
> >>>   	const uint8_t *in = static_cast<const uint8_t *>(input);
> >>> @@ -307,6 +315,30 @@ static const std::map<PixelFormat, FormatInfo> formatInfo = {
> >>>   		.packScanline = packScanlineSBGGR8,
> >>>   		.thumbScanline = thumbScanlineSBGGRxxP,
> >>>   	} },
> >>> +	{ formats::SBGGR16, {
> >>> +		.bitsPerSample = 16,
> >>> +		.pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },
> >>> +		.packScanline = packScanlineSBGGR16,
> >>> +		.thumbScanline = thumbScanlineSBGGRxxP,
> >> I had to look twice to understand why the BGGR16 function is used for
> >> every pattern. For me a "packScanlineRaw16" would be easier to
> >> understand.
> > Good idea.
> 
> Ack! I was kinda following the crowd here, we also have packScanlineSBGGR8, packScanlineSBGGR10P and 
> packScanlineSBGGR12P that are used regardless of the format's Bayer Order. Shall I rename those too?

Please :-)

> >> Whatever you prefer.
> >>
> >> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
> >>
> >>> +	} },
> >>> +	{ formats::SGBRG16, {
> >>> +		.bitsPerSample = 16,
> >>> +		.pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen },
> >>> +		.packScanline = packScanlineSBGGR16,
> >>> +		.thumbScanline = thumbScanlineSBGGRxxP,
> >>> +	} },
> >>> +	{ formats::SGRBG16, {
> >>> +		.bitsPerSample = 16,
> >>> +		.pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen },
> >>> +		.packScanline = packScanlineSBGGR16,
> >>> +		.thumbScanline = thumbScanlineSBGGRxxP,
> >>> +	} },
> >>> +	{ formats::SRGGB16, {
> >>> +		.bitsPerSample = 16,
> >>> +		.pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue },
> >>> +		.packScanline = packScanlineSBGGR16,
> >>> +		.thumbScanline = thumbScanlineSBGGRxxP,
> >>> +	} },
> >>>   	{ formats::SBGGR10_CSI2P, {
> >>>   		.bitsPerSample = 10,
> >>>   		.pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },

Patch
diff mbox series

diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp
index 59f1fa23..150782e8 100644
--- a/src/apps/common/dng_writer.cpp
+++ b/src/apps/common/dng_writer.cpp
@@ -134,6 +134,14 @@  void packScanlineSBGGR8(void *output, const void *input, unsigned int width)
 	std::copy(in, in + width, out);
 }
 
+void packScanlineSBGGR16(void *output, const void *input, unsigned int width)
+{
+	const uint16_t *in = static_cast<const uint16_t *>(input);
+	uint16_t *out = static_cast<uint16_t *>(output);
+
+	std::copy(in, in + width, out);
+}
+
 void packScanlineSBGGR10P(void *output, const void *input, unsigned int width)
 {
 	const uint8_t *in = static_cast<const uint8_t *>(input);
@@ -307,6 +315,30 @@  static const std::map<PixelFormat, FormatInfo> formatInfo = {
 		.packScanline = packScanlineSBGGR8,
 		.thumbScanline = thumbScanlineSBGGRxxP,
 	} },
+	{ formats::SBGGR16, {
+		.bitsPerSample = 16,
+		.pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },
+		.packScanline = packScanlineSBGGR16,
+		.thumbScanline = thumbScanlineSBGGRxxP,
+	} },
+	{ formats::SGBRG16, {
+		.bitsPerSample = 16,
+		.pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen },
+		.packScanline = packScanlineSBGGR16,
+		.thumbScanline = thumbScanlineSBGGRxxP,
+	} },
+	{ formats::SGRBG16, {
+		.bitsPerSample = 16,
+		.pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen },
+		.packScanline = packScanlineSBGGR16,
+		.thumbScanline = thumbScanlineSBGGRxxP,
+	} },
+	{ formats::SRGGB16, {
+		.bitsPerSample = 16,
+		.pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue },
+		.packScanline = packScanlineSBGGR16,
+		.thumbScanline = thumbScanlineSBGGRxxP,
+	} },
 	{ formats::SBGGR10_CSI2P, {
 		.bitsPerSample = 10,
 		.pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },