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

Backport palette packing asm from dav1d 1.3.0 #989

Merged
merged 3 commits into from
Apr 24, 2024

Conversation

fbossen
Copy link
Collaborator

@fbossen fbossen commented Apr 21, 2024

Move palette packing/edge-extension into a DSP function and provide asm and Rust equivalents

@fbossen
Copy link
Collaborator Author

fbossen commented Apr 21, 2024

This is on top of #987

@fbossen fbossen force-pushed the bp-1.3.0-0038 branch 2 times, most recently from da8504e to e32620e Compare April 21, 2024 18:43
@kkysen
Copy link
Collaborator

kkysen commented Apr 21, 2024

This is on top of #987

You can set the target branch to the branch for #987. You might have to move the branches to memorysafety/rav1d instead of fbossen/rav1d, though, not sure.

@fbossen
Copy link
Collaborator Author

fbossen commented Apr 21, 2024

You can set the target branch to the branch for #987. You might have to move the branches to memorysafety/rav1d instead of fbossen/rav1d, though, not sure.

Thank you for the suggestion. Indeed, I think I would have to move the branches to memorysafety/rav1d for this to work

@fbossen fbossen force-pushed the bp-1.3.0-0038 branch 5 times, most recently from 6ba4cc1 to 847035f Compare April 23, 2024 21:18
@kkysen kkysen self-assigned this Apr 24, 2024
@kkysen kkysen self-requested a review April 24, 2024 08:42
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.

Some little things. Rest looks generally good.

src/pal.rs Outdated
bh: c_int,
w: c_int,
h: 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.

Suggested change
) -> () {
) {

src/pal.rs Outdated Show resolved Hide resolved
src/pal.rs Outdated Show resolved Hide resolved
src/pal.rs Outdated Show resolved Hide resolved
src/pal.rs Outdated Show resolved Hide resolved
src/pal.rs Show resolved Hide resolved
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.

LGTM, but I'm not sure why it's causing some timeouts on aarch64 macOS, since these changes shouldn't make things slower.

@fbossen fbossen merged commit 83bbcc9 into memorysafety:main Apr 24, 2024
21 checks passed
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.

3 participants