Fix loggerd not rotating encoder if dcamera upload disabled (#2133)

* should only count after the check

* include no dcam

* no need to copy

* run with both param

* independent idx

* not break test if no device

* remove unused

* EON D is half

* needs test for qcamera too

Co-authored-by: Comma Device <device@comma.ai>

selfdrive/loggerd/loggerd.cc

@@ -134,6 +134,11 @@ void encoder_thread(bool is_streaming, bool raw_clips, int cam_idx) {
   EncoderState encoder_alt;
   bool has_encoder_alt = false;
 
+  pthread_mutex_lock(&s.rotate_lock);
+  int my_idx = s.num_encoder;
+  s.num_encoder += 1;
+  pthread_mutex_unlock(&s.rotate_lock);
+
   int encoder_segment = -1;
   int cnt = 0;
 
@@ -214,10 +219,10 @@ void encoder_thread(bool is_streaming, bool raw_clips, int cam_idx) {
                  && !do_exit) {
             s.cv.wait(lk);
           }
-          should_rotate = extra.frame_id > s.rotate_last_frame_id && encoder_segment < s.rotate_segment && s.rotate_seq_id == cam_idx;
+          should_rotate = extra.frame_id > s.rotate_last_frame_id && encoder_segment < s.rotate_segment && s.rotate_seq_id == my_idx;
         } else {
           // front camera is best effort
-          should_rotate = encoder_segment < s.rotate_segment && s.rotate_seq_id == cam_idx;
+          should_rotate = encoder_segment < s.rotate_segment && s.rotate_seq_id == my_idx;
         }
         if (do_exit) break;
 
@@ -226,7 +231,7 @@ void encoder_thread(bool is_streaming, bool raw_clips, int cam_idx) {
           LOG("rotate encoder to %s", s.segment_path);
 
           encoder_rotate(&encoder, s.segment_path, s.rotate_segment);
-          s.rotate_seq_id = (cam_idx + 1) % s.num_encoder;
+          s.rotate_seq_id = (my_idx + 1) % s.num_encoder;
 
           if (has_encoder_alt) {
             encoder_rotate(&encoder_alt, s.segment_path, s.rotate_segment);
@@ -249,7 +254,7 @@ void encoder_thread(bool is_streaming, bool raw_clips, int cam_idx) {
           pthread_mutex_unlock(&s.rotate_lock);
 
           while(s.should_close > 0 && s.should_close < s.num_encoder) {
-            // printf("%d waiting for others to reach close, %d/%d \n", cam_idx, s.should_close, s.num_encoder);
+            // printf("%d waiting for others to reach close, %d/%d \n", my_idx, s.should_close, s.num_encoder);
             s.cv.wait(lk);
           }
 
@@ -263,11 +268,17 @@ void encoder_thread(bool is_streaming, bool raw_clips, int cam_idx) {
           encoder_open(&encoder, encoder.next_path);
           encoder.segment = encoder.next_segment;
           encoder.rotating = false;
+          if (has_encoder_alt) {
+            encoder_close(&encoder_alt);
+            encoder_open(&encoder_alt, encoder_alt.next_path);
+            encoder_alt.segment = encoder_alt.next_segment;
+            encoder_alt.rotating = false;
+          }
           s.finish_close += 1;
           pthread_mutex_unlock(&s.rotate_lock);
 
           while(s.finish_close > 0 && s.finish_close < s.num_encoder) {
-            // printf("%d waiting for others to actually close, %d/%d \n", cam_idx, s.finish_close, s.num_encoder);
+            // printf("%d waiting for others to actually close, %d/%d \n", my_idx, s.finish_close, s.num_encoder);
             s.cv.wait(lk);
           }
           s.finish_close = 0;
@@ -607,16 +618,13 @@ int main(int argc, char** argv) {
 #ifndef DISABLE_ENCODER
   // rear camera
   std::thread encoder_thread_handle(encoder_thread, is_streaming, false, CAM_IDX_FCAM);
-  s.num_encoder += 1;
 
   // front camera
   std::thread front_encoder_thread_handle(encoder_thread, false, false, CAM_IDX_DCAM);
-  s.num_encoder += 1;
 
   #ifdef QCOM2
   // wide camera
   std::thread wide_encoder_thread_handle(encoder_thread, false, false, CAM_IDX_ECAM);
-  s.num_encoder += 1;
   #endif
 #endif
 

selfdrive/loggerd/tests/test_loggerd.py

@@ -6,11 +6,12 @@ import shutil
 import subprocess
 import time
 import unittest
+from parameterized import parameterized
 from pathlib import Path
+from tqdm import trange
 
 from common.params import Params
 from common.hardware import EON, TICI
-from common.timeout import Timeout
 from selfdrive.test.helpers import with_processes
 from selfdrive.loggerd.config import ROOT, CAMERA_FPS
 
@@ -21,9 +22,14 @@ if EON:
   CAMERAS = {
     "fcamera": FULL_SIZE,
     "dcamera": 770920,
+    "qcamera": 38533,
   }
 elif TICI:
   CAMERAS = {f"{c}camera": FULL_SIZE for c in ["f", "e", "d"]}
+else:
+  CAMERAS = {}
+
+ALL_CAMERA_COMBINATIONS = [(cameras,) for cameras in [CAMERAS, {k:CAMERAS[k] for k in CAMERAS if k!='dcamera'}]]
 
 FRAME_TOLERANCE = 2
 FILE_SIZE_TOLERANCE = 0.25
@@ -37,7 +43,6 @@ class TestLoggerd(unittest.TestCase):
       raise unittest.SkipTest
 
   def setUp(self):
-    Params().put("RecordFront", "1")
     self._clear_logs()
 
     self.segment_length = 2
@@ -55,28 +60,27 @@ class TestLoggerd(unittest.TestCase):
     last_route = sorted(Path(ROOT).iterdir(), key=os.path.getmtime)[-1]
     return os.path.join(ROOT, last_route)
 
-  # TODO: this should run faster than real time
   @with_processes(['camerad', 'loggerd'], init_time=5)
-  def test_log_rotation(self):
-    # wait for first seg to start being written
-    time.sleep(5)
-    route_prefix_path = self._get_latest_segment_path().rsplit("--", 1)[0]
+  def _log_data(self, t):
+    time.sleep(t)
+
+  # TODO: this should run faster than real time
+  @parameterized.expand(ALL_CAMERA_COMBINATIONS)
+  def test_log_rotation(self, cameras):
+    print("checking targets:", cameras)
+    Params().put("RecordFront", "1" if 'dcamera' in cameras else "0")
+    time.sleep(1)
 
     num_segments = random.randint(80, 150)
-    for i in range(num_segments):
-      if i < num_segments - 1:
-        with Timeout(self.segment_length*3, error_msg=f"timed out waiting for segment {i}"):
-          while True:
-            seg_num = int(self._get_latest_segment_path().rsplit("--", 1)[1])
-            if seg_num > i:
-              break
-            time.sleep(0.1)
-      else:
-        time.sleep(self.segment_length + 2)
+    self._log_data(self.segment_length * num_segments + 5)
+    time.sleep(5)
 
+    route_prefix_path = self._get_latest_segment_path().rsplit("--", 1)[0]
+    for i in trange(num_segments):
       # check each camera file size
-      for camera, size in CAMERAS.items():
-        file_path = f"{route_prefix_path}--{i}/{camera}.hevc"
+      for camera, size in cameras.items():
+        ext = "ts" if camera=='qcamera' else "hevc"
+        file_path = f"{route_prefix_path}--{i}/{camera}.{ext}"
 
         # check file size
         self.assertTrue(os.path.exists(file_path), f"couldn't find {file_path}")
@@ -84,10 +88,13 @@ class TestLoggerd(unittest.TestCase):
         self.assertTrue(math.isclose(file_size, size, rel_tol=FILE_SIZE_TOLERANCE),
                         f"{camera} failed size check: expected {size}, got {file_size}")
 
+        if camera == 'qcamera':
+          continue
+
         # check frame count
         cmd = f"ffprobe -v error -count_frames -select_streams v:0 -show_entries stream=nb_read_frames \
                -of default=nokey=1:noprint_wrappers=1 {file_path}"
-        expected_frames = self.segment_length * CAMERA_FPS
+        expected_frames = self.segment_length * CAMERA_FPS // 2 if (EON and camera=='dcamera') else self.segment_length * CAMERA_FPS
         frame_count = int(subprocess.check_output(cmd, shell=True, encoding='utf8').strip())
         self.assertTrue(abs(expected_frames - frame_count) <= FRAME_TOLERANCE,
                         f"{camera} failed frame count check: expected {expected_frames}, got {frame_count}")