[libcamera-devel] [PATCH v5 0/4] qcam: accelerate format conversion by OpenGL shader

[Show Liu]

Hi Laurent,


On Tue, Sep 8, 2020 at 7:23 PM Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:

> On Tue, Sep 08, 2020 at 05:35:27PM +0800, Show Liu wrote:
> > On Mon, Sep 7, 2020 at 12:25 AM Laurent Pinchart wrote:
> > > Hi Show,
> > >
> > > Thank you for the patches.
> > >
> > > On Fri, Sep 04, 2020 at 04:43:11PM +0800, Show Liu wrote:
> > > > This is patch set v5 for qcam accelerated format conversion by
> OpenGL shader.
> > > > It's based on viewfinderGL(patch set v3), please skip the v4.
> > > >
> > > > In this version, I changed the original viewfinder hierarchy
> including created
> > > > viewfinder base and move the original viewfinder as default Qt
> rendering and
> > > > created new viewfinderGL to handle OpenGL stuff and also move the
> OpenGL shader
> > > > code as Qt resource. All the changes are according to the previous
> review comments.
> > > >
> > > > Known issue:
> > > > * It's running well at start time, but when the stop button is
> pressed and
> > > >   starts again, there is no more capture event being triggered.
> > > >
> > > > Todo:
> > > > * Show the No camera icon when the capture stops being pressed.
> > >
> > > Nice work ! My head hurts with opengl now :-)
> > >
> > > Patches 1/4, 2/4 and 4/4 are mostly fine. I've sent more comments for
> > > patch 3/4, but nothing that should affect the architecture.
> >
> > Thanks for the review, I am trying to modify it according to your
> comments.
> >
> > > If there are
> > > issue you're not sure how to address, please let me know. I can try to
> > > help with the integration, letting you focus on the GL part. For
> > > instance I can have a look at the issues mentioned above,
> >
> > If so, that would be grateful!!  ...:-)
> >
> > > as well as the
> > > QT_NO_OPENGL conditional compilation. We also need to fall back to
> > > ViewFinderQt when the format isn't supported by ViewFinderGL, and
> that's
> > > also something I can handle.
> >
> > Actually I have thinks about this problem when I generating this series,
> > but the ViewFinderGL been generated before we got the format, currently
> > I am just have warning message and suggest to use render qt if
> setFormat()
> > is failed
>
> I'll handle this on top of the next version you'll submit.
>
> One point I failed to mention during the review, would it be possible to
> license the shaders and ViewFinderGL under (GPL-2.0-or-later OR
> LGPL-2.1-or-later) ?

Sure. No problem. I will change to "LGPL-2.1-or-later" .

I foresee a possibly need to use that code inside
> libcamera for a GPU-based ISP in the future

That sounds interesting.
And I believe the offscreen rendering would be more suitable for that....
just an idea.

Regards,
Show

, and also possibly in a
> generic format conversion library that could be part of the libcamera
> project but implemented in a separate .so. I have no plan to work on
> these in the very near future, but having that option would avoid going
> through a relicensing process in the future.
>
> > > > Show Liu (4):
> > > >   qcam: add OpenGL shader code as Qt resource
> > > >   qcam: new viewfinder hierarchy
> > > >   qcam: add viewfinderGL class to accelerate the format convert
> > > >   qcam: add additional command line option to select the render type
> > > >
> > > >  src/qcam/assets/shader/NV_2_planes_UV_f.glsl  |  32 ++
> > > >  src/qcam/assets/shader/NV_2_planes_VU_f.glsl  |  32 ++
> > > >  src/qcam/assets/shader/NV_3_planes_UV_f.glsl  |  33 ++
> > > >  src/qcam/assets/shader/NV_3_planes_VU_f.glsl  |  33 ++
> > > >  src/qcam/assets/shader/NV_vertex_shader.glsl  |  16 +
> > > >  src/qcam/assets/shader/shaders.qrc            |  10 +
> > > >  src/qcam/main.cpp                             |   3 +
> > > >  src/qcam/main_window.cpp                      |  29 +-
> > > >  src/qcam/main_window.h                        |   6 +
> > > >  src/qcam/meson.build                          |   7 +-
> > > >  src/qcam/viewfinder.h                         |  60 +--
> > > >  src/qcam/viewfinder_gl.cpp                    | 441
> ++++++++++++++++++
> > > >  src/qcam/viewfinder_gl.h                      |  97 ++++
> > > >  .../{viewfinder.cpp => viewfinder_qt.cpp}     |  24 +-
> > > >  src/qcam/viewfinder_qt.h                      |  67 +++
> > > >  15 files changed, 824 insertions(+), 66 deletions(-)
> > > >  create mode 100644 src/qcam/assets/shader/NV_2_planes_UV_f.glsl
> > > >  create mode 100644 src/qcam/assets/shader/NV_2_planes_VU_f.glsl
> > > >  create mode 100644 src/qcam/assets/shader/NV_3_planes_UV_f.glsl
> > > >  create mode 100644 src/qcam/assets/shader/NV_3_planes_VU_f.glsl
> > > >  create mode 100644 src/qcam/assets/shader/NV_vertex_shader.glsl
> > > >  create mode 100644 src/qcam/assets/shader/shaders.qrc
> > > >  create mode 100644 src/qcam/viewfinder_gl.cpp
> > > >  create mode 100644 src/qcam/viewfinder_gl.h
> > > >  rename src/qcam/{viewfinder.cpp => viewfinder_qt.cpp} (86%)
> > > >  create mode 100644 src/qcam/viewfinder_qt.h
>
> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20200909/382e6f7e/attachment.htm>