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

struct Dav1dFrameHeader: backport memory size reduction from dav1d 1.3.0 #1007

Merged
merged 2 commits into from
Apr 30, 2024

Conversation

fbossen
Copy link
Collaborator

@fbossen fbossen commented Apr 27, 2024

Shrinks Rav1dFrameHeader from 1664 to 1120 bytes.

@kkysen kkysen self-assigned this Apr 28, 2024
@kkysen kkysen self-requested a review April 28, 2024 05:04
Copy link
Collaborator

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

Mostly good, I just have some small comments about casts. And I think sharpness is supposed to be a u8 like in C.

src/lib.rs Outdated Show resolved Hide resolved
include/dav1d/headers.rs Outdated Show resolved Hide resolved
src/decode.rs Outdated Show resolved Hide resolved
src/decode.rs Outdated Show resolved Hide resolved
src/decode.rs Outdated Show resolved Hide resolved
src/lf_mask.rs Outdated Show resolved Hide resolved
src/thread_task.rs Outdated Show resolved Hide resolved
src/obu.rs Show resolved Hide resolved
src/obu.rs Outdated Show resolved Hide resolved
.frame_offset,
frame_offset,
.frame_offset as c_int,
frame_offset as c_int,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could make fn get_poc_diff and all of the poc fields/vars u8s. I can add that if you want (I checked to see if it it's possible).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but that's really beyond the scope of this PR. I'll let you create a new one.

@fbossen fbossen merged commit 3a57315 into memorysafety:main Apr 30, 2024
21 checks passed
@@ -358,7 +359,7 @@ unsafe fn read_tx_tree(
};
}

fn neg_deinterleave(diff: c_int, r#ref: c_int, max: c_int) -> c_int {
fn neg_deinterleave(diff: u8, r#ref: u8, max: u8) -> u8 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can't change these to unsigned, max - 1 may be negative.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does #1056 solve it?

fbossen pushed a commit to fbossen/rav1d that referenced this pull request May 8, 2024
`max` can be 0 and subtracting 1 causes underflow
fbossen pushed a commit to fbossen/rav1d that referenced this pull request May 8, 2024
`max` can be 0 and subtracting 1 causes underflow
fbossen pushed a commit that referenced this pull request May 9, 2024
`max` can be 0 and subtracting 1 causes underflow
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.

4 participants