Skip to content

Conversation

@TheSecurityDev
Copy link
Contributor

@TheSecurityDev TheSecurityDev commented Feb 7, 2026

This appears to be something that could be used for displaying individual thumbnails of the diff frames, but for now it's not used and makes things more complex.

Copilot AI review requested due to automatic review settings February 7, 2026 21:34
@github-actions github-actions bot added the ui label Feb 7, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2026

mici raylib UI Preview

✅ Videos are identical! View Diff Report

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR simplifies the UI diff replay HTML report generator by removing an unused mechanism for embedding per-frame thumbnails as base64 data URLs, reducing complexity in the diff analysis pipeline.

Changes:

  • Remove frame_to_data_url() and its base64 dependency.
  • Stop building/returning frame_data from find_differences() and update callers accordingly.
  • Update generate_html_report() signature to drop the unused frame_data parameter.
Comments suppressed due to low confidence (1)

selfdrive/ui/tests/diff/diff.py:76

  • generate_html_report builds chunks/current_chunk from different_frames, but the resulting chunks is never used in the generated HTML. This is dead code that adds complexity; consider removing the chunk-building block or wiring it into the report output.
  chunks = []
  if different_frames:
    current_chunk = [different_frames[0]]
    for i in range(1, len(different_frames)):
      if different_frames[i] == different_frames[i - 1] + 1:
        current_chunk.append(different_frames[i])
      else:
        chunks.append(current_chunk)
        current_chunk = [different_frames[i]]
    chunks.append(current_chunk)


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 171 to 173
if different_frames is None:
sys.exit(1)

Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

find_differences() always returns a (different_frames, total_frames) tuple (or raises on failure), so the if different_frames is None: branch is unreachable. Consider removing this check (and associated sys.exit(1)) or changing find_differences() to return None explicitly on failure if that’s intended.

Suggested change
if different_frames is None:
sys.exit(1)

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@TheSecurityDev TheSecurityDev Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should be removed but not important or relevant to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2026

raylib UI Preview

All Screenshots

@adeebshihadeh adeebshihadeh merged commit bcb13a7 into commaai:master Feb 7, 2026
22 of 25 checks passed
@TheSecurityDev TheSecurityDev deleted the remove-unused-frame-data branch February 7, 2026 22:28
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.

2 participants