| Message ID | 20191007085842.7608-1-jacopo@jmondi.org |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patches. On Mon, Oct 07, 2019 at 10:58:37AM +0200, Jacopo Mondi wrote: > With the small changes in patches 4 and 5: > - Define the FIFO path as a macro in 4/5 > - Adjust path printout in error paths in 5/5 > > I would push this version. So would I, but the new test fails on my system :-S Could you double-check on your side ? If you get no failure I'll investigate. > Jacopo Mondi (5): > ipa: vimc: Rename ipa_dummy to ipa_vimc > test: ipa: Rename the ipa_test to ipa_module_test > ipa: meson: Give IPAs access to internal libcamera APIs > ipa: vimc: Add support for tracing operations > test: ipa: Add test for the IPA Interface > > include/ipa/ipa_vimc.h | 22 +++ > src/ipa/ipa_dummy.cpp | 46 ------ > src/ipa/ipa_vimc.cpp | 112 ++++++++++++++ > src/ipa/meson.build | 18 ++- > test/ipa/ipa_interface_test.cpp | 142 ++++++++++++++++++ > .../ipa/{ipa_test.cpp => ipa_module_test.cpp} | 4 +- > test/ipa/meson.build | 3 +- > 7 files changed, 292 insertions(+), 55 deletions(-) > create mode 100644 include/ipa/ipa_vimc.h > delete mode 100644 src/ipa/ipa_dummy.cpp > create mode 100644 src/ipa/ipa_vimc.cpp > create mode 100644 test/ipa/ipa_interface_test.cpp > rename test/ipa/{ipa_test.cpp => ipa_module_test.cpp} (92%)
On Tue, Oct 08, 2019 at 04:49:50AM +0300, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patches. > > On Mon, Oct 07, 2019 at 10:58:37AM +0200, Jacopo Mondi wrote: > > With the small changes in patches 4 and 5: > > - Define the FIFO path as a macro in 4/5 > > - Adjust path printout in error paths in 5/5 > > > > I would push this version. > > So would I, but the new test fails on my system :-S Could you Oh really ? > double-check on your side ? If you get no failure I'll investigate. it works here.. I have a build failure with gcc which I should fix when applying but the test is ok diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp index 83eef7440fa4..cac6efeaf3ff 100644 --- a/test/ipa/ipa_interface_test.cpp +++ b/test/ipa/ipa_interface_test.cpp @@ -129,7 +129,7 @@ protected: private: void readFifo(EventNotifier *notifier) { - size_t s = read(notifier->fd(), &data_, 1); + ssize_t s = read(notifier->fd(), &data_, 1); if (s < 0) { cerr << "Failed to to read from IPA test fifo at: " << ipaFifoPath << strerror(errno) << endl; Could someone else try this ? Thanks j > > > Jacopo Mondi (5): > > ipa: vimc: Rename ipa_dummy to ipa_vimc > > test: ipa: Rename the ipa_test to ipa_module_test > > ipa: meson: Give IPAs access to internal libcamera APIs > > ipa: vimc: Add support for tracing operations > > test: ipa: Add test for the IPA Interface > > > > include/ipa/ipa_vimc.h | 22 +++ > > src/ipa/ipa_dummy.cpp | 46 ------ > > src/ipa/ipa_vimc.cpp | 112 ++++++++++++++ > > src/ipa/meson.build | 18 ++- > > test/ipa/ipa_interface_test.cpp | 142 ++++++++++++++++++ > > .../ipa/{ipa_test.cpp => ipa_module_test.cpp} | 4 +- > > test/ipa/meson.build | 3 +- > > 7 files changed, 292 insertions(+), 55 deletions(-) > > create mode 100644 include/ipa/ipa_vimc.h > > delete mode 100644 src/ipa/ipa_dummy.cpp > > create mode 100644 src/ipa/ipa_vimc.cpp > > create mode 100644 test/ipa/ipa_interface_test.cpp > > rename test/ipa/{ipa_test.cpp => ipa_module_test.cpp} (92%) > > -- > Regards, > > Laurent Pinchart
Hi again, On Tue, Oct 08, 2019 at 10:39:36AM +0200, Jacopo Mondi wrote: > On Tue, Oct 08, 2019 at 04:49:50AM +0300, Laurent Pinchart wrote: > > Hi Jacopo, > > > > Thank you for the patches. > > > > On Mon, Oct 07, 2019 at 10:58:37AM +0200, Jacopo Mondi wrote: > > > With the small changes in patches 4 and 5: > > > - Define the FIFO path as a macro in 4/5 > > > - Adjust path printout in error paths in 5/5 > > > > > > I would push this version. > > > > So would I, but the new test fails on my system :-S Could you > > Oh really ? > > > double-check on your side ? If you get no failure I'll investigate. > > it works here.. I have a build failure with gcc which I should fix > when applying but the test is ok > > diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp > index 83eef7440fa4..cac6efeaf3ff 100644 > --- a/test/ipa/ipa_interface_test.cpp > +++ b/test/ipa/ipa_interface_test.cpp > @@ -129,7 +129,7 @@ protected: > private: > void readFifo(EventNotifier *notifier) > { > - size_t s = read(notifier->fd(), &data_, 1); > + ssize_t s = read(notifier->fd(), &data_, 1); > if (s < 0) { > cerr << "Failed to to read from IPA test fifo at: " > << ipaFifoPath << strerror(errno) << endl; > Scratch this, I was testing a previous version :/ > Could someone else try this ? I now see it failing, but only when building with gcc o_0 I'll investigate, my bad I only tested the clang compiled version... > > Thanks > j > > > > > > Jacopo Mondi (5): > > > ipa: vimc: Rename ipa_dummy to ipa_vimc > > > test: ipa: Rename the ipa_test to ipa_module_test > > > ipa: meson: Give IPAs access to internal libcamera APIs > > > ipa: vimc: Add support for tracing operations > > > test: ipa: Add test for the IPA Interface > > > > > > include/ipa/ipa_vimc.h | 22 +++ > > > src/ipa/ipa_dummy.cpp | 46 ------ > > > src/ipa/ipa_vimc.cpp | 112 ++++++++++++++ > > > src/ipa/meson.build | 18 ++- > > > test/ipa/ipa_interface_test.cpp | 142 ++++++++++++++++++ > > > .../ipa/{ipa_test.cpp => ipa_module_test.cpp} | 4 +- > > > test/ipa/meson.build | 3 +- > > > 7 files changed, 292 insertions(+), 55 deletions(-) > > > create mode 100644 include/ipa/ipa_vimc.h > > > delete mode 100644 src/ipa/ipa_dummy.cpp > > > create mode 100644 src/ipa/ipa_vimc.cpp > > > create mode 100644 test/ipa/ipa_interface_test.cpp > > > rename test/ipa/{ipa_test.cpp => ipa_module_test.cpp} (92%) > > > > -- > > Regards, > > > > Laurent Pinchart > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi, -again- On Tue, Oct 08, 2019 at 10:39:36AM +0200, Jacopo Mondi wrote: > On Tue, Oct 08, 2019 at 04:49:50AM +0300, Laurent Pinchart wrote: > > Hi Jacopo, > > > > Thank you for the patches. > > > > On Mon, Oct 07, 2019 at 10:58:37AM +0200, Jacopo Mondi wrote: > > > With the small changes in patches 4 and 5: > > > - Define the FIFO path as a macro in 4/5 > > > - Adjust path printout in error paths in 5/5 > > > > > > I would push this version. > > > > So would I, but the new test fails on my system :-S Could you > > Oh really ? Just remove the stale dummy_ipa binary from your build directory, or it gets picked before the new ipa_vimc one :) I feel like I could push this, but I'll wait for you to test again. Thanks j > > > double-check on your side ? If you get no failure I'll investigate. > > it works here.. I have a build failure with gcc which I should fix > when applying but the test is ok > > diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp > index 83eef7440fa4..cac6efeaf3ff 100644 > --- a/test/ipa/ipa_interface_test.cpp > +++ b/test/ipa/ipa_interface_test.cpp > @@ -129,7 +129,7 @@ protected: > private: > void readFifo(EventNotifier *notifier) > { > - size_t s = read(notifier->fd(), &data_, 1); > + ssize_t s = read(notifier->fd(), &data_, 1); > if (s < 0) { > cerr << "Failed to to read from IPA test fifo at: " > << ipaFifoPath << strerror(errno) << endl; > > Could someone else try this ? > > Thanks > j > > > > > > Jacopo Mondi (5): > > > ipa: vimc: Rename ipa_dummy to ipa_vimc > > > test: ipa: Rename the ipa_test to ipa_module_test > > > ipa: meson: Give IPAs access to internal libcamera APIs > > > ipa: vimc: Add support for tracing operations > > > test: ipa: Add test for the IPA Interface > > > > > > include/ipa/ipa_vimc.h | 22 +++ > > > src/ipa/ipa_dummy.cpp | 46 ------ > > > src/ipa/ipa_vimc.cpp | 112 ++++++++++++++ > > > src/ipa/meson.build | 18 ++- > > > test/ipa/ipa_interface_test.cpp | 142 ++++++++++++++++++ > > > .../ipa/{ipa_test.cpp => ipa_module_test.cpp} | 4 +- > > > test/ipa/meson.build | 3 +- > > > 7 files changed, 292 insertions(+), 55 deletions(-) > > > create mode 100644 include/ipa/ipa_vimc.h > > > delete mode 100644 src/ipa/ipa_dummy.cpp > > > create mode 100644 src/ipa/ipa_vimc.cpp > > > create mode 100644 test/ipa/ipa_interface_test.cpp > > > rename test/ipa/{ipa_test.cpp => ipa_module_test.cpp} (92%) > > > > -- > > Regards, > > > > Laurent Pinchart > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On Tue, Oct 08, 2019 at 10:48:21AM +0200, Jacopo Mondi wrote: > On Tue, Oct 08, 2019 at 10:39:36AM +0200, Jacopo Mondi wrote: > > On Tue, Oct 08, 2019 at 04:49:50AM +0300, Laurent Pinchart wrote: > >> On Mon, Oct 07, 2019 at 10:58:37AM +0200, Jacopo Mondi wrote: > >>> With the small changes in patches 4 and 5: > >>> - Define the FIFO path as a macro in 4/5 > >>> - Adjust path printout in error paths in 5/5 > >>> > >>> I would push this version. > >> > >> So would I, but the new test fails on my system :-S Could you > > > > Oh really ? > > Just remove the stale dummy_ipa binary from your build directory, or > it gets picked before the new ipa_vimc one :) > > I feel like I could push this, but I'll wait for you to test again. Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Please push. > >> double-check on your side ? If you get no failure I'll investigate. > > > > it works here.. I have a build failure with gcc which I should fix > > when applying but the test is ok > > > > diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp > > index 83eef7440fa4..cac6efeaf3ff 100644 > > --- a/test/ipa/ipa_interface_test.cpp > > +++ b/test/ipa/ipa_interface_test.cpp > > @@ -129,7 +129,7 @@ protected: > > private: > > void readFifo(EventNotifier *notifier) > > { > > - size_t s = read(notifier->fd(), &data_, 1); > > + ssize_t s = read(notifier->fd(), &data_, 1); > > if (s < 0) { > > cerr << "Failed to to read from IPA test fifo at: " > > << ipaFifoPath << strerror(errno) << endl; > > > > Could someone else try this ? > > > >>> Jacopo Mondi (5): > >>> ipa: vimc: Rename ipa_dummy to ipa_vimc > >>> test: ipa: Rename the ipa_test to ipa_module_test > >>> ipa: meson: Give IPAs access to internal libcamera APIs > >>> ipa: vimc: Add support for tracing operations > >>> test: ipa: Add test for the IPA Interface > >>> > >>> include/ipa/ipa_vimc.h | 22 +++ > >>> src/ipa/ipa_dummy.cpp | 46 ------ > >>> src/ipa/ipa_vimc.cpp | 112 ++++++++++++++ > >>> src/ipa/meson.build | 18 ++- > >>> test/ipa/ipa_interface_test.cpp | 142 ++++++++++++++++++ > >>> .../ipa/{ipa_test.cpp => ipa_module_test.cpp} | 4 +- > >>> test/ipa/meson.build | 3 +- > >>> 7 files changed, 292 insertions(+), 55 deletions(-) > >>> create mode 100644 include/ipa/ipa_vimc.h > >>> delete mode 100644 src/ipa/ipa_dummy.cpp > >>> create mode 100644 src/ipa/ipa_vimc.cpp > >>> create mode 100644 test/ipa/ipa_interface_test.cpp > >>> rename test/ipa/{ipa_test.cpp => ipa_module_test.cpp} (92%)
Hi Laurent, On Tue, Oct 08, 2019 at 01:26:22PM +0300, Laurent Pinchart wrote: > Hi Jacopo, > > On Tue, Oct 08, 2019 at 10:48:21AM +0200, Jacopo Mondi wrote: > > On Tue, Oct 08, 2019 at 10:39:36AM +0200, Jacopo Mondi wrote: > > > On Tue, Oct 08, 2019 at 04:49:50AM +0300, Laurent Pinchart wrote: > > >> On Mon, Oct 07, 2019 at 10:58:37AM +0200, Jacopo Mondi wrote: > > >>> With the small changes in patches 4 and 5: > > >>> - Define the FIFO path as a macro in 4/5 > > >>> - Adjust path printout in error paths in 5/5 > > >>> > > >>> I would push this version. > > >> > > >> So would I, but the new test fails on my system :-S Could you > > > > > > Oh really ? > > > > Just remove the stale dummy_ipa binary from your build directory, or > > it gets picked before the new ipa_vimc one :) > > > > I feel like I could push this, but I'll wait for you to test again. > > Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Please push. Now pushed with your tag Thanks j > > > >> double-check on your side ? If you get no failure I'll investigate. > > > > > > it works here.. I have a build failure with gcc which I should fix > > > when applying but the test is ok > > > > > > diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp > > > index 83eef7440fa4..cac6efeaf3ff 100644 > > > --- a/test/ipa/ipa_interface_test.cpp > > > +++ b/test/ipa/ipa_interface_test.cpp > > > @@ -129,7 +129,7 @@ protected: > > > private: > > > void readFifo(EventNotifier *notifier) > > > { > > > - size_t s = read(notifier->fd(), &data_, 1); > > > + ssize_t s = read(notifier->fd(), &data_, 1); > > > if (s < 0) { > > > cerr << "Failed to to read from IPA test fifo at: " > > > << ipaFifoPath << strerror(errno) << endl; > > > > > > Could someone else try this ? > > > > > >>> Jacopo Mondi (5): > > >>> ipa: vimc: Rename ipa_dummy to ipa_vimc > > >>> test: ipa: Rename the ipa_test to ipa_module_test > > >>> ipa: meson: Give IPAs access to internal libcamera APIs > > >>> ipa: vimc: Add support for tracing operations > > >>> test: ipa: Add test for the IPA Interface > > >>> > > >>> include/ipa/ipa_vimc.h | 22 +++ > > >>> src/ipa/ipa_dummy.cpp | 46 ------ > > >>> src/ipa/ipa_vimc.cpp | 112 ++++++++++++++ > > >>> src/ipa/meson.build | 18 ++- > > >>> test/ipa/ipa_interface_test.cpp | 142 ++++++++++++++++++ > > >>> .../ipa/{ipa_test.cpp => ipa_module_test.cpp} | 4 +- > > >>> test/ipa/meson.build | 3 +- > > >>> 7 files changed, 292 insertions(+), 55 deletions(-) > > >>> create mode 100644 include/ipa/ipa_vimc.h > > >>> delete mode 100644 src/ipa/ipa_dummy.cpp > > >>> create mode 100644 src/ipa/ipa_vimc.cpp > > >>> create mode 100644 test/ipa/ipa_interface_test.cpp > > >>> rename test/ipa/{ipa_test.cpp => ipa_module_test.cpp} (92%) > > -- > Regards, > > Laurent Pinchart
With the small changes in patches 4 and 5: - Define the FIFO path as a macro in 4/5 - Adjust path printout in error paths in 5/5 I would push this version. Jacopo Mondi (5): ipa: vimc: Rename ipa_dummy to ipa_vimc test: ipa: Rename the ipa_test to ipa_module_test ipa: meson: Give IPAs access to internal libcamera APIs ipa: vimc: Add support for tracing operations test: ipa: Add test for the IPA Interface include/ipa/ipa_vimc.h | 22 +++ src/ipa/ipa_dummy.cpp | 46 ------ src/ipa/ipa_vimc.cpp | 112 ++++++++++++++ src/ipa/meson.build | 18 ++- test/ipa/ipa_interface_test.cpp | 142 ++++++++++++++++++ .../ipa/{ipa_test.cpp => ipa_module_test.cpp} | 4 +- test/ipa/meson.build | 3 +- 7 files changed, 292 insertions(+), 55 deletions(-) create mode 100644 include/ipa/ipa_vimc.h delete mode 100644 src/ipa/ipa_dummy.cpp create mode 100644 src/ipa/ipa_vimc.cpp create mode 100644 test/ipa/ipa_interface_test.cpp rename test/ipa/{ipa_test.cpp => ipa_module_test.cpp} (92%) -- 2.23.0