[libcamera-devel] [PATCH] libcamera: add hi846 camera sensor properties

Martin Kepplinger martin.kepplinger at puri.sm
Thu Sep 9 09:04:09 CEST 2021


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



More information about the libcamera-devel mailing list