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

Reuse me stats for lookahead #2956

Merged
merged 1 commit into from
Jun 2, 2022

Conversation

shssoichiro
Copy link
Collaborator

This partially addresses the comments in #2934 by removing the
unnecessary field coded_data.lookahead_me_stats. This data is never
modified between the time it is created in
compute_lookahead_motion_vectors and the time it is used in
compute_block_importances, so there is no need to keep a separate copy
of it. Instead, we can use the main copy of the data in
fs.frame_me_stats.

Results in ~1% peak memory usage reduction.

This partially addresses the comments in xiph#2934 by removing the
unnecessary field `coded_data.lookahead_me_stats`. This data is never
modified between the time it is created in
`compute_lookahead_motion_vectors` and the time it is used in
`compute_block_importances`, so there is no need to keep a separate copy
of it. Instead, we can use the main copy of the data in
`fs.frame_me_stats`.

Results in ~1% peak memory usage reduction.
@lu-zero
Copy link
Collaborator

lu-zero commented Jun 2, 2022

Does it fix the broken invariant?

@shssoichiro
Copy link
Collaborator Author

It fixes one usage of it. There's still another usage of frame_me_stats outside of the TileStateMut, I believe when we make the reference frames.

I wanted to create this fix separately since the solution for this usage is clear--we can remove it--while the fix for the other usage is unclear.

@shssoichiro shssoichiro merged commit efbcdc0 into xiph:master Jun 2, 2022
@shssoichiro shssoichiro deleted the remove-lookahead-me-stats branch June 2, 2022 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants