| Message ID | 20210902064416.6342-1-martin.kepplinger@puri.sm |
|---|---|
| State | Accepted |
| Commit | 06e53199c2563105030bda4c72752b853da7edc8 |
| Headers | show |
| Series |
|
| Related | show |
Hi Martin, Thank you for the patch. On Thu, Sep 02, 2021 at 08:44:16AM +0200, Martin Kepplinger wrote: > Add camera sensor properties for the Hynix hi846 sensor. > The part is also called YACG4D0C9SHC and a datasheet can be found at > https://product.skhynix.com/products/cis/cis.go > > This is the selfie camera in the Librem 5 phone. > > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> > --- > > Note that the driver is not yet merged into the mainline but being > reviewed: > https://lore.kernel.org/linux-media/20210831134344.1673318-1-martin.kepplinger@puri.sm/ It's well on its way to upstream so that's fine with me. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > libcamera doesn't warn about anything when using the camera. `cam` streams > and qcam shows a (non-debayered) image. qcam supports debayering when using the GLES rendered (-r gles), but doesn't yet support 16-bit formats. The following patch is completely untested but may help. Would you be able to give it a try ? diff --git a/src/qcam/assets/shader/bayer_8.frag b/src/qcam/assets/shader/bayer_8.frag index 4ece44ab5650..b924e8c6ff0a 100644 --- a/src/qcam/assets/shader/bayer_8.frag +++ b/src/qcam/assets/shader/bayer_8.frag @@ -22,10 +22,15 @@ varying vec4 center; varying vec4 yCoord; varying vec4 xCoord; +#if defined(BAYER_16BPP) +#define fetch(x, y) texture2D(tex_y, vec2(x, y)).a +#else +#define fetch(x, y) texture2D(tex_y, vec2(x, y)).r +#endif + void main(void) { - #define fetch(x, y) texture2D(tex_y, vec2(x, y)).r - float C = texture2D(tex_y, center.xy).r; // ( 0, 0) + float C = fetch(center.x, center.y); // ( 0, 0) const vec4 kC = vec4( 4.0, 6.0, 5.0, 5.0) / 8.0; // Determine which of four types of pixels we are on. diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp index 32232faa2ad8..83ee0c7d54bf 100644 --- a/src/qcam/viewfinder_gl.cpp +++ b/src/qcam/viewfinder_gl.cpp @@ -53,6 +53,11 @@ static const QList<libcamera::PixelFormat> supportedFormats{ libcamera::formats::SGBRG12_CSI2P, libcamera::formats::SGRBG12_CSI2P, libcamera::formats::SRGGB12_CSI2P, + /* Raw Bayer 16-bit */ + libcamera::formats::SBGGR16, + libcamera::formats::SGBRG16, + libcamera::formats::SGRBG16, + libcamera::formats::SRGGB16, }; ViewFinderGL::ViewFinderGL(QWidget *parent) @@ -226,6 +231,9 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format) fragmentShaderDefines_.append("#define RGB_PATTERN bgr"); fragmentShaderFile_ = ":RGB.frag"; break; + case libcamera::formats::SBGGR16: + fragmentShaderDefines_.append("#define BAYER_16BPP"); + [[fallthrough]]; case libcamera::formats::SBGGR8: firstRed_.setX(1.0); firstRed_.setY(1.0); @@ -233,6 +241,9 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format) fragmentShaderFile_ = ":bayer_8.frag"; textureMinMagFilters_ = GL_NEAREST; break; + case libcamera::formats::SGBRG16: + fragmentShaderDefines_.append("#define BAYER_16BPP"); + [[fallthrough]]; case libcamera::formats::SGBRG8: firstRed_.setX(0.0); firstRed_.setY(1.0); @@ -240,6 +251,9 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format) fragmentShaderFile_ = ":bayer_8.frag"; textureMinMagFilters_ = GL_NEAREST; break; + case libcamera::formats::SGRBG16: + fragmentShaderDefines_.append("#define BAYER_16BPP"); + [[fallthrough]]; case libcamera::formats::SGRBG8: firstRed_.setX(1.0); firstRed_.setY(0.0); @@ -247,6 +261,9 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format) fragmentShaderFile_ = ":bayer_8.frag"; textureMinMagFilters_ = GL_NEAREST; break; + case libcamera::formats::SRGGB16: + fragmentShaderDefines_.append("#define BAYER_16BPP"); + [[fallthrough]]; case libcamera::formats::SRGGB8: firstRed_.setX(0.0); firstRed_.setY(0.0); @@ -697,6 +714,36 @@ void ViewFinderGL::doRender() 1.0f / (size_.height() - 1)); break; + case libcamera::formats::SBGGR16: + case libcamera::formats::SGBRG16: + case libcamera::formats::SGRBG16: + case libcamera::formats::SRGGB16: + /* + * Raw Bayer 16-bit formats are stored in a GL_LUMINANCE_ALPHA + * texture. The texture width is equal to half the stride. + */ + glActiveTexture(GL_TEXTURE0); + configureTexture(*textures_[0]); + glTexImage2D(GL_TEXTURE_2D, + 0, + GL_LUMINANCE_ALPHA, + stride_ / 2, + size_.height(), + 0, + GL_LUMINANCE_ALPHA, + GL_UNSIGNED_BYTE, + image_->data(0).data()); + shaderProgram_.setUniformValue(textureUniformY_, 0); + shaderProgram_.setUniformValue(textureUniformBayerFirstRed_, + firstRed_); + shaderProgram_.setUniformValue(textureUniformSize_, + size_.width(), /* in pixels */ + size_.height()); + shaderProgram_.setUniformValue(textureUniformStep_, + 1.0f / (stride_ / 2 - 1), + 1.0f / (size_.height() - 1)); + break; + default: break; }; > src/libcamera/camera_sensor_properties.cpp | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > index 4ee45e72..39bb282d 100644 > --- a/src/libcamera/camera_sensor_properties.cpp > +++ b/src/libcamera/camera_sensor_properties.cpp > @@ -52,6 +52,24 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties) > const CameraSensorProperties *CameraSensorProperties::get(const std::string &sensor) > { > static const std::map<std::string, const CameraSensorProperties> sensorProps = { > + { "hi846", { > + .unitCellSize = { 1120, 1120 }, > + .testPatternModes = { > + { 0, controls::draft::TestPatternModeOff }, > + { 1, controls::draft::TestPatternModeSolidColor }, > + { 2, controls::draft::TestPatternModeColorBars }, > + { 3, controls::draft::TestPatternModeColorBarsFadeToGray }, > + { 4, controls::draft::TestPatternModePn9 }, > + /* > + * No corresponding test pattern mode for: > + * 5: "Gradient Horizontal" > + * 6: "Gradient Vertical" > + * 7: "Check Board" > + * 8: "Slant Pattern" > + * 9: "Resolution Pattern" > + */ > + }, > + } }, > { "imx219", { > .unitCellSize = { 1120, 1120 }, > .testPatternModes = {
Am Montag, dem 06.09.2021 um 04:37 +0300 schrieb Laurent Pinchart: > Hi Martin, > > Thank you for the patch. > > On Thu, Sep 02, 2021 at 08:44:16AM +0200, Martin Kepplinger wrote: > > Add camera sensor properties for the Hynix hi846 sensor. > > The part is also called YACG4D0C9SHC and a datasheet can be found > > at > > https://product.skhynix.com/products/cis/cis.go > > > > This is the selfie camera in the Librem 5 phone. > > > > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> > > --- > > > > Note that the driver is not yet merged into the mainline but being > > reviewed: > > https://lore.kernel.org/linux-media/20210831134344.1673318-1-martin.kepplinger@puri.sm/ > > It's well on its way to upstream so that's fine with me. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > libcamera doesn't warn about anything when using the camera. `cam` > > streams > > and qcam shows a (non-debayered) image. > > qcam supports debayering when using the GLES rendered (-r gles), but > doesn't yet support 16-bit formats. The following patch is completely > untested but may help. Would you be able to give it a try ? > > diff --git a/src/qcam/assets/shader/bayer_8.frag > b/src/qcam/assets/shader/bayer_8.frag > index 4ece44ab5650..b924e8c6ff0a 100644 > --- a/src/qcam/assets/shader/bayer_8.frag > +++ b/src/qcam/assets/shader/bayer_8.frag > @@ -22,10 +22,15 @@ varying vec4 center; > varying vec4 yCoord; > varying vec4 xCoord; > > +#if defined(BAYER_16BPP) > +#define fetch(x, y) texture2D(tex_y, vec2(x, y)).a > +#else > +#define fetch(x, y) texture2D(tex_y, vec2(x, y)).r > +#endif > + > void main(void) { > - #define fetch(x, y) texture2D(tex_y, vec2(x, y)).r > > - float C = texture2D(tex_y, center.xy).r; // ( 0, 0) > + float C = fetch(center.x, center.y); // ( 0, 0) > const vec4 kC = vec4( 4.0, 6.0, 5.0, 5.0) / 8.0; > > // Determine which of four types of pixels we are on. > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp > index 32232faa2ad8..83ee0c7d54bf 100644 > --- a/src/qcam/viewfinder_gl.cpp > +++ b/src/qcam/viewfinder_gl.cpp > @@ -53,6 +53,11 @@ static const QList<libcamera::PixelFormat> > supportedFormats{ > libcamera::formats::SGBRG12_CSI2P, > libcamera::formats::SGRBG12_CSI2P, > libcamera::formats::SRGGB12_CSI2P, > + /* Raw Bayer 16-bit */ > + libcamera::formats::SBGGR16, > + libcamera::formats::SGBRG16, > + libcamera::formats::SGRBG16, > + libcamera::formats::SRGGB16, > }; > > ViewFinderGL::ViewFinderGL(QWidget *parent) > @@ -226,6 +231,9 @@ bool ViewFinderGL::selectFormat(const > libcamera::PixelFormat &format) > fragmentShaderDefines_.append("#define RGB_PATTERN > bgr"); > fragmentShaderFile_ = ":RGB.frag"; > break; > + case libcamera::formats::SBGGR16: > + fragmentShaderDefines_.append("#define BAYER_16BPP"); > + [[fallthrough]]; > case libcamera::formats::SBGGR8: > firstRed_.setX(1.0); > firstRed_.setY(1.0); > @@ -233,6 +241,9 @@ bool ViewFinderGL::selectFormat(const > libcamera::PixelFormat &format) > fragmentShaderFile_ = ":bayer_8.frag"; > textureMinMagFilters_ = GL_NEAREST; > break; > + case libcamera::formats::SGBRG16: > + fragmentShaderDefines_.append("#define BAYER_16BPP"); > + [[fallthrough]]; > case libcamera::formats::SGBRG8: > firstRed_.setX(0.0); > firstRed_.setY(1.0); > @@ -240,6 +251,9 @@ bool ViewFinderGL::selectFormat(const > libcamera::PixelFormat &format) > fragmentShaderFile_ = ":bayer_8.frag"; > textureMinMagFilters_ = GL_NEAREST; > break; > + case libcamera::formats::SGRBG16: > + fragmentShaderDefines_.append("#define BAYER_16BPP"); > + [[fallthrough]]; > case libcamera::formats::SGRBG8: > firstRed_.setX(1.0); > firstRed_.setY(0.0); > @@ -247,6 +261,9 @@ bool ViewFinderGL::selectFormat(const > libcamera::PixelFormat &format) > fragmentShaderFile_ = ":bayer_8.frag"; > textureMinMagFilters_ = GL_NEAREST; > break; > + case libcamera::formats::SRGGB16: > + fragmentShaderDefines_.append("#define BAYER_16BPP"); > + [[fallthrough]]; > case libcamera::formats::SRGGB8: > firstRed_.setX(0.0); > firstRed_.setY(0.0); > @@ -697,6 +714,36 @@ void ViewFinderGL::doRender() > 1.0f / (size_.height() > - 1)); > break; > > + case libcamera::formats::SBGGR16: > + case libcamera::formats::SGBRG16: > + case libcamera::formats::SGRBG16: > + case libcamera::formats::SRGGB16: > + /* > + * Raw Bayer 16-bit formats are stored in a > GL_LUMINANCE_ALPHA > + * texture. The texture width is equal to half the > stride. > + */ > + glActiveTexture(GL_TEXTURE0); > + configureTexture(*textures_[0]); > + glTexImage2D(GL_TEXTURE_2D, > + 0, > + GL_LUMINANCE_ALPHA, > + stride_ / 2, > + size_.height(), > + 0, > + GL_LUMINANCE_ALPHA, > + GL_UNSIGNED_BYTE, > + image_->data(0).data()); builds when using data_ here (if that's correct?) without further inspection, qcam shows a black image stream with this (the ugly debayered image at least shows something so gain should be enough to see something). the fps are quite good though :) > + shaderProgram_.setUniformValue(textureUniformY_, 0); > + shaderProgram_.setUniformValue(textureUniformBayerFir > stRed_, > + firstRed_); > + shaderProgram_.setUniformValue(textureUniformSize_, > + size_.width(), /* in > pixels */ > + size_.height()); > + shaderProgram_.setUniformValue(textureUniformStep_, > + 1.0f / (stride_ / 2 - > 1), > + 1.0f / (size_.height() > - 1)); > + break; > + > default: > break; > }; > > > > src/libcamera/camera_sensor_properties.cpp | 18 ++++++++++++++++++ > > 1 file changed, 18 insertions(+) > > > > diff --git a/src/libcamera/camera_sensor_properties.cpp > > b/src/libcamera/camera_sensor_properties.cpp > > index 4ee45e72..39bb282d 100644 > > --- a/src/libcamera/camera_sensor_properties.cpp > > +++ b/src/libcamera/camera_sensor_properties.cpp > > @@ -52,6 +52,24 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties) > > const CameraSensorProperties *CameraSensorProperties::get(const > > std::string &sensor) > > { > > static const std::map<std::string, const > > CameraSensorProperties> sensorProps = { > > + { "hi846", { > > + .unitCellSize = { 1120, 1120 }, > > + .testPatternModes = { > > + { 0, > > controls::draft::TestPatternModeOff }, > > + { 1, > > controls::draft::TestPatternModeSolidColor }, > > + { 2, > > controls::draft::TestPatternModeColorBars }, > > + { 3, > > controls::draft::TestPatternModeColorBarsFadeToGray }, > > + { 4, > > controls::draft::TestPatternModePn9 }, > > + /* > > + * No corresponding test pattern > > mode for: > > + * 5: "Gradient Horizontal" > > + * 6: "Gradient Vertical" > > + * 7: "Check Board" > > + * 8: "Slant Pattern" > > + * 9: "Resolution Pattern" > > + */ > > + }, > > + } }, > > { "imx219", { > > .unitCellSize = { 1120, 1120 }, > > .testPatternModes = { >
Hi Martin, On Mon, Sep 06, 2021 at 02:51:47PM +0200, Martin Kepplinger wrote: > Am Montag, dem 06.09.2021 um 04:37 +0300 schrieb Laurent Pinchart: > > On Thu, Sep 02, 2021 at 08:44:16AM +0200, Martin Kepplinger wrote: > > > Add camera sensor properties for the Hynix hi846 sensor. > > > The part is also called YACG4D0C9SHC and a datasheet can be found > > > at > > > https://product.skhynix.com/products/cis/cis.go > > > > > > This is the selfie camera in the Librem 5 phone. > > > > > > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> > > > --- > > > > > > Note that the driver is not yet merged into the mainline but being > > > reviewed: > > > https://lore.kernel.org/linux-media/20210831134344.1673318-1-martin.kepplinger@puri.sm/ > > > > It's well on its way to upstream so that's fine with me. > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > libcamera doesn't warn about anything when using the camera. `cam` > > > streams > > > and qcam shows a (non-debayered) image. > > > > qcam supports debayering when using the GLES rendered (-r gles), but > > doesn't yet support 16-bit formats. The following patch is completely > > untested but may help. Would you be able to give it a try ? > > > > diff --git a/src/qcam/assets/shader/bayer_8.frag b/src/qcam/assets/shader/bayer_8.frag > > index 4ece44ab5650..b924e8c6ff0a 100644 > > --- a/src/qcam/assets/shader/bayer_8.frag > > +++ b/src/qcam/assets/shader/bayer_8.frag > > @@ -22,10 +22,15 @@ varying vec4 center; > > varying vec4 yCoord; > > varying vec4 xCoord; > > > > +#if defined(BAYER_16BPP) > > +#define fetch(x, y) texture2D(tex_y, vec2(x, y)).a > > +#else > > +#define fetch(x, y) texture2D(tex_y, vec2(x, y)).r > > +#endif > > + > > void main(void) { > > - #define fetch(x, y) texture2D(tex_y, vec2(x, y)).r > > > > - float C = texture2D(tex_y, center.xy).r; // ( 0, 0) > > + float C = fetch(center.x, center.y); // ( 0, 0) > > const vec4 kC = vec4( 4.0, 6.0, 5.0, 5.0) / 8.0; > > > > // Determine which of four types of pixels we are on. > > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp > > index 32232faa2ad8..83ee0c7d54bf 100644 > > --- a/src/qcam/viewfinder_gl.cpp > > +++ b/src/qcam/viewfinder_gl.cpp > > @@ -53,6 +53,11 @@ static const QList<libcamera::PixelFormat> supportedFormats{ > > libcamera::formats::SGBRG12_CSI2P, > > libcamera::formats::SGRBG12_CSI2P, > > libcamera::formats::SRGGB12_CSI2P, > > + /* Raw Bayer 16-bit */ > > + libcamera::formats::SBGGR16, > > + libcamera::formats::SGBRG16, > > + libcamera::formats::SGRBG16, > > + libcamera::formats::SRGGB16, > > }; > > > > ViewFinderGL::ViewFinderGL(QWidget *parent) > > @@ -226,6 +231,9 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format) > > fragmentShaderDefines_.append("#define RGB_PATTERN bgr"); > > fragmentShaderFile_ = ":RGB.frag"; > > break; > > + case libcamera::formats::SBGGR16: > > + fragmentShaderDefines_.append("#define BAYER_16BPP"); > > + [[fallthrough]]; > > case libcamera::formats::SBGGR8: > > firstRed_.setX(1.0); > > firstRed_.setY(1.0); > > @@ -233,6 +241,9 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format) > > fragmentShaderFile_ = ":bayer_8.frag"; > > textureMinMagFilters_ = GL_NEAREST; > > break; > > + case libcamera::formats::SGBRG16: > > + fragmentShaderDefines_.append("#define BAYER_16BPP"); > > + [[fallthrough]]; > > case libcamera::formats::SGBRG8: > > firstRed_.setX(0.0); > > firstRed_.setY(1.0); > > @@ -240,6 +251,9 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format) > > fragmentShaderFile_ = ":bayer_8.frag"; > > textureMinMagFilters_ = GL_NEAREST; > > break; > > + case libcamera::formats::SGRBG16: > > + fragmentShaderDefines_.append("#define BAYER_16BPP"); > > + [[fallthrough]]; > > case libcamera::formats::SGRBG8: > > firstRed_.setX(1.0); > > firstRed_.setY(0.0); > > @@ -247,6 +261,9 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format) > > fragmentShaderFile_ = ":bayer_8.frag"; > > textureMinMagFilters_ = GL_NEAREST; > > break; > > + case libcamera::formats::SRGGB16: > > + fragmentShaderDefines_.append("#define BAYER_16BPP"); > > + [[fallthrough]]; > > case libcamera::formats::SRGGB8: > > firstRed_.setX(0.0); > > firstRed_.setY(0.0); > > @@ -697,6 +714,36 @@ void ViewFinderGL::doRender() > > 1.0f / (size_.height() - 1)); > > break; > > > > + case libcamera::formats::SBGGR16: > > + case libcamera::formats::SGBRG16: > > + case libcamera::formats::SGRBG16: > > + case libcamera::formats::SRGGB16: > > + /* > > + * Raw Bayer 16-bit formats are stored in a GL_LUMINANCE_ALPHA > > + * texture. The texture width is equal to half the stride. > > + */ > > + glActiveTexture(GL_TEXTURE0); > > + configureTexture(*textures_[0]); > > + glTexImage2D(GL_TEXTURE_2D, > > + 0, > > + GL_LUMINANCE_ALPHA, > > + stride_ / 2, > > + size_.height(), > > + 0, > > + GL_LUMINANCE_ALPHA, > > + GL_UNSIGNED_BYTE, > > + image_->data(0).data()); > > builds when using data_ here (if that's correct?) It is. The diff was based on a branch that hasn't been merged yet, I forgot about it. Sorry. > without further inspection, qcam shows a black image stream with this > (the ugly debayered image at least shows something so gain should be > enough to see something). Do I recall correctly that the imx7 CSI bridge driver advertises 16-bit Bayer formats, but aligns the data to the right, not to the left ? That should then be fixed in the driver, and this patch will require more intrusive changes in the shader. As a quick test, could you drop the change to src/qcam/assets/shader/bayer_8.frag and try again ? I expect artifacts, but a visible image. > the fps are quite good though :) > > > + shaderProgram_.setUniformValue(textureUniformY_, 0); > > + shaderProgram_.setUniformValue(textureUniformBayerFirstRed_, > > + firstRed_); > > + shaderProgram_.setUniformValue(textureUniformSize_, > > + size_.width(), /* in pixels */ > > + size_.height()); > > + shaderProgram_.setUniformValue(textureUniformStep_, > > + 1.0f / (stride_ / 2 - 1), > > + 1.0f / (size_.height() - 1)); > > + break; > > + > > default: > > break; > > }; > > > > > src/libcamera/camera_sensor_properties.cpp | 18 ++++++++++++++++++ > > > 1 file changed, 18 insertions(+) > > > > > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > > > index 4ee45e72..39bb282d 100644 > > > --- a/src/libcamera/camera_sensor_properties.cpp > > > +++ b/src/libcamera/camera_sensor_properties.cpp > > > @@ -52,6 +52,24 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties) > > > const CameraSensorProperties *CameraSensorProperties::get(const std::string &sensor) > > > { > > > static const std::map<std::string, const CameraSensorProperties> sensorProps = { > > > + { "hi846", { > > > + .unitCellSize = { 1120, 1120 }, > > > + .testPatternModes = { > > > + { 0, controls::draft::TestPatternModeOff }, > > > + { 1, controls::draft::TestPatternModeSolidColor }, > > > + { 2, controls::draft::TestPatternModeColorBars }, > > > + { 3, controls::draft::TestPatternModeColorBarsFadeToGray }, > > > + { 4, controls::draft::TestPatternModePn9 }, > > > + /* > > > + * No corresponding test pattern mode for: > > > + * 5: "Gradient Horizontal" > > > + * 6: "Gradient Vertical" > > > + * 7: "Check Board" > > > + * 8: "Slant Pattern" > > > + * 9: "Resolution Pattern" > > > + */ > > > + }, > > > + } }, > > > { "imx219", { > > > .unitCellSize = { 1120, 1120 }, > > > .testPatternModes = {
Am Montag, dem 06.09.2021 um 21:04 +0300 schrieb Laurent Pinchart: > Hi Martin, > > On Mon, Sep 06, 2021 at 02:51:47PM +0200, Martin Kepplinger wrote: > > Am Montag, dem 06.09.2021 um 04:37 +0300 schrieb Laurent Pinchart: > > > On Thu, Sep 02, 2021 at 08:44:16AM +0200, Martin Kepplinger > > > wrote: > > > > Add camera sensor properties for the Hynix hi846 sensor. > > > > The part is also called YACG4D0C9SHC and a datasheet can be > > > > found > > > > at > > > > https://product.skhynix.com/products/cis/cis.go > > > > > > > > This is the selfie camera in the Librem 5 phone. > > > > > > > > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> > > > > --- > > > > > > > > Note that the driver is not yet merged into the mainline but > > > > being > > > > reviewed: > > > > https://lore.kernel.org/linux-media/20210831134344.1673318-1-martin.kepplinger@puri.sm/ > > > > > > It's well on its way to upstream so that's fine with me. > > > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > > > libcamera doesn't warn about anything when using the camera. > > > > `cam` > > > > streams > > > > and qcam shows a (non-debayered) image. > > > > > > qcam supports debayering when using the GLES rendered (-r gles), > > > but > > > doesn't yet support 16-bit formats. The following patch is > > > completely > > > untested but may help. Would you be able to give it a try ? > > > > > > diff --git a/src/qcam/assets/shader/bayer_8.frag > > > b/src/qcam/assets/shader/bayer_8.frag > > > index 4ece44ab5650..b924e8c6ff0a 100644 > > > --- a/src/qcam/assets/shader/bayer_8.frag > > > +++ b/src/qcam/assets/shader/bayer_8.frag > > > @@ -22,10 +22,15 @@ varying vec4 center; > > > varying vec4 yCoord; > > > varying vec4 xCoord; > > > > > > +#if defined(BAYER_16BPP) > > > +#define fetch(x, y) texture2D(tex_y, vec2(x, y)).a > > > +#else > > > +#define fetch(x, y) texture2D(tex_y, vec2(x, y)).r > > > +#endif > > > + > > > void main(void) { > > > - #define fetch(x, y) texture2D(tex_y, vec2(x, y)).r > > > > > > - float C = texture2D(tex_y, center.xy).r; // ( 0, 0) > > > + float C = fetch(center.x, center.y); // ( 0, 0) > > > const vec4 kC = vec4( 4.0, 6.0, 5.0, 5.0) / 8.0; > > > > > > // Determine which of four types of pixels we are on. > > > diff --git a/src/qcam/viewfinder_gl.cpp > > > b/src/qcam/viewfinder_gl.cpp > > > index 32232faa2ad8..83ee0c7d54bf 100644 > > > --- a/src/qcam/viewfinder_gl.cpp > > > +++ b/src/qcam/viewfinder_gl.cpp > > > @@ -53,6 +53,11 @@ static const QList<libcamera::PixelFormat> > > > supportedFormats{ > > > libcamera::formats::SGBRG12_CSI2P, > > > libcamera::formats::SGRBG12_CSI2P, > > > libcamera::formats::SRGGB12_CSI2P, > > > + /* Raw Bayer 16-bit */ > > > + libcamera::formats::SBGGR16, > > > + libcamera::formats::SGBRG16, > > > + libcamera::formats::SGRBG16, > > > + libcamera::formats::SRGGB16, > > > }; > > > > > > ViewFinderGL::ViewFinderGL(QWidget *parent) > > > @@ -226,6 +231,9 @@ bool ViewFinderGL::selectFormat(const > > > libcamera::PixelFormat &format) > > > fragmentShaderDefines_.append("#define > > > RGB_PATTERN bgr"); > > > fragmentShaderFile_ = ":RGB.frag"; > > > break; > > > + case libcamera::formats::SBGGR16: > > > + fragmentShaderDefines_.append("#define > > > BAYER_16BPP"); > > > + [[fallthrough]]; > > > case libcamera::formats::SBGGR8: > > > firstRed_.setX(1.0); > > > firstRed_.setY(1.0); > > > @@ -233,6 +241,9 @@ bool ViewFinderGL::selectFormat(const > > > libcamera::PixelFormat &format) > > > fragmentShaderFile_ = ":bayer_8.frag"; > > > textureMinMagFilters_ = GL_NEAREST; > > > break; > > > + case libcamera::formats::SGBRG16: > > > + fragmentShaderDefines_.append("#define > > > BAYER_16BPP"); > > > + [[fallthrough]]; > > > case libcamera::formats::SGBRG8: > > > firstRed_.setX(0.0); > > > firstRed_.setY(1.0); > > > @@ -240,6 +251,9 @@ bool ViewFinderGL::selectFormat(const > > > libcamera::PixelFormat &format) > > > fragmentShaderFile_ = ":bayer_8.frag"; > > > textureMinMagFilters_ = GL_NEAREST; > > > break; > > > + case libcamera::formats::SGRBG16: > > > + fragmentShaderDefines_.append("#define > > > BAYER_16BPP"); > > > + [[fallthrough]]; > > > case libcamera::formats::SGRBG8: > > > firstRed_.setX(1.0); > > > firstRed_.setY(0.0); > > > @@ -247,6 +261,9 @@ bool ViewFinderGL::selectFormat(const > > > libcamera::PixelFormat &format) > > > fragmentShaderFile_ = ":bayer_8.frag"; > > > textureMinMagFilters_ = GL_NEAREST; > > > break; > > > + case libcamera::formats::SRGGB16: > > > + fragmentShaderDefines_.append("#define > > > BAYER_16BPP"); > > > + [[fallthrough]]; > > > case libcamera::formats::SRGGB8: > > > firstRed_.setX(0.0); > > > firstRed_.setY(0.0); > > > @@ -697,6 +714,36 @@ void ViewFinderGL::doRender() > > > 1.0f / > > > (size_.height() - 1)); > > > break; > > > > > > + case libcamera::formats::SBGGR16: > > > + case libcamera::formats::SGBRG16: > > > + case libcamera::formats::SGRBG16: > > > + case libcamera::formats::SRGGB16: > > > + /* > > > + * Raw Bayer 16-bit formats are stored in a > > > GL_LUMINANCE_ALPHA > > > + * texture. The texture width is equal to half > > > the stride. > > > + */ > > > + glActiveTexture(GL_TEXTURE0); > > > + configureTexture(*textures_[0]); > > > + glTexImage2D(GL_TEXTURE_2D, > > > + 0, > > > + GL_LUMINANCE_ALPHA, > > > + stride_ / 2, > > > + size_.height(), > > > + 0, > > > + GL_LUMINANCE_ALPHA, > > > + GL_UNSIGNED_BYTE, > > > + image_->data(0).data()); > > > > builds when using data_ here (if that's correct?) > > It is. The diff was based on a branch that hasn't been merged yet, I > forgot about it. Sorry. > > > without further inspection, qcam shows a black image stream with > > this > > (the ugly debayered image at least shows something so gain should > > be > > enough to see something). > > Do I recall correctly that the imx7 CSI bridge driver advertises 16- > bit > Bayer formats, but aligns the data to the right, not to the left ? > That > should then be fixed in the driver, and this patch will require more > intrusive changes in the shader. exactly, the 16 bits start with zeroes for a 10-bit bayer format. > > As a quick test, could you drop the change to > src/qcam/assets/shader/bayer_8.frag and try again ? I expect > artifacts, > but a visible image. > > You're right. a visible image with artifacts now. thanks, martin
Am Montag, dem 06.09.2021 um 21:04 +0300 schrieb Laurent Pinchart: > Hi Martin, > > On Mon, Sep 06, 2021 at 02:51:47PM +0200, Martin Kepplinger wrote: > > Am Montag, dem 06.09.2021 um 04:37 +0300 schrieb Laurent Pinchart: > > > On Thu, Sep 02, 2021 at 08:44:16AM +0200, Martin Kepplinger > > > wrote: > > > > Add camera sensor properties for the Hynix hi846 sensor. > > > > The part is also called YACG4D0C9SHC and a datasheet can be > > > > found > > > > at > > > > https://product.skhynix.com/products/cis/cis.go > > > > > > > > This is the selfie camera in the Librem 5 phone. > > > > > > > > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> > > > > --- > > > > > > > > Note that the driver is not yet merged into the mainline but > > > > being > > > > reviewed: > > > > https://lore.kernel.org/linux-media/20210831134344.1673318-1-martin.kepplinger@puri.sm/ > > > > > > It's well on its way to upstream so that's fine with me. > > > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > > > libcamera doesn't warn about anything when using the camera. > > > > `cam` > > > > streams > > > > and qcam shows a (non-debayered) image. > > > > > > qcam supports debayering when using the GLES rendered (-r gles), > > > but > > > doesn't yet support 16-bit formats. The following patch is > > > completely > > > untested but may help. Would you be able to give it a try ? > > > > > > diff --git a/src/qcam/assets/shader/bayer_8.frag > > > b/src/qcam/assets/shader/bayer_8.frag > > > index 4ece44ab5650..b924e8c6ff0a 100644 > > > --- a/src/qcam/assets/shader/bayer_8.frag > > > +++ b/src/qcam/assets/shader/bayer_8.frag > > > @@ -22,10 +22,15 @@ varying vec4 center; > > > varying vec4 yCoord; > > > varying vec4 xCoord; > > > > > > +#if defined(BAYER_16BPP) > > > +#define fetch(x, y) texture2D(tex_y, vec2(x, y)).a > > > +#else > > > +#define fetch(x, y) texture2D(tex_y, vec2(x, y)).r > > > +#endif > > > + > > > void main(void) { > > > - #define fetch(x, y) texture2D(tex_y, vec2(x, y)).r > > > > > > - float C = texture2D(tex_y, center.xy).r; // ( 0, 0) > > > + float C = fetch(center.x, center.y); // ( 0, 0) > > > const vec4 kC = vec4( 4.0, 6.0, 5.0, 5.0) / 8.0; > > > > > > // Determine which of four types of pixels we are on. > > > diff --git a/src/qcam/viewfinder_gl.cpp > > > b/src/qcam/viewfinder_gl.cpp > > > index 32232faa2ad8..83ee0c7d54bf 100644 > > > --- a/src/qcam/viewfinder_gl.cpp > > > +++ b/src/qcam/viewfinder_gl.cpp > > > @@ -53,6 +53,11 @@ static const QList<libcamera::PixelFormat> > > > supportedFormats{ > > > libcamera::formats::SGBRG12_CSI2P, > > > libcamera::formats::SGRBG12_CSI2P, > > > libcamera::formats::SRGGB12_CSI2P, > > > + /* Raw Bayer 16-bit */ > > > + libcamera::formats::SBGGR16, > > > + libcamera::formats::SGBRG16, > > > + libcamera::formats::SGRBG16, > > > + libcamera::formats::SRGGB16, > > > }; > > > > > > ViewFinderGL::ViewFinderGL(QWidget *parent) > > > @@ -226,6 +231,9 @@ bool ViewFinderGL::selectFormat(const > > > libcamera::PixelFormat &format) > > > fragmentShaderDefines_.append("#define > > > RGB_PATTERN bgr"); > > > fragmentShaderFile_ = ":RGB.frag"; > > > break; > > > + case libcamera::formats::SBGGR16: > > > + fragmentShaderDefines_.append("#define > > > BAYER_16BPP"); > > > + [[fallthrough]]; > > > case libcamera::formats::SBGGR8: > > > firstRed_.setX(1.0); > > > firstRed_.setY(1.0); > > > @@ -233,6 +241,9 @@ bool ViewFinderGL::selectFormat(const > > > libcamera::PixelFormat &format) > > > fragmentShaderFile_ = ":bayer_8.frag"; > > > textureMinMagFilters_ = GL_NEAREST; > > > break; > > > + case libcamera::formats::SGBRG16: > > > + fragmentShaderDefines_.append("#define > > > BAYER_16BPP"); > > > + [[fallthrough]]; > > > case libcamera::formats::SGBRG8: > > > firstRed_.setX(0.0); > > > firstRed_.setY(1.0); > > > @@ -240,6 +251,9 @@ bool ViewFinderGL::selectFormat(const > > > libcamera::PixelFormat &format) > > > fragmentShaderFile_ = ":bayer_8.frag"; > > > textureMinMagFilters_ = GL_NEAREST; > > > break; > > > + case libcamera::formats::SGRBG16: > > > + fragmentShaderDefines_.append("#define > > > BAYER_16BPP"); > > > + [[fallthrough]]; > > > case libcamera::formats::SGRBG8: > > > firstRed_.setX(1.0); > > > firstRed_.setY(0.0); > > > @@ -247,6 +261,9 @@ bool ViewFinderGL::selectFormat(const > > > libcamera::PixelFormat &format) > > > fragmentShaderFile_ = ":bayer_8.frag"; > > > textureMinMagFilters_ = GL_NEAREST; > > > break; > > > + case libcamera::formats::SRGGB16: > > > + fragmentShaderDefines_.append("#define > > > BAYER_16BPP"); > > > + [[fallthrough]]; > > > case libcamera::formats::SRGGB8: > > > firstRed_.setX(0.0); > > > firstRed_.setY(0.0); > > > @@ -697,6 +714,36 @@ void ViewFinderGL::doRender() > > > 1.0f / > > > (size_.height() - 1)); > > > break; > > > > > > + case libcamera::formats::SBGGR16: > > > + case libcamera::formats::SGBRG16: > > > + case libcamera::formats::SGRBG16: > > > + case libcamera::formats::SRGGB16: > > > + /* > > > + * Raw Bayer 16-bit formats are stored in a > > > GL_LUMINANCE_ALPHA > > > + * texture. The texture width is equal to half > > > the stride. > > > + */ > > > + glActiveTexture(GL_TEXTURE0); > > > + configureTexture(*textures_[0]); > > > + glTexImage2D(GL_TEXTURE_2D, > > > + 0, > > > + GL_LUMINANCE_ALPHA, > > > + stride_ / 2, > > > + size_.height(), > > > + 0, > > > + GL_LUMINANCE_ALPHA, > > > + GL_UNSIGNED_BYTE, > > > + image_->data(0).data()); > > > > builds when using data_ here (if that's correct?) > > It is. The diff was based on a branch that hasn't been merged yet, I > forgot about it. Sorry. > > > without further inspection, qcam shows a black image stream with > > this > > (the ugly debayered image at least shows something so gain should > > be > > enough to see something). > > Do I recall correctly that the imx7 CSI bridge driver advertises 16- > bit > Bayer formats, but aligns the data to the right, not to the left ? > That > should then be fixed in the driver, and this patch will require more > intrusive changes in the shader. > What exactly would you want to fix in the driver here? The csi bridge driver? thanks, martin
diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp index 4ee45e72..39bb282d 100644 --- a/src/libcamera/camera_sensor_properties.cpp +++ b/src/libcamera/camera_sensor_properties.cpp @@ -52,6 +52,24 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties) const CameraSensorProperties *CameraSensorProperties::get(const std::string &sensor) { static const std::map<std::string, const CameraSensorProperties> sensorProps = { + { "hi846", { + .unitCellSize = { 1120, 1120 }, + .testPatternModes = { + { 0, controls::draft::TestPatternModeOff }, + { 1, controls::draft::TestPatternModeSolidColor }, + { 2, controls::draft::TestPatternModeColorBars }, + { 3, controls::draft::TestPatternModeColorBarsFadeToGray }, + { 4, controls::draft::TestPatternModePn9 }, + /* + * No corresponding test pattern mode for: + * 5: "Gradient Horizontal" + * 6: "Gradient Vertical" + * 7: "Check Board" + * 8: "Slant Pattern" + * 9: "Resolution Pattern" + */ + }, + } }, { "imx219", { .unitCellSize = { 1120, 1120 }, .testPatternModes = {
Add camera sensor properties for the Hynix hi846 sensor. The part is also called YACG4D0C9SHC and a datasheet can be found at https://product.skhynix.com/products/cis/cis.go This is the selfie camera in the Librem 5 phone. Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> --- Note that the driver is not yet merged into the mainline but being reviewed: https://lore.kernel.org/linux-media/20210831134344.1673318-1-martin.kepplinger@puri.sm/ libcamera doesn't warn about anything when using the camera. `cam` streams and qcam shows a (non-debayered) image. thanks, martin src/libcamera/camera_sensor_properties.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)