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

Fix image & video cache creating new entries when selecting data without explicit media type #7590

Merged
merged 2 commits into from
Oct 7, 2024

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Oct 4, 2024

What

That lengthy title is best explained in steps:

  • load a large video (or image!) by drag & droping it in
  • select it
  • observe how memory goes up again!

This happened because image & video both use the media type as part of their cache key entries, but we tend to sometimes resolve it ahead of time and sometimes not, so we get new cache entries.
We do want the media type in the cache entries so we can memorize which (user selected / override affected) media types won't work, but we have to make sure we don't waste memory because of when we resolve the media type.

Before:

before.mp4

After:

after.mp4

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!
  • If have noted any breaking changes to the log API in CHANGELOG.md and the migration guide

To run all checks from main, comment on the PR with @rerun-bot full-check.

@Wumpf Wumpf added 🪳 bug Something isn't working 📺 re_viewer affects re_viewer itself 🚀 performance Optimization, memory use, etc include in changelog 🎞️ video labels Oct 4, 2024
@teh-cmc teh-cmc self-requested a review October 7, 2024 07:16
Copy link
Member

@teh-cmc teh-cmc left a comment

Choose a reason for hiding this comment

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

I'm not sure why some things got renamed to MIME (in this PR, and previous ones)?

Not only is it inconsistent with the rest of the codebase which now sometimes calls things MIME and other times MediaType, but also these are certainly media types.

@Wumpf
Copy link
Member Author

Wumpf commented Oct 7, 2024

I'm not sure why some things got renamed to MIME

Tried to make a few of the errors more consistent 🤷

@Wumpf Wumpf merged commit c44c300 into main Oct 7, 2024
33 checks passed
@Wumpf Wumpf deleted the andreas/fix-video-cache branch October 7, 2024 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪳 bug Something isn't working include in changelog 🚀 performance Optimization, memory use, etc 📺 re_viewer affects re_viewer itself 🎞️ video
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Selecting a video takes up the memory of that video _again_
2 participants