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

Reduce memory usage of pyrdp-convert.py's conversion to video #353

Merged
merged 4 commits into from
Sep 2, 2021

Conversation

obilodeau
Copy link
Collaborator

@obilodeau obilodeau commented Aug 30, 2021

The first change is to use an MP4 streaming format instead of the default MP4 encoder.

The second makes our BitmapCache track the QImage backing buffer in addition to the QImage itself.

The last one plugs a memory leak in the run length encoding decoding routine in our C extension rle.c.

See #352 for the whole story including profiling and performance testing.

See doc comments for the benefits
@obilodeau obilodeau self-assigned this Aug 30, 2021
@obilodeau obilodeau added the enhancement New feature or request label Aug 30, 2021
@obilodeau obilodeau changed the title Reduce memory usage Reduce memory usage of pyrdp-convert.py's conversion to video Aug 30, 2021
@obilodeau
Copy link
Collaborator Author

Also see #352.

Tests are in-progress. Will post results once done.

This is required due to QT's implicit data sharing. Further updates to
unflushed buffers (ie: cache updates) would corrupt other portion of the
images.

The bug was not visible until we started to try to plug memory leaks
with our rle C extension.
Returns a bytearray and takes an immutable bytes object as input.
Reference counting was checked to be valid.

See #352 for the whole memory analysis
@obilodeau obilodeau merged commit 58d7f4f into master Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant