| Message ID | 20250821134141.83236-4-mzamazal@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi 2025. 08. 21. 15:41 keltezéssel, Milan Zamazal írta: > The window of the image that should be debayered must be aligned to the > bayer pattern size. This is already ensured for the window corner > coordinates but not for the window sizes. I think it's worth mentioning that this is solely a limitation of this implementation. And I think even then the width need not be constrained? Regards, Barnabás Pőcze > > We can either make the window sizes aligned similarly to how the window > corner coordinates are aligned or reject an unaligned configuration. > This patches chooses the latter as using a different window size than > the requested output size could lead to artefacts. Since such a > situation is not expected to occur in correctly set up environments, the > change should have no negative impact. > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/software_isp/debayer_cpu.cpp | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp > index 185edd814..3200b0c53 100644 > --- a/src/libcamera/software_isp/debayer_cpu.cpp > +++ b/src/libcamera/software_isp/debayer_cpu.cpp > @@ -539,7 +539,17 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg, > window_.y = ((inputCfg.size.height - outputCfg.size.height) / 2) & > ~(inputConfig_.patternSize.height - 1); > window_.width = outputCfg.size.width; > + if (window_.width % inputConfig_.patternSize.width != 0) { > + LOG(Debayer, Error) > + << "Output width is not a multiple of the bayer pattern width"; > + return -EINVAL; > + } > window_.height = outputCfg.size.height; > + if (window_.height % inputConfig_.patternSize.height != 0) { > + LOG(Debayer, Error) > + << "Output height is not a multiple of the bayer pattern height"; > + return -EINVAL; > + } > > /* > * Set the stats window to the whole processed window. Its coordinates are
Hi Barnabás, Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes: > Hi > > 2025. 08. 21. 15:41 keltezéssel, Milan Zamazal írta: >> The window of the image that should be debayered must be aligned to the >> bayer pattern size. This is already ensured for the window corner >> coordinates but not for the window sizes. > > I think it's worth mentioning that this is solely a limitation of this > implementation. And I think even then the width need not be constrained? I think that: - The bayer pattern size alignment is technically not necessary but it's a reasonable processing simplification and hardly a real problem. - Shrinking the original width is for the purpose of making the implementation easier and a bit faster, and somewhat annoying. > Regards, > Barnabás Pőcze > > >> We can either make the window sizes aligned similarly to how the window >> corner coordinates are aligned or reject an unaligned configuration. >> This patches chooses the latter as using a different window size than >> the requested output size could lead to artefacts. Since such a >> situation is not expected to occur in correctly set up environments, the >> change should have no negative impact. >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com> >> --- >> src/libcamera/software_isp/debayer_cpu.cpp | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp >> index 185edd814..3200b0c53 100644 >> --- a/src/libcamera/software_isp/debayer_cpu.cpp >> +++ b/src/libcamera/software_isp/debayer_cpu.cpp >> @@ -539,7 +539,17 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg, >> window_.y = ((inputCfg.size.height - outputCfg.size.height) / 2) & >> ~(inputConfig_.patternSize.height - 1); >> window_.width = outputCfg.size.width; >> + if (window_.width % inputConfig_.patternSize.width != 0) { >> + LOG(Debayer, Error) >> + << "Output width is not a multiple of the bayer pattern width"; >> + return -EINVAL; >> + } >> window_.height = outputCfg.size.height; >> + if (window_.height % inputConfig_.patternSize.height != 0) { >> + LOG(Debayer, Error) >> + << "Output height is not a multiple of the bayer pattern height"; >> + return -EINVAL; >> + } >> /* >> * Set the stats window to the whole processed window. Its coordinates are
Hi 2025. 09. 11. 14:36 keltezéssel, Milan Zamazal írta: > Hi Barnabás, > > Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes: > >> Hi >> >> 2025. 08. 21. 15:41 keltezéssel, Milan Zamazal írta: >>> The window of the image that should be debayered must be aligned to the >>> bayer pattern size. This is already ensured for the window corner >>> coordinates but not for the window sizes. >> >> I think it's worth mentioning that this is solely a limitation of this >> implementation. And I think even then the width need not be constrained? > > I think that: > > - The bayer pattern size alignment is technically not necessary but it's > a reasonable processing simplification and hardly a real problem. > > - Shrinking the original width is for the purpose of making the > implementation easier and a bit faster, and somewhat annoying. Please ignore my earlier message. On a second look, the width needs to be constrained as well due to the implementation. So nevermind, sorry. Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> Regards, Barnabás Pőcze > >> Regards, >> Barnabás Pőcze >> >> >>> We can either make the window sizes aligned similarly to how the window >>> corner coordinates are aligned or reject an unaligned configuration. >>> This patches chooses the latter as using a different window size than >>> the requested output size could lead to artefacts. Since such a >>> situation is not expected to occur in correctly set up environments, the >>> change should have no negative impact. >>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com> >>> --- >>> src/libcamera/software_isp/debayer_cpu.cpp | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp >>> index 185edd814..3200b0c53 100644 >>> --- a/src/libcamera/software_isp/debayer_cpu.cpp >>> +++ b/src/libcamera/software_isp/debayer_cpu.cpp >>> @@ -539,7 +539,17 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg, >>> window_.y = ((inputCfg.size.height - outputCfg.size.height) / 2) & >>> ~(inputConfig_.patternSize.height - 1); >>> window_.width = outputCfg.size.width; >>> + if (window_.width % inputConfig_.patternSize.width != 0) { >>> + LOG(Debayer, Error) >>> + << "Output width is not a multiple of the bayer pattern width"; >>> + return -EINVAL; >>> + } >>> window_.height = outputCfg.size.height; >>> + if (window_.height % inputConfig_.patternSize.height != 0) { >>> + LOG(Debayer, Error) >>> + << "Output height is not a multiple of the bayer pattern height"; >>> + return -EINVAL; >>> + } >>> /* >>> * Set the stats window to the whole processed window. Its coordinates are >
diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp index 185edd814..3200b0c53 100644 --- a/src/libcamera/software_isp/debayer_cpu.cpp +++ b/src/libcamera/software_isp/debayer_cpu.cpp @@ -539,7 +539,17 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg, window_.y = ((inputCfg.size.height - outputCfg.size.height) / 2) & ~(inputConfig_.patternSize.height - 1); window_.width = outputCfg.size.width; + if (window_.width % inputConfig_.patternSize.width != 0) { + LOG(Debayer, Error) + << "Output width is not a multiple of the bayer pattern width"; + return -EINVAL; + } window_.height = outputCfg.size.height; + if (window_.height % inputConfig_.patternSize.height != 0) { + LOG(Debayer, Error) + << "Output height is not a multiple of the bayer pattern height"; + return -EINVAL; + } /* * Set the stats window to the whole processed window. Its coordinates are
The window of the image that should be debayered must be aligned to the bayer pattern size. This is already ensured for the window corner coordinates but not for the window sizes. We can either make the window sizes aligned similarly to how the window corner coordinates are aligned or reject an unaligned configuration. This patches chooses the latter as using a different window size than the requested output size could lead to artefacts. Since such a situation is not expected to occur in correctly set up environments, the change should have no negative impact. Signed-off-by: Milan Zamazal <mzamazal@redhat.com> --- src/libcamera/software_isp/debayer_cpu.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+)