Integrate kwiver v2 #1818

mathstuf · 2024-01-17T15:24:23Z

Things not brought over:

rename of CMake variables (in order to do it once instead of at every merge conflict)
ffmpeg_video_input_clip needs ported

Removed parameters for format_config_block interface class. Added some back to derived class. Defining parameters in the interface will be special case requiring the use of a setter.

Update config using uncrustify --update-config -c .uncrustify.cfg and bringing the comments from the original file.

No need to include it everywhere with pluggable macros

Otherwise we get multiple definitions errors: vital/plugin_management/pluggable_macro_magic.h:41: multiple definition of `_test_opt_arg'; vital/algo/CMakeFiles/vital_algorithms.dir/algorithm.cxx.o:vital/plugin_management/pluggable_macro_magic .h:41: first defined here

Taken from https://github.com/Kitware/kwiver @7e4920796821476afb9be3b31b23791a1e1e76b6

algorithm.txx contains methods of the former (kwiver-v1) algorithm class that were using the interface name as a string

Taken from https://github.com/Kitware/kwiver @7e4920796821476afb9be3b31b23791a1e1e76b6

Brings the following from https://github.com/Kitware/kwiver @7e4920796821476afb9be3b31b23791a1e1e76b6 vital/any.h vital/exceptions.h vital/vital_types.h vital/types/color.h vital/types/geo_point.{cxx,h} vital/types/image.{cxx,h} vital/types/image_container.h vital/types/metadata.{cxx,h} vital/types/metadata_tags.h vital/types/metadata_traits.{cxx,h} vital/types/timestamp.{cxx,h} vital/types/metadata_map.h vital/types/video_raw_image.{cxx,h} vital/types/video_raw_metadata.{cxx,h} vital/types/video_settings.{cxx,h} vital/types/video_uninterpreted_data.{cxx,h} vital/util/visit.h

fixes: vision/kwiver-v2/kwiver-v2/vital/types/metadata.cxx:36:16: error: no match for ‘operator==’ (operand types are ‘const kwiver::vital::geo_polygon’ and ‘const kwiver ::vital::geo_polygon’) 36 | return lhs == rhs;

…er-v2 * commit 'f3ce9ab6b381cc44ebebad490701c6bb875d331e': Bring dump_klv

…e-kwiver-v2 * commit '95ccfcd11ac33c6046cfefeadcd4c0f08314afb1': Enable arrows/core applets

…ver-v2 * commit 'b18cacf70e7afbafce1c15ccb224f98a0fc38a58': Adapt dump_klv for new API

…grate-kwiver-v2 * commit '77f8b3dc07c347a80cf22f2d13b1da7f08cfdde8': cmake: add missing dependency for ffmpeg test

…egrate-kwiver-v2 * commit '7812ad62005505d3905755237328859c111d1064': compile: Add missing headers file in klv_util

…r-v2 * commit '91466d3e419322153ff5095f885d599855baac33': Add guide for importing kwiver algorithms

…wiver-v2 Ignore indentation changes. * commit 'ed09e8e86da1765c7bb076ee041ddd106d1b226b': Improve indentation in pluggable_macro_magic.h

…ate-kwiver-v2 * commit 'd45675ba7f5322a50cfcbb822b9e7a79e661a2c5': bring back //UNCRUST guard fix comment style in algorithm.{cxx,h}

…ver-v2 * commit 'cf9d4f14399b296cfe9cb514a5516b0b1192259f':

…kwiver-v2 * commit 'c2697c66ea52ee3e2d0555f1f48a11e1687e0727': vital_config.h: Add #define for method decorators

* commit '120dfbcf3c52c0082b0f79a1b8c7bdfa94003123': Bring rest of vital/types

…kwiver-v2 * commit '80a4b4ec830656b0355f844c15f6cb1c70ecd568': cmake: Update CMakeLists.txt for new files

…wiver-v2 * commit '4c0e5ae4bb93968934ba5c22f118cddd79ba7ab3': Bring dependencies of vital/types

…egrate-kwiver-v2 * commit 'a06f582bf3088f27729d4f08d730c02c316568b8': cmake: Update vital/types CMakeLists for new files

…e-kwiver-v2 * commit 'db38b3b1a1604cfcd65091268ec0a1e6652320b6': Add placeholders for moved files

…kwiver-v2 * commit '4a6a58c24327ed5067387c2bbabd16059019398d':

These changes had been done in `uncrustify` steps and ignored when merging. Apply their diffs.

* integrate-kwiver-v2-phase2: (175 commits) arrows/core/test: add missing link to `kwiver_algo_core` arrows/core: fix conflict resolution test_any_converter: resolve botched conflict resolution vital/algo: adapt for v2 vital: mask files not ported to v2 Add placeholders for moved files cmake: Update vital/types CMakeLists for new files Bring dependencies of vital/types cmake: Update CMakeLists.txt for new files Bring rest of vital/types vital_config.h: Add #define for method decorators sprokit: unignore the `dist` directory bring back //UNCRUST guard Improve indentation in pluggable_macro_magic.h Add guide for importing kwiver algorithms compile: Add missing headers file in klv_util cmake: add missing dependency for ffmpeg test Adapt dump_klv for new API Enable arrows/core applets Bring dump_klv ...

aronhelser

Couple of changes I needed to get this branch to configure on linux.

tools/CMakeLists.txt

vital/CMakeLists.txt

…on-adjustments FFmpeg corruption adjustments

* integrate-kwiver-v2-phase3: (177 commits) tools: fix botched CMake merge conflict cmake: remove duplicate `add_directory` arrows/core/test: add missing link to `kwiver_algo_core` arrows/core: fix conflict resolution test_any_converter: resolve botched conflict resolution vital/algo: adapt for v2 vital: mask files not ported to v2 Add placeholders for moved files cmake: Update vital/types CMakeLists for new files Bring dependencies of vital/types cmake: Update CMakeLists.txt for new files Bring rest of vital/types vital_config.h: Add #define for method decorators sprokit: unignore the `dist` directory bring back //UNCRUST guard Improve indentation in pluggable_macro_magic.h Add guide for importing kwiver algorithms compile: Add missing headers file in klv_util cmake: add missing dependency for ffmpeg test Adapt dump_klv for new API ...

aramSofthenge · 2024-02-07T11:04:52Z

arrows/core/metadata_map_io_csv.cxx

+  bool write_remaining_columns() { return parent.c_write_remaining_columns; }
+  bool write_enum_names() { return parent.c_write_enum_names; }
+  std::string names_string() { return parent.c_column_names; }


Why not define these functions as const?

Indeed this and similar methods can be const and just call the getter parent.get_write_enum_names.

This PR is integration; further PRs to improve const-correctness (clang-tidy can help here too) would be good.

aramSofthenge · 2024-02-07T11:06:41Z

arrows/core/metadata_map_io_csv.cxx

  std::vector< std::string > column_names;
-  std::string overrides_string;
+  std::string overrides_string() {return parent.c_column_overrides; }


Why not define these functions as const?

aramSofthenge · 2024-02-07T11:06:51Z

arrows/core/metadata_map_io_csv.cxx

  std::vector< std::string > column_overrides;
-  uint64_t every_n_microseconds{ 0 };
-  uint64_t every_n_frames{ 0 };
+  uint64_t every_n_microseconds() { return parent.c_every_n_microseconds; }


Why not define these functions as const?

aramSofthenge · 2024-02-07T11:12:08Z

arrows/core/video_input_filter.cxx

+  vital::frame_id_t c_start_at_frame() { return parent.c_start_at_frame; };
+  vital::frame_id_t c_stop_after_frame() { return parent.c_stop_after_frame;} ;
+  vital::frame_id_t c_frame_skip() { return parent.c_output_nth_frame; };
+  double c_frame_rate() { return parent.c_frame_rate; };


Why not define these functions as const?

aramSofthenge · 2024-02-07T11:12:26Z

arrows/core/video_input_filter.cxx


  // local state
  bool d_at_eov;

  // processing classes
-  vital::algo::video_input_sptr d_video_input;
+  vital::algo::video_input_sptr d_video_input() { return parent.c_video_input; };


Why not define this function as const?

aramSofthenge · 2024-02-07T12:39:40Z

arrows/ffmpeg/ffmpeg_video_input.cxx

+  const std::string& unknown_stream_behavior() { return parent.c_unknown_stream_behavior; };
+  const std::string& filter_description() { return parent.c_filter_desc;};
+  uint64_t retain_klv_duration() { return parent.c_retain_klv_duration; };
+  bool cuda_enabled() { return parent.c_cuda_enabled;} ;


Why not define this function as const?

aramSofthenge · 2024-02-07T12:55:44Z

python/kwiver/vital/config/module_config_helpers.cxx

+  catch ( kv::no_such_configuration_value_exception const& )
+  {
+    std::ostringstream sstr;
+
+    sstr << "\'" << key << "\'";
+
+    PyErr_SetString( PyExc_KeyError, sstr.str().c_str() );
+    throw py::error_already_set();
+  }


Would it be worth considering simplifying the catch block by directly concatenating the string inside PyErr_SetString()?

catch (const kv::no_such_configuration_value_exception&) { PyErr_SetString(PyExc_KeyError, ("'" + key + "'").c_str()); throw py::error_already_set(); }

aramSofthenge · 2024-02-07T12:56:14Z

python/kwiver/vital/config/module_config_helpers.cxx

+    sstr << "\'" << key << "\'";
+
+    PyErr_SetString( PyExc_KeyError, sstr.str().c_str() );
+    throw py::error_already_set();


Would it be worth considering simplifying the catch block by directly concatenating the string inside PyErr_SetString()?

catch (const kv::no_such_configuration_value_exception&) { PyErr_SetString(PyExc_KeyError, ("'" + key + "'").c_str()); throw py::error_already_set(); }

aramSofthenge · 2024-02-07T13:03:53Z

python/kwiver/vital/config/module_config_helpers.cxx

+void
+config_set_value( kwiver::vital::config_block_sptr self,
+                  kwiver::vital::config_block_key_t const&  key,
+                  kwiver::vital::config_block_key_t const&  value )


Is it possible that there's a typo in the function signature? It seems that the parameter value should be of type kwiver::vital::config_block_value_t

aramSofthenge · 2024-02-07T13:09:46Z

vital/types/set.h

+  ///
+  /// @return Shared pointer to specified element.
+  ///
+  /// @throws std::out_of_range if position is now within the range of objects


I think here it should be not instead of now

chetnieter and others added 30 commits April 4, 2023 17:15

Added setter macros to macro magic header

58aa7b0

Switched format_config_block to macro magic setup

8208747

Updated format_config_block implemementations to use macro magic

8b0da9b

Removed parameters for format_config_block interface

32b6041

Removed parameters for format_config_block interface class. Added some back to derived class. Defining parameters in the interface will be special case requiring the use of a setter.

Update .uncrustify.cfg to latest version (0.78.1)

bae7bdb

Update config using uncrustify --update-config -c .uncrustify.cfg and bringing the comments from the original file.

Apply uncrustify to whole code

7980c90

macros: Add missing namespace specifier

53fb3cb

macros: Add kwiver unique_ptr helpers

5596361

macros: always call initialize() in the constructor

6d01e54

macros: expand PLUGGABLE_VARIABLES only is variables are present

673f3e3

macros: fix test_interface classes

f6cee0e

macros: move test_interface to dedicated module

d4b8d48

No need to include it everywhere with pluggable macros

macros: Add missing type in setters

8c2d9fb

Bring kwiver/vital/algo/algorithm.[h,cxx]

f5fd07b

Taken from https://github.com/Kitware/kwiver @7e4920796821476afb9be3b31b23791a1e1e76b6

algorithm.txx: introduced templated algorithm methods based on INTERFACE

d87b62c

algorithm.txx contains methods of the former (kwiver-v1) algorithm class that were using the interface name as a string

Adapt format_config_block_plugin to new API

9eb78ce

Bring test files for vital:config_block_io test

37944a4

Taken from https://github.com/Kitware/kwiver @7e4920796821476afb9be3b31b23791a1e1e76b6

Adapt format_config_block_plugin for new pluggable API

4707583

Add config_helpers for set/get config in a uniform way

8153fcf

Adapt kwiver_applet to new API

34e2ee3

test_gtest.h bring definitions of CALL_TEST

3fbbab6

Taken from https://github.com/Kitware/kwiver @7e4920796821476afb9be3b31b23791a1e1e76b6

Bring video_input_filter for upstream

ad4ab24

Taken from https://github.com/Kitware/kwiver @7e4920796821476afb9be3b31b23791a1e1e76b6

cmake: Mirror kwiver test directory updates from kwiver-v1

d443b33

fix comment style in algorithm.{cxx,h}

bd1a06b

Adapt config_explorer for new API

ca4d9ed

Apply uncrustify to new files

d331b53

Add missing == operator in geo_polygon

903eb46

fixes: vision/kwiver-v2/kwiver-v2/vital/types/metadata.cxx:36:16: error: no match for ‘operator==’ (operand types are ‘const kwiver::vital::geo_polygon’ and ‘const kwiver ::vital::geo_polygon’) 36 | return lhs == rhs;

Replace VITAL_UNUSED with [[maybe_unused]] in vital/util/visit.h

34c9f8b

mathstuf and others added 24 commits January 12, 2024 00:27

Merge commit 'kwiver-v2-experiment-copy-dump_klv' into integrate-kwiv…

aebdf9f

…er-v2 * commit 'f3ce9ab6b381cc44ebebad490701c6bb875d331e': Bring dump_klv

Merge commit 'kwiver-v2-experiment-enable-core-applets' into integrat…

6f9ba71

…e-kwiver-v2 * commit '95ccfcd11ac33c6046cfefeadcd4c0f08314afb1': Enable arrows/core applets

Merge commit 'kwiver-v2-experiment-adapt-dump_klv' into integrate-kwi…

cee3a0f

…ver-v2 * commit 'b18cacf70e7afbafce1c15ccb224f98a0fc38a58': Adapt dump_klv for new API

Merge commit 'kwiver-v2-experiment-missing-ffmpeg-test-dep' into inte…

c1f0a95

…grate-kwiver-v2 * commit '77f8b3dc07c347a80cf22f2d13b1da7f08cfdde8': cmake: add missing dependency for ffmpeg test

Merge commit 'kwiver-v2-experiment-missing-headers-klv_util' into int…

aa26ea2

…egrate-kwiver-v2 * commit '7812ad62005505d3905755237328859c111d1064': compile: Add missing headers file in klv_util

Merge commit 'kwiver-v2-experiment-porting-docs' into integrate-kwive…

d087ddc

…r-v2 * commit '91466d3e419322153ff5095f885d599855baac33': Add guide for importing kwiver algorithms

Merge commit 'kwiver-v2-experiment-whitespace-diffs' into integrate-k…

f270e45

…wiver-v2 Ignore indentation changes. * commit 'ed09e8e86da1765c7bb076ee041ddd106d1b226b': Improve indentation in pluggable_macro_magic.h

Merge commit 'kwiver-v2-experiment-uncrustify-directives' into integr…

9b6880d

…ate-kwiver-v2 * commit 'd45675ba7f5322a50cfcbb822b9e7a79e661a2c5': bring back //UNCRUST guard fix comment style in algorithm.{cxx,h}

Merge commit 'kwiver-v2-experiment-merge-dump_klv' into integrate-kwi…

d1c0d13

…ver-v2 * commit 'cf9d4f14399b296cfe9cb514a5516b0b1192259f':

Merge commit 'kwiver-v2-experiment-method-decorators' into integrate-…

e1d76ee

…kwiver-v2 * commit 'c2697c66ea52ee3e2d0555f1f48a11e1687e0727': vital_config.h: Add #define for method decorators

Merge commit 'kwiver-v2-experiment-vital-types' into integrate-kwiver-v2

e228a33

* commit '120dfbcf3c52c0082b0f79a1b8c7bdfa94003123': Bring rest of vital/types

Merge commit 'kwiver-v2-experiment-vital-types-cmake' into integrate-…

5cdbf7b

…kwiver-v2 * commit '80a4b4ec830656b0355f844c15f6cb1c70ecd568': cmake: Update CMakeLists.txt for new files

Merge commit 'kwiver-v2-experiment-vital-types-deps' into integrate-k…

09f586e

…wiver-v2 * commit '4c0e5ae4bb93968934ba5c22f118cddd79ba7ab3': Bring dependencies of vital/types

Merge commit 'kwiver-v2-experiment-vital-types-cmake-update' into int…

696072a

…egrate-kwiver-v2 * commit 'a06f582bf3088f27729d4f08d730c02c316568b8': cmake: Update vital/types CMakeLists for new files

Merge commit 'kwiver-v2-experiment-placeholder-headers' into integrat…

dff6e53

…e-kwiver-v2 * commit 'db38b3b1a1604cfcd65091268ec0a1e6652320b6': Add placeholders for moved files

Merge commit 'kwiver-v2-experiment-vital-types-merge' into integrate-…

6336309

…kwiver-v2 * commit '4a6a58c24327ed5067387c2bbabd16059019398d':

vital: mask files not ported to v2

be77fb8

vital/algo: adapt for v2

80078fa

These changes had been done in `uncrustify` steps and ignored when merging. Apply their diffs.

test_any_converter: resolve botched conflict resolution

53cc4c9

arrows/core: fix conflict resolution

7204795

arrows/core/test: add missing link to kwiver_algo_core

c7d68b9

Ignore corrupt audio streams

f8bd1d7

Optimize performance on corrupt video

0daba15

aronhelser reviewed Jan 24, 2024

View reviewed changes

tools/CMakeLists.txt Outdated Show resolved Hide resolved

vital/CMakeLists.txt Outdated Show resolved Hide resolved

daniel-riehm and others added 4 commits January 24, 2024 18:35

Merge pull request Kitware#1817 from daniel-riehm/dev/ffmpeg-corrupti…

2ed70aa

…on-adjustments FFmpeg corruption adjustments

cmake: remove duplicate add_directory

9fac8d3

tools: fix botched CMake merge conflict

77d4aca

aramSofthenge suggested changes Feb 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate kwiver v2 #1818

Integrate kwiver v2 #1818

mathstuf commented Jan 17, 2024

aronhelser left a comment

aramSofthenge Feb 7, 2024

ChristosT Feb 8, 2024

mathstuf Feb 28, 2024

aramSofthenge Feb 7, 2024

aramSofthenge Feb 7, 2024

aramSofthenge Feb 7, 2024

aramSofthenge Feb 7, 2024

aramSofthenge Feb 7, 2024

aramSofthenge Feb 7, 2024

aramSofthenge Feb 7, 2024

aramSofthenge Feb 7, 2024

aramSofthenge Feb 7, 2024

Integrate kwiver v2 #1818

Are you sure you want to change the base?

Integrate kwiver v2 #1818

Conversation

mathstuf commented Jan 17, 2024

aronhelser left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment