cleanup

system/loggerd/encoder_writer.cc

@@ -39,17 +39,22 @@ size_t EncoderWriter::flush(LoggerState *logger) {
 void EncoderWriter::rotate(const std::string &path) {
   video_writer.reset();
   segment_path = path;
-  marked_ready_to_rotate = false;
+  if (marked_ready_to_rotate) {
+    marked_ready_to_rotate = false;
+    --ready_to_rotate;
+  }
   current_encoder_segment = -1;
 }
 
 size_t EncoderWriter::write_encoder_data(LoggerState *logger, const cereal::Event::Reader event) {
   auto edata = (event.*(info.get_encode_data_func))();
   const auto idx = edata.getIdx();
 
+  // write video
   if (info.record) {
     write_video(edata, idx);
   }
+
   // put it in log stream as the idx packet
   MessageBuilder msg;
   auto evt = msg.initEvent(event.getValid());
@@ -65,7 +70,7 @@ size_t EncoderWriter::write_video(const cereal::EncodeData::Reader &edata, const
   const bool is_key_frame = idx.getFlags() & V4L2_BUF_FLAG_KEYFRAME;
 
   if (!video_writer) {
-    if (is_key_frame) { // only create on iframe
+    if (is_key_frame) {  // only create on iframe
       if (dropped_frames) {
         // this should only happen for the first segment, maybe
         LOGW("%s: dropped %d non iframe packets before init", info.publish_name, dropped_frames);

system/loggerd/encoder_writer.h

@@ -11,8 +11,7 @@ class EncoderWriter {
   void rotate(const std::string &path);
   size_t write(LoggerState *logger, Message *msg);
   size_t flush(LoggerState *logger);
-
-  inline static int ready_to_rotate = 0;  // count of encoders ready to rotate
+  static int readyToRotate() { return ready_to_rotate; }
 
 private:
   size_t write_encoder_data(LoggerState *logger, const cereal::Event::Reader event);
@@ -26,4 +25,6 @@ class EncoderWriter {
   int dropped_frames = 0;
   bool marked_ready_to_rotate = false;
   EncoderInfo info;
+
+  inline static int ready_to_rotate = 0;  // count of encoders ready to rotate
 };

system/loggerd/loggerd.cc

@@ -20,15 +20,14 @@ void logger_rotate(LoggerdState *s) {
   for (auto &[_, encoder] : s->remote_encoders) {
     encoder->rotate(s->segment_path);
   }
-  EncoderWriter::ready_to_rotate = 0;
 
   s->last_rotate_tms = millis_since_boot();
   LOGW((s->logger.part == 0) ? "logging to %s" : "rotated to %s", s->segment_path);
 }
 
 void rotate_if_needed(LoggerdState *s) {
   // all encoders ready, trigger rotation
-  bool all_ready = s->remote_encoders.size() > 0 && (EncoderWriter::ready_to_rotate == s->remote_encoders.size());
+  bool all_ready = EncoderWriter::readyToRotate() > 0 && (EncoderWriter::readyToRotate() == s->remote_encoders.size());
 
   // fallback logic to prevent extremely long segments in the case of camera, encoder, etc. malfunctions
   bool timed_out = false;

system/loggerd/tests/test_encoder_writer.cc

@@ -7,28 +7,24 @@
 #include "system/loggerd/encoder_writer.h"
 
 static Message *generate_msg(const EncoderInfo &info, uint32_t segment_num, uint32_t frame_id, uint32_t flags = 0) {
-  cereal::EncodeData::Builder (cereal::Event::Builder::*initEncoder)();
-  if (strcmp(info.publish_name, "roadEncodeData") == 0) {
-    initEncoder = &cereal::Event::Builder::initRoadEncodeData;
-  } else if (strcmp(info.publish_name, "driverEncodeData") == 0) {
-    initEncoder = &cereal::Event::Builder::initDriverEncodeData;
+  cereal::EncodeData::Builder (cereal::Event::Builder::*initEncodeData)() = initEncodeData = &cereal::Event::Builder::initRoadEncodeData;
+  if (strcmp(info.publish_name, "driverEncodeData") == 0) {
+    initEncodeData = &cereal::Event::Builder::initDriverEncodeData;
   } else if (strcmp(info.publish_name, "wideRoadEncodeData") == 0) {
-    initEncoder = &cereal::Event::Builder::initWideRoadEncodeData;
+    initEncodeData = &cereal::Event::Builder::initWideRoadEncodeData;
   } else if (strcmp(info.publish_name, "qRoadEncodeData") == 0) {
-    initEncoder = &cereal::Event::Builder::initQRoadEncodeData;
+    initEncodeData = &cereal::Event::Builder::initQRoadEncodeData;
   }
 
   MessageBuilder msg;
   auto event = msg.initEvent(true);
-  cereal::EncodeData::Builder edat = (event.*initEncoder)();
+  cereal::EncodeData::Builder edat = (event.*initEncodeData)();
   auto edata = edat.initIdx();
-  struct timespec ts;
-  timespec_get(&ts, TIME_UTC);
-  uint64_t tt = (uint64_t)ts.tv_sec * 1000000000 + ts.tv_nsec;
-  edat.setUnixTimestampNanos(tt);
+  uint64_t ts = nanos_since_boot();
+  edat.setUnixTimestampNanos(ts);
   edata.setFrameId(frame_id);
-  edata.setTimestampSof(tt);
-  edata.setTimestampEof(tt);
+  edata.setTimestampSof(ts);
+  edata.setTimestampEof(ts);
   edata.setEncodeId(frame_id);
   edata.setSegmentNum(segment_num);
   edata.setSegmentId(frame_id);
@@ -41,7 +37,7 @@ static Message *generate_msg(const EncoderInfo &info, uint32_t segment_num, uint
 }
 
 static void test(LoggerState *logger, std::vector<std::unique_ptr<EncoderWriter>> &encoders, const std::vector<int> &segments) {
-  int segment = -1;
+  int logger_segment = -1;
   char segment_path[4096];
   const int frames = 10;
   int prev_seg = -1;
@@ -65,7 +61,7 @@ static void test(LoggerState *logger, std::vector<std::unique_ptr<EncoderWriter>
         if (e->ready_to_rotate == encoders.size()) {
           // q should not empty
           REQUIRE(e->q.size() > 0);
-          int err = logger_next(logger, LOG_ROOT.c_str(), segment_path, sizeof(segment_path), &segment);
+          int err = logger_next(logger, LOG_ROOT.c_str(), segment_path, sizeof(segment_path), &logger_segment);
           REQUIRE(err == 0);
           for (auto &en : encoders) {
             en->rotate(segment_path);
@@ -81,6 +77,8 @@ static void test(LoggerState *logger, std::vector<std::unique_ptr<EncoderWriter>
     e->flush(logger);
     REQUIRE(e->q.size() == 0);
   }
+
+  REQUIRE(segments.size() == logger_segment + 1);
 }
 
 TEST_CASE("EncoderWriter") {
@@ -104,18 +102,6 @@ TEST_CASE("EncoderWriter") {
     test(&logger, encoders, {10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20});
   }
   SECTION("encoderd restarted in the middle)") {
-    test(&logger, encoders, {0, 1, 2, 3, 4, 5, 0, 1, 2, 3, 4, 5});
+    test(&logger, encoders, {0, 1, 2, 3, 4, 5, 0, 1, 2, 3, 4, 5, 0, 1, 2, 3, 4, 5});
   }
-  SECTION("random encoder segment id)") {
-    std::random_device rd;
-    std::mt19937 gen(rd());
-    std::uniform_int_distribution<> distr(0, 50);
-    std::vector<int> segments;
-    for (int n = 0; n < 40; ++n) {
-      segments.push_back(distr(gen));
-    }
-    test(&logger, encoders, segments);
-  }
-
-  logger_close(&logger, nullptr);
 }