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 Rav1dWarpedMotionParams: Make abcd: [i16; 4] field atomic #662

Merged

Conversation

kkysen
Copy link
Collaborator

@kkysen kkysen commented Jan 3, 2024

This eliminates the only mutation of Rav1dFrameHeader, thus allowing it to be stored in an Arc without any borrowck problems.

This uses the atomig crate to make the [i16; 4] field atomic. The crate is quite simple, offering an Atomic<T> type that can only be implemented for actually lock-free atomic types through a simple pack and unpack into an atomic type (u64 in this case for [i16; 4]).

The alternatives would to use the atomic crate, which also offers an Atomic<T>, but doesn't guarantee a lock-free implementation, and whose implementation seems more complex. The other alternative would be to basically recreate what atomig is doing of packing and unpacking into an u64 for atomic operations, so I think using the existing atomig crate is simpler.

Update: I replaced atomig with an inline solution of about the same complexity.

@kkysen kkysen requested review from rinon and randomPoison January 3, 2024 11:27
@kkysen kkysen force-pushed the kkysen/struct-Rav1dITUTT35-Arcify branch from bc39b2b to 6ed6e8b Compare January 3, 2024 21:00
@kkysen kkysen force-pushed the kkysen/struct-Rav1dWarpedMotionParams-mutate-through-atomics branch from 4c5076a to cd8f316 Compare January 3, 2024 21:02
@kkysen kkysen changed the title struct Rav1dWarpedMotionParams: Make abcd: [i16; 4] field atomic struct Rav1dWarpedMotionParams: Make abcd: [i16; 4] field atomic Jan 3, 2024
include/dav1d/headers.rs Outdated Show resolved Hide resolved
Base automatically changed from kkysen/struct-Rav1dITUTT35-Arcify to main January 9, 2024 01:18
… which eliminates the only mutation of `Rav1dFrameHeader`.
@kkysen kkysen force-pushed the kkysen/struct-Rav1dWarpedMotionParams-mutate-through-atomics branch from cd8f316 to 88d9187 Compare January 9, 2024 01:19
@kkysen kkysen requested a review from randomPoison January 9, 2024 21:37
@kkysen kkysen merged commit 126be73 into main Jan 11, 2024
34 checks passed
@kkysen kkysen deleted the kkysen/struct-Rav1dWarpedMotionParams-mutate-through-atomics branch January 11, 2024 00:15
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