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

Mp4 crate/video types stores their content twice in RAM #7481

Open
Wumpf opened this issue Sep 24, 2024 · 2 comments
Open

Mp4 crate/video types stores their content twice in RAM #7481

Wumpf opened this issue Sep 24, 2024 · 2 comments
Labels
📉 performance Optimization, memory use, etc 🚜 refactor Change the code, not the functionality 🎞️ video

Comments

@Wumpf
Copy link
Member

Wumpf commented Sep 24, 2024

Right now we effectively hold an mp4 blob at least twice: Raw in the (arrow) store and then again upon parsing in our mp4 object.
All larger pieces should just be pointers back to the raw blob in order to conserve memory.

Also general cleanup of mp4 crate is in order, this is a good opportunity.

This won't fly for streaming mp4 easily, but for full mp4 it should be doable to do indices into the blob.

@Wumpf Wumpf added 📉 performance Optimization, memory use, etc 🚜 refactor Change the code, not the functionality 🎞️ video labels Sep 24, 2024
@emilk
Copy link
Member

emilk commented Sep 24, 2024

Does arrow bottom out in https://crates.io/crates/bytes ? That would be useful for storing RC-counted references across crates.

Otherwise we have to consider a few different alternatives:

  • re_mp4 uses bytes: Range<usize> instead of Vec<u8>
  • re_mp4 has some sort of trait ByteSource with a Box<dyn ByteSource> that we then implement for Blob
  • ???

@emilk
Copy link
Member

emilk commented Oct 8, 2024

I think the best path forward is to change re_mp4 to store byte ranges (Range<usize>) rather than the raw bytes, and then have re_video (or re_renderer) have some sort of trait ByteSource { fn read(&self, bytes: Range<usize>) -> Option<Vec<u8>> } that we then can wrap around a Blob.

It should be less than a days worth of work.

@emilk emilk changed the title Mp4 crate/video types shouldn't copy large amounts of data Mp4 crate/video types stores their content twice in RAM Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📉 performance Optimization, memory use, etc 🚜 refactor Change the code, not the functionality 🎞️ video
Projects
None yet
Development

No branches or pull requests

2 participants