[v2,2/9] test: ipa: libipa: Add tets for Interpolator
diff mbox series

Message ID 20240913075750.35115-3-stefan.klug@ideasonboard.com
State Superseded
Headers show
Series
  • Implement polynomial lsc support
Related show

Commit Message

Stefan Klug Sept. 13, 2024, 7:57 a.m. UTC 
Add tests for the Interpolator class.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 test/ipa/libipa/interpolator.cpp | 55 ++++++++++++++++++++++++++++++++
 test/ipa/libipa/meson.build      | 15 +++++++++
 test/ipa/meson.build             |  1 +
 3 files changed, 71 insertions(+)
 create mode 100644 test/ipa/libipa/interpolator.cpp
 create mode 100644 test/ipa/libipa/meson.build

Comments

Kieran Bingham Sept. 13, 2024, 9:06 a.m. UTC | #1 
Quoting Stefan Klug (2024-09-13 08:57:20)
> Add tests for the Interpolator class.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>  test/ipa/libipa/interpolator.cpp | 55 ++++++++++++++++++++++++++++++++
>  test/ipa/libipa/meson.build      | 15 +++++++++
>  test/ipa/meson.build             |  1 +
>  3 files changed, 71 insertions(+)
>  create mode 100644 test/ipa/libipa/interpolator.cpp
>  create mode 100644 test/ipa/libipa/meson.build
> 
> diff --git a/test/ipa/libipa/interpolator.cpp b/test/ipa/libipa/interpolator.cpp
> new file mode 100644
> index 000000000000..adb215386f0e
> --- /dev/null
> +++ b/test/ipa/libipa/interpolator.cpp
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2024, Paul Elder <paul.elder@ideasonboard.com>
> + *
> + * Miscellaneous utility tests
> + */
> +
> +#include <cmath>
> +#include <iostream>
> +#include <map>
> +#include <stdint.h>
> +#include <stdio.h>
> +
> +#include "../src/ipa/libipa/interpolator.h"
> +
> +#include "test.h"
> +
> +using namespace std;
> +using namespace libcamera;
> +using namespace ipa;
> +
> +#define ASSERT_EQ(a, b) \
> +       if((a) != (b)) { printf(#a  " != " #b "\n"); \
> +               return TestFail; \

We wouldn't normally hide return statements in a macro - but given the
small scope of the macro here, and how it helps improve readability, and
we're in tests- I think it's fine, and helpful



> +       }
> +
> +class InterpolatorTest : public Test
> +{
> +protected:
> +
> +       int run()
> +       {
> +               Interpolator<int> interpolator;
> +               interpolator.setData({{10,100},{20,200},{30,300}});
> +
> +               ASSERT_EQ(interpolator.getInterpolated(0), 100);
> +               ASSERT_EQ(interpolator.getInterpolated(10), 100);
> +               ASSERT_EQ(interpolator.getInterpolated(20), 200);
> +               ASSERT_EQ(interpolator.getInterpolated(25), 250);
> +               ASSERT_EQ(interpolator.getInterpolated(30), 300);
> +               ASSERT_EQ(interpolator.getInterpolated(40), 300);
> +
> +               interpolator.setQuantization(10);
> +               unsigned int q = 0;
> +               ASSERT_EQ(interpolator.getInterpolated(25, &q), 300);
> +               ASSERT_EQ(q, 30);
> +               ASSERT_EQ(interpolator.getInterpolated(24, &q), 200);
> +               ASSERT_EQ(q, 20);
> +               

nit: Delete a blank line

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> +
> +               return TestPass;
> +       }
> +};
> +
> +TEST_REGISTER(InterpolatorTest)
> diff --git a/test/ipa/libipa/meson.build b/test/ipa/libipa/meson.build
> new file mode 100644
> index 000000000000..4d2427dbd4e7
> --- /dev/null
> +++ b/test/ipa/libipa/meson.build
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +libipa_test = [
> +    {'name': 'interpolator', 'sources': ['interpolator.cpp']},
> +]
> +
> +foreach test : libipa_test
> +    exe = executable(test['name'], test['sources'],
> +                     dependencies : [libcamera_private, libipa_dep],
> +                     link_with : [test_libraries],
> +                     include_directories : [test_includes_internal,
> +                                            '../../../src/ipa/libipa/'])
> +
> +    test(test['name'], exe, suite : 'ipa')
> +endforeach
> diff --git a/test/ipa/meson.build b/test/ipa/meson.build
> index e9871aba44ee..63820de54899 100644
> --- a/test/ipa/meson.build
> +++ b/test/ipa/meson.build
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
> +subdir('libipa')
>  subdir('rkisp1')
>  
>  ipa_test = [
> -- 
> 2.43.0
>
Kieran Bingham Sept. 13, 2024, 9:06 a.m. UTC | #2 
Quoting Kieran Bingham (2024-09-13 10:06:00)
> Quoting Stefan Klug (2024-09-13 08:57:20)
> > Add tests for the Interpolator class.
> > 
> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > ---
> >  test/ipa/libipa/interpolator.cpp | 55 ++++++++++++++++++++++++++++++++
> >  test/ipa/libipa/meson.build      | 15 +++++++++
> >  test/ipa/meson.build             |  1 +
> >  3 files changed, 71 insertions(+)
> >  create mode 100644 test/ipa/libipa/interpolator.cpp
> >  create mode 100644 test/ipa/libipa/meson.build
> > 
> > diff --git a/test/ipa/libipa/interpolator.cpp b/test/ipa/libipa/interpolator.cpp
> > new file mode 100644
> > index 000000000000..adb215386f0e
> > --- /dev/null
> > +++ b/test/ipa/libipa/interpolator.cpp
> > @@ -0,0 +1,55 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2024, Paul Elder <paul.elder@ideasonboard.com>

I think you can claim this though if you prefer ;-)

> > + *
> > + * Miscellaneous utility tests
> > + */
> > +
> > +#include <cmath>
> > +#include <iostream>
> > +#include <map>
> > +#include <stdint.h>
> > +#include <stdio.h>
> > +
> > +#include "../src/ipa/libipa/interpolator.h"
> > +
> > +#include "test.h"
> > +
> > +using namespace std;
> > +using namespace libcamera;
> > +using namespace ipa;
> > +
> > +#define ASSERT_EQ(a, b) \
> > +       if((a) != (b)) { printf(#a  " != " #b "\n"); \
> > +               return TestFail; \
> 
> We wouldn't normally hide return statements in a macro - but given the
> small scope of the macro here, and how it helps improve readability, and
> we're in tests- I think it's fine, and helpful
> 
> 
> 
> > +       }
> > +
> > +class InterpolatorTest : public Test
> > +{
> > +protected:
> > +
> > +       int run()
> > +       {
> > +               Interpolator<int> interpolator;
> > +               interpolator.setData({{10,100},{20,200},{30,300}});
> > +
> > +               ASSERT_EQ(interpolator.getInterpolated(0), 100);
> > +               ASSERT_EQ(interpolator.getInterpolated(10), 100);
> > +               ASSERT_EQ(interpolator.getInterpolated(20), 200);
> > +               ASSERT_EQ(interpolator.getInterpolated(25), 250);
> > +               ASSERT_EQ(interpolator.getInterpolated(30), 300);
> > +               ASSERT_EQ(interpolator.getInterpolated(40), 300);
> > +
> > +               interpolator.setQuantization(10);
> > +               unsigned int q = 0;
> > +               ASSERT_EQ(interpolator.getInterpolated(25, &q), 300);
> > +               ASSERT_EQ(q, 30);
> > +               ASSERT_EQ(interpolator.getInterpolated(24, &q), 200);
> > +               ASSERT_EQ(q, 20);
> > +               
> 
> nit: Delete a blank line
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > +
> > +               return TestPass;
> > +       }
> > +};
> > +
> > +TEST_REGISTER(InterpolatorTest)
> > diff --git a/test/ipa/libipa/meson.build b/test/ipa/libipa/meson.build
> > new file mode 100644
> > index 000000000000..4d2427dbd4e7
> > --- /dev/null
> > +++ b/test/ipa/libipa/meson.build
> > @@ -0,0 +1,15 @@
> > +# SPDX-License-Identifier: CC0-1.0
> > +
> > +libipa_test = [
> > +    {'name': 'interpolator', 'sources': ['interpolator.cpp']},
> > +]
> > +
> > +foreach test : libipa_test
> > +    exe = executable(test['name'], test['sources'],
> > +                     dependencies : [libcamera_private, libipa_dep],
> > +                     link_with : [test_libraries],
> > +                     include_directories : [test_includes_internal,
> > +                                            '../../../src/ipa/libipa/'])
> > +
> > +    test(test['name'], exe, suite : 'ipa')
> > +endforeach
> > diff --git a/test/ipa/meson.build b/test/ipa/meson.build
> > index e9871aba44ee..63820de54899 100644
> > --- a/test/ipa/meson.build
> > +++ b/test/ipa/meson.build
> > @@ -1,5 +1,6 @@
> >  # SPDX-License-Identifier: CC0-1.0
> >  
> > +subdir('libipa')
> >  subdir('rkisp1')
> >  
> >  ipa_test = [
> > -- 
> > 2.43.0
> >
Paul Elder Sept. 13, 2024, 10:16 a.m. UTC | #3 
On Fri, Sep 13, 2024 at 09:57:20AM +0200, Stefan Klug wrote:
> Add tests for the Interpolator class.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>  test/ipa/libipa/interpolator.cpp | 55 ++++++++++++++++++++++++++++++++
>  test/ipa/libipa/meson.build      | 15 +++++++++
>  test/ipa/meson.build             |  1 +
>  3 files changed, 71 insertions(+)
>  create mode 100644 test/ipa/libipa/interpolator.cpp
>  create mode 100644 test/ipa/libipa/meson.build
> 
> diff --git a/test/ipa/libipa/interpolator.cpp b/test/ipa/libipa/interpolator.cpp
> new file mode 100644
> index 000000000000..adb215386f0e
> --- /dev/null
> +++ b/test/ipa/libipa/interpolator.cpp
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2024, Paul Elder <paul.elder@ideasonboard.com>

:D

> + *
> + * Miscellaneous utility tests
> + */
> +
> +#include <cmath>
> +#include <iostream>
> +#include <map>
> +#include <stdint.h>
> +#include <stdio.h>
> +
> +#include "../src/ipa/libipa/interpolator.h"
> +
> +#include "test.h"
> +
> +using namespace std;
> +using namespace libcamera;
> +using namespace ipa;
> +
> +#define ASSERT_EQ(a, b) \
> +	if((a) != (b)) { printf(#a  " != " #b "\n"); \

nit: newline?

> +		return TestFail; \
> +	}
> +
> +class InterpolatorTest : public Test
> +{
> +protected:
> +
> +	int run()
> +	{
> +		Interpolator<int> interpolator;
> +		interpolator.setData({{10,100},{20,200},{30,300}});

nit: spacing

Other than that, looks good to me


Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> +
> +		ASSERT_EQ(interpolator.getInterpolated(0), 100);
> +		ASSERT_EQ(interpolator.getInterpolated(10), 100);
> +		ASSERT_EQ(interpolator.getInterpolated(20), 200);
> +		ASSERT_EQ(interpolator.getInterpolated(25), 250);
> +		ASSERT_EQ(interpolator.getInterpolated(30), 300);
> +		ASSERT_EQ(interpolator.getInterpolated(40), 300);
> +
> +		interpolator.setQuantization(10);
> +		unsigned int q = 0;
> +		ASSERT_EQ(interpolator.getInterpolated(25, &q), 300);
> +		ASSERT_EQ(q, 30);
> +		ASSERT_EQ(interpolator.getInterpolated(24, &q), 200);
> +		ASSERT_EQ(q, 20);
> +		
> +
> +		return TestPass;
> +	}
> +};
> +
> +TEST_REGISTER(InterpolatorTest)
> diff --git a/test/ipa/libipa/meson.build b/test/ipa/libipa/meson.build
> new file mode 100644
> index 000000000000..4d2427dbd4e7
> --- /dev/null
> +++ b/test/ipa/libipa/meson.build
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +libipa_test = [
> +    {'name': 'interpolator', 'sources': ['interpolator.cpp']},
> +]
> +
> +foreach test : libipa_test
> +    exe = executable(test['name'], test['sources'],
> +                     dependencies : [libcamera_private, libipa_dep],
> +                     link_with : [test_libraries],
> +                     include_directories : [test_includes_internal,
> +                                            '../../../src/ipa/libipa/'])
> +
> +    test(test['name'], exe, suite : 'ipa')
> +endforeach
> diff --git a/test/ipa/meson.build b/test/ipa/meson.build
> index e9871aba44ee..63820de54899 100644
> --- a/test/ipa/meson.build
> +++ b/test/ipa/meson.build
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
> +subdir('libipa')
>  subdir('rkisp1')
>  
>  ipa_test = [
> -- 
> 2.43.0
>

Patch
diff mbox series 

diff --git a/test/ipa/libipa/interpolator.cpp b/test/ipa/libipa/interpolator.cpp
new file mode 100644
index 000000000000..adb215386f0e
--- /dev/null
+++ b/test/ipa/libipa/interpolator.cpp
@@ -0,0 +1,55 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2024, Paul Elder <paul.elder@ideasonboard.com>
+ *
+ * Miscellaneous utility tests
+ */
+
+#include <cmath>
+#include <iostream>
+#include <map>
+#include <stdint.h>
+#include <stdio.h>
+
+#include "../src/ipa/libipa/interpolator.h"
+
+#include "test.h"
+
+using namespace std;
+using namespace libcamera;
+using namespace ipa;
+
+#define ASSERT_EQ(a, b) \
+	if((a) != (b)) { printf(#a  " != " #b "\n"); \
+		return TestFail; \
+	}
+
+class InterpolatorTest : public Test
+{
+protected:
+
+	int run()
+	{
+		Interpolator<int> interpolator;
+		interpolator.setData({{10,100},{20,200},{30,300}});
+
+		ASSERT_EQ(interpolator.getInterpolated(0), 100);
+		ASSERT_EQ(interpolator.getInterpolated(10), 100);
+		ASSERT_EQ(interpolator.getInterpolated(20), 200);
+		ASSERT_EQ(interpolator.getInterpolated(25), 250);
+		ASSERT_EQ(interpolator.getInterpolated(30), 300);
+		ASSERT_EQ(interpolator.getInterpolated(40), 300);
+
+		interpolator.setQuantization(10);
+		unsigned int q = 0;
+		ASSERT_EQ(interpolator.getInterpolated(25, &q), 300);
+		ASSERT_EQ(q, 30);
+		ASSERT_EQ(interpolator.getInterpolated(24, &q), 200);
+		ASSERT_EQ(q, 20);
+		
+
+		return TestPass;
+	}
+};
+
+TEST_REGISTER(InterpolatorTest)
diff --git a/test/ipa/libipa/meson.build b/test/ipa/libipa/meson.build
new file mode 100644
index 000000000000..4d2427dbd4e7
--- /dev/null
+++ b/test/ipa/libipa/meson.build
@@ -0,0 +1,15 @@ 
+# SPDX-License-Identifier: CC0-1.0
+
+libipa_test = [
+    {'name': 'interpolator', 'sources': ['interpolator.cpp']},
+]
+
+foreach test : libipa_test
+    exe = executable(test['name'], test['sources'],
+                     dependencies : [libcamera_private, libipa_dep],
+                     link_with : [test_libraries],
+                     include_directories : [test_includes_internal,
+                                            '../../../src/ipa/libipa/'])
+
+    test(test['name'], exe, suite : 'ipa')
+endforeach
diff --git a/test/ipa/meson.build b/test/ipa/meson.build
index e9871aba44ee..63820de54899 100644
--- a/test/ipa/meson.build
+++ b/test/ipa/meson.build
@@ -1,5 +1,6 @@ 
 # SPDX-License-Identifier: CC0-1.0
 
+subdir('libipa')
 subdir('rkisp1')
 
 ipa_test = [