Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

encoderd: fix premature rotation issue on camera realignment #33112

Closed

Conversation

deanlee
Copy link
Contributor

@deanlee deanlee commented Jul 28, 2024

This PR fixes premature encoder rotation after camera realignment.

When a camera realignment occurs, the frame ID jumps ahead (e.g., from frame ID 100 to 106). The original rotation logic, relying on continuous frame IDs, caused early rotations. This could lead to crashes or memory exhaustion in loggerd due to a sync bug if realignment happens frequently.

The updated logic ensures correct rotation after the specified number of frames per segment (FRAMES_PER_SEGMENT), even with frame ID jumps. This change improves the robustness and stability of the encoder, preventing premature rotations and mitigating crash and memory issues.

Copy link
Contributor

Thanks for contributing to openpilot! In order for us to review your PR as quickly as possible, check the following:

  • Convert your PR to a draft unless it's ready to review
  • Read the contributing docs
  • Before marking as "ready for review", ensure:
    • the goal is clearly stated in the description
    • all the tests are passing
    • the change is something we merge
    • include a route or your device' dongle ID if relevant

@adeebshihadeh
Copy link
Contributor

We need a good test before we can merge anything like this.

@adeebshihadeh adeebshihadeh marked this pull request as draft August 2, 2024 19:17
@gregjhogan
Copy link
Member

gregjhogan commented Aug 7, 2024

I think this causes camera frames between camera files to be out of alignment for every segment after a frame drop? That is really annoying when trying to play video files in sync, and it causes any segments after a frame drop to not be eligible for training. It seems concerning this didn't cause any tests to fail considering it used to rotate using the logic in this PR and we changed it intentionally.

Copy link
Contributor

This PR has had no activity for 9 days. It will be automatically closed in 2 days if there is no activity.

@github-actions github-actions bot added the stale label Aug 17, 2024
Copy link
Contributor

This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes.

@github-actions github-actions bot closed this Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants