| Message ID | 20260615181127.97555-1-robert.mader@collabora.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Robert, thank you for the fix. Robert Mader <robert.mader@collabora.com> writes: > The debayer algorithm produces a sligtly smaller output resolution > compared to the input one, using a border at the edges depending on the > pattern. This wasn't considered when scaling to the final output buffer > size, resulting in garbage pixels at the right - or in case or 90 degree > rotated sensors - the bottom of the image. > > Use the native (i.e. unscaled) output size instead - the maximum output > buffer size. > > While on it use outputSize_ for better readability and adopt the scaling > comment slightly. > > Fixes: f520b29fe (libcamera: software_isp: debayer_egl: Add an eGL Debayer class) > Signed-off-by: Robert Mader <robert.mader@collabora.com> Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > --- > > Changes in V2: > - Instead of recomputing the native output size, save it in configure() > - Use outputSize_ instead of window_ for better readability > - Small changes to comment and commit message > --- > src/libcamera/software_isp/debayer_egl.cpp | 10 ++++++---- > src/libcamera/software_isp/debayer_egl.h | 1 + > 2 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp > index fd8de3942..521316657 100644 > --- a/src/libcamera/software_isp/debayer_egl.cpp > +++ b/src/libcamera/software_isp/debayer_egl.cpp > @@ -337,6 +337,7 @@ int DebayerEGL::configure(const StreamConfiguration &inputCfg, > > outputPixelFormat_ = outputCfg.pixelFormat; > outputSize_ = outputCfg.size; > + nativeOutputSize_ = outSizeRange.max; > > window_.x = ((inputCfg.size.width - outputCfg.size.width) / 2) & > ~(inputConfig_.patternSize.width - 1); > @@ -408,11 +409,12 @@ void DebayerEGL::setShaderVariableValues(const DebayerParams ¶ms) > 1.0f / (height_ - 1) }; > GLfloat Stride = (GLfloat)width_ / (shaderStridePixels_ / bytesPerPixel_); > /* > - * Scale input to output size, keeping the aspect ratio and preferring > - * cropping over black bars. > + * Scale the output size from the native size the algorithm produces for > + * the input size. Keep the aspect ratio and prefer cropping over black > + * bars. > */ > - GLfloat scale = std::max((GLfloat)window_.width / width_, > - (GLfloat)window_.height / height_); > + GLfloat scale = std::max((GLfloat)outputSize_.width / nativeOutputSize_.width, > + (GLfloat)outputSize_.height / nativeOutputSize_.height); > GLfloat trans = -(1.0f - scale); > GLfloat projMatrix[] = { > scale, 0, 0, 0, > diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h > index 943410fdd..fbd5430e4 100644 > --- a/src/libcamera/software_isp/debayer_egl.h > +++ b/src/libcamera/software_isp/debayer_egl.h > @@ -102,6 +102,7 @@ private: > /* Contrast */ > GLint contrastExpUniformDataIn_; > > + Size nativeOutputSize_; > Rectangle window_; > std::unique_ptr<SwStatsCpu> stats_; > eGL egl_;
Quoting Milan Zamazal (2026-06-16 10:41:53) > Hi Robert, > > thank you for the fix. > > Robert Mader <robert.mader@collabora.com> writes: > > > The debayer algorithm produces a sligtly smaller output resolution > > compared to the input one, using a border at the edges depending on the > > pattern. This wasn't considered when scaling to the final output buffer > > size, resulting in garbage pixels at the right - or in case or 90 degree > > rotated sensors - the bottom of the image. > > > > Use the native (i.e. unscaled) output size instead - the maximum output > > buffer size. > > > > While on it use outputSize_ for better readability and adopt the scaling > > comment slightly. > > > > Fixes: f520b29fe (libcamera: software_isp: debayer_egl: Add an eGL Debayer class) > > Signed-off-by: Robert Mader <robert.mader@collabora.com> > > Reviewed-by: Milan Zamazal <mzamazal@redhat.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > --- > > > > Changes in V2: > > - Instead of recomputing the native output size, save it in configure() > > - Use outputSize_ instead of window_ for better readability > > - Small changes to comment and commit message > > --- > > src/libcamera/software_isp/debayer_egl.cpp | 10 ++++++---- > > src/libcamera/software_isp/debayer_egl.h | 1 + > > 2 files changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp > > index fd8de3942..521316657 100644 > > --- a/src/libcamera/software_isp/debayer_egl.cpp > > +++ b/src/libcamera/software_isp/debayer_egl.cpp > > @@ -337,6 +337,7 @@ int DebayerEGL::configure(const StreamConfiguration &inputCfg, > > > > outputPixelFormat_ = outputCfg.pixelFormat; > > outputSize_ = outputCfg.size; > > + nativeOutputSize_ = outSizeRange.max; > > > > window_.x = ((inputCfg.size.width - outputCfg.size.width) / 2) & > > ~(inputConfig_.patternSize.width - 1); > > @@ -408,11 +409,12 @@ void DebayerEGL::setShaderVariableValues(const DebayerParams ¶ms) > > 1.0f / (height_ - 1) }; > > GLfloat Stride = (GLfloat)width_ / (shaderStridePixels_ / bytesPerPixel_); > > /* > > - * Scale input to output size, keeping the aspect ratio and preferring > > - * cropping over black bars. > > + * Scale the output size from the native size the algorithm produces for > > + * the input size. Keep the aspect ratio and prefer cropping over black > > + * bars. > > */ > > - GLfloat scale = std::max((GLfloat)window_.width / width_, > > - (GLfloat)window_.height / height_); > > + GLfloat scale = std::max((GLfloat)outputSize_.width / nativeOutputSize_.width, > > + (GLfloat)outputSize_.height / nativeOutputSize_.height); > > GLfloat trans = -(1.0f - scale); > > GLfloat projMatrix[] = { > > scale, 0, 0, 0, > > diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h > > index 943410fdd..fbd5430e4 100644 > > --- a/src/libcamera/software_isp/debayer_egl.h > > +++ b/src/libcamera/software_isp/debayer_egl.h > > @@ -102,6 +102,7 @@ private: > > /* Contrast */ > > GLint contrastExpUniformDataIn_; > > > > + Size nativeOutputSize_; > > Rectangle window_; > > std::unique_ptr<SwStatsCpu> stats_; > > eGL egl_; >
Quoting Robert Mader (2026-06-15 19:11:27) > The debayer algorithm produces a sligtly smaller output resolution > compared to the input one, using a border at the edges depending on the > pattern. This wasn't considered when scaling to the final output buffer > size, resulting in garbage pixels at the right - or in case or 90 degree > rotated sensors - the bottom of the image. > > Use the native (i.e. unscaled) output size instead - the maximum output > buffer size. > > While on it use outputSize_ for better readability and adopt the scaling > comment slightly. > > Fixes: f520b29fe (libcamera: software_isp: debayer_egl: Add an eGL Debayer class) checkstyle complains here: ----------------------------------------------------------------------------------------------- 915a82a0dfc084de82676ff140db17e741dbfd61 software_isp: debayer_egl: Consider border for scaling ----------------------------------------------------------------------------------------------- Malformed value 'f520b29fe (libcamera: software_isp: debayer_egl: Add an eGL Debayer class)' for commit trailer 'Fixes' Should be: Fixes: f520b29fe9e6 ("libcamera: software_isp: debayer_egl: Add an eGL Debayer class") Please add the commit hooks with: cp utils/hooks/post-commit .git/hooks/post-commit from your libcamera sources! I'll fix this one up here while applying. -- Regards Kieran > Signed-off-by: Robert Mader <robert.mader@collabora.com> > > --- > > Changes in V2: > - Instead of recomputing the native output size, save it in configure() > - Use outputSize_ instead of window_ for better readability > - Small changes to comment and commit message > --- > src/libcamera/software_isp/debayer_egl.cpp | 10 ++++++---- > src/libcamera/software_isp/debayer_egl.h | 1 + > 2 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp > index fd8de3942..521316657 100644 > --- a/src/libcamera/software_isp/debayer_egl.cpp > +++ b/src/libcamera/software_isp/debayer_egl.cpp > @@ -337,6 +337,7 @@ int DebayerEGL::configure(const StreamConfiguration &inputCfg, > > outputPixelFormat_ = outputCfg.pixelFormat; > outputSize_ = outputCfg.size; > + nativeOutputSize_ = outSizeRange.max; > > window_.x = ((inputCfg.size.width - outputCfg.size.width) / 2) & > ~(inputConfig_.patternSize.width - 1); > @@ -408,11 +409,12 @@ void DebayerEGL::setShaderVariableValues(const DebayerParams ¶ms) > 1.0f / (height_ - 1) }; > GLfloat Stride = (GLfloat)width_ / (shaderStridePixels_ / bytesPerPixel_); > /* > - * Scale input to output size, keeping the aspect ratio and preferring > - * cropping over black bars. > + * Scale the output size from the native size the algorithm produces for > + * the input size. Keep the aspect ratio and prefer cropping over black > + * bars. > */ > - GLfloat scale = std::max((GLfloat)window_.width / width_, > - (GLfloat)window_.height / height_); > + GLfloat scale = std::max((GLfloat)outputSize_.width / nativeOutputSize_.width, > + (GLfloat)outputSize_.height / nativeOutputSize_.height); > GLfloat trans = -(1.0f - scale); > GLfloat projMatrix[] = { > scale, 0, 0, 0, > diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h > index 943410fdd..fbd5430e4 100644 > --- a/src/libcamera/software_isp/debayer_egl.h > +++ b/src/libcamera/software_isp/debayer_egl.h > @@ -102,6 +102,7 @@ private: > /* Contrast */ > GLint contrastExpUniformDataIn_; > > + Size nativeOutputSize_; > Rectangle window_; > std::unique_ptr<SwStatsCpu> stats_; > eGL egl_; > -- > 2.54.0 >
diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp index fd8de3942..521316657 100644 --- a/src/libcamera/software_isp/debayer_egl.cpp +++ b/src/libcamera/software_isp/debayer_egl.cpp @@ -337,6 +337,7 @@ int DebayerEGL::configure(const StreamConfiguration &inputCfg, outputPixelFormat_ = outputCfg.pixelFormat; outputSize_ = outputCfg.size; + nativeOutputSize_ = outSizeRange.max; window_.x = ((inputCfg.size.width - outputCfg.size.width) / 2) & ~(inputConfig_.patternSize.width - 1); @@ -408,11 +409,12 @@ void DebayerEGL::setShaderVariableValues(const DebayerParams ¶ms) 1.0f / (height_ - 1) }; GLfloat Stride = (GLfloat)width_ / (shaderStridePixels_ / bytesPerPixel_); /* - * Scale input to output size, keeping the aspect ratio and preferring - * cropping over black bars. + * Scale the output size from the native size the algorithm produces for + * the input size. Keep the aspect ratio and prefer cropping over black + * bars. */ - GLfloat scale = std::max((GLfloat)window_.width / width_, - (GLfloat)window_.height / height_); + GLfloat scale = std::max((GLfloat)outputSize_.width / nativeOutputSize_.width, + (GLfloat)outputSize_.height / nativeOutputSize_.height); GLfloat trans = -(1.0f - scale); GLfloat projMatrix[] = { scale, 0, 0, 0, diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h index 943410fdd..fbd5430e4 100644 --- a/src/libcamera/software_isp/debayer_egl.h +++ b/src/libcamera/software_isp/debayer_egl.h @@ -102,6 +102,7 @@ private: /* Contrast */ GLint contrastExpUniformDataIn_; + Size nativeOutputSize_; Rectangle window_; std::unique_ptr<SwStatsCpu> stats_; eGL egl_;
The debayer algorithm produces a sligtly smaller output resolution compared to the input one, using a border at the edges depending on the pattern. This wasn't considered when scaling to the final output buffer size, resulting in garbage pixels at the right - or in case or 90 degree rotated sensors - the bottom of the image. Use the native (i.e. unscaled) output size instead - the maximum output buffer size. While on it use outputSize_ for better readability and adopt the scaling comment slightly. Fixes: f520b29fe (libcamera: software_isp: debayer_egl: Add an eGL Debayer class) Signed-off-by: Robert Mader <robert.mader@collabora.com> --- Changes in V2: - Instead of recomputing the native output size, save it in configure() - Use outputSize_ instead of window_ for better readability - Small changes to comment and commit message --- src/libcamera/software_isp/debayer_egl.cpp | 10 ++++++---- src/libcamera/software_isp/debayer_egl.h | 1 + 2 files changed, 7 insertions(+), 4 deletions(-)