replay: send frames based on encodeIdx packet

[pd0wm]

This should simplify things a bit. @deanlee what do you think?

[deanlee]

lgtm. the only problem is that the missing frames cannot be traced after this.are we going to do some special processing for lost frames in the future? for example, output a blank frame

[pd0wm]

What do you mean by missing frames? A frame that has an encodeIdx packet but is not in the hevc?

[deanlee]

What do you mean by missing frames? A frame that has an encodeIdx packet but is not in the hevc?

I mean there is no corresponding encode idx for getRoadCameraState().getFrameId()

[pd0wm]

Where do we still call getRoadCameraState? I think I made everything driven by encodeIdx now.

[deanlee]

Sorry for my bad expression, I mean, do we need to consider the situation where the frame does not has a corresponding encodeid? and do some special processing in the future, fox example: #22095

[pd0wm]

Think it's fine for replay if we only show frames that are actually in the video file for now. We will deal with dropped frames later.

[pd0wm]

After this change the demo route doesn't perform very well since it doesn't have eof/sof timestaps set correctly. This is no longer a problem on recent routes. Shouldn't be a problem since we will replace the demo route anyway by something that has modelV2 packets.

[deanlee]

I suggest to set the ENCODE_IDX's mono_time to SOF to ensure the events order and the frame sending time. This can solve all the side effects

 try {
      std::unique_ptr<Event> evt = std::make_unique<Event>(words);
      switch (evt->which) {
        case cereal::Event::ROAD_ENCODE_IDX:
          evt->mono_time = evt->event.getRoadEncodeIdx().getTimestampSof();
          break;
        case cereal::Event::DRIVER_ENCODE_IDX:
          evt->mono_time = evt->event.getDriverEncodeIdx().getTimestampSof();
          break;
        case cereal::Event::WIDE_ROAD_ENCODE_IDX:
          evt->mono_time = evt->event.getWideRoadEncodeIdx().getTimestampSof();
          break;
        default:
          break;
      }
      words = kj::arrayPtr(evt->reader.getEnd(), words.end());
      events.push_back(evt.release());
    } catch (const kj::Exception &e) {
      return false;
    }

[pd0wm]

Do you want to handle this outside of the event class? We still need to check for both eof and sof. C2 only has eof set, and in earlier routes none of them are set.

[deanlee]

How about put aside this simplification for now, back later and continue to do it?

[pd0wm]

This change is necessary for replay to work on qlogs, since they don't contain full frequency cameraState packets.

[deanlee]

hmmm...OIC

[pd0wm]

Let's merge this for now, but the real solution is probably to insert a special event type that only handles sending the frame data over visionIPC. That should allow us to send the encodeIdx packet at the correct logMonoTime, while also maintaining timing accuracy of the video.

[deanlee]

but the real solution is probably to insert a special event type that only handles sending the frame data over visionIPC. That should allow us to send the encodeIdx packet at the correct logMonoTime, while also maintaining timing accuracy of the video.

@pd0wm I have submitted a new PR #22364 after read your code. insert new custom encode events in logreader. all event's memory will be handled by logreader as before.
It should be able to meet the needs.and there will be no other side effects.

[deanlee]

@pd0wm It seems that there are duplicate frame eidx in the dmeo route, each frame has two identical eidx, as a result, every frame is published twice

[pd0wm]

It shouldn't since it checks for the type to be HEVC before publishing. We should really replace the demo route with something more recent though.