[libcamera-devel,GIT,PULL] : Raspberry Pi: IMX296 mono fixes
mbox series

Message ID CAEmqJPpo8OFGxT+1Pn71bX1LUhA0=xbO-BxuYgrnZy20=HwYCw@mail.gmail.com
State Accepted
Headers show
Series
  • [libcamera-devel,GIT,PULL] : Raspberry Pi: IMX296 mono fixes
Related show

Pull-request

https://github.com/naushir/libcamera.git

Message

Naushir Patuck July 10, 2023, 1:13 p.m. UTC 
The following changes since commit 7802471a3badb561f1e017c3ecdafc95a0a43d61:

  pipeline: rpi: Account for Bayer packing when validating format
(2023-07-07 11:39:34 +0300)

are available in the Git repository at:

  https://github.com/naushir/libcamera.git

for you to fetch changes up to e3ecd71894399b26e80d26b29078c82e6bfc5c63:

  ipa: rpi: imx296_mono: Disable all colour shading (2023-07-10 14:10:11 +0100)

----------------------------------------------------------------
Naushir Patuck (2):
      ipa: rpi: imx708: Fix mode switch drop frame count
      ipa: rpi: imx296_mono: Disable all colour shading

 src/ipa/rpi/cam_helper/cam_helper_imx708.cpp | 25 ++++++++++++++++++++++++-
 src/ipa/rpi/vc4/data/imx296_mono.json        | 48
++++++++++++++++++++++++------------------------
 2 files changed, 48 insertions(+), 25 deletions(-)

Comments

Kieran Bingham July 10, 2023, 1:18 p.m. UTC | #1 
Quoting Naushir Patuck (2023-07-10 14:13:56)
> The following changes since commit 7802471a3badb561f1e017c3ecdafc95a0a43d61:
> 
>   pipeline: rpi: Account for Bayer packing when validating format
> (2023-07-07 11:39:34 +0300)
> 
> are available in the Git repository at:
> 
>   https://github.com/naushir/libcamera.git
> 
> for you to fetch changes up to e3ecd71894399b26e80d26b29078c82e6bfc5c63:
> 
>   ipa: rpi: imx296_mono: Disable all colour shading (2023-07-10 14:10:11 +0100)
> 
> ----------------------------------------------------------------
> Naushir Patuck (2):
>       ipa: rpi: imx708: Fix mode switch drop frame count

clang++-11 -Isrc/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p -Isrc/ipa/rpi/cam_helper -I../../../src/libcamera/src/ipa/rpi/cam_helper -Isrc/ipa/rpi -I../../../src/libcamera/src/ipa/rpi -Iinclude -I../../../src/libcamera/include -Iinclude/libcamera/ipa -Iinclude/libcamera -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O0 -g -Wextra-semi -Wthread-safety -Wshadow -include /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-11/config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o -MF src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o.d -o src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o -c ../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp
../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp:62:15: error: 'hideFramesStartup' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override]
        unsigned int hideFramesStartup() const;
                     ^
../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper.h:98:23: note: overridden virtual function is here
        virtual unsigned int hideFramesStartup() const;
                             ^
1 error generated.

Means I can't do a direct git pull on this.

I'll fix up and merge.

--
Regards

Kieran


>       ipa: rpi: imx296_mono: Disable all colour shading
> 
>  src/ipa/rpi/cam_helper/cam_helper_imx708.cpp | 25 ++++++++++++++++++++++++-
>  src/ipa/rpi/vc4/data/imx296_mono.json        | 48
> ++++++++++++++++++++++++------------------------
>  2 files changed, 48 insertions(+), 25 deletions(-)
Naushir Patuck July 10, 2023, 1:19 p.m. UTC | #2 
On Mon, 10 Jul 2023 at 14:18, Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Quoting Naushir Patuck (2023-07-10 14:13:56)
> > The following changes since commit 7802471a3badb561f1e017c3ecdafc95a0a43d61:
> >
> >   pipeline: rpi: Account for Bayer packing when validating format
> > (2023-07-07 11:39:34 +0300)
> >
> > are available in the Git repository at:
> >
> >   https://github.com/naushir/libcamera.git
> >
> > for you to fetch changes up to e3ecd71894399b26e80d26b29078c82e6bfc5c63:
> >
> >   ipa: rpi: imx296_mono: Disable all colour shading (2023-07-10 14:10:11 +0100)
> >
> > ----------------------------------------------------------------
> > Naushir Patuck (2):
> >       ipa: rpi: imx708: Fix mode switch drop frame count
>
> clang++-11 -Isrc/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p -Isrc/ipa/rpi/cam_helper -I../../../src/libcamera/src/ipa/rpi/cam_helper -Isrc/ipa/rpi -I../../../src/libcamera/src/ipa/rpi -Iinclude -I../../../src/libcamera/include -Iinclude/libcamera/ipa -Iinclude/libcamera -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O0 -g -Wextra-semi -Wthread-safety -Wshadow -include /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-11/config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o -MF src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o.d -o src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o -c ../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp
> ../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp:62:15: error: 'hideFramesStartup' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override]
>         unsigned int hideFramesStartup() const;
>                      ^
> ../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper.h:98:23: note: overridden virtual function is here
>         virtual unsigned int hideFramesStartup() const;
>                              ^
> 1 error generated.
>
> Means I can't do a direct git pull on this.
>
> I'll fix up and merge.

Argh, you did warn me about this as well :(
Thanks for fixing it!

Naush


>
> --
> Regards
>
> Kieran
>
>
> >       ipa: rpi: imx296_mono: Disable all colour shading
> >
> >  src/ipa/rpi/cam_helper/cam_helper_imx708.cpp | 25 ++++++++++++++++++++++++-
> >  src/ipa/rpi/vc4/data/imx296_mono.json        | 48
> > ++++++++++++++++++++++++------------------------
> >  2 files changed, 48 insertions(+), 25 deletions(-)
Kieran Bingham July 10, 2023, 1:23 p.m. UTC | #3 
Quoting Naushir Patuck (2023-07-10 14:19:04)
> On Mon, 10 Jul 2023 at 14:18, Kieran Bingham
> <kieran.bingham@ideasonboard.com> wrote:
> >
> > Quoting Naushir Patuck (2023-07-10 14:13:56)
> > > The following changes since commit 7802471a3badb561f1e017c3ecdafc95a0a43d61:
> > >
> > >   pipeline: rpi: Account for Bayer packing when validating format
> > > (2023-07-07 11:39:34 +0300)
> > >
> > > are available in the Git repository at:
> > >
> > >   https://github.com/naushir/libcamera.git
> > >
> > > for you to fetch changes up to e3ecd71894399b26e80d26b29078c82e6bfc5c63:
> > >
> > >   ipa: rpi: imx296_mono: Disable all colour shading (2023-07-10 14:10:11 +0100)
> > >
> > > ----------------------------------------------------------------
> > > Naushir Patuck (2):
> > >       ipa: rpi: imx708: Fix mode switch drop frame count
> >
> > clang++-11 -Isrc/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p -Isrc/ipa/rpi/cam_helper -I../../../src/libcamera/src/ipa/rpi/cam_helper -Isrc/ipa/rpi -I../../../src/libcamera/src/ipa/rpi -Iinclude -I../../../src/libcamera/include -Iinclude/libcamera/ipa -Iinclude/libcamera -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O0 -g -Wextra-semi -Wthread-safety -Wshadow -include /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-11/config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o -MF src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o.d -o src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o -c ../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp
> > ../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp:62:15: error: 'hideFramesStartup' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override]
> >         unsigned int hideFramesStartup() const;
> >                      ^
> > ../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper.h:98:23: note: overridden virtual function is here
> >         virtual unsigned int hideFramesStartup() const;
> >                              ^
> > 1 error generated.
> >
> > Means I can't do a direct git pull on this.
> >
> > I'll fix up and merge.
> 
> Argh, you did warn me about this as well :(
> Thanks for fixing it!

No worries. Merged!

--
Kieran


> 
> Naush
> 
> 
> >
> > --
> > Regards
> >
> > Kieran
> >
> >
> > >       ipa: rpi: imx296_mono: Disable all colour shading
> > >
> > >  src/ipa/rpi/cam_helper/cam_helper_imx708.cpp | 25 ++++++++++++++++++++++++-
> > >  src/ipa/rpi/vc4/data/imx296_mono.json        | 48
> > > ++++++++++++++++++++++++------------------------
> > >  2 files changed, 48 insertions(+), 25 deletions(-)
Naushir Patuck July 10, 2023, 1:25 p.m. UTC | #4 
On Mon, 10 Jul 2023 at 14:23, Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Quoting Naushir Patuck (2023-07-10 14:19:04)
> > On Mon, 10 Jul 2023 at 14:18, Kieran Bingham
> > <kieran.bingham@ideasonboard.com> wrote:
> > >
> > > Quoting Naushir Patuck (2023-07-10 14:13:56)
> > > > The following changes since commit 7802471a3badb561f1e017c3ecdafc95a0a43d61:
> > > >
> > > >   pipeline: rpi: Account for Bayer packing when validating format
> > > > (2023-07-07 11:39:34 +0300)
> > > >
> > > > are available in the Git repository at:
> > > >
> > > >   https://github.com/naushir/libcamera.git
> > > >
> > > > for you to fetch changes up to e3ecd71894399b26e80d26b29078c82e6bfc5c63:
> > > >
> > > >   ipa: rpi: imx296_mono: Disable all colour shading (2023-07-10 14:10:11 +0100)
> > > >
> > > > ----------------------------------------------------------------
> > > > Naushir Patuck (2):
> > > >       ipa: rpi: imx708: Fix mode switch drop frame count
> > >
> > > clang++-11 -Isrc/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p -Isrc/ipa/rpi/cam_helper -I../../../src/libcamera/src/ipa/rpi/cam_helper -Isrc/ipa/rpi -I../../../src/libcamera/src/ipa/rpi -Iinclude -I../../../src/libcamera/include -Iinclude/libcamera/ipa -Iinclude/libcamera -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O0 -g -Wextra-semi -Wthread-safety -Wshadow -include /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-11/config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o -MF src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o.d -o src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o -c ../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp
> > > ../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp:62:15: error: 'hideFramesStartup' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override]
> > >         unsigned int hideFramesStartup() const;
> > >                      ^
> > > ../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper.h:98:23: note: overridden virtual function is here
> > >         virtual unsigned int hideFramesStartup() const;
> > >                              ^
> > > 1 error generated.
> > >
> > > Means I can't do a direct git pull on this.
> > >
> > > I'll fix up and merge.
> >
> > Argh, you did warn me about this as well :(
> > Thanks for fixing it!
>
> No worries. Merged!

You'll be happy to learn that I've now switched to clang as my default compiler
so this will never happen again ;-)

>
> --
> Kieran
>
>
> >
> > Naush
> >
> >
> > >
> > > --
> > > Regards
> > >
> > > Kieran
> > >
> > >
> > > >       ipa: rpi: imx296_mono: Disable all colour shading
> > > >
> > > >  src/ipa/rpi/cam_helper/cam_helper_imx708.cpp | 25 ++++++++++++++++++++++++-
> > > >  src/ipa/rpi/vc4/data/imx296_mono.json        | 48
> > > > ++++++++++++++++++++++++------------------------
> > > >  2 files changed, 48 insertions(+), 25 deletions(-)
Laurent Pinchart July 12, 2023, 9:29 p.m. UTC | #5 
On Mon, Jul 10, 2023 at 02:25:09PM +0100, Naushir Patuck via libcamera-devel wrote:
> On Mon, 10 Jul 2023 at 14:23, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote:
> > Quoting Naushir Patuck (2023-07-10 14:19:04)
> > > On Mon, 10 Jul 2023 at 14:18, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote:
> > > > Quoting Naushir Patuck (2023-07-10 14:13:56)
> > > > > The following changes since commit 7802471a3badb561f1e017c3ecdafc95a0a43d61:
> > > > >
> > > > >   pipeline: rpi: Account for Bayer packing when validating format
> > > > > (2023-07-07 11:39:34 +0300)
> > > > >
> > > > > are available in the Git repository at:
> > > > >
> > > > >   https://github.com/naushir/libcamera.git
> > > > >
> > > > > for you to fetch changes up to e3ecd71894399b26e80d26b29078c82e6bfc5c63:
> > > > >
> > > > >   ipa: rpi: imx296_mono: Disable all colour shading (2023-07-10 14:10:11 +0100)
> > > > >
> > > > > ----------------------------------------------------------------
> > > > > Naushir Patuck (2):
> > > > >       ipa: rpi: imx708: Fix mode switch drop frame count
> > > >
> > > > clang++-11 -Isrc/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p -Isrc/ipa/rpi/cam_helper -I../../../src/libcamera/src/ipa/rpi/cam_helper -Isrc/ipa/rpi -I../../../src/libcamera/src/ipa/rpi -Iinclude -I../../../src/libcamera/include -Iinclude/libcamera/ipa -Iinclude/libcamera -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O0 -g -Wextra-semi -Wthread-safety -Wshadow -include /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-11/config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o -MF src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o.d -o src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o -c ../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp
> > > > ../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp:62:15: error: 'hideFramesStartup' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override]
> > > >         unsigned int hideFramesStartup() const;
> > > >                      ^
> > > > ../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper.h:98:23: note: overridden virtual function is here
> > > >         virtual unsigned int hideFramesStartup() const;
> > > >                              ^
> > > > 1 error generated.
> > > >
> > > > Means I can't do a direct git pull on this.
> > > >
> > > > I'll fix up and merge.
> > >
> > > Argh, you did warn me about this as well :(
> > > Thanks for fixing it!
> >
> > No worries. Merged!
> 
> You'll be happy to learn that I've now switched to clang as my default compiler
> so this will never happen again ;-)

But you may then not notice gcc-specific issues :-) Would you be able to
compile-test with both compilers before sending patches ?

> > > > >       ipa: rpi: imx296_mono: Disable all colour shading
> > > > >
> > > > >  src/ipa/rpi/cam_helper/cam_helper_imx708.cpp | 25 ++++++++++++++++++++++++-
> > > > >  src/ipa/rpi/vc4/data/imx296_mono.json        | 48 ++++++++++++++++++++++++------------------------
> > > > >  2 files changed, 48 insertions(+), 25 deletions(-)
Naushir Patuck July 13, 2023, 7:02 a.m. UTC | #6 
On Wed, 12 Jul 2023 at 22:29, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Mon, Jul 10, 2023 at 02:25:09PM +0100, Naushir Patuck via libcamera-devel wrote:
> > On Mon, 10 Jul 2023 at 14:23, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote:
> > > Quoting Naushir Patuck (2023-07-10 14:19:04)
> > > > On Mon, 10 Jul 2023 at 14:18, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote:
> > > > > Quoting Naushir Patuck (2023-07-10 14:13:56)
> > > > > > The following changes since commit 7802471a3badb561f1e017c3ecdafc95a0a43d61:
> > > > > >
> > > > > >   pipeline: rpi: Account for Bayer packing when validating format
> > > > > > (2023-07-07 11:39:34 +0300)
> > > > > >
> > > > > > are available in the Git repository at:
> > > > > >
> > > > > >   https://github.com/naushir/libcamera.git
> > > > > >
> > > > > > for you to fetch changes up to e3ecd71894399b26e80d26b29078c82e6bfc5c63:
> > > > > >
> > > > > >   ipa: rpi: imx296_mono: Disable all colour shading (2023-07-10 14:10:11 +0100)
> > > > > >
> > > > > > ----------------------------------------------------------------
> > > > > > Naushir Patuck (2):
> > > > > >       ipa: rpi: imx708: Fix mode switch drop frame count
> > > > >
> > > > > clang++-11 -Isrc/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p -Isrc/ipa/rpi/cam_helper -I../../../src/libcamera/src/ipa/rpi/cam_helper -Isrc/ipa/rpi -I../../../src/libcamera/src/ipa/rpi -Iinclude -I../../../src/libcamera/include -Iinclude/libcamera/ipa -Iinclude/libcamera -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O0 -g -Wextra-semi -Wthread-safety -Wshadow -include /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-11/config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o -MF src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o.d -o src/ipa/rpi/cam_helper/librpi_ipa_cam_helper.a.p/cam_helper_imx708.cpp.o -c ../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp
> > > > > ../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp:62:15: error: 'hideFramesStartup' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override]
> > > > >         unsigned int hideFramesStartup() const;
> > > > >                      ^
> > > > > ../../../src/libcamera/src/ipa/rpi/cam_helper/cam_helper.h:98:23: note: overridden virtual function is here
> > > > >         virtual unsigned int hideFramesStartup() const;
> > > > >                              ^
> > > > > 1 error generated.
> > > > >
> > > > > Means I can't do a direct git pull on this.
> > > > >
> > > > > I'll fix up and merge.
> > > >
> > > > Argh, you did warn me about this as well :(
> > > > Thanks for fixing it!
> > >
> > > No worries. Merged!
> >
> > You'll be happy to learn that I've now switched to clang as my default compiler
> > so this will never happen again ;-)
>
> But you may then not notice gcc-specific issues :-) Would you be able to
> compile-test with both compilers before sending patches ?

Yes, I'll keep gcc as a compile test step.
It seems like clang is so much more picky compared to gcc on compiler warnings,
it seems sensible to use it as the default.

Regards,
Naush


>
> > > > > >       ipa: rpi: imx296_mono: Disable all colour shading
> > > > > >
> > > > > >  src/ipa/rpi/cam_helper/cam_helper_imx708.cpp | 25 ++++++++++++++++++++++++-
> > > > > >  src/ipa/rpi/vc4/data/imx296_mono.json        | 48 ++++++++++++++++++++++++------------------------
> > > > > >  2 files changed, 48 insertions(+), 25 deletions(-)
>
> --
> Regards,
>
> Laurent Pinchart