More clan-tidy leftovers

Summary: More clang-tidy warnings...

Reviewed By: kiminoue7

Differential Revision: D52824643

vrs/DataPieceTypes.h

@@ -36,12 +36,14 @@ using std::size_t;
 /// https://stackoverflow.com/questions/17794569/why-isnt-vectorbool-a-stl-container
 class Bool {
  public:
-  /*implicit*/ Bool(bool value = false) : value_(value) {}
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
+  Bool(bool value = false) : value_(value) {}
   Bool& operator=(bool value) {
     value_ = value;
     return *this;
   }
-  /*implicit*/ operator bool() const {
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
+  operator bool() const {
     return value_;
   }
   const bool* operator&() const {
@@ -77,7 +79,8 @@ struct PointND {
   static constexpr size_t kSize = N;
 
   PointND() : dim{} {}
-  /*implicit*/ PointND(const T arr[N]) {
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
+  PointND(const T arr[N]) {
     operator=(arr);
   }
 
@@ -179,7 +182,8 @@ struct MatrixND {
   static constexpr size_t kMatrixSize = N;
 
   MatrixND() = default;
-  /*implicit*/ MatrixND(const T arr[N][N]) {
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
+  MatrixND(const T arr[N][N]) {
     operator=(arr);
   }
 

vrs/DataReference.h

@@ -73,7 +73,7 @@ class DataReference {
   /// @param data: Pointer to second block of bytes.
   /// @param size: Size of second block of bytes.
   template <class T>
-  DataReference(vector<T>& vectorT, void* data = nullptr, uint32_t size = 0)
+  explicit DataReference(vector<T>& vectorT, void* data = nullptr, uint32_t size = 0)
       : DataReference(
             vectorT.data(),
             static_cast<uint32_t>(sizeof(T) * vectorT.size()),
@@ -104,7 +104,7 @@ class DataReference {
   /// @param data: Pointer to second block of bytes.
   /// @param size: Size of second block of bytes.
   template <class T>
-  DataReference(T& object, void* data = nullptr, uint32_t size = 0)
+  explicit DataReference(T& object, void* data = nullptr, uint32_t size = 0)
       : DataReference(&object, sizeof(T), data, size) {}
 
   /// @param data1: Pointer to first block of bytes.

vrs/DataSource.h

@@ -78,7 +78,8 @@ class DataSourceChunk {
   DataSourceChunk(const void* _data, size_t _size);
   /// Constructor for a vector<T> of objects of POD type T.
   template <typename T>
-  /* implicit */ DataSourceChunk(const vector<T>& vectorT)
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
+  DataSourceChunk(const vector<T>& vectorT)
       : data_{vectorT.data()}, size_{sizeof(T) * vectorT.size()} {}
   virtual ~DataSourceChunk() = default;
 
@@ -89,7 +90,8 @@ class DataSourceChunk {
       std::enable_if_t<std::is_trivially_copyable<T>::value, int> = 0,
       // T may not be a pointer.
       typename = typename enable_if<!is_pointer<T>::value, T>::type>
-  /* implicit */ DataSourceChunk(const T& object) : data_{&object}, size_{sizeof(T)} {}
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
+  DataSourceChunk(const T& object) : data_{&object}, size_{sizeof(T)} {}
 
   /// Copy the data (if any), and update the provided buffer pointer accordingly.
   /// The number of bytes copied *must be the exact size* specified in the constructor.

vrs/Record.h

@@ -144,6 +144,7 @@ class Record final {
 
   /// Public for testing
   struct uninitialized_byte final {
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-type-member-init, modernize-use-equals-default)
     uninitialized_byte() {} // do not use '= default' as it will initialize byte!
     uint8_t byte;
   };

vrs/RecordFileWriter.cpp

@@ -148,15 +148,9 @@ struct CompressionThreadsData {
 // structure holding the background save thread's data, if any
 struct WriterThreadData {
   WriterThreadData()
-      : fileError{0},
-        shouldEndThread{false},
-        writeEventChannel{
+      : writeEventChannel{
             "WriterThreadDataWriteEventChannel",
-            os::EventChannel::NotificationMode::UNICAST},
-        hasRecordsReadyToWrite{false},
-        maxTimestampProvider{nullptr},
-        autoCollectDelay{0},
-        nextAutoCollectTime{0} {
+            os::EventChannel::NotificationMode::UNICAST} {
     // Do *not* start the thread here, as this will create race conditions.
     // The thread may start before the constructor even finished and returned, which means the
     // main thread may not even have a reference to this object.
@@ -168,15 +162,15 @@ struct WriterThreadData {
     }
   }
 
-  atomic<int> fileError; // 0, or last write error
-  atomic<bool> shouldEndThread; // set when this thread should finish-up & end
+  atomic<int> fileError{}; // 0, or last write error
+  atomic<bool> shouldEndThread{}; // set when this thread should finish-up & end
   os::EventChannel writeEventChannel; // wake the background thread to write prepared records
 
   recursive_mutex mutex; // mutex to protect access to the following fields
-  RecordFileWriter::RecordBatches recordsReadyToWrite;
-  atomic<bool> hasRecordsReadyToWrite;
+  RecordFileWriter::RecordBatches recordsReadyToWrite{};
+  atomic<bool> hasRecordsReadyToWrite{};
   function<double()> maxTimestampProvider; // for auto-writing records
-  atomic<double> autoCollectDelay; // for auto-writing records
+  atomic<double> autoCollectDelay{}; // for auto-writing records
   double nextAutoCollectTime{}; // for auto-writing records
 
   CompressionThreadsData compressionThreadsData_;

vrs/RecordFormat.h

@@ -163,6 +163,7 @@ class ImageContentBlockSpec {
       uint32_t keyFrameIndex = 0);
 
   /// Image formats with encoding (png, jpeg, etc).
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
   ImageContentBlockSpec(ImageFormat imageFormat, uint32_t width = 0, uint32_t height = 0);
 
   /// Raw pixels image formats.
@@ -343,6 +344,7 @@ class AudioContentBlockSpec {
   /// For audio formats with encoding (mp3, flac, etc).
   explicit AudioContentBlockSpec(AudioFormat audioFormat, uint8_t channelCount = 0);
   /// For PCM audio formats.
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
   AudioContentBlockSpec(
       AudioSampleFormat sampleFormat,
       uint8_t channelCount = 0,
@@ -479,12 +481,14 @@ class ContentBlock {
   static const size_t kSizeUnknown;
 
   /// Very generic block description.
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
   ContentBlock(ContentType type = ContentType::EMPTY, size_t size = kSizeUnknown);
 
   /// Factory-style reconstruct from persisted description as string on disk.
   explicit ContentBlock(const string& formatStr);
 
   /// Image formats with encoding (png, jpeg, etc).
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
   ContentBlock(ImageFormat imageFormat, uint32_t width = 0, uint32_t height = 0);
 
   /// Image formats with custom codec encoding.
@@ -505,16 +509,19 @@ class ContentBlock {
       uint32_t stride = 0,
       uint32_t stride2 = 0);
 
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
   ContentBlock(const ImageContentBlockSpec& imageSpec, size_t size = kSizeUnknown);
   ContentBlock(
       const ContentBlock& imageContentBlock,
       double keyFrameTimestamp,
       uint32_t keyFrameIndex);
 
   /// Very generic audio block description.
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
   ContentBlock(AudioFormat audioFormat, uint8_t channelCount = 0);
 
   /// PCM audio block description.
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
   ContentBlock(
       AudioSampleFormat sampleFormat,
       uint8_t numChannels = 0,
@@ -645,25 +652,30 @@ class RecordFormat {
   /// Empty record format definition.
   RecordFormat() = default;
   /// Default copy constructor
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
   RecordFormat(const RecordFormat&) = default;
 
   /// Build a RecordFormat from a string description.
   /// This constructor is meant for internal VRS usage only.
   /// @internal
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
   RecordFormat(const string& format) {
     set(format);
   }
   /// Build a RecordFormat from a string description.
   /// This constructor is meant for internal VRS usage only.
   /// @internal
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
   RecordFormat(const char* format) {
     set(format);
   }
   /// Build a RecordFormat from a single ContentBlock.
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
   RecordFormat(const ContentBlock& block) {
     blocks_.emplace_back(block);
   }
   /// Build a RecordFormat from a single ContentBlock.
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
   RecordFormat(ContentBlock&& block) {
     blocks_.emplace_back(block);
   }
@@ -681,6 +693,7 @@ class RecordFormat {
   /// @param type: Content type of the block.
   /// @param size: Size of the block, or ContentBlock::kSizeUnknown, if the size of the block isn't
   /// known/fixed.
+  // NOLINTNEXTLINE(hicpp-explicit-conversions)
   RecordFormat(ContentType type, size_t size = ContentBlock::kSizeUnknown) {
     blocks_.emplace_back(type, size);
   }

vrs/helpers/MemBuffer.h

@@ -32,6 +32,7 @@ namespace helpers {
 
 class MemBuffer {
   struct uninitialized_byte final {
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-type-member-init, modernize-use-equals-default)
     uninitialized_byte() {} // do not use '= default' as it will initialize byte!
     uint8_t byte;
   };

vrs/helpers/Rapidjson.hpp

@@ -45,9 +45,11 @@ using JValue = vrs_rapidjson::GenericValue<JUtf8Encoding, JCrtAllocator>;
 using JStringRef = vrs_rapidjson::GenericStringRef<char>;
 
 static inline JStringRef jStringRef(const char* str) {
+  // NOLINTNEXTLINE(modernize-return-braced-init-list)
   return JStringRef(str, strlen(str));
 }
 static inline JStringRef jStringRef(const string& str) {
+  // NOLINTNEXTLINE(modernize-return-braced-init-list)
   return JStringRef(str.c_str(), str.size());
 }
 template <class T>

vrs/test/DataLayoutFormatTest.cpp

@@ -350,6 +350,7 @@ TEST_F(DataLayoutFormatTester, DataLayoutFormatTest) {
   os::remove(fileName);
 }
 
+// NOLINTNEXTLINE(modernize-macro-to-enum)
 #define SAMPLE_EPOCH_TIME 2000000000
 
 TEST_F(DataLayoutFormatTester, FormatValuesTest) {

vrs/test/RecordTest.cpp

@@ -292,6 +292,7 @@ static uint8_t f(uint8_t k) {
 const size_t kSize = 9; // odd, to expose padding issues
 
 union ArrayUnion {
+  // NOLINTNEXTLINE(cppcoreguidelines-pro-type-member-init)
   ArrayUnion() {} // required, do not use '= default' which would initialize the fields!
   Record::uninitialized_byte uninitialized_bytes[kSize];
   uint8_t initialized_bytes[kSize];

vrs/utils/AudioExtractor.cpp

@@ -18,6 +18,7 @@
 
 #include <array>
 #include <iostream>
+#include <utility>
 
 #define DEFAULT_LOG_CHANNEL "AudioExtractor"
 #include <logging/Log.h>
@@ -136,8 +137,8 @@ int AudioExtractor::closeWavFile(DiskFile& inFile) {
   return inFile.close();
 }
 
-AudioExtractor::AudioExtractor(const string& folderPath, StreamId id, uint32_t& counter)
-    : folderPath_{folderPath},
+AudioExtractor::AudioExtractor(string folderPath, StreamId id, uint32_t& counter)
+    : folderPath_{std::move(folderPath)},
       id_{id},
       cumulativeOutputAudioFileCount_{counter},
       currentAudioContentBlockSpec_{AudioFormat::UNDEFINED} {}

vrs/utils/AudioExtractor.h

@@ -27,7 +27,7 @@ namespace vrs::utils {
 
 class AudioExtractor : public RecordFormatStreamPlayer {
  public:
-  AudioExtractor(const string& folderPath, StreamId id, uint32_t& counter);
+  AudioExtractor(string folderPath, StreamId id, uint32_t& counter);
 
   ~AudioExtractor() override;
 

vrs/utils/AudioTrackExtractor.cpp

@@ -16,8 +16,8 @@
 
 #include "AudioTrackExtractor.h"
 
-#include <iostream>
 #include <limits>
+#include <utility>
 
 #define DEFAULT_LOG_CHANNEL "AudioTrackExtractor"
 #include <logging/Log.h>
@@ -37,8 +37,10 @@ using namespace vrs::helpers;
 
 namespace vrs::utils {
 
-AudioTrackExtractor::AudioTrackExtractor(const string& wavFilePath, bool& outStop)
-    : wavFilePath_{wavFilePath}, stop_{outStop}, fileAudioSpec_{AudioFormat::UNDEFINED} {}
+AudioTrackExtractor::AudioTrackExtractor(string wavFilePath, bool& outStop)
+    : wavFilePath_{std::move(wavFilePath)},
+      stop_{outStop},
+      fileAudioSpec_{AudioFormat::UNDEFINED} {}
 
 AudioTrackExtractor::~AudioTrackExtractor() {
   AudioExtractor::closeWavFile(wavFile_);

vrs/utils/AudioTrackExtractor.h

@@ -24,7 +24,7 @@ namespace vrs::utils {
 
 class AudioTrackExtractor : public RecordFormatStreamPlayer {
  public:
-  AudioTrackExtractor(const string& wavFilePath, bool& outStop);
+  AudioTrackExtractor(string wavFilePath, bool& outStop);
   ~AudioTrackExtractor() override;
 
   bool onAudioRead(const CurrentRecord& record, size_t, const ContentBlock& audioBlock) override;

vrs/utils/DataExtractor.cpp

@@ -16,6 +16,8 @@
 
 #include "DataExtractor.h"
 
+#include <utility>
+
 #include <fmt/format.h>
 
 #define DEFAULT_LOG_CHANNEL "DataExtractor"
@@ -39,8 +41,8 @@ using namespace vrs;
 
 DataExtractor::DataExtractorStreamPlayer::DataExtractorStreamPlayer(
     ofstream& output,
-    const string& outputFolder)
-    : output_{output}, outputFolder_{outputFolder} {}
+    string outputFolder)
+    : output_{output}, outputFolder_{std::move(outputFolder)} {}
 
 bool DataExtractor::DataExtractorStreamPlayer::writeImage(
     const CurrentRecord& record,

vrs/utils/DataExtractor.h

@@ -61,7 +61,7 @@ class DataExtractor {
 
   class DataExtractorStreamPlayer : public utils::VideoRecordFormatStreamPlayer {
    public:
-    DataExtractorStreamPlayer(std::ofstream& output, const string& outputFolder);
+    DataExtractorStreamPlayer(std::ofstream& output, string outputFolder);
     bool onDataLayoutRead(const CurrentRecord&, size_t blockIndex, DataLayout&) override;
     bool onImageRead(const CurrentRecord&, size_t blockIndex, const ContentBlock&) override;
     bool onAudioRead(const CurrentRecord& record, size_t, const ContentBlock& audioBlock) override;

vrs/utils/FilterCopyHelpers.h

@@ -40,7 +40,8 @@ struct TagOverrider {
 
 // Optional parameters for copy (or merge) operations, to override defaults
 struct CopyOptions {
-  CopyOptions(bool showProgress = true) : showProgress{showProgress} {}
+  CopyOptions() = default;
+  explicit CopyOptions(bool showProgress) : showProgress{showProgress} {}
   CopyOptions(const CopyOptions& rhs);
 
   // Compression preset of the output file. Use this method to set the user's explicit choice.
@@ -164,7 +165,7 @@ class ContentBlockChunk : public ContentChunk {
 
 class FilteredChunksSource : public DataSource {
  public:
-  FilteredChunksSource(deque<unique_ptr<ContentChunk>>& chunks)
+  explicit FilteredChunksSource(deque<unique_ptr<ContentChunk>>& chunks)
       : DataSource(getFilteredChunksSize(chunks)), chunks_{chunks} {}
   void copyTo(uint8_t* buffer) const override;
 

vrs/utils/ThrottleHelpers.h

@@ -30,7 +30,7 @@ class ThrottledFileDelegate;
 /// using a RecordFileWriter object.
 class ThrottledWriter {
  public:
-  ThrottledWriter(const CopyOptions& options);
+  explicit ThrottledWriter(const CopyOptions& options);
   ThrottledWriter(const CopyOptions& options, ThrottledFileDelegate& fileDelegate);
 
   /// Init writer with latest copy options values (if they were changed since constructor)

vrs/utils/Validation.cpp

@@ -666,9 +666,7 @@ class RecordMaster : public StreamPlayer {
         id_{matchingId},
         reader_{matchingReader},
         holder_{holder},
-        index_{reader_.getIndex(id_)},
-        lastRecord_{0},
-        readCounter_{0} {}
+        index_{reader_.getIndex(id_)} {}
   bool processRecordHeader(const CurrentRecord& record, DataReference& outDataReference) override {
     buffer_.resize(record.recordSize);
     outDataReference.useVector(buffer_);

vrs/utils/converters/Raw10ToGrey10Converter.cpp

@@ -62,7 +62,7 @@ void convertVectorized(
   if (contiguous) {
     const int numFullOperations = (heightInPixels * widthInPixels) / 8;
     // The width must be a multiple of 4, so there is at most one leftover pixel group.
-    bool hasLeftover = (heightInPixels * widthInPixels) % 8 ? true : false;
+    bool hasLeftover = (heightInPixels * widthInPixels) % 8;
     for (int op = 0; op < numFullOperations; ++op) {
       const uint8x16_t encoded = vld1q_u8(src);
       const uint8x16_t r = vqtbl1q_u8(encoded, rshuf);