Quick code cleanup

- Use references instead of pointers
- Use shared ptr for Frame
- Minor changes in config verification
- Update of clang-tidy checks

Author: Louis Fréneau

CMakeLists.txt

@@ -35,7 +35,7 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)
 set(CMAKE_CXX_EXTENSIONS OFF)
 
 if(${USE_CLANG_TIDY})
-  set(CLANG_TIDY_DEFAULT "clang-tidy;-checks=*,-llvmlibc-*,-llvm-*,-modernize-*,-fuchsia-*,-altera-*,-hicpp-use-auto,-bugprone-easily-swappable-parameters,-performance-avoid-endl,-cppcoreguidelines-pro-bounds-constant-array-index,-cppcoreguidelines-avoid-magic-numbers,-readability-magic-numbers,-readability-identifier-length,-readability-math-missing-parentheses,-readability-function-cognitive-complexity,-boost-use-ranges,-misc-non-private-member-variables-in-classes,-cppcoreguidelines-avoid-non-const-global-variables,-abseil-string-find-str-contains,-cppcoreguidelines-avoid-do-while,-readability-use-std-min-max,-google-build-using-namespace")
+  set(CLANG_TIDY_DEFAULT "clang-tidy;-checks=*,-llvmlibc-*,-llvm-*,-modernize-*,-fuchsia-*,-altera-*,-hicpp-use-auto,-bugprone-easily-swappable-parameters,-performance-avoid-endl,-cppcoreguidelines-pro-bounds-constant-array-index,-cppcoreguidelines-avoid-magic-numbers,-readability-magic-numbers,-readability-identifier-length,-readability-math-missing-parentheses,-readability-function-cognitive-complexity,-boost-use-ranges,-misc-non-private-member-variables-in-classes,-cppcoreguidelines-avoid-non-const-global-variables,-abseil-string-find-str-contains,-cppcoreguidelines-avoid-do-while,-readability-use-std-min-max,-google-build-using-namespace,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-hicpp-braces-around-statements,-readability-braces-around-statements")
 endif()
 
 # Useful for debugging

src/app/uvgVPCCencAppExample.cpp

@@ -65,12 +65,12 @@ std::binary_semaphore filled_input_slots{0};
 
 struct input_handler_args {
     // Parameters passed from main thread to input thread.
-    const cli::opts_t* opts;
+    const cli::opts_t& opts;
 
     // Picture and thread status passed from input thread to main thread.
     std::shared_ptr<uvgvpcc_enc::Frame> frame_in;
     int retval;
-    input_handler_args(const cli::opts_t* opts, std::shared_ptr<uvgvpcc_enc::Frame> frame, int retval)
+    input_handler_args(const cli::opts_t& opts, std::shared_ptr<uvgvpcc_enc::Frame> frame, int retval)
         : opts(opts), frame_in(std::move(frame)), retval(retval) {}
 };
 
@@ -93,7 +93,7 @@ void create_bytes(uint64_t value, char* dst, size_t len) {
 
 /// @brief Simple wrapper for the miniply library for parsing a .ply file.
 /// @param frame 
-void loadFrameFromPlyFile(std::shared_ptr<uvgvpcc_enc::Frame>& frame) {
+void loadFrameFromPlyFile(const std::shared_ptr<uvgvpcc_enc::Frame>& frame) {
     // uvgVPCCenc currently support only geometry of type unsigned int
     uvgvpcc_enc::Logger::log(uvgvpcc_enc::LogLevel::TRACE, "APPLICATION",
                              "Loading frame " + std::to_string(frame->frameId) + " from " + frame->pointCloudPath + "\n");
@@ -147,7 +147,7 @@ void loadFrameFromPlyFile(std::shared_ptr<uvgvpcc_enc::Frame>& frame) {
 /// @param args 
 void inputReadThread(const std::shared_ptr<input_handler_args>& args) {
     double inputReadTimerTotal = uvgvpcc_enc::p_->timerLog ? uvgvpcc_enc::global_timer.elapsed() : 0.0;
-    const cli::opts_t& appParameters = *args->opts;
+    const cli::opts_t& appParameters = args->opts;
     size_t frameId = 0;
     const size_t frameLimit = appParameters.frames * appParameters.loop_input;
     bool run = true;
@@ -315,7 +315,7 @@ int main(const int argc, const char* const argv[]) {
     }
 
     // Initialize the application input and output threads
-    const std::shared_ptr<input_handler_args> in_args = std::make_shared<input_handler_args>(&appParameters, nullptr, RETVAL_RUNNING);
+    const std::shared_ptr<input_handler_args> in_args = std::make_shared<input_handler_args>(appParameters, nullptr, RETVAL_RUNNING);
     std::thread inputTh(&inputReadThread, in_args);
     size_t frameRead = 0;
     std::shared_ptr<uvgvpcc_enc::Frame> currFrame = nullptr;

src/lib/bitstreamGeneration/atlas_context.cpp

@@ -106,7 +106,7 @@ atlas_tile_header atlas_context::create_atlas_tile_header(size_t frameIndex, siz
     return ath;
 }
 
-atlas_tile_data_unit atlas_context::create_atlas_tile_data_unit(const uvgvpcc_enc::Parameters& paramUVG, const uvgvpcc_enc::Frame& frameUVG,
+atlas_tile_data_unit atlas_context::create_atlas_tile_data_unit(const uvgvpcc_enc::Parameters& paramUVG, const std::shared_ptr<uvgvpcc_enc::Frame>& frameUVG,
                                                                 atlas_tile_header& ath) const {
     (void)paramUVG;
 
@@ -117,9 +117,9 @@ atlas_tile_data_unit atlas_context::create_atlas_tile_data_unit(const uvgvpcc_en
     const size_t levelOfDetailX = 1;  // lf addition, in TMC2 those are patch parameters. However, they are also global parameters.
     const size_t levelOfDetailY = 1;  // TODO(lf): check if those are really constant, and do not depends on other parameters
 
-    for (size_t patch_index = 0; patch_index < frameUVG.patchList.size(); ++patch_index) {
+    for (size_t patch_index = 0; patch_index < frameUVG->patchList.size(); ++patch_index) {
         // std::cout << "-- DEBUG: Creating atlas patch, index: " << patch_index << std::endl;
-        const uvgvpcc_enc::Patch& patchUVG = frameUVG.patchList[patch_index];
+        const uvgvpcc_enc::Patch& patchUVG = frameUVG->patchList[patch_index];
         const uint8_t patchMode = static_cast<uint8_t>(ATDU_PATCH_MODE_I_TILE::I_INTRA);
         patch_information_data pid;
         pid.patchMode = patchMode;
@@ -162,7 +162,7 @@ atlas_tile_data_unit atlas_context::create_atlas_tile_data_unit(const uvgvpcc_en
 
 atlas_tile_layer_rbsp atlas_context::create_atlas_tile_layer_rbsp(size_t frameIndex, size_t tileIndex,
                                                                   const uvgvpcc_enc::Parameters& paramUVG,
-                                                                  const uvgvpcc_enc::Frame& frameUVG) {
+                                                                  const std::shared_ptr<uvgvpcc_enc::Frame>& frameUVG) {
     atlas_tile_layer_rbsp rbsp;
 
     // This should be enough for now
@@ -351,7 +351,7 @@ void atlas_context::initialize_atlas_context(const std::shared_ptr<uvgvpcc_enc::
     // Create create_atlas_tile_layer_rbsp for each atlas frame/NAL unit
     for (size_t frame_index = 0; frame_index < gofUVG->nbFrames; ++frame_index) {
         // std::cout << "DEBUG: Creating atlas RBSP, index: " << frame_index << std::endl;
-        auto& frameUVG = *(gofUVG->frames[frame_index]);
+        auto& frameUVG = gofUVG->frames[frame_index];
 
         const size_t tile_index = 0;  // Always 0, because we only have one tile per frame
         const atlas_tile_layer_rbsp rbsp = create_atlas_tile_layer_rbsp(frame_index, tile_index, paramUVG, frameUVG);

src/lib/bitstreamGeneration/atlas_context.hpp

@@ -82,10 +82,10 @@ class atlas_context {
     atlas_frame_parameter_set create_atlas_frame_parameter_set();
     atlas_frame_tile_information create_atlas_frame_tile_information() const;
     atlas_tile_header create_atlas_tile_header(size_t frameIndex, size_t tileIndex, const uvgvpcc_enc::Parameters& paramUVG) const;
-    atlas_tile_data_unit create_atlas_tile_data_unit(const uvgvpcc_enc::Parameters& paramUVG, const uvgvpcc_enc::Frame& frameUVG,
+    atlas_tile_data_unit create_atlas_tile_data_unit(const uvgvpcc_enc::Parameters& paramUVG, const std::shared_ptr<uvgvpcc_enc::Frame>& frameUVG,
                                                      atlas_tile_header& ath) const;
     atlas_tile_layer_rbsp create_atlas_tile_layer_rbsp(size_t frameIndex, size_t tileIndex, const uvgvpcc_enc::Parameters& paramUVG,
-                                                       const uvgvpcc_enc::Frame& frameUVG);
+                                                       const std::shared_ptr<uvgvpcc_enc::Frame>& frameUVG);
 
     // -------------- Functions to write data structures to bitstream --------------
     static void write_nal_hdr(bitstream_t* stream, const uint8_t nal_type, const uint8_t nal_layer_id, const uint8_t nal_temporal_id_plus1);

src/lib/bitstreamGeneration/bitstream_util.cpp

@@ -39,7 +39,7 @@
 #include <cstring>
 #include <new>
 
-// NOLINTBEGIN(cppcoreguidelines-owning-memory,cppcoreguidelines-no-malloc,hicpp-no-malloc,cppcoreguidelines-pro-bounds-pointer-arithmetic) //
+// NOLINTBEGIN(cppcoreguidelines-owning-memory,cppcoreguidelines-no-malloc,hicpp-no-malloc) //
 // TODO(gg) : lf : Currently we manually handle most of the memory object in the bitstream generation. Consider reducing alloc use to minimum
 
 const std::array<uint32_t, 32> uvg_bit_set_mask = {
@@ -264,4 +264,4 @@ void uvg_bitstream_copy_bytes(bitstream_t *const stream, const uint8_t *bytes, u
 
 uint32_t uvg_bitstream_peek_last_byte(bitstream_t *const stream) { return stream->data; }
 
-// NOLINTEND(cppcoreguidelines-owning-memory,cppcoreguidelines-no-malloc,hicpp-no-malloc,cppcoreguidelines-pro-bounds-pointer-arithmetic)
+// NOLINTEND(cppcoreguidelines-owning-memory,cppcoreguidelines-no-malloc,hicpp-no-malloc)

src/lib/bitstreamGeneration/gof.cpp

@@ -46,7 +46,7 @@
 #include "video_sub_bitstream.hpp"
 #include <cstdint>
 
-// NOLINTBEGIN(cppcoreguidelines-pro-type-reinterpret-cast,cppcoreguidelines-owning-memory,cppcoreguidelines-pro-bounds-pointer-arithmetic,cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays)
+// NOLINTBEGIN(cppcoreguidelines-pro-type-reinterpret-cast,cppcoreguidelines-owning-memory,cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays)
 
 void v3c_gof::write_v3c_chunk(uvgvpcc_enc::API::v3c_unit_stream* out) {
     // --------------- Write V3C unit stream -----------------------------------------------------
@@ -322,4 +322,4 @@ void v3c_gof::write_v3c_ld_chunk(const std::vector<nal_info>& ovd_nals, const st
                              "New V3C LD chunk created, " + std::to_string(out->v3c_chunks.size()) + " chunk(s) in buffer. \n");
 }
 
-// NOLINTEND(cppcoreguidelines-pro-type-reinterpret-cast,cppcoreguidelines-owning-memory,cppcoreguidelines-pro-bounds-pointer-arithmetic,cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays)
+// NOLINTEND(cppcoreguidelines-pro-type-reinterpret-cast,cppcoreguidelines-owning-memory,cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays)

src/lib/bitstreamGeneration/video_sub_bitstream.cpp

@@ -41,7 +41,7 @@
 #include <string>
 #include <vector>
 
-// NOLINTBEGIN(cppcoreguidelines-pro-type-reinterpret-cast,cppcoreguidelines-owning-memory,cppcoreguidelines-pro-bounds-pointer-arithmetic)
+// NOLINTBEGIN(cppcoreguidelines-pro-type-reinterpret-cast,cppcoreguidelines-owning-memory)
 
 namespace {
 
@@ -142,4 +142,4 @@ void find_nals(std::vector<uint8_t> &input_data, std::vector<nal_info> &nals) {
     }
 }
 
-// NOLINTEND(cppcoreguidelines-pro-type-reinterpret-cast,cppcoreguidelines-owning-memory,cppcoreguidelines-pro-bounds-pointer-arithmetic)
+// NOLINTEND(cppcoreguidelines-pro-type-reinterpret-cast,cppcoreguidelines-owning-memory)

src/lib/include/uvgvpcc/threadqueue.hpp

@@ -81,7 +81,7 @@ class Job : public std::enable_shared_from_this<Job> {
     using JobFunction = std::function<void()>;
     // Variadic template constructor
     template <typename Func, typename... Args>
-    Job(std::string name, size_t priority, Func&& func, Args&&... args)
+    Job(const std::string& name, const size_t& priority, Func&& func, Args&&... args)
         : name_(name),
           func_(std::bind(std::forward<Func>(func), std::forward<Args>(args)...)),
           state_(threadqueue_job_state::THREADQUEUE_JOB_STATE_PAUSED),
@@ -100,7 +100,7 @@ class Job : public std::enable_shared_from_this<Job> {
     void complete();
     std::string getName() const { return name_; }
     threadqueue_job_state getState() const { return state_; }
-    void setState(threadqueue_job_state state) { state_ = state; }
+    void setState(const threadqueue_job_state& state) { state_ = state; }
 
     mutable std::mutex mtx_;
     std::vector<std::shared_ptr<Job>> reverseDependencies_;

src/lib/include/uvgvpcc/uvgvpcc.hpp

@@ -289,7 +289,7 @@ struct v3c_unit_stream {
 
 void initializeEncoder();
 void setParameter(const std::string& parameterName,const std::string& parameterValue);
-void encodeFrame(std::shared_ptr<Frame> frame, v3c_unit_stream* output);
+void encodeFrame(std::shared_ptr<Frame>& frame, v3c_unit_stream* output);
 void emptyFrameQueue();
 void stopEncoder();
 

src/lib/mapEncoding/abstract2DMapEncoder.hpp

@@ -45,7 +45,7 @@ class Abstract2DMapEncoder {
     Abstract2DMapEncoder(const ENCODER_TYPE& encoderType): encoderType_(encoderType) {};
     virtual ~Abstract2DMapEncoder() = default;
 
-    virtual void encodeGOFMaps(std::shared_ptr<uvgvpcc_enc::GOF>& gof) = 0;
+    virtual void encodeGOFMaps(const std::shared_ptr<uvgvpcc_enc::GOF>& gof) = 0;
 
 protected:
     const ENCODER_TYPE encoderType_;